Re: [Qemu-devel] [PATCH v2] net: mipsnet: check transmit buffer size before sending
Hello Jason, +-- On Mon, 13 Jun 2016, Jason Wang wrote --+ | > case MIPSNET_TX_DATA_BUFFER: | > s->tx_buffer[s->tx_written++] = val; | | I believe we may still have a buffer overflow here, no? No, this is the overflow that the patch is meant to fix. | > -if (s->tx_written == s->tx_count) { | > +if ((s->tx_written >= MAX_ETH_FRAME_SIZE) | > +|| (s->tx_written == s->tx_count)) { | > /* Send buffer. */ Earlier, send buffer would occur when if 'tx_written' reached 'tx_count'. With this patch, it'll also occur when 'tx_written' reaches maximum frame size. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Re: [Qemu-devel] [PATCH v2 13/22] hw/intc/arm_gicv3: Wire up distributor and redistributor MMIO regions
On 2016/5/26 22:55, Peter Maydell wrote: > Wire up the MMIO functions exposed by the distributor and the > redistributor into MMIO regions exposed by the GICv3 device. > > Signed-off-by: Peter Maydell Reviewed-by: Shannon Zhao > --- > hw/intc/arm_gicv3.c | 15 ++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c > index 7c4bee6..e8f6766 100644 > --- a/hw/intc/arm_gicv3.c > +++ b/hw/intc/arm_gicv3.c > @@ -324,6 +324,19 @@ static void arm_gicv3_post_load(GICv3State *s) > gicv3_cache_all_target_cpustates(s); > } > > +static const MemoryRegionOps gic_ops[] = { > +{ > +.read_with_attrs = gicv3_dist_read, > +.write_with_attrs = gicv3_dist_write, > +.endianness = DEVICE_NATIVE_ENDIAN, > +}, > +{ > +.read_with_attrs = gicv3_redist_read, > +.write_with_attrs = gicv3_redist_write, > +.endianness = DEVICE_NATIVE_ENDIAN, > +} > +}; > + > static void arm_gic_realize(DeviceState *dev, Error **errp) > { > /* Device instance realize function for the GIC sysbus device */ > @@ -337,7 +350,7 @@ static void arm_gic_realize(DeviceState *dev, Error > **errp) > return; > } > > -gicv3_init_irqs_and_mmio(s, gicv3_set_irq, NULL); > +gicv3_init_irqs_and_mmio(s, gicv3_set_irq, gic_ops); > } > > static void arm_gicv3_class_init(ObjectClass *klass, void *data) > -- Shannon
Re: [Qemu-devel] [PATCH v2 1/3] error: Remove NULL checks on error_propagate() calls
On Fri, 10 Jun 2016 17:12:16 -0300 Eduardo Habkost wrote: > error_propagate() already ignores local_err==NULL, so there's no > need to check it before calling. > > Coccinelle patch used to perform the changes added to > scripts/coccinelle/error_propagate_null.cocci. > > Signed-off-by: Eduardo Habkost > --- > block.c | 20 +-- > block/qcow2.c | 4 +--- > block/quorum.c| 4 +--- > block/raw-posix.c | 16 --- > block/raw_bsd.c | 4 +--- > block/snapshot.c | 4 +--- > blockdev.c| 12 +++- > bootdevice.c | 4 +--- > dump.c| 4 +--- > hw/ide/qdev.c | 4 +--- > hw/net/ne2000-isa.c | 4 +--- > hw/s390x/virtio-ccw.c | 28 > +++ For virtio-ccw: Acked-by: Cornelia Huck > hw/usb/dev-storage.c | 4 +--- > qga/commands-win32.c | 8 ++-- > qom/object.c | 4 +--- > scripts/coccinelle/error_propagate_null.cocci | 10 ++ > 16 files changed, 41 insertions(+), 93 deletions(-) > create mode 100644 scripts/coccinelle/error_propagate_null.cocci
Re: [Qemu-devel] [PATCH 09/10] ppc: Move exception generation code out of line
On 13.06.2016 07:24, Cédric Le Goater wrote: > From: Benjamin Herrenschmidt > > There's no point inlining this, if you hit the exception case you exit > anyway, and not inlining saves about 100K of code size (and cache > footprint). > > Signed-off-by: Benjamin Herrenschmidt > --- > target-ppc/translate.c | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index f211d175c09c..600d5db2bb9a 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -283,7 +283,8 @@ void gen_update_current_nip(void *opaque) > tcg_gen_movi_tl(cpu_nip, ctx->nip); > } > > -static inline void gen_exception_err(DisasContext *ctx, uint32_t excp, > uint32_t error) > +static void __attribute__((noinline)) > +gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error) > { > TCGv_i32 t0, t1; > if (ctx->exception == POWERPC_EXCP_NONE) { > @@ -297,7 +298,8 @@ static inline void gen_exception_err(DisasContext *ctx, > uint32_t excp, uint32_t > ctx->exception = (excp); > } > > -static inline void gen_exception(DisasContext *ctx, uint32_t excp) > +static void __attribute__((noinline)) > +gen_exception(DisasContext *ctx, uint32_t excp) > { > TCGv_i32 t0; > if (ctx->exception == POWERPC_EXCP_NONE) { > @@ -309,7 +311,8 @@ static inline void gen_exception(DisasContext *ctx, > uint32_t excp) > ctx->exception = (excp); > } > > -static inline void gen_debug_exception(DisasContext *ctx) > +static void __attribute__((noinline)) > +gen_debug_exception(DisasContext *ctx) > { > TCGv_i32 t0; Do you get the same results if you just remove the "inline" keyword, without adding the "__attribute__((noinline))" ? If yes, I'd suggest to do this patch without the "__attribute__((noinline))" - that's easier to read, and the compiler can still decide to inline something in case it's better one a certain architecture. Thomas
Re: [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables
On Fri, 10 Jun 2016 17:12:17 -0300 Eduardo Habkost wrote: > This patch simplifies code that uses a local_err variable just to > immediately use it for an error_propagate() call. > > Coccinelle patch used to perform the changes added to > scripts/coccinelle/remove_local_err.cocci. > > Signed-off-by: Eduardo Habkost > --- > block.c | 8 ++-- > block/raw-posix.c | 8 ++-- > block/raw_bsd.c | 4 +--- > blockdev.c| 16 +--- > hw/s390x/s390-virtio-ccw.c| 5 + > hw/s390x/virtio-ccw.c | 28 +++- For the two virtio-ccw files: Acked-by: Cornelia Huck > scripts/coccinelle/remove_local_err.cocci | 27 +++ > target-i386/cpu.c | 4 +--- > 8 files changed, 46 insertions(+), 54 deletions(-) > create mode 100644 scripts/coccinelle/remove_local_err.cocci
Re: [Qemu-devel] [PATCH 1/2] Fix confusing argument names of do_unaligned_access() functions
On 2016-06-10 19:26, Sergey Sorokin wrote: > There are functions cpu_unaligned_access() and do_unaligned_access() that > are called with access type and mmu index arguments. But these arguments > are named 'is_write' and 'is_user' in their declarations. > The patch fixes the names to avoid a confusion. Unless I missed something, it seems that the is_user/mmu_idx argument is never used. Should we maybe just drop it? Otherwise it looks fine. -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH v2 14/22] hw/intc/arm_gicv3: Implement gicv3_set_irq()
On 2016/5/26 22:55, Peter Maydell wrote: > Implement the code which updates the GIC state when an interrupt > input into the GIC is asserted. > > Signed-off-by: Peter Maydell > --- > hw/intc/arm_gicv3.c| 20 +++- > hw/intc/arm_gicv3_dist.c | 21 + > hw/intc/arm_gicv3_redist.c | 21 + > hw/intc/gicv3_internal.h | 2 ++ > trace-events | 2 ++ > 5 files changed, 65 insertions(+), 1 deletion(-) > > diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c > index e8f6766..e770409 100644 > --- a/hw/intc/arm_gicv3.c > +++ b/hw/intc/arm_gicv3.c > @@ -311,7 +311,25 @@ static void gicv3_set_irq(void *opaque, int irq, int > level) > * [N+32..N+63] : PPI (internal interrupts for CPU 1 > * ... > */ > -/* Do nothing for now */ > +GICv3State *s = opaque; > + > +if (irq < (s->num_irq - GIC_INTERNAL)) { > +/* external interrupt (SPI) */ > +gicv3_dist_set_irq(s, irq + GIC_INTERNAL, level); > +} else { > +/* per-cpu interrupt (PPI) */ > +int cpu; > + > +irq -= (s->num_irq - GIC_INTERNAL); > +cpu = irq / GIC_INTERNAL; > +irq %= GIC_INTERNAL; > +assert(cpu < s->num_cpu); > +/* Raising SGIs via this function would be a bug in how the board > + * model wires up interrupts. > + */ > +assert(irq >= 16 && irq < 32); Nit: Use GIC_NR_SGIS instead and irq < 32 is unnecessary. Reviewed-by: Shannon Zhao -- Shannon
Re: [Qemu-devel] [Xen-devel] [PULL 3/4] xen: add pvUSB backend
On 10/06/16 12:38, Gerd Hoffmann wrote: > On Di, 2016-06-07 at 10:35 +0200, Olaf Hering wrote: >> On Mon, May 23, Gerd Hoffmann wrote: >> >>> +++ b/hw/usb/Makefile.objs >>> +common-obj-$(CONFIG_XEN_BACKEND) += xen-usb.o >>> +++ b/hw/usb/xen-usb.c >>> +usb_bus_new(&usbif->bus, sizeof(usbif->bus), &xen_usb_bus_ops, >>> xen_sysdev); >> >> xen_sysdev is in an i386-only file, as a result qemu fails to link. > > Ping. Juergen? Xen folks? Can anyone have a look please? Sorry, just was 3 weeks offline due to moving across Germany (900 km). > I think the fix would be s/xen_sysdev/xendev/, but I can't test that > myself. I think the proper solution is to move the xen_sysdev definition and initialization from hw/xenpv/xen_machine_pv.c to hw/xen/xen_backend.c I'll send a patch. Juergen
Re: [Qemu-devel] [PATCH v3 1/2] target-i386: KVM: add basic Intel LMCE support
On 06/08/16 13:32, Paolo Bonzini wrote: > > > On 03/06/2016 17:57, Radim Krčmář wrote: > >> > +cenv->msr_ia32_feature_control |= > >> > +MSR_IA32_FEATURE_CONTROL_LMCE | > >> > +MSR_IA32_FEATURE_CONTROL_LOCKED; > > Locking right from the start breaks nested KVM, because nested relies on > > setting VMXON feature from inside of the guest. > > > > Do we keep it unlocked, or move everything into QEMU? > > > > (The latter seems simpler.) > > I think it should be moved into the firmware, with QEMU publishing the > desired setting via fw_cfg. The same as what is done in real hardware, > that's the KVM mantra. :) > > For v4 it's okay to just remove this. > > Paolo Currently, only VMX bits (bit 1 & 2), LMCE bit (bit 20) as well as lock bit (bit 0) in MSR_IA32_FEATURE_CONTROL are used for guest. The availability of features indicated by those bits (except the lock bit) can be discovered from cpuid and other MSR, so it looks not necessary to publish them via fw_cfg. Or do you have other concerns? Thanks, Haozhong
Re: [Qemu-devel] [PATCH v2 16/22] hw/intc/arm_gicv3: Implement gicv3_cpuif_update()
On 2016/5/26 22:55, Peter Maydell wrote: > +void gicv3_cpuif_update(GICv3CPUState *cs) > +{ > +/* Tell the CPU about its highest priority pending interrupt */ > +int irqlevel = 0; > +int fiqlevel = 0; > +ARMCPU *cpu = ARM_CPU(cs->cpu); > +CPUARMState *env = &cpu->env; > + > +trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq, > + cs->hppi.grp, cs->hppi.prio); > + > +if (cs->hppi.grp == GICV3_G1 && !arm_feature(env, ARM_FEATURE_EL3)) { > +/* If a Security-enabled GIC sends a G1S interrupt to a > + * Security-disabled CPU, we must treat it as if it were G0. > + */ > +cs->hppi.grp = GICV3_G0; > +} > + > +if (icc_hppi_can_preempt(cs)) { > +/* We have an interrupt: should we signal it as IRQ or FIQ? > + * This is described in the GICv3 spec section 4.6.2. > + */ > +bool isfiq; > + > +switch (cs->hppi.grp) { > +case GICV3_G0: > +isfiq = true; > +break; > +case GICV3_G1: > +isfiq = (!arm_is_secure(env) || > + (arm_current_el(env) == 3 && arm_el_is_aa64(env, 3))); > +break; > +case GICV3_G1NS: > +isfiq = arm_is_secure(env); > +break; > +default: > +g_assert_not_reached(); > +} > + > +if (isfiq) { > +fiqlevel = 1; > +} else { > +irqlevel = 1; > +} > +} > + > +trace_gicv3_cpuif_set_irqs(gicv3_redist_affid(cs), fiqlevel, irqlevel); > + > +qemu_set_irq(cs->parent_fiq, fiqlevel); > +qemu_set_irq(cs->parent_irq, irqlevel); Does it need to set both fiq and irq for one interrupt? I think it should be called differently based on the value of isfiq. Thanks, -- Shannon
[Qemu-devel] [PATCH] scsi-disk: Use (unsigned long) typecasts when using "%lu" format string
Some source code analyzers like cppcheck spill out a warning if the sign of the argument does not match the format string. Ticket: https://bugs.launchpad.net/qemu/+bug/1589564 Signed-off-by: Thomas Huth --- hw/scsi/scsi-disk.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 1881969..36f8a85 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2060,13 +2060,13 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) } break; case MODE_SELECT: -DPRINTF("Mode Select(6) (len %lu)\n", (long)r->req.cmd.xfer); +DPRINTF("Mode Select(6) (len %lu)\n", (unsigned long)r->req.cmd.xfer); break; case MODE_SELECT_10: -DPRINTF("Mode Select(10) (len %lu)\n", (long)r->req.cmd.xfer); +DPRINTF("Mode Select(10) (len %lu)\n", (unsigned long)r->req.cmd.xfer); break; case UNMAP: -DPRINTF("Unmap (len %lu)\n", (long)r->req.cmd.xfer); +DPRINTF("Unmap (len %lu)\n", (unsigned long)r->req.cmd.xfer); break; case VERIFY_10: case VERIFY_12: @@ -2080,7 +2080,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf) case WRITE_SAME_16: DPRINTF("WRITE SAME %d (len %lu)\n", req->cmd.buf[0] == WRITE_SAME_10 ? 10 : 16, -(long)r->req.cmd.xfer); +(unsigned long)r->req.cmd.xfer); break; default: DPRINTF("Unknown SCSI command (%2.2x=%s)\n", buf[0], -- 1.8.3.1
Re: [Qemu-devel] [PATCH] exec: Fix qemu_ram_block_from_host for Xen
On 09/06/2016 17:56, Anthony PERARD wrote: > Since f615f39 (exec: remove ram_addr argument from > qemu_ram_block_from_host), migration under Xen is likely to fail, with a > SEGV of QEMU. But the commit only reveal a bug with the calculation of > the offset value in qemu_ram_block_from_host(). > > This patch calculates the offset from the ram_addr as > qemu_ram_addr_from_host() will later calculate the ram_addr from the > offset. > > Signed-off-by: Anthony PERARD > --- > exec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/exec.c b/exec.c > index f2c9e37..f13106d 100644 > --- a/exec.c > +++ b/exec.c > @@ -1935,7 +1935,7 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool > round_offset, > ram_addr = xen_ram_addr_from_mapcache(ptr); > block = qemu_get_ram_block(ram_addr); > if (block) { > -*offset = (host - block->host); > +*offset = ram_addr - block->offset; > } > rcu_read_unlock(); > return block; > Acked-by: Paolo Bonzini Feel free to send a pull request yourself! Thanks, and sorry for the breakage. Indeed the broken code comes from commit 422148d3e56c3c9a07c0cf36c1e0a0b76f09c357. Paolo
Re: [Qemu-devel] [PATCH 0/7] s390x: next batch of patches
On 06/08/2016 01:02 PM, Cornelia Huck wrote: > More s390x patches, this time mostly dealing with channel I/O: > Bugfixes and cleanups, and dequeue pending interrupts after > machine checks. > > Christian Borntraeger (3): > virtio-ccw: Provide traces for indicator changes > s390x: Limit s390-ccw machines to 248 CPUs > s390x/kvm: Fixup interrupt type for non-adapter I/O interrupts > > Cornelia Huck (2): > linux-headers: update > s390x/css: introduce property type for device ids > > Halil Pasic (2): > s390x/kvm: add interface for clearing IO irqs > s390x/css: clear IO irqs when generating IPI CRW > > hw/intc/s390_flic.c| 8 ++ > hw/intc/s390_flic_kvm.c| 26 ++ > hw/s390x/css.c | 124 > - > hw/s390x/s390-virtio-ccw.c | 2 +- > hw/s390x/virtio-ccw.c | 80 +++- > hw/s390x/virtio-ccw.h | 2 +- > include/hw/s390x/css.h | 17 > include/hw/s390x/s390_flic.h | 2 + > include/standard-headers/linux/pci_regs.h | 20 +++- > include/standard-headers/linux/virtio_config.h | 2 + > linux-headers/asm-arm/unistd.h | 2 + > linux-headers/asm-arm64/unistd.h | 3 + > linux-headers/asm-powerpc/unistd.h | 2 + > linux-headers/asm-s390/kvm.h | 1 + > linux-headers/asm-s390/unistd.h| 4 +- > linux-headers/asm-x86/kvm.h| 6 +- > linux-headers/asm-x86/unistd_x32.h | 2 + > linux-headers/linux/kvm.h | 1 + > target-s390x/kvm.c | 5 +- > trace-events | 1 + > 20 files changed, 251 insertions(+), 59 deletions(-) > Selection looks good to me.
Re: [Qemu-devel] [PULL 01/31] Add optionrom compatible with fw_cfg DMA version
On 10/06/2016 23:45, Richard W.M. Jones wrote: > Hi Paolo, > > Did this patch get dropped again? It hasn't appeared upstream. Yes, some versions of clang don't support -m16. It needs to be tested. I still want to get it in 2.7, but I have to set aside some time. Paolo
Re: [Qemu-devel] [PATCH v4 17/28] qapi: Factor out JSON number formatting
Eric Blake writes: > On 06/03/2016 03:02 AM, Markus Armbruster wrote: > Suggest: * Return 0 if the number is finite, as required by RFC 7159, else -1. The return value makes some sense only for symmetry with qstring_append_json_string(). Without that, I'd ask you to keep this function simple. Callers could just as easily test isfinite() themselves. >>> >>> I'm actually thinking of modifying this, given the recent thread >>> pointing out that libvirt chokes hard on JSON extensions: >>> >>> https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg04398.html >>> >>> That is, for symmetry with qstring_append_json_string(), I'm thinking of >>> changing NaN to 0 and Inf to DBL_MAX, and always outputting a finite >>> value, in addition to returning -1 to inform the caller that a >>> substitution was made, so that the output is always strict JSON. >> >> Mapping infinities to DBL_MIN and DBL_MAX is debatable, but mapping NaN >> to zero is outright wrong. > > How about this alternative: > > Finite values remain numbers: > "number":1 > > But non-finite values are output as strings, so that our output is > always valid JSON - the recipient may not be expecting a string in place > of a number, but at least should be able to parse the output rather than > choking hard. > "number":"nan" > > The return value -1 then indicates that a stringized replacement was > used, so that any later patch can use a strict flag on whether to allow > the replacement output or assert. I dislike this a lot less than mapping non-finite numbers to finite ones. I still dislike it, because it defeats fitting a schema to QMP: instead of the true JSON type "number", we'd need the sum type of "number" and "string", which this really isn't: only a few special strings are valid, and they're not actually strings. If the schema language can do sum types, we'd even be stuck with their common super-type. The most practical solution isn't always a likable one, though. >> If we decide QMP should stick to JSON here and avoid non-finite numbers, >> we need to treat an attempt to emit a non-finite number as a bug: >> assert(isfinite(...)). Making sure nothing ever attempts to emit such >> numbers will be tedious. >> >> If we decide QMP should remain as it is, we need to document non-finite >> numbers among its JSON extensions. We should also fix our QMP parsers >> to accept non-finite numbers then. Including the one in libvirt. >> Attempts to emit non-finite numbers then *may* be bugs. Really no >> different than finite numbers outside their intended range, such as a >> negative size. Catching these bugs is of course also tedious. The >> difference is that they manifest in QMP as semantic instead of lexical >> errors. Lexical errors are the worst to handle gracefully. > > I may still try to tackle fixing the QMP parser to accept NaN and > infinity on input (since it's hand-written, we at least have control > over that) Making json-lexer.c recognize infinities and NaNs in strtod() syntax shouldn't be hard. I'd omit nan(n-char-sequence-opt), because its semantics are implementation defined. I'd also omit all spellings other than inf and nan. That leaves inf, +inf, -inf, nan, +nan, -nan. >- it will certainly be easier than getting libvirt to parse > non-finite numbers (libvirt uses libyajl, and my emails to the yajl > mailing list have gone unanswered, making me think the project is not > very vibrant and thus not very patchable). Nobody likes to carry downstream patches, but an unresponsive upstream may leave you no choice. > But with my proposal of > producing a stringized non-finite value, we at least convert lexical > into semantic errors, which I agree with your assessment is a nicer way > of dealing with it. > > Of course, a policy change of outputting stringized non-finite numbers > should be separate from refactoring patches that just move functions around. Yes.
Re: [Qemu-devel] [PATCH v4 00/28] Add qapi-to-JSON and clone visitors
Eric Blake writes: > On 06/03/2016 06:09 AM, Markus Armbruster wrote: >> Most patches are ready as is or with minor touch-ups. >> >> I'd probably drop PATCH 01. The file name collision no longer exists, >> and moving files just to improve their names a bit doesn't seem worth >> the bother. > > It's a bit more churn to the rest of the series, but I can live with it. > It's also more related to the JSON visitor than the clone visitor, so > even if I do want to keep it, it makes more sense to keep it with the > second half of the series. Thanks. >> There are memory leaks in PATCH 12 and 24 (see review of PATCH 24). >> >> PATCH 13 needs more work than I'm comfortable to do on commit. Mostly >> missing comment updates, but also a crash bug. > > I've got that fix ready to post. > >> >> On PATCH 21, I have minor interface design doubts. >> >> The only difficult question is what to do about invalid UTF-8 strings >> and non-finite numbers. Patches before PATCH 29 show opinions, but a >> decision isn't made until PATCH 29. Good, because it leaves most of >> this series not blocked by the debate on what we should do. >> >> I guess I could take PATCH 02-12 now, and let you respin the rest. But >> perhaps its easier if you respin all of them. > > I'm posting v5 of 2-15 now, then focusing on your comments on the JSON > visitor for a later posting. Posted as "[PATCH v5 00/15] Add clone visitor". Will review as soon as I can. > I also think my subset F series (adding a 'box' parameter) is relatively > orthogonal, if you want to start reviewing that: > https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg03569.html > > I'd still LOVE to get blockdev-add working for ALL block devices before > 2.7 soft freeze (there's patches for NBD that I need to review, and I > know the gluster folks were waiting on qapi patches to be able to use a > SocketAddress union as the basis for the gluster branch of the > blockdev-add union - and my qapi patches are almost all the way there to > permitting it). I'd very much like to finish this huge task, too. Let's try.
Re: [Qemu-devel] [PATCH 1/2] Fix confusing argument names of do_unaligned_access() functions
On 10/06/2016 19:26, Sergey Sorokin wrote: > cpu-common.h is not included in qom/cpu.h what do you think? Should > it be included? Or may be MMUAccessType should be just moved into > another header. For example into exec/memattrs.h You can move it to qom/cpu.h. Paolo
Re: [Qemu-devel] [PATCH v3 1/2] target-i386: KVM: add basic Intel LMCE support
On 13/06/2016 09:55, Haozhong Zhang wrote: > Currently, only VMX bits (bit 1 & 2), LMCE bit (bit 20) as well as > lock bit (bit 0) in MSR_IA32_FEATURE_CONTROL are used for guest. The > availability of features indicated by those bits (except the lock bit) > can be discovered from cpuid and other MSR, so it looks not necessary > to publish them via fw_cfg. Or do you have other concerns? I would prefer to avoid having to change the firmware (SeaBIOS and OVMF) every time a new bit is added. Using fw_cfg makes it possible to develop the feature in the firmware once and for all. Paolo
Re: [Qemu-devel] [PATCH] net: mipsnet: check transmit buffer size before sending
On 2016-06-02 10:28, Peter Maydell wrote: > On 2 June 2016 at 07:44, P J P wrote: > > From: Prasad J Pandit > > > > When processing MIPSnet I/O port write operation, it uses a > > transmit buffer tx_buffer[MAX_ETH_FRAME_SIZE=1514]. Two indices > > 's->tx_written' and 's->tx_count' are used to control data written > > to this buffer. If the two were to be equal before writing, it'd > > lead to an OOB write access beyond tx_buffer. Add check to avoid it. > > > > Reported-by: Li Qiang > > Signed-off-by: Prasad J Pandit > > --- > > hw/net/mipsnet.c | 10 ++ > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/hw/net/mipsnet.c b/hw/net/mipsnet.c > > index 740cd98..8d5e5bf 100644 > > --- a/hw/net/mipsnet.c > > +++ b/hw/net/mipsnet.c > > @@ -158,7 +158,7 @@ static void mipsnet_ioport_write(void *opaque, hwaddr > > addr, > > trace_mipsnet_write(addr, val); > > switch (addr) { > > case MIPSNET_TX_DATA_COUNT: > > - s->tx_count = (val <= MAX_ETH_FRAME_SIZE) ? val : 0; > > +s->tx_count = (val < MAX_ETH_FRAME_SIZE) ? val : > > MAX_ETH_FRAME_SIZE; > > s->tx_written = 0; > > This is a behaviour change -- the register will now read > back as MAX_ETH_FRAME_SIZE rather than 0 if written with > an overlarge value. > > Do we have any documentation on how this (simulated) > device is supposed to behave in this case? This device is not supported by the linux kernel for more than 2.5 years (since v3.7). Do we want to keep this device in QEMU? Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 09/10] ppc: Move exception generation code out of line
On 06/13/2016 09:44 AM, Thomas Huth wrote: > On 13.06.2016 07:24, Cédric Le Goater wrote: >> From: Benjamin Herrenschmidt >> >> There's no point inlining this, if you hit the exception case you exit >> anyway, and not inlining saves about 100K of code size (and cache >> footprint). >> >> Signed-off-by: Benjamin Herrenschmidt >> --- >> target-ppc/translate.c | 9 ++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c >> index f211d175c09c..600d5db2bb9a 100644 >> --- a/target-ppc/translate.c >> +++ b/target-ppc/translate.c >> @@ -283,7 +283,8 @@ void gen_update_current_nip(void *opaque) >> tcg_gen_movi_tl(cpu_nip, ctx->nip); >> } >> >> -static inline void gen_exception_err(DisasContext *ctx, uint32_t excp, >> uint32_t error) >> +static void __attribute__((noinline)) >> +gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error) >> { >> TCGv_i32 t0, t1; >> if (ctx->exception == POWERPC_EXCP_NONE) { >> @@ -297,7 +298,8 @@ static inline void gen_exception_err(DisasContext *ctx, >> uint32_t excp, uint32_t >> ctx->exception = (excp); >> } >> >> -static inline void gen_exception(DisasContext *ctx, uint32_t excp) >> +static void __attribute__((noinline)) >> +gen_exception(DisasContext *ctx, uint32_t excp) >> { >> TCGv_i32 t0; >> if (ctx->exception == POWERPC_EXCP_NONE) { >> @@ -309,7 +311,8 @@ static inline void gen_exception(DisasContext *ctx, >> uint32_t excp) >> ctx->exception = (excp); >> } >> >> -static inline void gen_debug_exception(DisasContext *ctx) >> +static void __attribute__((noinline)) >> +gen_debug_exception(DisasContext *ctx) >> { >> TCGv_i32 t0; > > Do you get the same results if you just remove the "inline" keyword, > without adding the "__attribute__((noinline))" ? If yes, I'd suggest to > do this patch without the "__attribute__((noinline))" - that's easier to > read, and the compiler can still decide to inline something in case it's > better one a certain architecture. Yes. They are no differences. The interesting part though is that the .text is about the same size. There is even a slight increase of ~2K with gcc 4.9.2 (intel host) and a slight decrease of ~1K with gcc 5.3.1 (ppc64le host). I guess we can just drop that patch. It does not seem to bring much. Thanks, C.
Re: [Qemu-devel] [PATCH 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property
On 10/06/2016 19:52, Lluís Vilanova wrote: > Fair point. But now I feel tempted to change both trace_events_dstate and > trace_events_enabled_count into unsigned int... it burns me when I see signed > types used only on their positives by design. > > But don't worry, I'll change trace_events_dstate into int :) unsigned int would be fine too. Paolo
Re: [Qemu-devel] [PATCH 3/3] KVM: use KVM_CAP_MAX_VCPU_ID
On Fri, 27 May 2016 13:16:49 +0200 Greg Kurz wrote: > On Fri, 27 May 2016 13:58:28 +1000 > David Gibson wrote: > > > On Thu, May 26, 2016 at 10:02:23AM +0200, Greg Kurz wrote: > > > As stated in linux/Documentation/virtual/kvm/api.txt: > > > > > > The maximum possible value for max_vcpu_id can be retrieved using the > > > KVM_CAP_MAX_VCPU_ID of the KVM_CHECK_EXTENSION ioctl() at run-time. > > > > > > If the KVM_CAP_MAX_VCPU_ID does not exist, you should assume that > > > max_vcpu_id is the same as the value returned from KVM_CAP_MAX_VCPUS. > > > > > > Signed-off-by: Greg Kurz > > > > Reviewed-by: David Gibson > > > > Paolo, > > FYI Patch 1/3 was in David's pull request earlier today. It is now upstream. > > You may now apply 2/3 and 3/3. > > Thanks ! > Hi Paolo, Just a friendly reminder. Without this patch, we cannot create more than 128 single-threaded vCPUs on POWER8 if the host has unsplit cores. Thanks ! -- Greg > -- > Greg > > > > --- > > > kvm-all.c |8 +++- > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/kvm-all.c b/kvm-all.c > > > index e56f38527815..e74e0c6e2352 100644 > > > --- a/kvm-all.c > > > +++ b/kvm-all.c > > > @@ -1459,10 +1459,16 @@ static int kvm_max_vcpus(KVMState *s) > > > return (ret) ? ret : kvm_recommended_vcpus(s); > > > } > > > > > > +static int kvm_max_vcpu_id(KVMState *s) > > > +{ > > > +int ret = kvm_check_extension(s, KVM_CAP_MAX_VCPU_ID); > > > +return (ret) ? ret : kvm_max_vcpus(s); > > > +} > > > + > > > bool kvm_vcpu_id_is_valid(int vcpu_id) > > > { > > > KVMState *s = KVM_STATE(current_machine->accelerator); > > > -return vcpu_id >= 0 && vcpu_id < kvm_max_vcpus(s); > > > +return vcpu_id >= 0 && vcpu_id < kvm_max_vcpu_id(s); > > > } > > > > > > static int kvm_init(MachineState *ms) > > > > > > >
Re: [Qemu-devel] [PATCH v8 16/17] vmxnet3: remove unnecessary internal msi state flag
Cao jin writes: > Internal flag msi_used is unnecessary, it has the same effect as > msi_enabled(). > msi_uninit() could be called directly without risk. > > cc: Dmitry Fleytman > cc: Jason Wang > cc: Markus Armbruster > cc: Marcel Apfelbaum > cc: Michael S. Tsirkin > > Signed-off-by: Cao jin > --- > hw/net/vmxnet3.c | 15 +-- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index 63f8904..3ed4335 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -280,8 +280,6 @@ typedef struct { > > /* Whether MSI-X support was installed successfully */ > bool msix_used; > -/* Whether MSI support was installed successfully */ > -bool msi_used; > hwaddr drv_shmem; > hwaddr temp_shared_guest_driver_memory; > > @@ -363,7 +361,7 @@ static bool _vmxnet3_assert_interrupt_line(VMXNET3State > *s, uint32_t int_idx) > msix_notify(d, int_idx); > return false; > } > -if (s->msi_used && msi_enabled(d)) { > +if (msi_enabled(d)) { > VMW_IRPRN("Sending MSI notification for vector %u", int_idx); > msi_notify(d, int_idx); > return false; > @@ -387,7 +385,7 @@ static void _vmxnet3_deassert_interrupt_line(VMXNET3State > *s, int lidx) > * This function should never be called for MSI(X) interrupts > * because deassertion never required for message interrupts > */ > -assert(!s->msi_used || !msi_enabled(d)); > +assert(!msi_enabled(d)); > > VMW_IRPRN("Deasserting line for interrupt %u", lidx); > pci_irq_deassert(d); > @@ -424,7 +422,7 @@ static void vmxnet3_trigger_interrupt(VMXNET3State *s, > int lidx) > goto do_automask; > } > > -if (s->msi_used && msi_enabled(d) && s->auto_int_masking) { > +if (msi_enabled(d) && s->auto_int_masking) { > goto do_automask; > } > > @@ -1409,7 +1407,7 @@ static void vmxnet3_update_features(VMXNET3State *s) > > static bool vmxnet3_verify_intx(VMXNET3State *s, int intx) > { > -return s->msix_used || s->msi_used || (intx == > +return s->msix_used || msi_enabled(PCI_DEVICE(s)) || (intx == > (pci_get_byte(s->parent_obj.config + PCI_INTERRUPT_PIN) - 1)); If you need to respin for some other reason, you could clean up the distasteful line break here, and drop the superfluous parenthesis: return s->msix_used || msi_enabled(PCI_DEVICE(s)) || intx == pci_get_byte(s->parent_obj.config + PCI_INTERRUPT_PIN) - 1; > } > > @@ -2202,9 +2200,7 @@ vmxnet3_cleanup_msi(VMXNET3State *s) > { > PCIDevice *d = PCI_DEVICE(s); > > -if (s->msi_used) { > -msi_uninit(d); > -} > +msi_uninit(d); > } > > static void > @@ -2295,7 +2291,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, > Error **errp) > /* Any error other than -ENOTSUP(board's MSI support is broken) > * is a programming error. Fall back to INTx silently on -ENOTSUP */ > assert(!ret || ret == -ENOTSUP); > -s->msi_used = !ret; > > if (!vmxnet3_init_msix(s)) { > VMW_WRPRN("Failed to initialize MSI-X, configuration is > inconsistent.");
Re: [Qemu-devel] [PATCH v8 00/17] Add param Error ** for msi_init()
Cao jin writes: > v8 changelog: > 1. address all stylistic remarks (Markus) > 2. add two new patches(16&17) which I missed in last round, shoot the > unnecessary >internal msi flag. > 3. rebase on the upstream, fix trivial conflict in vmxnet3: > -#include "vmxnet_tx_pkt.h" > -#include "vmxnet_rx_pkt.h" > +#include "net_tx_pkt.h" > +#include "net_rx_pkt.h" > 4. There is a new device "e1000e" added, need to cover it in patch 12. > > Hi Markus, I add your R-b in first 15 patches, maybe you want to take a glance > at e1000e part in patch 12. > > The last two new patches need some review. > Will take care of msix flag cleanup when tackle msix_init(). Series Reviewed-by: Markus Armbruster
[Qemu-devel] [Bug 1568589] Re: Compile for os x host failed
According to the discussion for the patch that you've mentioned, the problem should go away after a "make distclean". Can you still reproduce it after doing a fresh build? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1568589 Title: Compile for os x host failed Status in QEMU: New Bug description: Hello QEMU, I try compile qemu from git pulled by me today and have a troubles: GEN trace/generated-helpers.c CCaarch64-softmmu/trace/generated-helpers.o LINK aarch64-softmmu/qemu-system-aarch64 Undefined symbols for architecture x86_64: "_event_notifier_init_fd", referenced from: _process_msg in ivshmem.o ld: symbol(s) not found for architecture x86_64 clang: error: linker command failed with exit code 1 (use -v to see invocation) make[1]: *** [qemu-system-aarch64] Error 1 make: *** [subdir-aarch64-softmmu] Error 2 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1568589/+subscriptions
Re: [Qemu-devel] [RFC Patch 0/3] Accept passed in socket 'fd' open from outside for unix socket
On 09.06.2016 11:16, Daniel P. Berrange wrote: > On Wed, Jun 08, 2016 at 05:48:57PM -0400, Aaron Conole wrote: >> Flavio Leitner writes: >> >>> Adding Aaron who is fixing exactly that on the OVS side. >>> >>> Aaron, please see the last question in the bottom of this email. >>> >>> On Wed, Jun 08, 2016 at 06:07:29AM -0400, Amnon Ilan wrote: - Original Message - > From: "Michal Privoznik" > To: "Daniel P. Berrange" > Cc: qemu-devel@nongnu.org, "amit shah" , > jasow...@redhat.com, "Wei Xu" , > arm...@redhat.com > Sent: Thursday, June 2, 2016 2:38:53 PM > Subject: Re: [Qemu-devel] [RFC Patch 0/3] Accept passed in socket > 'fd' open from outside for unix socket > > On 02.06.2016 10:29, Daniel P. Berrange wrote: >> On Thu, Jun 02, 2016 at 09:41:56AM +0200, Michal Privoznik wrote: >>> On 01.06.2016 18:16, Wei Xu wrote: On 2016年06月01日 00:44, Daniel P. Berrange wrote: > On Wed, Jun 01, 2016 at 12:30:44AM +0800, w...@redhat.com wrote: >> From: Wei Xu >> >> Recently I'm working on a fd passing issue, selinux forbids qemu to >> create a unix socket for a chardev when managing VMs with libvirt, >> because qemu don't have sufficient permissions in this case, and >> proposal from libvirt team is opening the 'fd' in libvirt and merely >> passing it to qemu. > > This sounds like a bug in libvirt, or selinux, or a mistaken > configuration > of the guest. It is entirely possible for QEMU to create a unix socket > - not > least because that is exactly what QEMU uses for its QMP monitor > backend. > Looking at your example command line, I think the issue is simply that > you > should be putting the sockets in a different location. ie at > /var/lib/libvirt/qemu/$guest-vhost-user1.sock where QEMU has > permission to > create sockets already. ah.. adjusting permission or file location can solve this problem, i'm guessing maybe this is a more security concern, the socket is used as a network interface for a vm, similar as the qcow image file, thus should prevent it to be arbitrarily accessed. Michael, do you have any comment on this? >>> >>> I haven't seen the patches. But in libvirt we allow users to create a >>> vhostuser interface and even specify where the socket should be placed: >>> >>> >>> >>> >>> >>> >>> >>> The following cmd line is generated by libvirt then: >>> >>> -chardev socket,id=charnet1,path=/tmp/vhost1.sock,server \ >>> -netdev type=vhost-user,id=hostnet1,chardev=charnet1 \ >>> -device >>> virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:ee:96:6c,bus=pci.0,\ >>> >>> Now, if we accept only /var/run/openvwitch path in >>> /interface/source/@path (or whatever path to OVS is), we don't need this >>> and have users manually label the dir (unless already labeled). But >>> since we accept just any path in there, we should make sure that qemu is >>> then able to create the socket. One possible fix would be to allow qemu >>> create sockets just anywhere in the system. This, however, brings huge >>> security risks and it's not acceptable IMO. The other option would be >>> that libvirt would create the socket, and pass its FD to qemu (since >>> libvirt already is allowed to create sockets anywhere). >> >> There are plenty of other places where we allow arbitrary paths in the >> XML, but which have restrictions imposed by the security drivers. Not >> least the devices which have the exact same scenario as this >> network device, and require use of /var/lib/libvirt/qemu as the directory >> for the sockets. We certainly do not want to allow QEMU to create sockets >> anywhere. >> >> I don't think we want to grant QEMU svirtt permission to create sockets >> in the /var/run/openvswitch directory either really.IMHO, users of vhost >> user should really be using /var/lib/libvirt/qemu, as is used for all >> other UNIX sockets we create wrt other devices. > > Okay. I can live with that; but in that case we should document it > somewhere, that we guarantee only paths under /var/lib/libvirt/ to be > accessible and for the rest we do our best but maybe require sys admin > intervention (e.g. to label the whole tree for a non-standard location). Does OVS has some limit for it's sockets to be only in /var/run/openvswitch ? >> >> As of a recent commit, it can only be in /var/run/openvswitch or a >> subdirectory therein (found in the openvswitch database). Well, this changes game rules for libvirt. The documentation I've suggested above won't any good. Therefore I think we need to be able to have libvirt opening the socke
[Qemu-devel] [Bug 1553999] Re: OpenGL support is disabled
Since you're talking about a pre-compiled binary, I assume you wanted to open this bug against Ubuntu's QEMU package, not against the QEMU project? ** Project changed: qemu => qemu (Ubuntu) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1553999 Title: OpenGL support is disabled Status in qemu package in Ubuntu: New Bug description: $ qemu-system-x86_64 -enable-kvm -display sdl,gl=on -vga qxl SDL1 display code has no opengl support. Please recompile qemu with SDL2, using ./configure --enable-sdl --with-sdlabi=2.0 qemu-system-x86_64: OpenGL support is disabled Can you please recompile qemu with support for opengl. The -display mode allows for opengl support. To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1553999/+subscriptions
Re: [Qemu-devel] [PATCH] vl: Error messages need to go to stderr, fix some
Paolo, will you pick this up, or do you want me to take it through my (currently empty) error-next branch? Markus Armbruster writes: > We print a few fatal error messages to stdout instead of stderr. > Reproducer: > > $ qemu-system-x86_64 -g 1024x768 > Option g not supported for this target > $ qemu-system-x86_64 -g 1024x768 >/dev/null > > Fix by printing them with error_report(). This also improves the messages. > The above one becomes > > qemu-system-x86_64: -g 1024x768: Option not supported for this target > > Reported-by: Tobi {github.com/tobimensch} > Signed-off-by: Markus Armbruster > --- > vl.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/vl.c b/vl.c > index 18d1423..7800b31 100644 > --- a/vl.c > +++ b/vl.c > @@ -3075,7 +3075,7 @@ int main(int argc, char **argv, char **envp) > > popt = lookup_opt(argc, argv, &optarg, &optind); > if (!(popt->arch_mask & arch_type)) { > -printf("Option %s not supported for this target\n", > popt->name); > +error_report("Option not supported for this target"); > exit(1); > } > switch(popt->index) { > @@ -3849,21 +3849,21 @@ int main(int argc, char **argv, char **envp) > break; > case QEMU_OPTION_xen_domid: > if (!(xen_available())) { > -printf("Option %s not supported for this target\n", > popt->name); > +error_report("Option not supported for this target"); > exit(1); > } > xen_domid = atoi(optarg); > break; > case QEMU_OPTION_xen_create: > if (!(xen_available())) { > -printf("Option %s not supported for this target\n", > popt->name); > +error_report("Option not supported for this target"); > exit(1); > } > xen_mode = XEN_CREATE; > break; > case QEMU_OPTION_xen_attach: > if (!(xen_available())) { > -printf("Option %s not supported for this target\n", > popt->name); > +error_report("Option not supported for this target"); > exit(1); > } > xen_mode = XEN_ATTACH;
Re: [Qemu-devel] [PATCH 3/6] [trivial] trace: Cosmetic changes on fast-path tracing
On 25/02/2016 16:03, Lluís Vilanova wrote: > static inline bool trace_event_get_state_dynamic(TraceEvent *ev) > { > -int id = trace_event_get_id(ev); > +TraceEventID id; > +assert(ev != NULL); Please don't add "!= NULL" asserts. The reason of a crash would be pretty obvious from the backtrace. In fact a NULL event is especially unlikely given that all events are held in an array... Paolo > +assert(trace_event_get_state_static(ev)); > +id = trace_event_get_id(ev); > return trace_event_get_state_dynamic_by_id(id); > } >
[Qemu-devel] [Bug 1591611] Re: chroot using qemu-x86_64-static fails on ppc64el
I don't know of any plans to do so. They would not be difficult to implement (500 lines of code or so at most I guess), but on the other hand they've been unimplemented for some years. They fall into the category of "nobody who wants them has cared enough to write the code yet", I'm afraid. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1591611 Title: chroot using qemu-x86_64-static fails on ppc64el Status in QEMU: New Bug description: When attempting to use qemu-x86_64-static from qemu 2.5.0 on a ppc64el host to chroot into an amd64 environment, all commands fail with an assertion error. /usr/bin/qemu-x86_64-static from the host was copied into the chroot /usr/bin, and the host has multiformat support in the kernel. Sample output illustrating the problem, as well as bash builtins working: # chroot /virtualbox/scratchdisks_local_001/amd64_chroot qemu-x86_64-static /bin/bash # ls bash: ../sysdeps/nptl/fork.c:136: __libc_fork: Assertion `({ __typeof (self->tid) __value; if (sizeof (__value) == 1) asm volatile ("movb %%fs:%P2,%b0" : "=q" (__value) : "0" (0), "i" (__builtin_offsetof (struct pthread, tid))); else if (sizeof (__value) == 4) asm volatile ("movl %%fs:%P1,%0" : "=r" (__value) : "i" (__builtin_offsetof (struct pthread, tid))); else { if (sizeof (__value) != 8) abort (); asm volatile ("movq %%fs:%P1,%q0" : "=r" (__value) : "i" (__builtin_offsetof (struct pthread, tid))); } __value; }) != ppid' failed. setup_frame: not implemented setup_frame: not implemented qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segmentation fault setup_frame: not implemented setup_frame: not implemented # echo TEST TEST # cat test bash: ../sysdeps/nptl/fork.c:136: __libc_fork: Assertion `({ __typeof (self->tid) __value; if (sizeof (__value) == 1) asm volatile ("movb %%fs:%P2,%b0" : "=q" (__value) : "0" (0), "i" (__builtin_offsetof (struct pthread, tid))); else if (sizeof (__value) == 4) asm volatile ("movl %%fs:%P1,%0" : "=r" (__value) : "i" (__builtin_offsetof (struct pthread, tid))); else { if (sizeof (__value) != 8) abort (); asm volatile ("movq %%fs:%P1,%q0" : "=r" (__value) : "i" (__builtin_offsetof (struct pthread, tid))); } __value; }) != ppid' failed. qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segmentation fault It is currently unknown if other host architectures (e.g. aarch64) are also affected. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1591611/+subscriptions
Re: [Qemu-devel] [PATCH v2 11/22] hw/intc/arm_gicv3: Implement GICv3 distributor registers
On 13 June 2016 at 07:27, Shannon Zhao wrote: > > > On 2016/5/26 22:55, Peter Maydell wrote: >> +static uint8_t gicd_read_ipriorityr(GICv3State *s, MemTxAttrs attrs, int >> irq) >> +{ >> +/* Read the value of GICD_IPRIORITYR for the specified interrupt, >> + * honouring security state (these are RAZ/WI for Group 0 or Secure >> + * Group 1 interrupts). >> + */ >> +uint32_t prio; >> + >> +if (irq < GIC_INTERNAL || irq >= s->num_irq) { >> +return 0; >> +} >> + >> +prio = s->gicd_ipriority[irq]; >> + >> +if (!attrs.secure && !(s->gicd_ctlr & GICD_CTLR_DS)) { >> +if (!gicv3_gicd_group_test(s, irq)) { >> +/* Fields for Group 0 or Secure Group 1 interrupts are RAZ/WI */ > Here this check assure this interrupt belongs to Group 0 and NS access > is not permitted, so it should return 0. But it doesn't say anything > about Secure Group 1. We're testing the GICD_IGROUPR bit here. If DS is zero (security enabled), then IGROUPR == 0 means "Group 0 or Secure Group 1", which is what the comment says we're testing. (If you care which of 0 and S1 it is then you look at IGRPMODR, but for security checks like these we don't need to.) >> +return 0; >> +} >> +/* NS view of the interrupt priority */ >> +prio = (prio << 1) & 0xff; >> +} > So maybe here it should check if attrs.secure is true and the Group is > 1, then return 0. I'm confused. If attrs.secure is true this is a secure access which should be allowed to see anything, shouldn't it? >> +return prio; >> +} thanks -- PMM
Re: [Qemu-devel] [PATCH v2 14/22] hw/intc/arm_gicv3: Implement gicv3_set_irq()
On 13 June 2016 at 08:49, Shannon Zhao wrote: > > > On 2016/5/26 22:55, Peter Maydell wrote: >> Implement the code which updates the GIC state when an interrupt >> input into the GIC is asserted. >> >> Signed-off-by: Peter Maydell >> --- >> hw/intc/arm_gicv3.c| 20 +++- >> hw/intc/arm_gicv3_dist.c | 21 + >> hw/intc/arm_gicv3_redist.c | 21 + >> hw/intc/gicv3_internal.h | 2 ++ >> trace-events | 2 ++ >> 5 files changed, 65 insertions(+), 1 deletion(-) >> >> diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c >> index e8f6766..e770409 100644 >> --- a/hw/intc/arm_gicv3.c >> +++ b/hw/intc/arm_gicv3.c >> @@ -311,7 +311,25 @@ static void gicv3_set_irq(void *opaque, int irq, int >> level) >> * [N+32..N+63] : PPI (internal interrupts for CPU 1 >> * ... >> */ >> -/* Do nothing for now */ >> +GICv3State *s = opaque; >> + >> +if (irq < (s->num_irq - GIC_INTERNAL)) { >> +/* external interrupt (SPI) */ >> +gicv3_dist_set_irq(s, irq + GIC_INTERNAL, level); >> +} else { >> +/* per-cpu interrupt (PPI) */ >> +int cpu; >> + >> +irq -= (s->num_irq - GIC_INTERNAL); >> +cpu = irq / GIC_INTERNAL; >> +irq %= GIC_INTERNAL; >> +assert(cpu < s->num_cpu); >> +/* Raising SGIs via this function would be a bug in how the board >> + * model wires up interrupts. >> + */ >> +assert(irq >= 16 && irq < 32); > Nit: Use GIC_NR_SGIS instead and irq < 32 is unnecessary. Fixed, thanks. -- PMM
[Qemu-devel] [RFC PATCH v1 2/4] exec.c: Remove static allocation of sub_section of sub_page
From: Vijaya Kumar K Allocate sub_section dynamically. Remove dependency on TARGET_PAGE_SIZE to make run-time page size detection for arm platforms. Signed-off-by: Vijaya Kumar K --- exec.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/exec.c b/exec.c index a9d465b..e803a41 100644 --- a/exec.c +++ b/exec.c @@ -154,7 +154,7 @@ typedef struct subpage_t { MemoryRegion iomem; AddressSpace *as; hwaddr base; -uint16_t sub_section[TARGET_PAGE_SIZE]; +uint16_t *sub_section; } subpage_t; #define PHYS_SECTION_UNASSIGNED 0 @@ -1151,6 +1151,7 @@ static void phys_section_destroy(MemoryRegion *mr) if (have_sub_page) { subpage_t *subpage = container_of(mr, subpage_t, iomem); object_unref(OBJECT(&subpage->iomem)); +g_free(subpage->sub_section); g_free(subpage); } } @@ -2272,7 +2273,7 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr base) subpage_t *mmio; mmio = g_malloc0(sizeof(subpage_t)); - +mmio->sub_section = g_malloc0(TARGET_PAGE_SIZE * sizeof(uint16_t)); mmio->as = as; mmio->base = base; memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio, -- 1.7.9.5
Re: [Qemu-devel] [PATCH v2 16/22] hw/intc/arm_gicv3: Implement gicv3_cpuif_update()
On 13 June 2016 at 08:56, Shannon Zhao wrote: > > > On 2016/5/26 22:55, Peter Maydell wrote: >> +void gicv3_cpuif_update(GICv3CPUState *cs) >> +{ >> +/* Tell the CPU about its highest priority pending interrupt */ >> +int irqlevel = 0; >> +int fiqlevel = 0; >> +ARMCPU *cpu = ARM_CPU(cs->cpu); >> +CPUARMState *env = &cpu->env; >> + >> +trace_gicv3_cpuif_update(gicv3_redist_affid(cs), cs->hppi.irq, >> + cs->hppi.grp, cs->hppi.prio); >> + >> +if (cs->hppi.grp == GICV3_G1 && !arm_feature(env, ARM_FEATURE_EL3)) { >> +/* If a Security-enabled GIC sends a G1S interrupt to a >> + * Security-disabled CPU, we must treat it as if it were G0. >> + */ >> +cs->hppi.grp = GICV3_G0; >> +} >> + >> +if (icc_hppi_can_preempt(cs)) { >> +/* We have an interrupt: should we signal it as IRQ or FIQ? >> + * This is described in the GICv3 spec section 4.6.2. >> + */ >> +bool isfiq; >> + >> +switch (cs->hppi.grp) { >> +case GICV3_G0: >> +isfiq = true; >> +break; >> +case GICV3_G1: >> +isfiq = (!arm_is_secure(env) || >> + (arm_current_el(env) == 3 && arm_el_is_aa64(env, 3))); >> +break; >> +case GICV3_G1NS: >> +isfiq = arm_is_secure(env); >> +break; >> +default: >> +g_assert_not_reached(); >> +} >> + >> +if (isfiq) { >> +fiqlevel = 1; >> +} else { >> +irqlevel = 1; >> +} >> +} >> + >> +trace_gicv3_cpuif_set_irqs(gicv3_redist_affid(cs), fiqlevel, irqlevel); >> + >> +qemu_set_irq(cs->parent_fiq, fiqlevel); >> +qemu_set_irq(cs->parent_irq, irqlevel); > Does it need to set both fiq and irq for one interrupt? I think it > should be called differently based on the value of isfiq. If the state of the CPU changes then an asserted interrupt may switch from "must assert FIQ" to "must assert IRQ". This means we need to call qemu_set_irq() for both (in one case with a 0 level and in the other case with a 1 level); we can't just ignore the IRQ/FIQ line which doesn't apply to this interrupt. There's also the case of "icc_hppi_can_preempt() returns false" where we need to make both calls with a 0 level, so we can't simplify to one call with isfiq and one call with !isfiq. thanks -- PMM
Re: [Qemu-devel] [PATCH v8 00/17] Add param Error ** for msi_init()
On 06/13/2016 04:48 PM, Markus Armbruster wrote: Cao jin writes: v8 changelog: 1. address all stylistic remarks (Markus) 2. add two new patches(16&17) which I missed in last round, shoot the unnecessary internal msi flag. 3. rebase on the upstream, fix trivial conflict in vmxnet3: -#include "vmxnet_tx_pkt.h" -#include "vmxnet_rx_pkt.h" +#include "net_tx_pkt.h" +#include "net_rx_pkt.h" 4. There is a new device "e1000e" added, need to cover it in patch 12. Hi Markus, I add your R-b in first 15 patches, maybe you want to take a glance at e1000e part in patch 12. The last two new patches need some review. Will take care of msix flag cleanup when tackle msix_init(). Series Reviewed-by: Markus Armbruster Thanks very much. I will send a single patch 16 in-reply-to the current in the series, to fix the stylistic issue. -- Yours Sincerely, Cao jin
[Qemu-devel] [PATCH] xen: move xen_sysdev to xen_backend.c
Commit 9432e53a5bc88681b2d3aec4dac9db07c5476d1b added xen_sysdev as a system device to serve as an anchor for removable virtual buses. This introduced a build failure for non-x86 builds with CONFIG_XEN_BACKEND set, as xen_sysdev was defined in a x86 specific file while being consumed in an architecture independent source. Move the xen_sysdev definition and initialization to xen_backend.c to avoid the build failure. Signed-off-by: Juergen Gross --- hw/xen/xen_backend.c | 41 + hw/xenpv/xen_machine_pv.c | 40 2 files changed, 41 insertions(+), 40 deletions(-) diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c index c63f9df..6e52474 100644 --- a/hw/xen/xen_backend.c +++ b/hw/xen/xen_backend.c @@ -27,12 +27,17 @@ #include #include "hw/hw.h" +#include "hw/sysbus.h" #include "sysemu/char.h" #include "qemu/log.h" #include "hw/xen/xen_backend.h" #include +#define TYPE_XENSYSDEV "xensysdev" + +DeviceState *xen_sysdev; + /* - */ /* public */ @@ -763,6 +768,10 @@ int xen_be_init(void) /* Check if xen_init() have been called */ goto err; } + +xen_sysdev = qdev_create(NULL, TYPE_XENSYSDEV); +qdev_init_nofail(xen_sysdev); + return 0; err: @@ -863,3 +872,35 @@ void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, ... } qemu_log_flush(); } + +static int xen_sysdev_init(SysBusDevice *dev) +{ +return 0; +} + +static Property xen_sysdev_properties[] = { +{/* end of property list */}, +}; + +static void xen_sysdev_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); + +k->init = xen_sysdev_init; +dc->props = xen_sysdev_properties; +} + +static const TypeInfo xensysdev_info = { +.name = TYPE_XENSYSDEV, +.parent= TYPE_SYS_BUS_DEVICE, +.instance_size = sizeof(SysBusDevice), +.class_init= xen_sysdev_class_init, +}; + +static void xenbe_register_types(void) +{ +type_register_static(&xensysdev_info); +} + +type_init(xenbe_register_types); diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c index f68cf48..48f725c 100644 --- a/hw/xenpv/xen_machine_pv.c +++ b/hw/xenpv/xen_machine_pv.c @@ -25,15 +25,10 @@ #include "qemu/osdep.h" #include "hw/hw.h" #include "hw/boards.h" -#include "hw/sysbus.h" #include "hw/xen/xen_backend.h" #include "xen_domainbuild.h" #include "sysemu/block-backend.h" -#define TYPE_XENSYSDEV "xensysdev" - -DeviceState *xen_sysdev; - static void xen_init_pv(MachineState *machine) { DriveInfo *dinfo; @@ -72,9 +67,6 @@ static void xen_init_pv(MachineState *machine) break; } -xen_sysdev = qdev_create(NULL, TYPE_XENSYSDEV); -qdev_init_nofail(xen_sysdev); - xen_be_register("console", &xen_console_ops); xen_be_register("vkbd", &xen_kbdmouse_ops); xen_be_register("vfb", &xen_framebuffer_ops); @@ -112,38 +104,6 @@ static void xen_init_pv(MachineState *machine) xen_init_display(xen_domid); } -static int xen_sysdev_init(SysBusDevice *dev) -{ -return 0; -} - -static Property xen_sysdev_properties[] = { -{/* end of property list */}, -}; - -static void xen_sysdev_class_init(ObjectClass *klass, void *data) -{ -DeviceClass *dc = DEVICE_CLASS(klass); -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - -k->init = xen_sysdev_init; -dc->props = xen_sysdev_properties; -} - -static const TypeInfo xensysdev_info = { -.name = TYPE_XENSYSDEV, -.parent= TYPE_SYS_BUS_DEVICE, -.instance_size = sizeof(SysBusDevice), -.class_init= xen_sysdev_class_init, -}; - -static void xenpv_register_types(void) -{ -type_register_static(&xensysdev_info); -} - -type_init(xenpv_register_types); - static void xenpv_machine_init(MachineClass *mc) { mc->desc = "Xen Para-virtualized PC"; -- 2.6.6
Re: [Qemu-devel] [PATCH] vl: Error messages need to go to stderr, fix some
On 13/06/2016 11:03, Markus Armbruster wrote: > Paolo, will you pick this up, or do you want me to take it through my > (currently empty) error-next branch? I can pick it up. Paolo > Markus Armbruster writes: > >> We print a few fatal error messages to stdout instead of stderr. >> Reproducer: >> >> $ qemu-system-x86_64 -g 1024x768 >> Option g not supported for this target >> $ qemu-system-x86_64 -g 1024x768 >/dev/null >> >> Fix by printing them with error_report(). This also improves the messages. >> The above one becomes >> >> qemu-system-x86_64: -g 1024x768: Option not supported for this target >> >> Reported-by: Tobi {github.com/tobimensch} >> Signed-off-by: Markus Armbruster >> --- >> vl.c | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index 18d1423..7800b31 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -3075,7 +3075,7 @@ int main(int argc, char **argv, char **envp) >> >> popt = lookup_opt(argc, argv, &optarg, &optind); >> if (!(popt->arch_mask & arch_type)) { >> -printf("Option %s not supported for this target\n", >> popt->name); >> +error_report("Option not supported for this target"); >> exit(1); >> } >> switch(popt->index) { >> @@ -3849,21 +3849,21 @@ int main(int argc, char **argv, char **envp) >> break; >> case QEMU_OPTION_xen_domid: >> if (!(xen_available())) { >> -printf("Option %s not supported for this target\n", >> popt->name); >> +error_report("Option not supported for this target"); >> exit(1); >> } >> xen_domid = atoi(optarg); >> break; >> case QEMU_OPTION_xen_create: >> if (!(xen_available())) { >> -printf("Option %s not supported for this target\n", >> popt->name); >> +error_report("Option not supported for this target"); >> exit(1); >> } >> xen_mode = XEN_CREATE; >> break; >> case QEMU_OPTION_xen_attach: >> if (!(xen_available())) { >> -printf("Option %s not supported for this target\n", >> popt->name); >> +error_report("Option not supported for this target"); >> exit(1); >> } >> xen_mode = XEN_ATTACH;
Re: [Qemu-devel] [PATCH 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property
First of all, a generic problem I see with your patches is that the newly-introduced APIs are not providing a good abstraction. If something is only used internally, as is the case for trace_event_get_cpu_id, you don't need accessors. On the other hand, when you have a repeated expression such as trace_event_get_cpu_id(ev) != trace_event_cpu_count() then you need an API such as trace_event_is_vcpu(ev). Another small ugliness is that you are using "vcpu" in trace-events and in the generated files, but "cpu" in the C file. My suggestion is to prefix functions with vcpu_trace_event if they refer to per-VCPU trace events, and only use the VCPU ids in those functions. On 25/02/2016 16:03, Lluís Vilanova wrote: > +static inline bool trace_event_get_cpu_state_dynamic(CPUState *cpu, > + TraceEvent *ev) > { > -int id = trace_event_get_id(ev); > +TraceEventVCPUID id; > +assert(cpu != NULL); > assert(ev != NULL); Please do not add more "!= NULL" asserts. In fact, we should remove the others; in this case the ev != NULL assertion is particularly pointless since it comes after a dereference. > assert(trace_event_get_state_static(ev)); > -trace_events_enabled_count += state - trace_events_dstate[id]; > -trace_events_dstate[id] = state; > +assert(trace_event_get_cpu_id(ev) != trace_event_cpu_count()); > +id = trace_event_get_cpu_id(ev); > +return trace_event_get_cpu_state_dynamic_by_cpu_id(cpu, id); Based on the above suggestion regarding APIs: assert(trace_event_is_vcpu(ev)); return vcpu_trace_event_get_state_dynamic(cpu, ev->cpu_id); > } > > #endif /* TRACE__CONTROL_INTERNAL_H */ > diff --git a/trace/control-stub.c b/trace/control-stub.c > new file mode 100644 > index 000..858b13e > --- /dev/null > +++ b/trace/control-stub.c > @@ -0,0 +1,29 @@ > +/* > + * Interface for configuring and controlling the state of tracing events. > + * > + * Copyright (C) 2014-2016 Lluís Vilanova > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#include "qemu/osdep.h" > +#include "trace/control.h" This is not a stub, in fact it has a bunch of duplicate code with trace/control.c. The actual stubs are trace_event_set_cpu_state_dynamic() (which I'd rename to vcpu_trace_event_set_state_dynamic) and vcpu_trace_event_set_state_dynamic_all that does a CPU_FOREACH. That said, I am skeptical about the benefit of the interfaces you are adding. They add a lot of complication and overhead (especially regarding the memory/cache overhead of the dstate array) without a clear use case, in my opinion; all the processing you do at run-time is just as well suited for later filtering. I also believe that it's a bad idea to add "stuff" to trace-tool without a user; unless I'm mistaken neither "vcpu" nor "tcg" trace events are unused in qemu.git, and this means that the ~400 lines added in this series are actually dead code. Paolo
Re: [Qemu-devel] [Xen-devel] [PATCH] xen: move xen_sysdev to xen_backend.c
On 13/06/16 11:12, Juergen Gross wrote: > Commit 9432e53a5bc88681b2d3aec4dac9db07c5476d1b added xen_sysdev as a > system device to serve as an anchor for removable virtual buses. This > introduced a build failure for non-x86 builds with CONFIG_XEN_BACKEND > set, as xen_sysdev was defined in a x86 specific file while being > consumed in an architecture independent source. > > Move the xen_sysdev definition and initialization to xen_backend.c to > avoid the build failure. > > Signed-off-by: Juergen Gross Sorry, used old mail address of Stefano. Corrected. Juergen > --- > hw/xen/xen_backend.c | 41 + > hw/xenpv/xen_machine_pv.c | 40 > 2 files changed, 41 insertions(+), 40 deletions(-) > > diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c > index c63f9df..6e52474 100644 > --- a/hw/xen/xen_backend.c > +++ b/hw/xen/xen_backend.c > @@ -27,12 +27,17 @@ > #include > > #include "hw/hw.h" > +#include "hw/sysbus.h" > #include "sysemu/char.h" > #include "qemu/log.h" > #include "hw/xen/xen_backend.h" > > #include > > +#define TYPE_XENSYSDEV "xensysdev" > + > +DeviceState *xen_sysdev; > + > /* - */ > > /* public */ > @@ -763,6 +768,10 @@ int xen_be_init(void) > /* Check if xen_init() have been called */ > goto err; > } > + > +xen_sysdev = qdev_create(NULL, TYPE_XENSYSDEV); > +qdev_init_nofail(xen_sysdev); > + > return 0; > > err: > @@ -863,3 +872,35 @@ void xen_be_printf(struct XenDevice *xendev, int > msg_level, const char *fmt, ... > } > qemu_log_flush(); > } > + > +static int xen_sysdev_init(SysBusDevice *dev) > +{ > +return 0; > +} > + > +static Property xen_sysdev_properties[] = { > +{/* end of property list */}, > +}; > + > +static void xen_sysdev_class_init(ObjectClass *klass, void *data) > +{ > +DeviceClass *dc = DEVICE_CLASS(klass); > +SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > + > +k->init = xen_sysdev_init; > +dc->props = xen_sysdev_properties; > +} > + > +static const TypeInfo xensysdev_info = { > +.name = TYPE_XENSYSDEV, > +.parent= TYPE_SYS_BUS_DEVICE, > +.instance_size = sizeof(SysBusDevice), > +.class_init= xen_sysdev_class_init, > +}; > + > +static void xenbe_register_types(void) > +{ > +type_register_static(&xensysdev_info); > +} > + > +type_init(xenbe_register_types); > diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c > index f68cf48..48f725c 100644 > --- a/hw/xenpv/xen_machine_pv.c > +++ b/hw/xenpv/xen_machine_pv.c > @@ -25,15 +25,10 @@ > #include "qemu/osdep.h" > #include "hw/hw.h" > #include "hw/boards.h" > -#include "hw/sysbus.h" > #include "hw/xen/xen_backend.h" > #include "xen_domainbuild.h" > #include "sysemu/block-backend.h" > > -#define TYPE_XENSYSDEV "xensysdev" > - > -DeviceState *xen_sysdev; > - > static void xen_init_pv(MachineState *machine) > { > DriveInfo *dinfo; > @@ -72,9 +67,6 @@ static void xen_init_pv(MachineState *machine) > break; > } > > -xen_sysdev = qdev_create(NULL, TYPE_XENSYSDEV); > -qdev_init_nofail(xen_sysdev); > - > xen_be_register("console", &xen_console_ops); > xen_be_register("vkbd", &xen_kbdmouse_ops); > xen_be_register("vfb", &xen_framebuffer_ops); > @@ -112,38 +104,6 @@ static void xen_init_pv(MachineState *machine) > xen_init_display(xen_domid); > } > > -static int xen_sysdev_init(SysBusDevice *dev) > -{ > -return 0; > -} > - > -static Property xen_sysdev_properties[] = { > -{/* end of property list */}, > -}; > - > -static void xen_sysdev_class_init(ObjectClass *klass, void *data) > -{ > -DeviceClass *dc = DEVICE_CLASS(klass); > -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > - > -k->init = xen_sysdev_init; > -dc->props = xen_sysdev_properties; > -} > - > -static const TypeInfo xensysdev_info = { > -.name = TYPE_XENSYSDEV, > -.parent= TYPE_SYS_BUS_DEVICE, > -.instance_size = sizeof(SysBusDevice), > -.class_init= xen_sysdev_class_init, > -}; > - > -static void xenpv_register_types(void) > -{ > -type_register_static(&xensysdev_info); > -} > - > -type_init(xenpv_register_types); > - > static void xenpv_machine_init(MachineClass *mc) > { > mc->desc = "Xen Para-virtualized PC"; >
Re: [Qemu-devel] [PATCH v2 0/3] Support building qemu-user powered docker test images
Fam Zheng writes: > On Wed, 06/08 17:35, Alex Bennée wrote: >> Hi, >> >> This is a re-spin of the previous series built on top of >> fam/docker.next. I've made the changes suggested in the last review >> and split the first patch apart to separate (and fix) the build >> directory changes first. >> >> Now it no longer messes with the docker file you can actually >> cross-build tests. First ensure you build the debian-bootstrap image: >> >> DEB_ARCH=armhf DEB_TYPE=testing \ >> ./tests/docker/docker.py build qemu:debian-bootstrap \ >> ./tests/docker/dockerfiles/debian-bootstrap.docker \ >> --include-executable=./arm-linux-user/qemu-arm > > This can take long depending on the network - I had to explicitly use a close > mirror in the pre script to test this, not sure why the mirror redirector > doesn't work. Eventually I get this error once the pre script succeeds and > docker build starts: > > Sending build context to Docker daemon 235.1 MB > Step 1 : FROM scratch > ---> > Step 2 : ADD . / > ---> 807bfa810b0c > Removing intermediate container e57ded00b227 > Step 3 : RUN sed -i 's/in_target mount/echo not for docker in_target mount/g' > /debootstrap/functions > ---> Running in fd80232b38fc > rpc error: code = 2 desc = "oci runtime error: exec format error" > > IIUC the sed is an armhf binary? Is something missing in the docker file? I > cannot find where the copied qemu-arm is used... Ahh I realise I missed out the implicit binfmt_misc needs to be set up. So on my Ubuntu system just having qemu-user installed means the host system binfmt_misc is set up for armhf binaries to run /usr/bin/qemu-arm. I haven't experimented with explicitly setting up binfmt_misc in the container because it was already working :-/ > > Fam > >> >> And then run the test quick target: >> >> make docker-test-quick@debian-bootstrap J=9 V=1 >> >> I'll leave it up to you how we cleanly integrate multi-arch builds >> into the Make system ;-) >> >> Alex Bennée (3): >> tests/docker/docker.py: docker_dir outside build >> tests/docker/docker.py: support --include-executable >> add debian-bootstrap.docker target (and pre script) >> >> tests/docker/docker.py | 68 >> +--- >> tests/docker/dockerfiles/debian-bootstrap.docker | 21 >> tests/docker/dockerfiles/debian-bootstrap.pre| 5 ++ >> 3 files changed, 86 insertions(+), 8 deletions(-) >> create mode 100644 tests/docker/dockerfiles/debian-bootstrap.docker >> create mode 100755 tests/docker/dockerfiles/debian-bootstrap.pre >> >> -- >> 2.7.4 >> -- Alex Bennée
Re: [Qemu-devel] [PATCH v2 3/3] add debian-bootstrap.docker target (and pre script)
Fam Zheng writes: > On Wed, 06/08 17:35, Alex Bennée wrote: >> Together with the debian-bootstrap.pre script can now build an arbitrary >> architecture of Debian using debootstrap. The docker script will now >> search for an associated $dockerfile.pre script which gets run in the >> same build context as the dockerfile will be. This allows debootstrap to >> set up its first stage before the container is built. >> >> To build a container you need a command line like: >> >> DEB_ARCH=armhf DEB_TYPE=testing \ >> ./tests/docker/docker.py build \ >> --include-executable=arm-linux-user/qemu-arm debian:armhf \ >> ./tests/docker/dockerfiles/debian-bootstrap.docker >> >> Signed-off-by: Alex Bennée >> >> --- >> v2 >> - use .pre script instead of embedded HOST_CMD >> - make default image include all QEMU build-deps >> --- >> tests/docker/docker.py | 5 + > > Could you split the docker.py into a separate patch? Sure. > >> tests/docker/dockerfiles/debian-bootstrap.docker | 21 + >> tests/docker/dockerfiles/debian-bootstrap.pre| 5 + >> 3 files changed, 31 insertions(+) >> create mode 100644 tests/docker/dockerfiles/debian-bootstrap.docker >> create mode 100755 tests/docker/dockerfiles/debian-bootstrap.pre >> >> diff --git a/tests/docker/docker.py b/tests/docker/docker.py >> index ed6fa45..43041eb 100755 >> --- a/tests/docker/docker.py >> +++ b/tests/docker/docker.py >> @@ -205,6 +205,11 @@ class BuildCommand(SubCommand): >> # Create a docker context directory for the build >> docker_dir = tempfile.mkdtemp(prefix="docker_build") >> >> +# Is there a .pre file to run in the build context? >> +docker_pre = os.path.splitext(args.dockerfile)[0]+".pre" >> +if os.path.exists(docker_pre): >> +subprocess.call(os.path.realpath(docker_pre), >> cwd=docker_dir) > > We should probably abort here if exit code is non-zero. Use > subprocess.check_call? OK. > > Fam > >> + >> # Do we include a extra binary? >> if args.include_executable: >> _copy_binary_with_libs(args.include_executable, >> diff --git a/tests/docker/dockerfiles/debian-bootstrap.docker >> b/tests/docker/dockerfiles/debian-bootstrap.docker >> new file mode 100644 >> index 000..3a9125e >> --- /dev/null >> +++ b/tests/docker/dockerfiles/debian-bootstrap.docker >> @@ -0,0 +1,21 @@ >> +# Create Debian Bootstrap Image >> +# >> +# This is intended to be pre-poluated by: >> +# - a first stage debootstrap (see debian-bootstrap.pre) >> +# - a native qemu-$arch that binfmt_misc will run >> +FROM scratch >> + >> +# Add everything from the context into the container >> +ADD . / >> + >> +# Patch all mounts as docker already has stuff set up >> +RUN sed -i 's/in_target mount/echo not for docker in_target mount/g' >> /debootstrap/functions >> + >> +# Run stage 2 >> +RUN /debootstrap/debootstrap --second-stage >> + >> +# At this point we can install additional packages if we want >> +# Duplicate deb line as deb-src >> +RUN cat /etc/apt/sources.list | sed "s/deb/deb-src/" >> >> /etc/apt/sources.list >> +RUN apt-get update >> +RUN apt-get -y build-dep qemu >> diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre >> b/tests/docker/dockerfiles/debian-bootstrap.pre >> new file mode 100755 >> index 000..6f42da6 >> --- /dev/null >> +++ b/tests/docker/dockerfiles/debian-bootstrap.pre >> @@ -0,0 +1,5 @@ >> +#!/bin/sh >> +# >> +# Simple wrapper for debootstrap, run in the docker build context >> +# >> +fakeroot debootstrap --variant=buildd --foreign --arch=$DEB_ARCH $DEB_TYPE >> . http://httpredir.debian.org/debian >> -- >> 2.7.4 >> -- Alex Bennée
Re: [Qemu-devel] [PATCH v2 2/3] tests/docker/docker.py: support --include-executable
Fam Zheng writes: > On Wed, 06/08 17:35, Alex Bennée wrote: >> When passed the path to a binary we copy it and any linked libraries >> into the docker build context. These can then be included by a >> dockerfile with the line: >> >> # Copy all of context into container >> ADD . / >> >> This is mainly intended for setting up foreign architecture docker >> images which use qemu-$arch to do cross-architecture linux-user >> execution. It also relies on the host and guest file-system following >> reasonable multi-arch layouts so the copied libraries don't clash with >> the guest ones. >> >> Signed-off-by: Alex Bennée >> >> --- >> v2 >> - change name of option >> - require full path to executable >> - clean-up the copy code >> --- >> tests/docker/docker.py | 42 ++ >> 1 file changed, 42 insertions(+) >> >> diff --git a/tests/docker/docker.py b/tests/docker/docker.py >> index ae40bb3..ed6fa45 100755 >> --- a/tests/docker/docker.py >> +++ b/tests/docker/docker.py >> @@ -20,6 +20,7 @@ import atexit >> import uuid >> import argparse >> import tempfile >> +import re >> from shutil import copy, rmtree >> >> def _text_checksum(text): >> @@ -38,6 +39,38 @@ def _guess_docker_command(): >> raise Exception("Cannot find working docker command. Tried:\n%s" % \ >> commands_txt) >> >> +def _copy_with_mkdir(src, root_dir, sub_path): >> +"""Copy src into root_dir, creating sub_path as needed.""" >> +dest_dir = os.path.normpath("%s/%s" % (root_dir, sub_path)) >> +try: >> +os.makedirs(dest_dir) >> +except OSError: >> +print "%s already created" % (dest_dir) > > Is this error necessary? If it doesn't hurt (i.e. multiple libraries are > copied > into it), I think we can safely say "pass". Or, add "if not > os.path.isdir(dest_dir):" above os.makedirs. OK. > >> + >> +dest_file = "%s/%s" % (dest_dir, os.path.basename(src)) >> +copy(src, dest_file) >> + >> + >> +def _copy_binary_with_libs(src, dest_dir): >> +"""Copy a binary executable and all its dependant libraries. >> + >> +This does rely on the host file-system being fairly multi-arch >> +aware so the file don't clash with the guests layout.""" >> + >> +_copy_with_mkdir(src, dest_dir, "/usr/bin") >> + >> +# do ldd bit here >> +ldd_re = re.compile(r"(/.*/)(\S*)") >> +ldd_output = subprocess.check_output(["ldd", src]) >> +for line in ldd_output.split("\n"): >> +search = ldd_re.search(line) >> +if search and len(search.groups()) == 2: >> +so_path = search.groups()[0] >> +so_lib = search.groups()[1] >> +_copy_with_mkdir("%s/%s" % (so_path, so_lib), >> + dest_dir, so_path) >> + >> + >> class Docker(object): >> """ Running Docker commands """ >> def __init__(self): >> @@ -151,6 +184,10 @@ class BuildCommand(SubCommand): >> """ Build docker image out of a dockerfile. Arguments: >> """ >> name = "build" >> def args(self, parser): >> +parser.add_argument("--include-executable", "-e", >> +help="""Specify a binary that will be copied to >> the >> +container together with all its dependent >> +libraries""") >> parser.add_argument("tag", >> help="Image Tag") >> parser.add_argument("dockerfile", >> @@ -168,6 +205,11 @@ class BuildCommand(SubCommand): >> # Create a docker context directory for the build >> docker_dir = tempfile.mkdtemp(prefix="docker_build") >> >> +# Do we include a extra binary? > > s/a extra/an extra/ OK > >> +if args.include_executable: >> +_copy_binary_with_libs(args.include_executable, >> + docker_dir) >> + >> dkr.build_image(tag, docker_dir, dockerfile, >> quiet=args.quiet, argv=argv) >> >> -- >> 2.7.4 >> Thanks -- Alex Bennée
[Qemu-devel] [PATCH v8 16/17] vmxnet3: remove unnecessary internal msi state flag
Internal flag msi_used is unnecessary, it has the same effect as msi_enabled(). msi_uninit() could be called directly without risk. cc: Dmitry Fleytman cc: Jason Wang cc: Markus Armbruster cc: Marcel Apfelbaum cc: Michael S. Tsirkin Reviewed-by: Markus Armbruster Signed-off-by: Cao jin --- changelog: fix stylistic issue. hw/net/vmxnet3.c | 17 ++--- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index 63f8904..9d439db 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -280,8 +280,6 @@ typedef struct { /* Whether MSI-X support was installed successfully */ bool msix_used; -/* Whether MSI support was installed successfully */ -bool msi_used; hwaddr drv_shmem; hwaddr temp_shared_guest_driver_memory; @@ -363,7 +361,7 @@ static bool _vmxnet3_assert_interrupt_line(VMXNET3State *s, uint32_t int_idx) msix_notify(d, int_idx); return false; } -if (s->msi_used && msi_enabled(d)) { +if (msi_enabled(d)) { VMW_IRPRN("Sending MSI notification for vector %u", int_idx); msi_notify(d, int_idx); return false; @@ -387,7 +385,7 @@ static void _vmxnet3_deassert_interrupt_line(VMXNET3State *s, int lidx) * This function should never be called for MSI(X) interrupts * because deassertion never required for message interrupts */ -assert(!s->msi_used || !msi_enabled(d)); +assert(!msi_enabled(d)); VMW_IRPRN("Deasserting line for interrupt %u", lidx); pci_irq_deassert(d); @@ -424,7 +422,7 @@ static void vmxnet3_trigger_interrupt(VMXNET3State *s, int lidx) goto do_automask; } -if (s->msi_used && msi_enabled(d) && s->auto_int_masking) { +if (msi_enabled(d) && s->auto_int_masking) { goto do_automask; } @@ -1409,8 +1407,8 @@ static void vmxnet3_update_features(VMXNET3State *s) static bool vmxnet3_verify_intx(VMXNET3State *s, int intx) { -return s->msix_used || s->msi_used || (intx == - (pci_get_byte(s->parent_obj.config + PCI_INTERRUPT_PIN) - 1)); +return s->msix_used || msi_enabled(PCI_DEVICE(s)) +|| intx == pci_get_byte(s->parent_obj.config + PCI_INTERRUPT_PIN) - 1; } static void vmxnet3_validate_interrupt_idx(bool is_msix, int idx) @@ -2202,9 +2200,7 @@ vmxnet3_cleanup_msi(VMXNET3State *s) { PCIDevice *d = PCI_DEVICE(s); -if (s->msi_used) { -msi_uninit(d); -} +msi_uninit(d); } static void @@ -2295,7 +2291,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp) /* Any error other than -ENOTSUP(board's MSI support is broken) * is a programming error. Fall back to INTx silently on -ENOTSUP */ assert(!ret || ret == -ENOTSUP); -s->msi_used = !ret; if (!vmxnet3_init_msix(s)) { VMW_WRPRN("Failed to initialize MSI-X, configuration is inconsistent."); -- 2.1.0
Re: [Qemu-devel] [RFC 03/10] cpus: Introduce async_wait_run_on_cpu()
alvise rigo writes: > I think that async_safe_run_on_cpu() does a different thing: it > queries a job to the target vCPU and wants all the other to "observe" > the submitted task. However, we will have the certainty that only the > target vCPU observed the task, the other might still be running in the > guest code. For the code to have run every will have come out of the run loop and synced up at that point. No safe work is run with guest code executing. > > alvise > > On Wed, Jun 8, 2016 at 5:20 PM, Alex Bennée wrote: >> >> Sergey Fedorov writes: >> >>> On 08/06/16 17:10, alvise rigo wrote: Using run_on_cpu() we might deadlock QEMU if other vCPUs are waiting for the current vCPU. We need to exit from the vCPU loop in order to avoid this. >>> >>> I see, we could deadlock indeed. Alternatively, we may want fix >>> run_on_cpu() to avoid waiting for completion by itself when called from >>> CPU loop. >> >> async_safe_run_on_cpu can't deadlock as all vCPUs are suspended (or >> waiting) for the work to complete. The tasks are run in strict order so >> if you queued async tasks for other vCPUs first you could ensure >> everything is in the state you want it when you finally service the >> calling vCPU. >> >>> >>> Kind regards, >>> Sergey >> >> >> -- >> Alex Bennée -- Alex Bennée
[Qemu-devel] linux-user: drop support for "unknown" host CPUs (ie hppa and m68k) ?
Currently our configure script allows linux-user targets to be built for "unknown" host CPU architectures (which must be using the TCI interpreter). However, code like user-exec.c has host-architecture #ifdef ladders which in practice mean you can't build linux-user for an arbitrary host architecture. The only two host architectures which have code in user-exec.c but aren't recognised by configure are HPPA and m68k. I'd like to move to an "every supported host architecture has a linux-user/host/$(ARCH)/hostdep.h header" model (this fixes a problem in the code currently in master where make doesn't notice it needs to rebuild if a host arch moves from "using generic/" to "using $(ARCH)"). Host architectures which we sort-of-but-don't support are an annoying corner case I'd like to be able to drop entirely, by either: (1) just dropping the support outright (2) promoting them to at least being recognised by configure as a known architecture, even if one without a tcg backend Does this seem like a good idea? I think HPPA should definitely be in category (1) -- we dropped the TCG backend years back, and I don't think any of us has a machine to test changes on,. Which category should m68k host support go into? Does anybody actually use m68k host + TCI interpreter linux-user ? My guess is this isn't actually used, because the m68k-specific code in user-exec.c doesn't set the "is_write" flag correctly, which means that guests which do self-modifying code won't work (including anything using a signal handler with an on-stack trampoline). So my preference would be to drop the m68k-host code too. thanks -- PMM
Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type
On 13/06/2016 11:08, vija...@caviumnetworks.com wrote: > + */ > +#define TARGET_AARCH64_PAGE_BITS 12 > /* The ARM MMU allows 1k pages. */ > /* ??? Linux doesn't actually use these, and they're deprecated in recent > architecture revisions. Maybe a configure option to disable them. */ > -#define TARGET_PAGE_BITS 10 > -#endif > +#define TARGET_ARM_PAGE_BITS 10 > +#define TARGET_PAGE_BITS arm_get_target_page_bits() Please avoid the function call and just make it a variable target_page_bits. Paolo
Re: [Qemu-devel] [PATCH] Makefile: Fix tag file generation targets
On 09/06/2016 19:58, Sergey Fedorov wrote: > From: Sergey Fedorov > > "ctags" produces a file named "tags", not "ctags". It doesn't look > reasonable to use phony target name as a file name to remove. Just use > exact file names to remove in "ctags" and "TAGS" target receipts. > > Signed-off-by: Sergey Fedorov > Signed-off-by: Sergey Fedorov > --- > Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index b8563db68677..61bf1bf9e2bb 100644 > --- a/Makefile > +++ b/Makefile > @@ -478,12 +478,12 @@ test speed: all > > .PHONY: ctags > ctags: > - rm -f $@ > + rm -f tags > find "$(SRC_PATH)" -name '*.[hc]' -exec ctags --append {} + > > .PHONY: TAGS > TAGS: > - rm -f $@ > + rm -f TAGS > find "$(SRC_PATH)" -name '*.[hc]' -exec etags --append {} + > > cscope: > Queued, thanks. Paolo
Re: [Qemu-devel] [PATCH v3 0/2] AVX2 configure fixes
On 10/06/2016 13:16, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > Hi, > This pair of patches fixes a couple of issues that we found during test. > The first is that the configure test is pessimistic when compiled with -O2, > the second is that the explicit 4.9 gcc test is a bit coarse; I've removed > that test but beefed up the ./configure test to actually use the avx2 > intrinsics and that fails in the same way as the main code, so it works > in the same way as the explicit check but allows older gcc's to work > when -save-temps isn't used. > > Dave > > v3 > Use compile_object rather than compile_prog as suggested by Peter to > hopefully fail on his ccache setup. Queued, thanks. Paolo > v2 > Remove the explicit version check > Split the patches > > > Dr. David Alan Gilbert (2): > Make avx2 configure test work with -O2 > avx2 configure: Use primitives in test > > configure | 17 - > util/cutils.c | 8 +--- > 2 files changed, 13 insertions(+), 12 deletions(-) >
Re: [Qemu-devel] [RFC PATCH v1 3/4] translate-all.c: Compute L1 page table properties at runtime
On 13/06/2016 11:08, vija...@caviumnetworks.com wrote: > From: Vijaya Kumar K > > Remove L1 page mapping table properties computing > statically using macros which is dependent on > TARGET_PAGE_BITS. Drop macros V_L1_SIZE, V_L1_SHIFT, > V_L1_BITS macros and replace with variables which are > computed at early stage of VM boot. > > Removing dependency can help to make TARGET_PAGE_BITS > dynamic. > > Signed-off-by: Vijaya Kumar K > --- > include/qemu-common.h |1 + > translate-all.c | 57 > ++--- > vl.c |3 +++ > 3 files changed, 39 insertions(+), 22 deletions(-) > > diff --git a/include/qemu-common.h b/include/qemu-common.h > index 1f2cb94..d5f0450 100644 > --- a/include/qemu-common.h > +++ b/include/qemu-common.h > @@ -129,6 +129,7 @@ int parse_debug_env(const char *name, int max, int > initial); > > const char *qemu_ether_ntoa(const MACAddr *mac); > void page_size_init(void); > +void init_l1_page_table_param(void); > > /* returns non-zero if dump is in progress, otherwise zero is > * returned. */ > diff --git a/translate-all.c b/translate-all.c > index 118e7d3..a580ca9 100644 > --- a/translate-all.c > +++ b/translate-all.c > @@ -57,6 +57,7 @@ > #include "qemu/bitmap.h" > #include "qemu/timer.h" > #include "exec/log.h" > +#include "qemu/error-report.h" > > //#define DEBUG_TB_INVALIDATE > //#define DEBUG_FLUSH > @@ -99,25 +100,18 @@ typedef struct PageDesc { > #define V_L2_BITS 10 > #define V_L2_SIZE (1 << V_L2_BITS) > > -/* The bits remaining after N lower levels of page tables. */ > -#define V_L1_BITS_REM \ > -((L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS) % V_L2_BITS) > - > -#if V_L1_BITS_REM < 4 > -#define V_L1_BITS (V_L1_BITS_REM + V_L2_BITS) > -#else > -#define V_L1_BITS V_L1_BITS_REM > -#endif > - > -#define V_L1_SIZE ((target_ulong)1 << V_L1_BITS) > - > -#define V_L1_SHIFT (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - V_L1_BITS) > - > uintptr_t qemu_host_page_size; > intptr_t qemu_host_page_mask; > > +/* > + * L1 Mapping properties > + */ > +static unsigned long v_l1_bits; > +static unsigned long v_l1_size; > +static unsigned long v_l1_shift; Please make these uint8_t. > /* The bottom level has pointers to PageDesc */ > -static void *l1_map[V_L1_SIZE]; > +static void *l1_map; You can make this array have a static V_L2_SIZE * 16 size too. Peter, what do you think? > /* code generation context */ > TCGContext tcg_ctx; > @@ -127,6 +121,25 @@ TCGContext tcg_ctx; > __thread int have_tb_lock; > #endif > > +void init_l1_page_table_param(void) > +{ > +uint32_t v_l1_bits_rem; > + > +assert(TARGET_PAGE_BITS); > +/* The bits remaining after N lower levels of page tables. */ > +v_l1_bits_rem = ((L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS) % > V_L2_BITS); > +if (v_l1_bits_rem < 4) > +v_l1_bits = (v_l1_bits_rem + V_L2_BITS); > +else > +v_l1_bits = v_l1_bits_rem; > + > +v_l1_size = ((target_ulong)1 << v_l1_bits); > +v_l1_shift = (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - v_l1_bits); Please assert that v_l1_shift % V_L2_BITS == 0. > +l1_map = g_malloc0(v_l1_size * sizeof(void *)); > +if (!l1_map) > +error_report("Allocation faile for L1 MAP table\n"); > +} > + > void tb_lock(void) > { > #ifdef CONFIG_USER_ONLY > @@ -408,10 +421,10 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, > int alloc) > int i; > > /* Level 1. Always allocated. */ > -lp = l1_map + ((index >> V_L1_SHIFT) & (V_L1_SIZE - 1)); > +lp = l1_map + ((index >> v_l1_shift) & (v_l1_size - 1)); > > /* Level 2..N-1. */ > -for (i = V_L1_SHIFT / V_L2_BITS - 1; i > 0; i--) { > +for (i = v_l1_shift / V_L2_BITS - 1; i > 0; i--) { Please cache v_l1_shift / V_L2_BITS - 1 into a new variable v_l2_levels. > void **p = atomic_rcu_read(lp); > > if (p == NULL) { > @@ -819,8 +832,8 @@ static void page_flush_tb(void) > { > int i; > > -for (i = 0; i < V_L1_SIZE; i++) { > -page_flush_tb_1(V_L1_SHIFT / V_L2_BITS - 1, l1_map + i); > +for (i = 0; i < v_l1_size; i++) { > +page_flush_tb_1(v_l1_shift / V_L2_BITS - 1, l1_map + i); > } > } > > @@ -1825,9 +1838,9 @@ int walk_memory_regions(void *priv, > walk_memory_regions_fn fn) > data.start = -1u; > data.prot = 0; > > -for (i = 0; i < V_L1_SIZE; i++) { > -int rc = walk_memory_regions_1(&data, (target_ulong)i << (V_L1_SHIFT > + TARGET_PAGE_BITS), > - V_L1_SHIFT / V_L2_BITS - 1, l1_map + > i); > +for (i = 0; i < v_l1_size; i++) { > +int rc = walk_memory_regions_1(&data, (target_ulong)i << (v_l1_shift > + TARGET_PAGE_BITS), > + v_l1_shift / V_L2_BITS - 1, l1_map + > i); > if (rc != 0) { > return rc; > } > diff --git a/vl.c b/vl.c > index b0bcc25..b6da265 100644 > --- a/vl.c > +++ b
Re: [Qemu-devel] [RFC PATCH v1 2/4] exec.c: Remove static allocation of sub_section of sub_page
On 13 June 2016 at 10:08, wrote: > From: Vijaya Kumar K > > Allocate sub_section dynamically. Remove dependency > on TARGET_PAGE_SIZE to make run-time page size detection > for arm platforms. > > Signed-off-by: Vijaya Kumar K > --- > exec.c |5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/exec.c b/exec.c > index a9d465b..e803a41 100644 > --- a/exec.c > +++ b/exec.c > @@ -154,7 +154,7 @@ typedef struct subpage_t { > MemoryRegion iomem; > AddressSpace *as; > hwaddr base; > -uint16_t sub_section[TARGET_PAGE_SIZE]; > +uint16_t *sub_section; > } subpage_t; > > #define PHYS_SECTION_UNASSIGNED 0 > @@ -1151,6 +1151,7 @@ static void phys_section_destroy(MemoryRegion *mr) > if (have_sub_page) { > subpage_t *subpage = container_of(mr, subpage_t, iomem); > object_unref(OBJECT(&subpage->iomem)); > +g_free(subpage->sub_section); > g_free(subpage); > } > } > @@ -2272,7 +2273,7 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr > base) > subpage_t *mmio; > > mmio = g_malloc0(sizeof(subpage_t)); > - > +mmio->sub_section = g_malloc0(TARGET_PAGE_SIZE * sizeof(uint16_t)); You can write this as = g_new0(uint16_t, TARGET_PAGE_SIZE); thanks -- PMM
Re: [Qemu-devel] [PATCH v8 10/17] pci bridge dev: change msi property type
On 06/10/2016 12:54 PM, Cao jin wrote: From bit to enum OnOffAuto. cc: Michael S. Tsirkin cc: Markus Armbruster cc: Marcel Apfelbaum Reviewed-by: Markus Armbruster Signed-off-by: Cao jin --- hw/pci-bridge/pci_bridge_dev.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c index 41ca47b..0fbecc4 100644 --- a/hw/pci-bridge/pci_bridge_dev.c +++ b/hw/pci-bridge/pci_bridge_dev.c @@ -42,9 +42,10 @@ struct PCIBridgeDev { MemoryRegion bar; uint8_t chassis_nr; -#define PCI_BRIDGE_DEV_F_MSI_REQ 0 -#define PCI_BRIDGE_DEV_F_SHPC_REQ 1 +#define PCI_BRIDGE_DEV_F_SHPC_REQ 0 uint32_t flags; + +OnOffAuto msi; }; typedef struct PCIBridgeDev PCIBridgeDev; @@ -66,7 +67,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev) } } else { /* MSI is not applicable without SHPC */ -bridge_dev->flags &= ~(1 << PCI_BRIDGE_DEV_F_MSI_REQ); +bridge_dev->msi = ON_OFF_AUTO_OFF; } err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0); @@ -74,7 +75,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev) goto slotid_error; } -if ((bridge_dev->flags & (1 << PCI_BRIDGE_DEV_F_MSI_REQ)) && +if (bridge_dev->msi != ON_OFF_AUTO_OFF && msi_nonbroken) { err = msi_init(dev, 0, 1, true, true); if (err < 0) { @@ -147,8 +148,8 @@ static Property pci_bridge_dev_properties[] = { /* Note: 0 is not a legal chassis number. */ DEFINE_PROP_UINT8(PCI_BRIDGE_DEV_PROP_CHASSIS_NR, PCIBridgeDev, chassis_nr, 0), -DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_MSI, PCIBridgeDev, flags, -PCI_BRIDGE_DEV_F_MSI_REQ, true), +DEFINE_PROP_ON_OFF_AUTO(PCI_BRIDGE_DEV_PROP_MSI, PCIBridgeDev, msi, +ON_OFF_AUTO_AUTO), DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags, PCI_BRIDGE_DEV_F_SHPC_REQ, true), DEFINE_PROP_END_OF_LIST(), Reviewed-by: Marcel Apfelbaum Thanks, Marcel
Re: [Qemu-devel] [PATCH] configure: Remove unused CONFIG_SIGEV_THREAD_ID switch
On 10/06/2016 17:04, Thomas Huth wrote: > The CONFIG_SIGEV_THREAD_ID switch is unused since the related code > has been removed by commit 6d327171551a12b937c5718073b9848d0274c74d > ("aio / timers: Remove alarm timers"), so it can safely be removed > nowadays. Queued, thanks. Paolo > Signed-off-by: Thomas Huth > --- > configure | 22 -- > 1 file changed, 22 deletions(-) > > diff --git a/configure b/configure > index 8c2f90b..469aa45 100755 > --- a/configure > +++ b/configure > @@ -4178,24 +4178,6 @@ if compile_prog "" "" ; then > fi > > ## > -# check if we have usable SIGEV_THREAD_ID > - > -sigev_thread_id=no > -cat > $TMPC << EOF > -#include > -int main(void) { > - struct sigevent ev; > - ev.sigev_notify = SIGEV_THREAD_ID; > - ev._sigev_un._tid = 0; > - asm volatile("" : : "g"(&ev)); > - return 0; > -} > -EOF > -if compile_prog "" "" ; then > -sigev_thread_id=yes > -fi > - > -## > # check if trace backend exists > > $python "$source_path/scripts/tracetool.py" "--backends=$trace_backends" > --check-backends > /dev/null 2> /dev/null > @@ -4862,7 +4844,6 @@ echo "preadv support$preadv" > echo "fdatasync $fdatasync" > echo "madvise $madvise" > echo "posix_madvise $posix_madvise" > -echo "sigev_thread_id $sigev_thread_id" > echo "uuid support $uuid" > echo "libcap-ng support $cap_ng" > echo "vhost-net support $vhost_net" > @@ -5277,9 +5258,6 @@ fi > if test "$posix_madvise" = "yes" ; then >echo "CONFIG_POSIX_MADVISE=y" >> $config_host_mak > fi > -if test "$sigev_thread_id" = "yes" ; then > - echo "CONFIG_SIGEV_THREAD_ID=y" >> $config_host_mak > -fi > > if test "$spice" = "yes" ; then >echo "CONFIG_SPICE=y" >> $config_host_mak >
Re: [Qemu-devel] [PATCH] nbd: Don't use *_to_cpup() functions
On 10/06/2016 17:00, Peter Maydell wrote: > The *_to_cpup() functions are not very useful, as they simply do > a pointer dereference and then a *_to_cpu(). Instead use either: > * ld*_*_p(), if the data is at an address that might not be >correctly aligned for the load > * a local dereference and *_to_cpu(), if the pointer is >the correct type and known to be correctly aligned > > Signed-off-by: Peter Maydell > --- > The motivation is to be able to drop *_to_cpup() entirely; > we don't have many places that use it. Good idea indeed. Queued both patches. Paolo > --- > nbd/client.c | 8 > nbd/server.c | 10 +- > qemu-nbd.c | 4 ++-- > 3 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/nbd/client.c b/nbd/client.c > index 31b88f3..bb8981f 100644 > --- a/nbd/client.c > +++ b/nbd/client.c > @@ -572,7 +572,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char > *name, uint32_t *flags, > error_setg(errp, "Failed to read export flags"); > goto fail; > } > -*flags = be32_to_cpup(flags); > +*flags = be32_to_cpu(*flags); > } else { > error_setg(errp, "Bad magic received"); > goto fail; > @@ -726,9 +726,9 @@ ssize_t nbd_receive_reply(QIOChannel *ioc, struct > nbd_reply *reply) > [ 7 .. 15]handle > */ > > -magic = be32_to_cpup((uint32_t*)buf); > -reply->error = be32_to_cpup((uint32_t*)(buf + 4)); > -reply->handle = be64_to_cpup((uint64_t*)(buf + 8)); > +magic = ldl_be_p(buf); > +reply->error = ldl_be_p(buf + 4); > +reply->handle = ldq_be_p(buf + 8); > > reply->error = nbd_errno_to_system_errno(reply->error); > > diff --git a/nbd/server.c b/nbd/server.c > index b2cfeb9..91471f1 100644 > --- a/nbd/server.c > +++ b/nbd/server.c > @@ -646,11 +646,11 @@ static ssize_t nbd_receive_request(QIOChannel *ioc, > struct nbd_request *request) > [24 .. 27] len > */ > > -magic = be32_to_cpup((uint32_t*)buf); > -request->type = be32_to_cpup((uint32_t*)(buf + 4)); > -request->handle = be64_to_cpup((uint64_t*)(buf + 8)); > -request->from = be64_to_cpup((uint64_t*)(buf + 16)); > -request->len = be32_to_cpup((uint32_t*)(buf + 24)); > +magic = ldl_be_p(buf); > +request->type = ldl_be_p(buf + 4); > +request->handle = ldq_be_p(buf + 8); > +request->from = ldq_be_p(buf + 16); > +request->len= ldl_be_p(buf + 24); > > TRACE("Got request: " >"{ magic = 0x%x, .type = %d, from = %" PRIu64" , len = %u }", > diff --git a/qemu-nbd.c b/qemu-nbd.c > index 6554f0a..9519db3 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -154,8 +154,8 @@ static void read_partition(uint8_t *p, struct > partition_record *r) > r->end_cylinder = p[7] | ((p[6] << 2) & 0x300); > r->end_sector = p[6] & 0x3f; > > -r->start_sector_abs = le32_to_cpup((uint32_t *)(p + 8)); > -r->nb_sectors_abs = le32_to_cpup((uint32_t *)(p + 12)); > +r->start_sector_abs = ldl_le_p(p + 8); > +r->nb_sectors_abs = ldl_le_p(p + 12); > } > > static int find_partition(BlockBackend *blk, int partition, >
Re: [Qemu-devel] [PATCH v8 11/17] msi_init: change return value to 0 on success
On 06/10/2016 12:54 PM, Cao jin wrote: No caller use its return value as msi capability offset, also in order to make its return behaviour consistent with msix_init(). cc: Michael S. Tsirkin cc: Paolo Bonzini cc: Hannes Reinecke cc: Markus Armbruster cc: Marcel Apfelbaum Acked-by: Hannes Reinecke Reviewed-by: Markus Armbruster Signed-off-by: Cao jin --- hw/pci/msi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Hi Hannes, This version changed, If is ok with you, I will let your A-b still here. diff --git a/hw/pci/msi.c b/hw/pci/msi.c index 359058e..ed79225 100644 --- a/hw/pci/msi.c +++ b/hw/pci/msi.c @@ -173,8 +173,7 @@ bool msi_enabled(const PCIDevice *dev) * If @msi64bit, make the device capable of sending a 64-bit message * address. * If @msi_per_vector_mask, make the device support per-vector masking. - * Return the offset of capability MSI in config space on success, - * return -errno on error. + * Return 0 on success, return -errno on error. * * -ENOTSUP means lacking msi support for a msi-capable platform. * -EINVAL means capability overlap, happens when @offset is non-zero, @@ -236,7 +235,8 @@ int msi_init(struct PCIDevice *dev, uint8_t offset, pci_set_long(dev->wmask + msi_mask_off(dev, msi64bit), 0x >> (PCI_MSI_VECTORS_MAX - nr_vectors)); } -return config_offset; + +return 0; } void msi_uninit(struct PCIDevice *dev) Reviewed-by: Marcel Apfelbaum Thanks, Marcel
Re: [Qemu-devel] [PATCH v2 11/22] hw/intc/arm_gicv3: Implement GICv3 distributor registers
On 2016/6/13 17:04, Peter Maydell wrote: > On 13 June 2016 at 07:27, Shannon Zhao wrote: >> > >> > >> > On 2016/5/26 22:55, Peter Maydell wrote: >>> >> +static uint8_t gicd_read_ipriorityr(GICv3State *s, MemTxAttrs attrs, >>> >> int irq) >>> >> +{ >>> >> +/* Read the value of GICD_IPRIORITYR for the specified interrupt, >>> >> + * honouring security state (these are RAZ/WI for Group 0 or Secure >>> >> + * Group 1 interrupts). >>> >> + */ >>> >> +uint32_t prio; >>> >> + >>> >> +if (irq < GIC_INTERNAL || irq >= s->num_irq) { >>> >> +return 0; >>> >> +} >>> >> + >>> >> +prio = s->gicd_ipriority[irq]; >>> >> + >>> >> +if (!attrs.secure && !(s->gicd_ctlr & GICD_CTLR_DS)) { >>> >> +if (!gicv3_gicd_group_test(s, irq)) { >>> >> +/* Fields for Group 0 or Secure Group 1 interrupts are >>> >> RAZ/WI */ >> > Here this check assure this interrupt belongs to Group 0 and NS access >> > is not permitted, so it should return 0. But it doesn't say anything >> > about Secure Group 1. > We're testing the GICD_IGROUPR bit here. If DS is zero (security > enabled), then IGROUPR == 0 means "Group 0 or Secure Group 1", which > is what the comment says we're testing. (If you care which of 0 and S1 > it is then you look at IGRPMODR, but for security checks like these > we don't need to.) > Oh, right. -- Shannon
Re: [Qemu-devel] [RFC PATCH v1 3/4] translate-all.c: Compute L1 page table properties at runtime
On 13 June 2016 at 10:25, Paolo Bonzini wrote: > > > On 13/06/2016 11:08, vija...@caviumnetworks.com wrote: >> From: Vijaya Kumar K >> >> Remove L1 page mapping table properties computing >> statically using macros which is dependent on >> TARGET_PAGE_BITS. Drop macros V_L1_SIZE, V_L1_SHIFT, >> V_L1_BITS macros and replace with variables which are >> computed at early stage of VM boot. >> >> Removing dependency can help to make TARGET_PAGE_BITS >> dynamic. >> >> Signed-off-by: Vijaya Kumar K >> --- >> include/qemu-common.h |1 + >> translate-all.c | 57 >> ++--- >> vl.c |3 +++ >> 3 files changed, 39 insertions(+), 22 deletions(-) >> >> diff --git a/include/qemu-common.h b/include/qemu-common.h >> index 1f2cb94..d5f0450 100644 >> --- a/include/qemu-common.h >> +++ b/include/qemu-common.h >> @@ -129,6 +129,7 @@ int parse_debug_env(const char *name, int max, int >> initial); >> >> const char *qemu_ether_ntoa(const MACAddr *mac); >> void page_size_init(void); >> +void init_l1_page_table_param(void); >> >> /* returns non-zero if dump is in progress, otherwise zero is >> * returned. */ >> diff --git a/translate-all.c b/translate-all.c >> index 118e7d3..a580ca9 100644 >> --- a/translate-all.c >> +++ b/translate-all.c >> @@ -57,6 +57,7 @@ >> #include "qemu/bitmap.h" >> #include "qemu/timer.h" >> #include "exec/log.h" >> +#include "qemu/error-report.h" >> >> //#define DEBUG_TB_INVALIDATE >> //#define DEBUG_FLUSH >> @@ -99,25 +100,18 @@ typedef struct PageDesc { >> #define V_L2_BITS 10 >> #define V_L2_SIZE (1 << V_L2_BITS) >> >> -/* The bits remaining after N lower levels of page tables. */ >> -#define V_L1_BITS_REM \ >> -((L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS) % V_L2_BITS) >> - >> -#if V_L1_BITS_REM < 4 >> -#define V_L1_BITS (V_L1_BITS_REM + V_L2_BITS) >> -#else >> -#define V_L1_BITS V_L1_BITS_REM >> -#endif >> - >> -#define V_L1_SIZE ((target_ulong)1 << V_L1_BITS) >> - >> -#define V_L1_SHIFT (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - V_L1_BITS) >> - >> uintptr_t qemu_host_page_size; >> intptr_t qemu_host_page_mask; >> >> +/* >> + * L1 Mapping properties >> + */ >> +static unsigned long v_l1_bits; >> +static unsigned long v_l1_size; >> +static unsigned long v_l1_shift; > > Please make these uint8_t. > >> /* The bottom level has pointers to PageDesc */ >> -static void *l1_map[V_L1_SIZE]; >> +static void *l1_map; > > You can make this array have a static V_L2_SIZE * 16 size too. Peter, > what do you think? I don't know this code well enough to have an informed view, but we only allocate this once at startup, right? I'm not sure why making it a static array would be better? thanks -- PMM
[Qemu-devel] [PATCH] nbd: simplify the nbd_request and nbd_reply structs
These structs are never used to represent the bytes that go over the network. The big-endian network data is built into a uint8_t array in nbd_{receive,send}_{request,reply}. Remove the unused magic field, reorder the struct to avoid holes, and remove the packed attribute. Signed-off-by: Paolo Bonzini --- include/block/nbd.h | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index b86a976..747bb0a 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -25,19 +25,20 @@ #include "io/channel-socket.h" #include "crypto/tlscreds.h" +/* Note: these are _NOT_ the same as the network representation of an NBD + * request and reply! + */ struct nbd_request { -uint32_t magic; -uint32_t type; uint64_t handle; uint64_t from; uint32_t len; -} QEMU_PACKED; +uint32_t type; +}; struct nbd_reply { -uint32_t magic; -uint32_t error; uint64_t handle; -} QEMU_PACKED; +uint32_t error; +}; #define NBD_FLAG_HAS_FLAGS (1 << 0)/* Flags are there */ #define NBD_FLAG_READ_ONLY (1 << 1)/* Device is read-only */ -- 2.5.5
Re: [Qemu-devel] [Qemu-block] [PATCH] macio: Use blk_drain instead of blk_drain_all
Am 12.06.2016 um 08:56 hat Fam Zheng geschrieben: > We only care about the associated backend, so blk_drain is more > appropriate here. > > Signed-off-by: Fam Zheng [ Cc: John ] > --- > hw/ide/macio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/ide/macio.c b/hw/ide/macio.c > index 78c10a0..a8c7321 100644 > --- a/hw/ide/macio.c > +++ b/hw/ide/macio.c > @@ -400,7 +400,7 @@ static void pmac_ide_flush(DBDMA_io *io) > IDEState *s = idebus_active_if(&m->bus); > > if (s->bus->dma->aiocb) { > -blk_drain_all(); > +blk_drain(s->blk); > } > } Looks good to me: Reviewed-by: Kevin Wolf However, even this is still doing too much. We only need to drain the requests that come from this device and can ignore e.g. block job requests. Now the part that I'm not completely sure about is whether the problem is here in the IDE emulation and it should track its own requests or whether it is blk_drain() that actually shouldn't drain the BDS but just all requests that came in through this specific BB. I'm leaning towards the latter, but I'm unsure whether we have cases where we actually need to drain the whole root BDS. Any opinions? Kevin
Re: [Qemu-devel] [PATCH V3] block/iscsi: allow caching of the allocation map
On 24/05/2016 10:40, Peter Lieven wrote: > until now the allocation map was used only as a hint if a cluster > is allocated or not. If a block was not allocated (or Qemu had > no info about the allocation status) a get_block_status call was > issued to check the allocation status and possibly avoid > a subsequent read of unallocated sectors. If a block known to be > allocated the get_block_status call was omitted. In the other case > a get_block_status call was issued before every read to avoid > the necessity for a consistent allocation map. To avoid the > potential overhead of calling get_block_status for each and > every read request this took only place for the bigger requests. > > This patch enhances this mechanism to cache the allocation > status and avoid calling get_block_status for blocks where > the allocation status has been queried before. This allows > for bypassing the read request even for smaller requests and > additionally omits calling get_block_status for known to be > unallocated blocks. > > Signed-off-by: Peter Lieven > --- > v2->v3: - fix wording errors [Fam] > - reinit allocmap only if allocmap is present in > bdrv_reopen_commit > v1->v2: - add more comments [Fam] > - free allocmap if allocation of allocmap_valid fails [Fam] > - fix indent and whitespace errors [Fam] > - account for cache mode changes on reopen > > block/iscsi.c | 231 > +- > 1 file changed, 182 insertions(+), 49 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 2ca8e72..fb308f6 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -2,7 +2,7 @@ > * QEMU Block driver for iSCSI images > * > * Copyright (c) 2010-2011 Ronnie Sahlberg > - * Copyright (c) 2012-2015 Peter Lieven > + * Copyright (c) 2012-2016 Peter Lieven > * > * Permission is hereby granted, free of charge, to any person obtaining a > copy > * of this software and associated documentation files (the "Software"), to > deal > @@ -62,7 +62,23 @@ typedef struct IscsiLun { > struct scsi_inquiry_logical_block_provisioning lbp; > struct scsi_inquiry_block_limits bl; > unsigned char *zeroblock; > -unsigned long *allocationmap; > +/* The allocmap tracks which clusters (pages) on the iSCSI target are > + * allocated and which are not. In case a target returns zeros for > + * unallocated pages (iscsilun->lprz) we can directly return zeros > instead > + * of reading zeros over the wire if a read request falls within an > + * unallocated block. As there are 3 possible states we need 2 bitmaps to > + * track. allocmap_valid keeps track if QEMU's information about a page > is > + * valid. allocmap tracks if a page is allocated or not. In case QEMU > has no > + * valid information about a page the corresponding allocmap entry > should be > + * switched to unallocated as well to force a new lookup of the > allocation > + * status as lookups are generally skipped if a page is suspect to be > + * allocated. If a iSCSI target is opened with cache.direct = on the > + * allocmap_valid does not exist turning all cached information invalid > so > + * that a fresh lookup is made for any page even if allocmap entry > returns > + * it's unallocated. */ > +unsigned long *allocmap; > +unsigned long *allocmap_valid; > +long allocmap_size; > int cluster_sectors; > bool use_16_for_rw; > bool write_protected; > @@ -415,37 +431,132 @@ static bool is_request_lun_aligned(int64_t sector_num, > int nb_sectors, > return 1; > } > > -static unsigned long *iscsi_allocationmap_init(IscsiLun *iscsilun) > +static void iscsi_allocmap_free(IscsiLun *iscsilun) > { > -return bitmap_try_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks, > - iscsilun), > - iscsilun->cluster_sectors)); > +g_free(iscsilun->allocmap); > +g_free(iscsilun->allocmap_valid); > +iscsilun->allocmap = NULL; > +iscsilun->allocmap_valid = NULL; > } > > -static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num, > -int nb_sectors) > + > +static int iscsi_allocmap_init(IscsiLun *iscsilun, int open_flags) > { > -if (iscsilun->allocationmap == NULL) { > -return; > +iscsi_allocmap_free(iscsilun); > + > +iscsilun->allocmap_size = > +DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks, iscsilun), > + iscsilun->cluster_sectors); > + > +iscsilun->allocmap = bitmap_try_new(iscsilun->allocmap_size); > +if (!iscsilun->allocmap) { > +return -ENOMEM; > +} > + > +if (open_flags & BDRV_O_NOCACHE) { > +/* in case that cache.direct = on all allocmap entries are > + * treated as invalid to force a relookup of the block > + * status on
Re: [Qemu-devel] [PATCH v5 0/2] trace: Add event for vCPU memory accesses
On Thu, Jun 09, 2016 at 07:31:36PM +0200, Lluís Vilanova wrote: > This series adds an event to track information related to memory accesses > performed by the guest CPUs ("guest_mem_before"). > > A future series might extend this to contain the physical address and memory > value (e.g., "guest_mem_after"). > > Signed-off-by: Lluís Vilanova > --- > > Changes in v5 > - > > * Simplify tracing code in "tcg/tcg-opc.c" [Richard Henderson] > > > Changes in v4 > - > > * Clarify alignment info is not on the trace. > * Add event information on commit log. [Richard Henderson] > > > Changes in v3 > - > > * Set "tcg_ctx.cpu" to NULL when unused. [Paolo Bonzini] > * Clarify how the 'info' field is interpreted. > * Fix argument size in 'info' field when using ld/st handlers. > * Fix reset of unused bits in 'info' field. > > > Changes in v2 > - > > * Rebase on bfc766d. > * Rename "guest_vmem" to "guest_mem_before" > * Add memory access information. [suggested by Peter Maydell] > * Drop event "guest_vmem_user_syscall". [suggested by Peter Maydell] > > > Lluís Vilanova (2): > exec: [tcg] Track which vCPU is performing translation and execution > trace: [all] Add "guest_mem_before" event > > > include/exec/cpu_ldst_template.h | 25 > include/exec/cpu_ldst_useronly_template.h | 22 ++ > target-alpha/translate.c |1 + > target-arm/translate.c|1 + > target-cris/translate.c |1 + > target-cris/translate_v10.c |1 + > target-i386/translate.c |1 + > target-lm32/translate.c |1 + > target-m68k/translate.c |1 + > target-microblaze/translate.c |1 + > target-mips/translate.c |1 + > target-moxie/translate.c |1 + > target-openrisc/translate.c |1 + > target-ppc/translate.c|1 + > target-s390x/translate.c |1 + > target-sh4/translate.c|1 + > target-sparc/translate.c |1 + > target-tilegx/translate.c |1 + > target-tricore/translate.c|1 + > target-unicore32/translate.c |1 + > target-xtensa/translate.c |1 + > tcg/tcg-op.c | 10 ++ > tcg/tcg.h |4 +++ > trace-events | 22 ++ > trace/mem-internal.h | 46 > + > trace/mem.h | 34 + > translate-all.c |2 + > 27 files changed, 184 insertions(+) > create mode 100644 trace/mem-internal.h > create mode 100644 trace/mem.h > > > To: qemu-devel@nongnu.org > Cc: Stefan Hajnoczi > Cc: Peter Maydell > Cc: Richard Henderson Thanks, applied to my tracing tree: https://github.com/stefanha/qemu/commits/tracing Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] linux-user: drop support for "unknown" host CPUs (ie hppa and m68k) ?
Le 13/06/2016 à 11:26, Peter Maydell a écrit : > Currently our configure script allows linux-user targets to be built > for "unknown" host CPU architectures (which must be using the TCI > interpreter). However, code like user-exec.c has host-architecture > #ifdef ladders which in practice mean you can't build linux-user for > an arbitrary host architecture. The only two host architectures which > have code in user-exec.c but aren't recognised by configure are > HPPA and m68k. > > I'd like to move to an "every supported host architecture has a > linux-user/host/$(ARCH)/hostdep.h header" model (this fixes a problem > in the code currently in master where make doesn't notice it needs to > rebuild if a host arch moves from "using generic/" to "using $(ARCH)"). > Host architectures which we sort-of-but-don't support are an annoying > corner case I'd like to be able to drop entirely, by either: > (1) just dropping the support outright > (2) promoting them to at least being recognised by configure as >a known architecture, even if one without a tcg backend > > Does this seem like a good idea? > > I think HPPA should definitely be in category (1) -- we dropped > the TCG backend years back, and I don't think any of us has a > machine to test changes on,. I have an HPPA server sleeping in my garage, if someone needs it... > Which category should m68k host support go into? Does anybody > actually use m68k host + TCI interpreter linux-user ? My guess > is this isn't actually used, because the m68k-specific code in > user-exec.c doesn't set the "is_write" flag correctly, which means > that guests which do self-modifying code won't work (including > anything using a signal handler with an on-stack trampoline). > So my preference would be to drop the m68k-host code too. I've never tried to use an m68k computer as a QEMU host, and I think it should be horribly slow (for 68040 at 40 Mhz, and I don't have coldfire). IMHO, I see no problem to remove m68k-host (1) while it does not break qemu-m68k and qemu-system-m68k. Thanks, Laurent
Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type
On 13 June 2016 at 10:08, wrote: > From: Vijaya Kumar K > > Replace TARGET_PAGE_BITS with arm_target_page_size function > in order to fetch page size at run-time. > > Introduced MachineClass callback to compute target page > size at the early boot before memory initialization. > This callback is currently implemented for ARM platforms. > Based on cpu_model, the page size is updated in > target_page_bits which is defined as TARGET_PAGE_BITS. > > Signed-off-by: Vijaya Kumar K > --- > hw/arm/virt.c | 48 > include/hw/boards.h |1 + > target-arm/cpu.h| 12 +++- > vl.c|7 +++ > 4 files changed, 63 insertions(+), 5 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 73113cf..37aab33 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -90,6 +90,12 @@ typedef struct { > int32_t gic_version; > } VirtMachineState; > > +/* > + * Holds TARGET_AARCH_64_PAGE_BITS or TARGET_ARM_PAGE_BITS > + * based on the the cpu type emulated at runtime. > + */ > +static uint32_t target_page_bits; The CPU page size is not specific to the 'virt' board, so this is the wrong place to do this. You should identify the page size in arm_cpu_realizefn() based on the set of feature bits the CPU has: anything with ARM_FEATURE_V7 has a 4K page table (this includes a lot of 32-bit CPUs). CPU realize is pretty late in startup so you may need to rearrange some other stuff to be sure that it will work OK. If that absolutely can't work then we could do this in CPU init, but that would be a bit messier. > --- a/vl.c > +++ b/vl.c > @@ -4045,6 +4045,13 @@ int main(int argc, char **argv, char **envp) > object_property_add_child(object_get_root(), "machine", >OBJECT(current_machine), &error_abort); > > +/* > + * Compute target page size dynamically if arch supports > + * multiple page sizes. Ex: ARM > + */ > +if (machine_class->update_target_page_size) > +machine_class->update_target_page_size(cpu_model); > + > init_l1_page_table_param(); Page size isn't board specific so you don't need any of these hook functions in the machine class. thanks -- PMM
Re: [Qemu-devel] [RFC PATCH v1 2/4] exec.c: Remove static allocation of sub_section of sub_page
On 13/06/2016 11:08, vija...@caviumnetworks.com wrote: > From: Vijaya Kumar K > > Allocate sub_section dynamically. Remove dependency > on TARGET_PAGE_SIZE to make run-time page size detection > for arm platforms. > > Signed-off-by: Vijaya Kumar K > --- > exec.c |5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/exec.c b/exec.c > index a9d465b..e803a41 100644 > --- a/exec.c > +++ b/exec.c > @@ -154,7 +154,7 @@ typedef struct subpage_t { > MemoryRegion iomem; > AddressSpace *as; > hwaddr base; > -uint16_t sub_section[TARGET_PAGE_SIZE]; > +uint16_t *sub_section; Please make this a flexible array member instead, so that you can avoid the extra pointer dereference. Thanks, Paolo > } subpage_t; > > #define PHYS_SECTION_UNASSIGNED 0 > @@ -1151,6 +1151,7 @@ static void phys_section_destroy(MemoryRegion *mr) > if (have_sub_page) { > subpage_t *subpage = container_of(mr, subpage_t, iomem); > object_unref(OBJECT(&subpage->iomem)); > +g_free(subpage->sub_section); > g_free(subpage); > } > } > @@ -2272,7 +2273,7 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr > base) > subpage_t *mmio; > > mmio = g_malloc0(sizeof(subpage_t)); > - > +mmio->sub_section = g_malloc0(TARGET_PAGE_SIZE * sizeof(uint16_t)); > mmio->as = as; > mmio->base = base; > memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio, >
[Qemu-devel] [PATCH trivial 1/2] hw/bt/hci: spelling fix: endianness
Signed-off-by: Michael Tokarev --- hw/bt/hci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/bt/hci.c b/hw/bt/hci.c index 7d52205..963f683 100644 --- a/hw/bt/hci.c +++ b/hw/bt/hci.c @@ -421,7 +421,7 @@ static void bt_submit_raw_acl(struct bt_piconet_s *net, int length, uint8_t *dat /* HCI layer emulation */ -/* Note: we could ignore endiannes because unswapped handles will still +/* Note: we could ignore endianness because unswapped handles will still * be valid as connection identifiers for the guest - they don't have to * be continuously allocated. We do it though, to preserve similar * behaviour between hosts. Some things, like the BD_ADDR cannot be -- 2.1.4
Re: [Qemu-devel] [PATCH V3] block/iscsi: allow caching of the allocation map
On 30/05/2016 08:33, Peter Lieven wrote: > > The idea of the allocmap in cache.direct = on mode is that we can > still speed up block jobs by skipping large unallocated areas. In this case > the allocmap has only a hint character. If we don't know the status > we issue a get_block_status request and verify the status. If its > unallocated > we return zeroes. If we new through an earlier get block status request > that the area is allocated we can skip the useless get_block_status > request. > This is the old behaviour without this patch. I'm adding a note like this: diff --git a/block/iscsi.c b/block/iscsi.c index fa03028..299b23c 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -523,6 +523,9 @@ static void iscsi_allocmap_set_unallocated(IscsiLun *iscsilun, int64_t sector_num, int nb_sectors) { +/* Note: if cache.direct=on the third argument to iscsi_allocmap_update + * is ignored, so this will in effect be an iscsi_allocmap_set_invalid. + */ iscsi_allocmap_update(iscsilun, sector_num, nb_sectors, false, true); } Paolo
[Qemu-devel] [PATCH] arm: spelling fix: mismatch
Signed-off-by: Michael Tokarev --- target-arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 862e780..5571ad6 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -7354,7 +7354,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, * is unpredictable. Flag this as a guest error. */ if (sign != sext) { qemu_log_mask(LOG_GUEST_ERROR, - "AArch32: VTCR.S / VTCR.T0SZ[3] missmatch\n"); + "AArch32: VTCR.S / VTCR.T0SZ[3] mismatch\n"); } } t1sz = extract32(tcr->raw_tcr, 16, 6); -- 2.1.4
[Qemu-devel] [PATCH trivial 2/2] hw/dma/omap: spelling fix: endianness
Signed-off-by: Michael Tokarev --- hw/dma/omap_dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c index 700cd6b..f6f86f9 100644 --- a/hw/dma/omap_dma.c +++ b/hw/dma/omap_dma.c @@ -1975,7 +1975,7 @@ static void omap_dma4_write(void *opaque, hwaddr addr, ch->endian[1] =(value >> 19) & 1; ch->endian_lock[1] =(value >> 18) & 1; if (ch->endian[0] != ch->endian[1]) -fprintf(stderr, "%s: DMA endiannes conversion enable attempt\n", +fprintf(stderr, "%s: DMA endianness conversion enable attempt\n", __FUNCTION__); ch->write_mode = (value >> 16) & 3; ch->burst[1] = (value & 0xc000) >> 14; -- 2.1.4
Re: [Qemu-devel] [PATCH v2] mirror: follow AioContext change gracefully
On 12/06/2016 08:51, Fam Zheng wrote: > From: Stefan Hajnoczi > > When dataplane is enabled or disabled the drive switches to a new > AioContext. The mirror block job must also move to the new AioContext > so that drive accesses are always made within its AioContext. > > This patch partially achieves that by draining target and source > requests to reach a quiescent point. The job is resumed in the new > AioContext after moving s->target into the new AioContext. > > The quiesce_requested flag is added to deal with yield points in > block_job_sleep_ns(), bdrv_is_allocated_above(), and > bdrv_get_block_status_above(). Previously they continue executing in > the old AioContext. The nested aio_poll in mirror_detach_aio_context > will drive the mirror coroutine upto fixed yield points, where > mirror_check_for_quiesce is called. > > Cc: Fam Zheng > Cc: Paolo Bonzini > Cc: Jeff Cody > Signed-off-by: Stefan Hajnoczi > [Drain source as well, and add s->quiesce_requested flag. -- Fam] > Signed-off-by: Fam Zheng As discussed on IRC, perhaps we can reuse the pausing/job->busy mechanism to detect quiescence. There's no synchronous pause function, but it can be realized with block_job_pause and aio_poll. Also while discussing this patch on IRC Fam noticed that resume currently clears the job's iostatus. I think that functionality can be moved to the QMP command. Thanks, Paolo > --- > > v2: Picked up Stefan's RFC patch and move on towards a more complete > fix. Please review! > > Jason: it would be nice if you could test this version again. It differs > from the previous version. > --- > block/mirror.c | 45 - > 1 file changed, 44 insertions(+), 1 deletion(-) > > diff --git a/block/mirror.c b/block/mirror.c > index 80fd3c7..142199a 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -63,6 +63,8 @@ typedef struct MirrorBlockJob { > int ret; > bool unmap; > bool waiting_for_io; > +bool quiesce_requested; /* temporarily detached to move AioContext, > + don't do more I/O */ > int target_cluster_sectors; > int max_iov; > } MirrorBlockJob; > @@ -119,7 +121,7 @@ static void mirror_iteration_done(MirrorOp *op, int ret) > qemu_iovec_destroy(&op->qiov); > g_free(op); > > -if (s->waiting_for_io) { > +if (s->waiting_for_io && !s->quiesce_requested) { > qemu_coroutine_enter(s->common.co, NULL); > } > } > @@ -307,6 +309,14 @@ static void mirror_do_zero_or_discard(MirrorBlockJob *s, > } > } > > +static void coroutine_fn mirror_check_for_quiesce(MirrorBlockJob *s) > +{ > +if (s->quiesce_requested) { > +s->quiesce_requested = false; > +qemu_coroutine_yield(); > +} > +} > + > static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) > { > BlockDriverState *source = blk_bs(s->common.blk); > @@ -331,6 +341,7 @@ static uint64_t coroutine_fn > mirror_iteration(MirrorBlockJob *s) > mirror_wait_for_io(s); > } > > +mirror_check_for_quiesce(s); > /* Find the number of consective dirty chunks following the first dirty > * one, and wait for in flight requests in them. */ > while (nb_chunks * sectors_per_chunk < (s->buf_size >> > BDRV_SECTOR_BITS)) { > @@ -442,6 +453,31 @@ static void mirror_drain(MirrorBlockJob *s) > } > } > > +static void mirror_attached_aio_context(AioContext *new_context, void > *opaque) > +{ > +MirrorBlockJob *s = opaque; > + > +blk_set_aio_context(s->target, new_context); > + > +/* Resume execution */ > +assert(!s->quiesce_requested); > +if (s->waiting_for_io) { > +qemu_coroutine_enter(s->common.co, NULL); > +} > +} > + > +static void mirror_detach_aio_context(void *opaque) > +{ > +MirrorBlockJob *s = opaque; > + > +/* Complete pending write requests */ > +assert(!s->quiesce_requested); > +s->quiesce_requested = true; > +while (s->quiesce_requested || s->in_flight) { > +aio_poll(blk_get_aio_context(s->common.blk), true); > +} > +} > + > typedef struct { > int ret; > } MirrorExitData; > @@ -491,6 +527,8 @@ static void mirror_exit(BlockJob *job, void *opaque) > if (replace_aio_context) { > aio_context_release(replace_aio_context); > } > +blk_remove_aio_context_notifier(s->common.blk, > mirror_attached_aio_context, > +mirror_detach_aio_context, s); > g_free(s->replaces); > bdrv_op_unblock_all(target_bs, s->common.blocker); > blk_unref(s->target); > @@ -583,6 +621,7 @@ static void coroutine_fn mirror_run(void *opaque) > block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, 0); > } > > +mirror_check_for_quiesce(s); > if (block_job_is_cancelled(&s->common)) { > goto immediate_exit; > } > @@ -612,6 +651,7 @@ static void coroutine_fn mirror_run(vo
Re: [Qemu-devel] [PATCH v3 1/2] target-i386: KVM: add basic Intel LMCE support
On 06/13/16 10:33, Paolo Bonzini wrote: > > > On 13/06/2016 09:55, Haozhong Zhang wrote: > > Currently, only VMX bits (bit 1 & 2), LMCE bit (bit 20) as well as > > lock bit (bit 0) in MSR_IA32_FEATURE_CONTROL are used for guest. The > > availability of features indicated by those bits (except the lock bit) > > can be discovered from cpuid and other MSR, so it looks not necessary > > to publish them via fw_cfg. Or do you have other concerns? > > I would prefer to avoid having to change the firmware (SeaBIOS and OVMF) > every time a new bit is added. Using fw_cfg makes it possible to > develop the feature in the firmware once and for all. > Thanks for the explanation! Is it proper to add a key in fw_cfg for this purpose, e.g FW_CFG_MSR_FEATURE_CONTROL to pass bits that are supposed to be set by firmware? Thanks, Haozhong
Re: [Qemu-devel] [PATCH] target-i386: kvm: cache KVM_GET_SUPPORTED_CPUID data
On 13/06/2016 04:21, Chao Peng wrote: > KVM_GET_SUPPORTED_CPUID ioctl is called frequently when initializing > CPU. Depends on CPU features and CPU count, the number of calls can be > extremely high which slows down QEMU booting significantly. In our > testing, we saw 5922 calls with switches: > > -cpu SandyBridge -smp 6,sockets=6,cores=1,threads=1 > > This ioctl takes more than 100ms, which is almost half of the total > QEMU startup time. > > While for most cases the data returned from two different invocations > are not changed, that means, we can cache the data to avoid trapping > into kernel for the second time. To make sure the cache safe one > assumption is desirable: the ioctl is stateless. This is not true > however, at least for some CPUID leaves. Which are the CPUID leaves for which KVM_GET_SUPPORTED_CPUID is not stateless? I cannot find any. > The good part is even the ioctl is not fully stateless, we can still > cache the return value if we know the data is unchanged for the leaves > we are interested in. Actually this should be true for most invocations > and looks all the places in current code hold true. > > A non-cached version can be introduced if refresh is required in the > future. [...] > > +static Notifier kvm_exit_notifier; > +static void kvm_arch_destroy(Notifier *n, void *unused) > +{ > +g_free(cpuid_cache); > +} > + > int kvm_arch_init(MachineState *ms, KVMState *s) > { > uint64_t identity_base = 0xfffbc000; > @@ -1165,6 +1176,9 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > smram_machine_done.notify = register_smram_listener; > qemu_add_machine_init_done_notifier(&smram_machine_done); > } > + > +kvm_exit_notifier.notify = kvm_arch_destroy; > +qemu_add_exit_notifier(&kvm_exit_notifier); > return 0; This part is unnecessary; the OS takes care of freeing the heap on exit. Thanks, Paolo
Re: [Qemu-devel] [PATCH] scsi-disk: Use (unsigned long) typecasts when using "%lu" format string
On 13/06/2016 10:10, Thomas Huth wrote: > Some source code analyzers like cppcheck spill out a warning if > the sign of the argument does not match the format string. > > Ticket: https://bugs.launchpad.net/qemu/+bug/1589564 > Signed-off-by: Thomas Huth > --- > hw/scsi/scsi-disk.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c > index 1881969..36f8a85 100644 > --- a/hw/scsi/scsi-disk.c > +++ b/hw/scsi/scsi-disk.c > @@ -2060,13 +2060,13 @@ static int32_t scsi_disk_emulate_command(SCSIRequest > *req, uint8_t *buf) > } > break; > case MODE_SELECT: > -DPRINTF("Mode Select(6) (len %lu)\n", (long)r->req.cmd.xfer); > +DPRINTF("Mode Select(6) (len %lu)\n", (unsigned > long)r->req.cmd.xfer); > break; > case MODE_SELECT_10: > -DPRINTF("Mode Select(10) (len %lu)\n", (long)r->req.cmd.xfer); > +DPRINTF("Mode Select(10) (len %lu)\n", (unsigned > long)r->req.cmd.xfer); > break; > case UNMAP: > -DPRINTF("Unmap (len %lu)\n", (long)r->req.cmd.xfer); > +DPRINTF("Unmap (len %lu)\n", (unsigned long)r->req.cmd.xfer); > break; > case VERIFY_10: > case VERIFY_12: > @@ -2080,7 +2080,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest > *req, uint8_t *buf) > case WRITE_SAME_16: > DPRINTF("WRITE SAME %d (len %lu)\n", > req->cmd.buf[0] == WRITE_SAME_10 ? 10 : 16, > -(long)r->req.cmd.xfer); > +(unsigned long)r->req.cmd.xfer); > break; > default: > DPRINTF("Unknown SCSI command (%2.2x=%s)\n", buf[0], > Queued, thanks. Paolo
Re: [Qemu-devel] [RFC PATCH v1 3/4] translate-all.c: Compute L1 page table properties at runtime
On 13/06/2016 11:36, Peter Maydell wrote: >>> /* The bottom level has pointers to PageDesc */ >>> >> -static void *l1_map[V_L1_SIZE]; >>> >> +static void *l1_map; >> > >> > You can make this array have a static V_L2_SIZE * 16 size too. Peter, >> > what do you think? > I don't know this code well enough to have an informed view, > but we only allocate this once at startup, right? I'm not sure > why making it a static array would be better? It makes accesses faster by avoiding a pointer load. On one hand it might be just microoptimization, on the other hand... death by one thousand papercuts... Paolo
Re: [Qemu-devel] [PATCH v3 1/2] target-i386: KVM: add basic Intel LMCE support
On 13/06/2016 12:01, Haozhong Zhang wrote: > > I would prefer to avoid having to change the firmware (SeaBIOS and OVMF) > > every time a new bit is added. Using fw_cfg makes it possible to > > develop the feature in the firmware once and for all. > > Thanks for the explanation! Is it proper to add a key in fw_cfg for > this purpose, e.g FW_CFG_MSR_FEATURE_CONTROL to pass bits that are > supposed to be set by firmware? We usually add new files now, so it would be a new file named "etc/msr-feature-control". Thanks, Paolo
Re: [Qemu-devel] [PATCH v6 1/1] Introduce "xen-load-devices-state"
On Fri, 3 Jun 2016, Anthony PERARD wrote: > On Fri, Jun 03, 2016 at 05:58:34PM +0800, Changlong Xie wrote: > > From: Wen Congyang > > > > Introduce a "xen-load-devices-state" QAPI command that can be used to > > load the state of all devices, but not the RAM or the block devices of > > the VM. > > > > We only have hmp commands savevm/loadvm, and qmp commands > > xen-save-devices-state. > > > > We use this new command for COLO: > > 1. suspend both primary vm and secondary vm > > 2. sync the state > > 3. resume both primary vm and secondary vm > > > > In such case, we need to update all devices' state in any time. > > > > Signed-off-by: Wen Congyang > > Signed-off-by: Changlong Xie > > Reviewed-by: Anthony PERARD David, can we still have your Ack on this patch? > > --- > > migration/savevm.c | 37 + > > qapi-schema.json | 14 ++ > > qmp-commands.hx| 27 +++ > > 3 files changed, 78 insertions(+) > > > > diff --git a/migration/savevm.c b/migration/savevm.c > > index 6c21231..ae2ef8b 100644 > > --- a/migration/savevm.c > > +++ b/migration/savevm.c > > @@ -31,6 +31,7 @@ > > #include "hw/boards.h" > > #include "hw/hw.h" > > #include "hw/qdev.h" > > +#include "hw/xen/xen.h" > > #include "net/net.h" > > #include "monitor/monitor.h" > > #include "sysemu/sysemu.h" > > @@ -1754,6 +1755,12 @@ qemu_loadvm_section_start_full(QEMUFile *f, > > MigrationIncomingState *mis) > > return -EINVAL; > > } > > > > +/* Validate if it is a device's state */ > > +if (xen_enabled() && se->is_ram) { > > +error_report("loadvm: %s RAM loading not allowed on Xen", idstr); > > +return -EINVAL; > > +} > > + > > /* Add entry */ > > le = g_malloc0(sizeof(*le)); > > > > @@ -2064,6 +2071,36 @@ void qmp_xen_save_devices_state(const char > > *filename, Error **errp) > > } > > } > > > > +void qmp_xen_load_devices_state(const char *filename, Error **errp) > > +{ > > +QEMUFile *f; > > +QIOChannelFile *ioc; > > +int ret; > > + > > +/* Guest must be paused before loading the device state; the RAM state > > + * will already have been loaded by xc > > + */ > > +if (runstate_is_running()) { > > +error_setg(errp, "Cannot update device state while vm is running"); > > +return; > > +} > > +vm_stop(RUN_STATE_RESTORE_VM); > > + > > +ioc = qio_channel_file_new_path(filename, O_RDONLY | O_BINARY, 0, > > errp); > > +if (!ioc) { > > +return; > > +} > > +f = qemu_fopen_channel_input(QIO_CHANNEL(ioc)); > > + > > +migration_incoming_state_new(f); > > +ret = qemu_loadvm_state(f); > > +qemu_fclose(f); > > +if (ret < 0) { > > +error_setg(errp, QERR_IO_ERROR); > > +} > > +migration_incoming_state_destroy(); > > +} > > + > > int load_vmstate(const char *name) > > { > > BlockDriverState *bs, *bs_vm_state; > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 8483bdf..48c3a6f 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -4201,6 +4201,20 @@ > >'data': [ 'none', 'record', 'play' ] } > > > > ## > > +# @xen-load-devices-state: > > +# > > +# Load the state of all devices from file. The RAM and the block devices > > +# of the VM are not loaded by this command. > > +# > > +# @filename: the file to load the state of the devices from as binary > > +# data. See xen-save-devices-state.txt for a description of the binary > > +# format. > > +# > > +# Since: 2.7 > > +## > > +{ 'command': 'xen-load-devices-state', 'data': {'filename': 'str'} } > > + > > +## > > # @GICCapability: > > # > > # The struct describes capability for a specific GIC (Generic > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index 28801a2..780e7f2 100644 > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -587,6 +587,33 @@ Example: > > EQMP > > > > { > > +.name = "xen-load-devices-state", > > +.args_type = "filename:F", > > +.mhandler.cmd_new = qmp_marshal_xen_load_devices_state, > > +}, > > + > > +SQMP > > +xen-load-devices-state > > +-- > > + > > +Load the state of all devices from file. The RAM and the block devices > > +of the VM are not loaded by this command. > > + > > +Arguments: > > + > > +- "filename": the file to load the state of the devices from as binary > > +data. See xen-save-devices-state.txt for a description of the binary > > +format. > > + > > +Example: > > + > > +-> { "execute": "xen-load-devices-state", > > + "arguments": { "filename": "/tmp/resume" } } > > +<- { "return": {} } > > + > > +EQMP > > + > > +{ > > .name = "xen-set-global-dirty-log", > > .args_type = "enable:b", > > .mhandler.cmd_new = qmp_marshal_xen_set_global_dirty_log, > > -- > > 1.9.3 > > > > > > > > -- > Anthony PERARD >
Re: [Qemu-devel] [PATCH v3 1/2] target-i386: KVM: add basic Intel LMCE support
On 06/13/16 12:07, Paolo Bonzini wrote: > > > On 13/06/2016 12:01, Haozhong Zhang wrote: > > > I would prefer to avoid having to change the firmware (SeaBIOS and OVMF) > > > every time a new bit is added. Using fw_cfg makes it possible to > > > develop the feature in the firmware once and for all. > > > > Thanks for the explanation! Is it proper to add a key in fw_cfg for > > this purpose, e.g FW_CFG_MSR_FEATURE_CONTROL to pass bits that are > > supposed to be set by firmware? > > We usually add new files now, so it would be a new file named > "etc/msr-feature-control". > Got it, thanks! Haozhong
Re: [Qemu-devel] [RFC PATCH v1 4/4] target-arm: Compute page size based on ARM target cpu type
On 13 June 2016 at 10:43, Peter Maydell wrote: > On 13 June 2016 at 10:08, wrote: >> +/* >> + * Holds TARGET_AARCH_64_PAGE_BITS or TARGET_ARM_PAGE_BITS >> + * based on the the cpu type emulated at runtime. >> + */ >> +static uint32_t target_page_bits; > > The CPU page size is not specific to the 'virt' board, so this > is the wrong place to do this. You should identify the > page size in arm_cpu_realizefn() based on the set of feature > bits the CPU has: anything with ARM_FEATURE_V7 has a 4K page > table (this includes a lot of 32-bit CPUs). Actually that should be "with ARM_FEATURE_V7 and not ARM_FEATURE_MPU", or we'll break the PMSA code. Note that you'll also need to handle systems where the different CPUs in it disagree about the preferred target page size -- the xlnx-ep108 board can have both Cortex-A53 (prefers 4K) and Cortex-R5 (prefers 1K) CPUs in it. "Use the smallest value required by any CPU on the board" is probably the best approach. thanks -- PMM
Re: [Qemu-devel] [PULL v2 00/15] tb hash improvements
On 12 June 2016 at 01:20, Richard Henderson wrote: > Pull v2, with the fixed ppc cpu_relax, and the two %lu fixes. > Not re-sending the rest of the patch set. > > > r~ > > > The following changes since commit a93c1bdf0bd4689287094ddb2aae3dc907da3535: > > Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20160610-1' into > staging (2016-06-10 15:47:17 +0100) > > are available in the git repository at: > > git://github.com/rth7680/qemu.git tags/pull-tcg-20160611 > > for you to fetch changes up to 329844d4bc3d5a11f1e63938d66f74c9584c7abc: > > translate-all: add tb hash bucket info to 'info jit' dump (2016-06-11 > 17:11:16 -0700) > > > TB hashing improvements Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v8 12/17] pci: Convert msi_init() to Error and fix callers to check it
On 06/10/2016 12:54 PM, Cao jin wrote: msi_init() reports errors with error_report(), which is wrong when it's used in realize(). Fix by converting it to Error. Fix its callers to handle failure instead of ignoring it. For those callers who don't handle the failure, it might happen: when user want msi on, but he doesn't get what he want because of msi_init fails silently. cc: Gerd Hoffmann cc: John Snow cc: Dmitry Fleytman cc: Jason Wang cc: Michael S. Tsirkin cc: Hannes Reinecke cc: Paolo Bonzini cc: Alex Williamson cc: Markus Armbruster cc: Marcel Apfelbaum Reviewed-by: Markus Armbruster Signed-off-by: Cao jin --- hw/audio/intel-hda.c | 24 hw/ide/ich.c | 15 +-- hw/net/e1000e.c| 8 ++-- hw/net/vmxnet3.c | 37 - hw/pci-bridge/ioh3420.c| 6 +- hw/pci-bridge/pci_bridge_dev.c | 20 hw/pci-bridge/xio3130_downstream.c | 6 +- hw/pci-bridge/xio3130_upstream.c | 6 +- hw/pci/msi.c | 11 --- hw/scsi/megasas.c | 26 +- hw/scsi/mptsas.c | 31 --- hw/scsi/vmw_pvscsi.c | 2 +- hw/usb/hcd-xhci.c | 23 +++ hw/vfio/pci.c | 7 +-- include/hw/pci/msi.h | 3 ++- 15 files changed, 154 insertions(+), 71 deletions(-) diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c index 6b4dda0..82101f8 100644 --- a/hw/audio/intel-hda.c +++ b/hw/audio/intel-hda.c @@ -1135,6 +1135,8 @@ static void intel_hda_realize(PCIDevice *pci, Error **errp) { IntelHDAState *d = INTEL_HDA(pci); uint8_t *conf = d->pci.config; +Error *err = NULL; +int ret; d->name = object_get_typename(OBJECT(d)); @@ -1143,13 +1145,27 @@ static void intel_hda_realize(PCIDevice *pci, Error **errp) /* HDCTL off 0x40 bit 0 selects signaling mode (1-HDA, 0 - Ac97) 18.1.19 */ conf[0x40] = 0x01; +if (d->msi != ON_OFF_AUTO_OFF) { +ret = msi_init(&d->pci, d->old_msi_addr ? 0x50 : 0x60, + 1, true, false, &err); +/* Any error other than -ENOTSUP(board's MSI support is broken) + * is a programming error */ +assert(!ret || ret == -ENOTSUP); +if (ret && d->msi == ON_OFF_AUTO_ON) { +/* Can't satisfy user's explicit msi=on request, fail */ +error_append_hint(&err, "You have to use msi=auto (default) or " +"msi=off with this machine type.\n"); +error_propagate(errp, err); +return; +} +assert(!err || d->msi == ON_OFF_AUTO_AUTO); +/* With msi=auto, we fall back to MSI off silently */ +error_free(err); +} + memory_region_init_io(&d->mmio, OBJECT(d), &intel_hda_mmio_ops, d, "intel-hda", 0x4000); pci_register_bar(&d->pci, 0, 0, &d->mmio); -if (d->msi != ON_OFF_AUTO_OFF) { - /* TODO check for errors */ -msi_init(&d->pci, d->old_msi_addr ? 0x50 : 0x60, 1, true, false); -} hda_codec_bus_init(DEVICE(pci), &d->codecs, sizeof(d->codecs), intel_hda_response, intel_hda_xfer); diff --git a/hw/ide/ich.c b/hw/ide/ich.c index 0a13334..084bef8 100644 --- a/hw/ide/ich.c +++ b/hw/ide/ich.c @@ -68,7 +68,6 @@ #include #include "sysemu/block-backend.h" #include "sysemu/dma.h" - #include #include @@ -111,6 +110,15 @@ static void pci_ich9_ahci_realize(PCIDevice *dev, Error **errp) int sata_cap_offset; uint8_t *sata_cap; d = ICH_AHCI(dev); +int ret; + +/* Although the AHCI 1.3 specification states that the first capability + * should be PMCAP, the Intel ICH9 data sheet specifies that the ICH9 + * AHCI device puts the MSI capability first, pointing to 0x80. */ +ret = msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false, NULL); +/* Any error other than -ENOTSUP(board's MSI support is broken) + * is a programming error. Fall back to INTx silently on -ENOTSUP */ +assert(!ret || ret == -ENOTSUP); ahci_realize(&d->ahci, DEVICE(dev), pci_get_address_space(dev), 6); @@ -142,11 +150,6 @@ static void pci_ich9_ahci_realize(PCIDevice *dev, Error **errp) pci_set_long(sata_cap + SATA_CAP_BAR, (ICH9_IDP_BAR + 0x4) | (ICH9_IDP_INDEX_LOG2 << 4)); d->ahci.idp_offset = ICH9_IDP_INDEX; - -/* Although the AHCI 1.3 specification states that the first capability - * should be PMCAP, the Intel ICH9 data sheet specifies that the ICH9 - * AHCI device puts the MSI capability first, pointing to 0x80. */ -msi_init(dev, ICH9_MSI_CAP_OFFSET, 1, true, false); } static void pci_ich9_uninit(PCIDevice *dev) diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c index 692283f..a
Re: [Qemu-devel] [PATCH v2 2/3] hw/iommu: enable iommu with -device
On 06/12/2016 07:27 AM, Peter Xu wrote: On Thu, Jun 02, 2016 at 11:15:54PM +0300, Marcel Apfelbaum wrote: [...] static void vtd_realize(DeviceState *dev, Error **errp) { +PCIBus *bus = PC_MACHINE(qdev_get_machine())->bus; IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev); VTD_DPRINTF(GENERAL, ""); @@ -2029,6 +2043,9 @@ static void vtd_realize(DeviceState *dev, Error **errp) s->vtd_as_by_busptr = g_hash_table_new_full(vtd_uint64_hash, vtd_uint64_equal, g_free, g_free); vtd_init(s); +sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, Q35_HOST_BRIDGE_IOMMU_ADDR); +bus->iommu_fn = vtd_host_dma_iommu; +bus->iommu_opaque = dev; Here, shall we still use pci_setup_iommu() to keep the two fields private for pci framework? I've already spotted it and took care of it, thanks :) ! Btw, I am rebasing Intel IR work onto this patchset, but encountered issues (guest hang, or errornous interrupts) when guest specify more than 1 vcpus (everything is cool as long as vcpu=1). Maybe there is something wrong during the rebase, still investigating. Please shoot if there is any clue. I am running with 2 vcpus and I didn't see any problem, I'll let you know if can reproduce. Thanks, Marcel Thanks, -- peterx
[Qemu-devel] [QEMU 1/7] balloon: speed up inflating & deflating process
The implementation of the current virtio-balloon is not very efficient, Bellow is test result of time spends on inflating the balloon to 3GB of a 4GB idle guest: a. allocating pages (6.5%, 103ms) b. sending PFNs to host (68.3%, 787ms) c. address translation (6.1%, 96ms) d. madvise (19%, 300ms) It takes about 1577ms for the whole inflating process to complete. The test shows that the bottle neck is the stage b and stage d. If using a bitmap to send the page info instead of the PFNs, we can reduce the overhead spends on stage b quite a lot. Furthermore, it's possible to do the address translation and do the madvise with a bulk of pages, instead of the current page per page way, so the overhead of stage c and stage d can also be reduced a lot. This patch is the QEMU side implementation which is intended to speed up the inflating & deflating process by adding a new feature to the virtio-balloon device. And now, inflating the balloon to 3GB of a 4GB idle guest only takes 210ms, it's about 8 times as fast as before. TODO: optimize stage a by allocating/freeing a chunk of pages instead of a single page at a time. Signed-off-by: Liang Li --- hw/virtio/virtio-balloon.c | 159 include/standard-headers/linux/virtio_balloon.h | 1 + 2 files changed, 139 insertions(+), 21 deletions(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 8c15e09..8cf74c2 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -47,6 +47,76 @@ static void balloon_page(void *addr, int deflate) #endif } +static void do_balloon_bulk_pages(ram_addr_t base_pfn, int page_shift, + unsigned long len, bool deflate) +{ +ram_addr_t size, processed, chunk, base; +void *addr; +MemoryRegionSection section = {.mr = NULL}; + +size = (len << page_shift); +base = (base_pfn << page_shift); + +for (processed = 0; processed < size; processed += chunk) { +chunk = size - processed; +while (chunk >= TARGET_PAGE_SIZE) { +section = memory_region_find(get_system_memory(), + base + processed, chunk); +if (!section.mr) { +chunk = QEMU_ALIGN_DOWN(chunk / 2, TARGET_PAGE_SIZE); +} else { +break; +} +} + +if (section.mr && +(int128_nz(section.size) && memory_region_is_ram(section.mr))) { +addr = section.offset_within_region + + memory_region_get_ram_ptr(section.mr); +qemu_madvise(addr, chunk, + deflate ? QEMU_MADV_WILLNEED : QEMU_MADV_DONTNEED); +} else { +fprintf(stderr, "can't find the chunk, skip\n"); +chunk = TARGET_PAGE_SIZE; +} +} +} + +static void balloon_bulk_pages(ram_addr_t base_pfn, unsigned long *bitmap, + unsigned long len, int page_shift, bool deflate) +{ +#if defined(__linux__) +unsigned long end = len * 8; +unsigned long current = 0; + +if (!qemu_balloon_is_inhibited() && (!kvm_enabled() || + kvm_has_sync_mmu())) { +while (current < end) { +unsigned long one = find_next_bit(bitmap, end, current); + +if (one < end) { +unsigned long zero = find_next_zero_bit(bitmap, end, one + 1); +unsigned long page_length; + +if (zero >= end) { +page_length = end - one; +} else { +page_length = zero - one; +} + +if (page_length) { +do_balloon_bulk_pages(base_pfn + one, page_shift, + page_length, deflate); +} +current = one + page_length; +} else { +current = one; +} +} +} +#endif +} + static const char *balloon_stat_names[] = { [VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in", [VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out", @@ -78,6 +148,12 @@ static bool balloon_stats_supported(const VirtIOBalloon *s) return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_STATS_VQ); } +static bool balloon_page_bitmap_supported(const VirtIOBalloon *s) +{ +VirtIODevice *vdev = VIRTIO_DEVICE(s); +return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_BITMAP); +} + static bool balloon_stats_enabled(const VirtIOBalloon *s) { return s->stats_poll_interval > 0; @@ -224,27 +300,66 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq) return; } -while (iov_to_buf(elem->out_sg, elem->out_num, offset, &pfn, 4) == 4) { -ram_addr_t pa; -ram_addr_t addr; -int p = virtio_ldl_p(vdev, &pfn); - -pa = (ram_addr_t) p << VIRTIO_BALLOON_PFN_SHIFT; -of
[Qemu-devel] [QEMU 0/7] Fast balloon and fast live migration
This patch set is intended to speed up the inflating/deflating process of virtio-balloon and speed up live migration by skipping process guest's free pages. The virtio-balloon is extended to support some new features, so as to make things faster. Liang Li (7): balloon: speed up inflating & deflating process virtio-balloon: add drop cache support Add the hmp and qmp interface for dropping cache balloon: get free page info from guest bitmap: Add a new bitmap_move function kvm: Add two new arch specific functions migration: skip free pages during live migration balloon.c | 51 +++- hmp-commands.hx | 15 ++ hmp.c | 22 ++ hmp.h | 3 + hw/virtio/virtio-balloon.c | 315 ++-- include/hw/virtio/virtio-balloon.h | 23 +- include/qemu/bitmap.h | 13 + include/standard-headers/linux/virtio_balloon.h | 2 + include/sysemu/balloon.h| 13 +- include/sysemu/kvm.h| 2 + migration/ram.c | 93 +++ monitor.c | 18 ++ qapi-schema.json| 35 +++ qmp-commands.hx | 23 ++ target-arm/kvm.c| 14 ++ target-i386/kvm.c | 35 +++ target-mips/kvm.c | 14 ++ target-ppc/kvm.c| 14 ++ target-s390x/kvm.c | 14 ++ 19 files changed, 693 insertions(+), 26 deletions(-) -- 1.9.1
[Qemu-devel] [QEMU 2/7] virtio-balloon: add drop cache support
virtio-balloon can make use of the amount of free memory to determine the amount of memory to be filled in the balloon, but the amount of free memory will be effected by the page cache, which can be reclaimed. Drop the cache before getting the amount of free memory will be very helpful to relect the exact amount of memroy that can be reclaimed. This patch add a new feature to the balloon device to support this operation, hypervisor can request the VM to drop it's cache, so as to reclaim more memory. Signed-off-by: Liang Li --- balloon.c | 10 ++- hw/virtio/virtio-balloon.c | 85 - include/hw/virtio/virtio-balloon.h | 19 +- include/standard-headers/linux/virtio_balloon.h | 1 + include/sysemu/balloon.h| 5 +- 5 files changed, 115 insertions(+), 5 deletions(-) diff --git a/balloon.c b/balloon.c index f2ef50c..0fb34bf 100644 --- a/balloon.c +++ b/balloon.c @@ -36,6 +36,7 @@ static QEMUBalloonEvent *balloon_event_fn; static QEMUBalloonStatus *balloon_stat_fn; +static QEMUBalloonDropCache *balloon_drop_cache_fn; static void *balloon_opaque; static bool balloon_inhibited; @@ -65,9 +66,12 @@ static bool have_balloon(Error **errp) } int qemu_add_balloon_handler(QEMUBalloonEvent *event_func, - QEMUBalloonStatus *stat_func, void *opaque) + QEMUBalloonStatus *stat_func, + QEMUBalloonDropCache *drop_cache_func, + void *opaque) { -if (balloon_event_fn || balloon_stat_fn || balloon_opaque) { +if (balloon_event_fn || balloon_stat_fn || balloon_drop_cache_fn +|| balloon_opaque) { /* We're already registered one balloon handler. How many can * a guest really have? */ @@ -75,6 +79,7 @@ int qemu_add_balloon_handler(QEMUBalloonEvent *event_func, } balloon_event_fn = event_func; balloon_stat_fn = stat_func; +balloon_drop_cache_fn = drop_cache_func; balloon_opaque = opaque; return 0; } @@ -86,6 +91,7 @@ void qemu_remove_balloon_handler(void *opaque) } balloon_event_fn = NULL; balloon_stat_fn = NULL; +balloon_drop_cache_fn = NULL; balloon_opaque = NULL; } diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 8cf74c2..4757ba5 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -36,6 +36,10 @@ #define BALLOON_PAGE_SIZE (1 << VIRTIO_BALLOON_PFN_SHIFT) +enum balloon_req_id { + BALLOON_DROP_CACHE, +}; + static void balloon_page(void *addr, int deflate) { #if defined(__linux__) @@ -154,6 +158,12 @@ static bool balloon_page_bitmap_supported(const VirtIOBalloon *s) return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_BITMAP); } +static bool balloon_misc_supported(const VirtIOBalloon *s) +{ +VirtIODevice *vdev = VIRTIO_DEVICE(s); +return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_MISC); +} + static bool balloon_stats_enabled(const VirtIOBalloon *s) { return s->stats_poll_interval > 0; @@ -420,6 +430,39 @@ out: } } +static void virtio_balloon_handle_resp(VirtIODevice *vdev, VirtQueue *vq) +{ +VirtIOBalloon *s = VIRTIO_BALLOON(vdev); +VirtQueueElement *elem; +size_t offset = 0; +uint32_t tmp32, id = 0; + +elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); +if (!elem) { +s->req_status = REQ_ERROR; +return; +} + +s->misc_vq_elem = elem; + +if (!elem->out_num) { +return; +} + +iov_to_buf(elem->out_sg, elem->out_num, offset, + &tmp32, sizeof(uint32_t)); +id = virtio_ldl_p(vdev, &tmp32); +offset += sizeof(uint32_t); +switch (id) { +case BALLOON_DROP_CACHE: +s->req_status = REQ_DONE; +break; +default: +break; +} + +} + static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) { VirtIOBalloon *dev = VIRTIO_BALLOON(vdev); @@ -490,6 +533,7 @@ static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f, f |= dev->host_features; virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ); virtio_add_feature(&f, VIRTIO_BALLOON_F_PAGE_BITMAP); +virtio_add_feature(&f, VIRTIO_BALLOON_F_MISC); return f; } @@ -500,6 +544,36 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) VIRTIO_BALLOON_PFN_SHIFT); } +static int virtio_balloon_drop_cache(void *opaque, unsigned long type) +{ +VirtIOBalloon *s = opaque; +VirtIODevice *vdev = VIRTIO_DEVICE(s); +VirtQueueElement *elem = s->misc_vq_elem; +int len; + +if (!balloon_misc_supported(s)) { +return REQ_UNSUPPORT; +} + +if (elem == NULL || !elem->in_num) { +elem = virtqueue_pop(s->mvq, sizeof(VirtQueueElement)); +if (!elem) { +return REQ_ERROR; +
[Qemu-devel] [QEMU 3/7] Add the hmp and qmp interface for dropping cache
Add the hmp and qmp interface to drop vm's page cache, users can control the type of cache they want vm to drop. Signed-off-by: Liang Li --- balloon.c| 19 +++ hmp-commands.hx | 15 +++ hmp.c| 22 ++ hmp.h| 3 +++ monitor.c| 18 ++ qapi-schema.json | 35 +++ qmp-commands.hx | 23 +++ 7 files changed, 135 insertions(+) diff --git a/balloon.c b/balloon.c index 0fb34bf..3d96111 100644 --- a/balloon.c +++ b/balloon.c @@ -122,3 +122,22 @@ void qmp_balloon(int64_t target, Error **errp) trace_balloon_event(balloon_opaque, target); balloon_event_fn(balloon_opaque, target); } + +void qmp_balloon_drop_cache(DropCacheType type, Error **errp) +{ +if (!have_balloon(errp)) { +return; +} + +if (!balloon_drop_cache_fn) { +error_setg(errp, QERR_UNSUPPORTED); +return; +} +if (type < 0 && type >= DROP_CACHE_TYPE__MAX) { +error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type", + "a value in range[0, 3]"); +return; +} + +balloon_drop_cache_fn(balloon_opaque, type); +} diff --git a/hmp-commands.hx b/hmp-commands.hx index 98b4b1a..c73572c 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1378,6 +1378,21 @@ Request VM to change its memory allocation to @var{value} (in MB). ETEXI { +.name = "balloon_drop_cache", +.args_type = "type:s", +.params = "type", +.help = "request VM to drop its page caches", +.mhandler.cmd = hmp_balloon_drop_cache, +.command_completion = balloon_drop_cache_completion +}, + +STEXI +@item balloon_drop_cache @var{type} +@findex balloon_drop_cache +Request VM to dorp its page caches. +ETEXI + +{ .name = "set_link", .args_type = "name:s,up:b", .params = "name on|off", diff --git a/hmp.c b/hmp.c index a4b1d3d..3aa1062 100644 --- a/hmp.c +++ b/hmp.c @@ -1061,6 +1061,28 @@ void hmp_balloon(Monitor *mon, const QDict *qdict) } } +void hmp_balloon_drop_cache(Monitor *mon, const QDict *qdict) +{ +const char *type = qdict_get_str(qdict, "type"); +Error *err = NULL; +int i; + +for (i = 0; i < DROP_CACHE_TYPE__MAX; i++) { +if (strcmp(type, DropCacheType_lookup[i]) == 0) { +qmp_balloon_drop_cache(1 + i, &err); +break; +} +} + +if (i == DROP_CACHE_TYPE__MAX) { +error_setg(&err, QERR_INVALID_PARAMETER, type); +} + +if (err) { +error_report_err(err); +} +} + void hmp_block_resize(Monitor *mon, const QDict *qdict) { const char *device = qdict_get_str(qdict, "device"); diff --git a/hmp.h b/hmp.h index 093d65f..6bb6499 100644 --- a/hmp.h +++ b/hmp.h @@ -55,6 +55,7 @@ void hmp_nmi(Monitor *mon, const QDict *qdict); void hmp_set_link(Monitor *mon, const QDict *qdict); void hmp_block_passwd(Monitor *mon, const QDict *qdict); void hmp_balloon(Monitor *mon, const QDict *qdict); +void hmp_balloon_drop_cache(Monitor *mon, const QDict *qdict); void hmp_block_resize(Monitor *mon, const QDict *qdict); void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict); void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict); @@ -120,6 +121,8 @@ void watchdog_action_completion(ReadLineState *rs, int nb_args, const char *str); void migrate_set_capability_completion(ReadLineState *rs, int nb_args, const char *str); +void balloon_drop_cache_completion(ReadLineState *rs, int nb_args, + const char *str); void migrate_set_parameter_completion(ReadLineState *rs, int nb_args, const char *str); void host_net_add_completion(ReadLineState *rs, int nb_args, const char *str); diff --git a/monitor.c b/monitor.c index a27e115..eefdf3d 100644 --- a/monitor.c +++ b/monitor.c @@ -3367,6 +3367,24 @@ void migrate_set_parameter_completion(ReadLineState *rs, int nb_args, } } +void balloon_drop_cache_completion(ReadLineState *rs, int nb_args, + const char *str) +{ +size_t len; + +len = strlen(str); +readline_set_completion_index(rs, len); +if (nb_args == 2) { +int i; +for (i = 0; i < DROP_CACHE_TYPE__MAX; i++) { +const char *name = DropCacheType_lookup[i]; +if (!strncmp(str, name, len)) { +readline_add_completion(rs, name); +} +} +} +} + void host_net_add_completion(ReadLineState *rs, int nb_args, const char *str) { int i; diff --git a/qapi-schema.json b/qapi-schema.json index 8483bdf..117f70a 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1655,6 +1655,41 @@ { 'command': 'balloon', 'data': {'value': 'int'} } ## +# @DropCacheType +# +# Cache types enumeration +# +# @
[Qemu-devel] [QEMU 6/7] kvm: Add two new arch specific functions
Add a new function to get the vm's max pfn and a new function to filter out the holes to get a tight free page bitmap. They are implemented on X86, and all the arches should implement them for live migration optimization. Signed-off-by: Liang Li --- include/sysemu/kvm.h | 2 ++ target-arm/kvm.c | 14 ++ target-i386/kvm.c| 35 +++ target-mips/kvm.c| 14 ++ target-ppc/kvm.c | 14 ++ target-s390x/kvm.c | 14 ++ 6 files changed, 93 insertions(+) diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index ad6f837..50915f9 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -230,6 +230,8 @@ int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr, target_ulong len, int type); void kvm_remove_all_breakpoints(CPUState *cpu); int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap); +unsigned long *tighten_guest_free_page_bmap(unsigned long *bmap); +unsigned long get_guest_max_pfn(void); #ifndef _WIN32 int kvm_set_signal_mask(CPUState *cpu, const sigset_t *sigset); #endif diff --git a/target-arm/kvm.c b/target-arm/kvm.c index 83da447..6464542 100644 --- a/target-arm/kvm.c +++ b/target-arm/kvm.c @@ -627,3 +627,17 @@ int kvm_arch_msi_data_to_gsi(uint32_t data) { return (data - 32) & 0x; } + +unsigned long get_guest_max_pfn(void) +{ +/* To be done */ + +return 0; +} + +unsigned long *tighten_guest_free_page_bmap(unsigned long *bmap) +{ +/* To be done */ + +return bmap; +} diff --git a/target-i386/kvm.c b/target-i386/kvm.c index abf50e6..0b394cb 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -3327,3 +3327,38 @@ int kvm_arch_msi_data_to_gsi(uint32_t data) { abort(); } + +unsigned long get_guest_max_pfn(void) +{ +PCMachineState *pcms = PC_MACHINE(current_machine); +ram_addr_t above_4g_mem = pcms->above_4g_mem_size; +unsigned long max_pfn; + +if (above_4g_mem) { +max_pfn = ((1ULL << 32) + above_4g_mem) >> TARGET_PAGE_BITS; +} else { +max_pfn = pcms->below_4g_mem_size >> TARGET_PAGE_BITS; +} + +return max_pfn; +} + +unsigned long *tighten_guest_free_page_bmap(unsigned long *bmap) +{ +PCMachineState *pcms = PC_MACHINE(current_machine); +ram_addr_t above_4g_mem = pcms->above_4g_mem_size; + +if (above_4g_mem) { +unsigned long *src, *dst, len, pos; +ram_addr_t below_4g_mem = pcms->below_4g_mem_size; +src = bmap + ((1ULL << 32) >> TARGET_PAGE_BITS) / BITS_PER_LONG; +dst = bmap + (below_4g_mem >> TARGET_PAGE_BITS) / BITS_PER_LONG; +bitmap_move(dst, src, above_4g_mem >> TARGET_PAGE_BITS); + +pos = (above_4g_mem + below_4g_mem) >> TARGET_PAGE_BITS; +len = ((1ULL << 32) - below_4g_mem) >> TARGET_PAGE_BITS; +bitmap_clear(bmap, pos, len); +} + +return bmap; +} diff --git a/target-mips/kvm.c b/target-mips/kvm.c index a854e4d..89a54e5 100644 --- a/target-mips/kvm.c +++ b/target-mips/kvm.c @@ -1048,3 +1048,17 @@ int kvm_arch_msi_data_to_gsi(uint32_t data) { abort(); } + +unsigned long get_guest_max_pfn(void) +{ +/* To be done */ + +return 0; +} + +unsigned long *tighten_guest_free_page_bmap(unsigned long *bmap) +{ +/* To be done */ + +return bmap; +} diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index 24d6032..e222b31 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -2579,3 +2579,17 @@ int kvmppc_enable_hwrng(void) return kvmppc_enable_hcall(kvm_state, H_RANDOM); } + +unsigned long get_guest_max_pfn(void) +{ +/* To be done */ + +return 0; +} + +unsigned long *tighten_guest_free_page_bmap(unsigned long *bmap) +{ +/* To be done */ + +return bmap; +} diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 8f46fd0..893755b 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -2271,3 +2271,17 @@ int kvm_arch_msi_data_to_gsi(uint32_t data) { abort(); } + +unsigned long get_guest_max_pfn(void) +{ +/* To be done */ + +return 0; +} + +unsigned long *tighten_guest_free_page_bmap(unsigned long *bmap) +{ +/* To be done */ + +return bmap; +} -- 1.9.1
[Qemu-devel] [QEMU 4/7] balloon: get free page info from guest
Add a new feature to get the free page information from guest, the free page information is saved in a bitmap. Please note that 'free page' only means these pages are free before the request, some of the pages will become no free during the process of sending the free page bitmap to QEMU. Signed-off-by: Liang Li --- balloon.c | 24 +++- hw/virtio/virtio-balloon.c | 75 +- include/hw/virtio/virtio-balloon.h | 4 ++ include/sysemu/balloon.h | 8 4 files changed, 108 insertions(+), 3 deletions(-) diff --git a/balloon.c b/balloon.c index 3d96111..c74c472 100644 --- a/balloon.c +++ b/balloon.c @@ -37,6 +37,7 @@ static QEMUBalloonEvent *balloon_event_fn; static QEMUBalloonStatus *balloon_stat_fn; static QEMUBalloonDropCache *balloon_drop_cache_fn; +static QEMUBalloonGetFreePage *balloon_get_free_page_fn; static void *balloon_opaque; static bool balloon_inhibited; @@ -68,10 +69,11 @@ static bool have_balloon(Error **errp) int qemu_add_balloon_handler(QEMUBalloonEvent *event_func, QEMUBalloonStatus *stat_func, QEMUBalloonDropCache *drop_cache_func, + QEMUBalloonGetFreePage *get_free_page_func, void *opaque) { if (balloon_event_fn || balloon_stat_fn || balloon_drop_cache_fn -|| balloon_opaque) { +|| balloon_get_free_page_fn || balloon_opaque) { /* We're already registered one balloon handler. How many can * a guest really have? */ @@ -80,6 +82,7 @@ int qemu_add_balloon_handler(QEMUBalloonEvent *event_func, balloon_event_fn = event_func; balloon_stat_fn = stat_func; balloon_drop_cache_fn = drop_cache_func; +balloon_get_free_page_fn = get_free_page_func; balloon_opaque = opaque; return 0; } @@ -92,6 +95,7 @@ void qemu_remove_balloon_handler(void *opaque) balloon_event_fn = NULL; balloon_stat_fn = NULL; balloon_drop_cache_fn = NULL; +balloon_get_free_page_fn = NULL; balloon_opaque = NULL; } @@ -141,3 +145,21 @@ void qmp_balloon_drop_cache(DropCacheType type, Error **errp) balloon_drop_cache_fn(balloon_opaque, type); } + +bool balloon_free_pages_support(void) +{ +return balloon_get_free_page_fn ? true : false; +} + +BalloonReqStatus balloon_get_free_pages(unsigned long *bitmap, unsigned long len) +{ +if (!balloon_get_free_page_fn) { +return REQ_UNSUPPORT; +} + +if (!bitmap) { +return REQ_INVALID_PARAM; +} + +return balloon_get_free_page_fn(balloon_opaque, bitmap, len); +} diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 4757ba5..30ba074 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -38,6 +38,7 @@ enum balloon_req_id { BALLOON_DROP_CACHE, + BALLOON_GET_FREE_PAGES, }; static void balloon_page(void *addr, int deflate) @@ -435,7 +436,8 @@ static void virtio_balloon_handle_resp(VirtIODevice *vdev, VirtQueue *vq) VirtIOBalloon *s = VIRTIO_BALLOON(vdev); VirtQueueElement *elem; size_t offset = 0; -uint32_t tmp32, id = 0; +uint32_t tmp32, id = 0, page_shift; +uint64_t base_pfn, tmp64, bmap_len; elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); if (!elem) { @@ -457,6 +459,32 @@ static void virtio_balloon_handle_resp(VirtIODevice *vdev, VirtQueue *vq) case BALLOON_DROP_CACHE: s->req_status = REQ_DONE; break; +case BALLOON_GET_FREE_PAGES: +iov_to_buf(elem->out_sg, elem->out_num, offset, + &tmp32, sizeof(uint32_t)); +page_shift = virtio_ldl_p(vdev, &tmp32); +offset += sizeof(uint32_t); +s->page_shift = page_shift; + +iov_to_buf(elem->out_sg, elem->out_num, offset, + &tmp64, sizeof(uint64_t)); +base_pfn = virtio_ldq_p(vdev, &tmp64); +offset += sizeof(uint64_t); +s->base_pfn = base_pfn; + +iov_to_buf(elem->out_sg, elem->out_num, offset, + &tmp64, sizeof(uint64_t)); +bmap_len = virtio_ldq_p(vdev, &tmp64); +offset += sizeof(uint64_t); +if (s->bmap_len < bmap_len) { + s->req_status = REQ_INVALID_PARAM; + return; +} + +iov_to_buf(elem->out_sg, elem->out_num, offset, + s->free_page_bmap, bmap_len); +s->req_status = REQ_DONE; + break; default: break; } @@ -574,6 +602,48 @@ static int virtio_balloon_drop_cache(void *opaque, unsigned long type) return REQ_DONE; } +static BalloonReqStatus virtio_balloon_free_pages(void *opaque, + unsigned long *bitmap, + unsigned long bmap_len) +{ +VirtIOBalloon *s = opaque; +VirtIODevice *vdev = VIRTIO_DEVICE(s); +VirtQueueElement *el
[Qemu-devel] [QEMU 5/7] bitmap: Add a new bitmap_move function
Sometimes, it is need to move a portion of bitmap to another place in a large bitmap, if overlap happens, the bitmap_copy can't not work correctly, we need a new function to do this work. Signed-off-by: Liang Li --- include/qemu/bitmap.h | 13 + 1 file changed, 13 insertions(+) diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h index ec5146f..6ac89ca 100644 --- a/include/qemu/bitmap.h +++ b/include/qemu/bitmap.h @@ -37,6 +37,7 @@ * bitmap_set(dst, pos, nbits) Set specified bit area * bitmap_set_atomic(dst, pos, nbits) Set specified bit area with atomic ops * bitmap_clear(dst, pos, nbits) Clear specified bit area + * bitmap_move(dst, src, nbits) Move *src to *dst * bitmap_test_and_clear_atomic(dst, pos, nbits)Test and clear area * bitmap_find_next_zero_area(buf, len, pos, n, mask) Find bit free area */ @@ -136,6 +137,18 @@ static inline void bitmap_copy(unsigned long *dst, const unsigned long *src, } } +static inline void bitmap_move(unsigned long *dst, const unsigned long *src, + long nbits) +{ +if (small_nbits(nbits)) { +unsigned long tmp = *src; +*dst = tmp; +} else { +long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); +memmove(dst, src, len); +} +} + static inline int bitmap_and(unsigned long *dst, const unsigned long *src1, const unsigned long *src2, long nbits) { -- 1.9.1
[Qemu-devel] [QEMU 7/7] migration: skip free pages during live migration
After sending out the request for free pages, live migration process will start without waiting for the free page bitmap is ready. If the free page bitmap is not ready when doing the 1st migration_bitmap_sync() after ram_save_setup(), the free page bitmap will be ignored, this means the free pages will not be filtered out in this case. The current implementation can not work with post copy, if post copy is enabled, we simply ignore the free pages. Will make it work later. Signed-off-by: Liang Li --- migration/ram.c | 93 + 1 file changed, 93 insertions(+) diff --git a/migration/ram.c b/migration/ram.c index 844ea46..5f1c3ff 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -43,6 +43,8 @@ #include "trace.h" #include "exec/ram_addr.h" #include "qemu/rcu_queue.h" +#include "sysemu/balloon.h" +#include "sysemu/kvm.h" #ifdef DEBUG_MIGRATION_RAM #define DPRINTF(fmt, ...) \ @@ -228,6 +230,7 @@ static QemuMutex migration_bitmap_mutex; static uint64_t migration_dirty_pages; static uint32_t last_version; static bool ram_bulk_stage; +static bool ignore_freepage_rsp; /* used by the search for pages to send */ struct PageSearchStatus { @@ -244,6 +247,7 @@ static struct BitmapRcu { struct rcu_head rcu; /* Main migration bitmap */ unsigned long *bmap; +unsigned long *free_page_bmap; /* bitmap of pages that haven't been sent even once * only maintained and used in postcopy at the moment * where it's used to send the dirtymap at the start @@ -639,6 +643,7 @@ static void migration_bitmap_sync(void) rcu_read_unlock(); qemu_mutex_unlock(&migration_bitmap_mutex); +ignore_freepage_rsp = true; trace_migration_bitmap_sync_end(migration_dirty_pages - num_dirty_pages_init); num_dirty_pages_period += migration_dirty_pages - num_dirty_pages_init; @@ -1417,6 +1422,7 @@ static void migration_bitmap_free(struct BitmapRcu *bmap) { g_free(bmap->bmap); g_free(bmap->unsentmap); +g_free(bmap->free_page_bmap); g_free(bmap); } @@ -1487,6 +1493,85 @@ void migration_bitmap_extend(ram_addr_t old, ram_addr_t new) } } +static void filter_out_guest_free_page(unsigned long *free_page_bmap, + long nbits) +{ +long i, page_count = 0, len; +unsigned long *bitmap; + +tighten_guest_free_page_bmap(free_page_bmap); +qemu_mutex_lock(&migration_bitmap_mutex); +bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap; +slow_bitmap_complement(bitmap, free_page_bmap, nbits); + +len = (last_ram_offset() >> TARGET_PAGE_BITS) / BITS_PER_LONG; +for (i = 0; i < len; i++) { +page_count += hweight_long(bitmap[i]); +} + +migration_dirty_pages = page_count; +qemu_mutex_unlock(&migration_bitmap_mutex); +} + +static void ram_request_free_page(unsigned long *bmap, unsigned long max_pfn) +{ +BalloonReqStatus status; + +status = balloon_get_free_pages(bmap, max_pfn); +switch (status) { +case REQ_DONE: +ignore_freepage_rsp = false; +break; +case REQ_ERROR: +error_report("Errro happend when request free page"); +break; +default: +error_report("unexpected response status: %d", status); +break; +} +} + +static void ram_handle_free_page(void) +{ +unsigned long nbits; +RAMBlock *pc_ram_block; +BalloonReqStatus status; + +status = balloon_get_free_pages(migration_bitmap_rcu->free_page_bmap, +get_guest_max_pfn()); +switch (status) { +case REQ_DONE: +rcu_read_lock(); +pc_ram_block = QLIST_FIRST_RCU(&ram_list.blocks); +nbits = pc_ram_block->used_length >> TARGET_PAGE_BITS; +filter_out_guest_free_page(migration_bitmap_rcu->free_page_bmap, nbits); +rcu_read_unlock(); + +qemu_mutex_lock_iothread(); +migration_bitmap_sync(); +qemu_mutex_unlock_iothread(); +/* + * bulk stage assumes in (migration_bitmap_find_and_reset_dirty) that + * every page is dirty, that's no longer ture at this point. + */ +ram_bulk_stage = false; +last_seen_block = NULL; +last_sent_block = NULL; +last_offset = 0; +break; +case REQ_ERROR: +ignore_freepage_rsp = true; +error_report("failed to get free page"); +break; +case REQ_INVALID_PARAM: +ignore_freepage_rsp = true; +error_report("buffer overflow"); +break; +default: +break; +} +} + /* * 'expected' is the value you expect the bitmap mostly to be full * of; it won't bother printing lines that are all this value. @@ -1950,6 +2035,11 @@ static int ram_save_setup(QEMUFile *f, void *opaque) qemu_mutex_unlock_ramlist(); qemu_mutex_unlock_iothread(); +if (balloon_free_pages_support() && !migrate_postcopy_ram()) { +u
[Qemu-devel] [PATCH 04/12] hw/i2c: QOM'ify versatile_i2c.c
Drop the old SysBus init function and use instance_init Signed-off-by: xiaoqiang zhao --- hw/i2c/versatile_i2c.c | 19 ++- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/hw/i2c/versatile_i2c.c b/hw/i2c/versatile_i2c.c index 0bce524..da9f298 100644 --- a/hw/i2c/versatile_i2c.c +++ b/hw/i2c/versatile_i2c.c @@ -79,32 +79,25 @@ static const MemoryRegionOps versatile_i2c_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static int versatile_i2c_init(SysBusDevice *sbd) +static void versatile_i2c_init(Object *obj) { -DeviceState *dev = DEVICE(sbd); -VersatileI2CState *s = VERSATILE_I2C(dev); +DeviceState *dev = DEVICE(obj); +VersatileI2CState *s = VERSATILE_I2C(obj); +SysBusDevice *sbd = SYS_BUS_DEVICE(obj); I2CBus *bus; bus = i2c_init_bus(dev, "i2c"); s->bitbang = bitbang_i2c_init(bus); -memory_region_init_io(&s->iomem, OBJECT(s), &versatile_i2c_ops, s, +memory_region_init_io(&s->iomem, obj, &versatile_i2c_ops, s, "versatile_i2c", 0x1000); sysbus_init_mmio(sbd, &s->iomem); -return 0; -} - -static void versatile_i2c_class_init(ObjectClass *klass, void *data) -{ -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - -k->init = versatile_i2c_init; } static const TypeInfo versatile_i2c_info = { .name = TYPE_VERSATILE_I2C, .parent= TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(VersatileI2CState), -.class_init= versatile_i2c_class_init, +.instance_init = versatile_i2c_init, }; static void versatile_i2c_register_types(void) -- 2.1.4
[Qemu-devel] [PATCH 03/12] hw/i2c: QOM'ify omap_i2c.c
* Split the omap_i2c_init into an instance_init and realize function * Drop the old SysBus init function and use instance_init Signed-off-by: xiaoqiang zhao --- hw/i2c/omap_i2c.c | 44 ++-- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/hw/i2c/omap_i2c.c b/hw/i2c/omap_i2c.c index 67fbbff..f7c92ea 100644 --- a/hw/i2c/omap_i2c.c +++ b/hw/i2c/omap_i2c.c @@ -22,6 +22,7 @@ #include "hw/arm/omap.h" #include "hw/sysbus.h" #include "qemu/error-report.h" +#include "qapi/error.h" #define TYPE_OMAP_I2C "omap_i2c" #define OMAP_I2C(obj) OBJECT_CHECK(OMAPI2CState, (obj), TYPE_OMAP_I2C) @@ -445,29 +446,35 @@ static const MemoryRegionOps omap_i2c_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static int omap_i2c_init(SysBusDevice *sbd) +static void omap_i2c_init(Object *obj) { -DeviceState *dev = DEVICE(sbd); -OMAPI2CState *s = OMAP_I2C(dev); - -if (!s->fclk) { -error_report("omap_i2c: fclk not connected"); -return -1; -} -if (s->revision >= OMAP2_INTR_REV && !s->iclk) { -/* Note that OMAP1 doesn't have a separate interface clock */ -error_report("omap_i2c: iclk not connected"); -return -1; -} +DeviceState *dev = DEVICE(obj); +OMAPI2CState *s = OMAP_I2C(obj); +SysBusDevice *sbd = SYS_BUS_DEVICE(obj); sysbus_init_irq(sbd, &s->irq); sysbus_init_irq(sbd, &s->drq[0]); sysbus_init_irq(sbd, &s->drq[1]); -memory_region_init_io(&s->iomem, OBJECT(s), &omap_i2c_ops, s, "omap.i2c", - (s->revision < OMAP2_INTR_REV) ? 0x800 : 0x1000); sysbus_init_mmio(sbd, &s->iomem); s->bus = i2c_init_bus(dev, NULL); -return 0; +} + +static void omap_i2c_realize(DeviceState *dev, Error **errp) +{ +OMAPI2CState *s = OMAP_I2C(dev); + +memory_region_init_io(&s->iomem, OBJECT(dev), &omap_i2c_ops, s, "omap.i2c", + (s->revision < OMAP2_INTR_REV) ? 0x800 : 0x1000); + +if (!s->fclk) { +error_setg(errp, "omap_i2c: fclk not connected"); +return; +} +if (s->revision >= OMAP2_INTR_REV && !s->iclk) { +/* Note that OMAP1 doesn't have a separate interface clock */ +error_setg(errp, "omap_i2c: iclk not connected"); +return; +} } static Property omap_i2c_properties[] = { @@ -480,18 +487,19 @@ static Property omap_i2c_properties[] = { static void omap_i2c_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); -k->init = omap_i2c_init; + dc->props = omap_i2c_properties; dc->reset = omap_i2c_reset; /* Reason: pointer properties "iclk", "fclk" */ dc->cannot_instantiate_with_device_add_yet = true; +dc->realize = omap_i2c_realize; } static const TypeInfo omap_i2c_info = { .name = TYPE_OMAP_I2C, .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(OMAPI2CState), +.instance_init = omap_i2c_init, .class_init = omap_i2c_class_init, }; -- 2.1.4
[Qemu-devel] [PATCH 06/12] hw/gpio: QOM'ify pl061.c
* Merge the pl061_initfn into pl061_init * Drop the old SysBus init function and use instance_init Signed-off-by: xiaoqiang zhao --- hw/gpio/pl061.c | 24 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c index 44faeb2..4ae2aa1 100644 --- a/hw/gpio/pl061.c +++ b/hw/gpio/pl061.c @@ -341,20 +341,6 @@ static const MemoryRegionOps pl061_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static int pl061_initfn(SysBusDevice *sbd) -{ -DeviceState *dev = DEVICE(sbd); -PL061State *s = PL061(dev); - -memory_region_init_io(&s->iomem, OBJECT(s), &pl061_ops, s, "pl061", 0x1000); -sysbus_init_mmio(sbd, &s->iomem); -sysbus_init_irq(sbd, &s->irq); -qdev_init_gpio_in(dev, pl061_set_irq, 8); -qdev_init_gpio_out(dev, s->out, 8); - -return 0; -} - static void pl061_luminary_init(Object *obj) { PL061State *s = PL061(obj); @@ -366,17 +352,23 @@ static void pl061_luminary_init(Object *obj) static void pl061_init(Object *obj) { PL061State *s = PL061(obj); +DeviceState *dev = DEVICE(obj); +SysBusDevice *sbd = SYS_BUS_DEVICE(obj); s->id = pl061_id; s->rsvd_start = 0x424; + +memory_region_init_io(&s->iomem, obj, &pl061_ops, s, "pl061", 0x1000); +sysbus_init_mmio(sbd, &s->iomem); +sysbus_init_irq(sbd, &s->irq); +qdev_init_gpio_in(dev, pl061_set_irq, 8); +qdev_init_gpio_out(dev, s->out, 8); } static void pl061_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); -k->init = pl061_initfn; dc->vmsd = &vmstate_pl061; dc->reset = &pl061_reset; } -- 2.1.4
[Qemu-devel] [PATCH 01/12] hw/i2c: QOM'ify bitbang_i2c.c
Drop the old SysBus init function and use instance_init Signed-off-by: xiaoqiang zhao --- hw/i2c/bitbang_i2c.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/hw/i2c/bitbang_i2c.c b/hw/i2c/bitbang_i2c.c index 6ed2060..d3a2989 100644 --- a/hw/i2c/bitbang_i2c.c +++ b/hw/i2c/bitbang_i2c.c @@ -210,13 +210,14 @@ static void bitbang_i2c_gpio_set(void *opaque, int irq, int level) } } -static int gpio_i2c_init(SysBusDevice *sbd) +static void gpio_i2c_init(Object *obj) { -DeviceState *dev = DEVICE(sbd); -GPIOI2CState *s = GPIO_I2C(dev); +DeviceState *dev = DEVICE(obj); +GPIOI2CState *s = GPIO_I2C(obj); +SysBusDevice *sbd = SYS_BUS_DEVICE(obj); I2CBus *bus; -memory_region_init(&s->dummy_iomem, OBJECT(s), "gpio_i2c", 0); +memory_region_init(&s->dummy_iomem, obj, "gpio_i2c", 0); sysbus_init_mmio(sbd, &s->dummy_iomem); bus = i2c_init_bus(dev, "i2c"); @@ -224,16 +225,12 @@ static int gpio_i2c_init(SysBusDevice *sbd) qdev_init_gpio_in(dev, bitbang_i2c_gpio_set, 2); qdev_init_gpio_out(dev, &s->out, 1); - -return 0; } static void gpio_i2c_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); -k->init = gpio_i2c_init; set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); dc->desc = "Virtual GPIO to I2C bridge"; } @@ -242,6 +239,7 @@ static const TypeInfo gpio_i2c_info = { .name = TYPE_GPIO_I2C, .parent= TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(GPIOI2CState), +.instance_init = gpio_i2c_init, .class_init= gpio_i2c_class_init, }; -- 2.1.4
[Qemu-devel] [PATCH 08/12] hw/misc: QOM'ify arm_l2x0.c
Drop the old SysBus init function and use instance_init Signed-off-by: xiaoqiang zhao --- hw/misc/arm_l2x0.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hw/misc/arm_l2x0.c b/hw/misc/arm_l2x0.c index 4442227..66a0787 100644 --- a/hw/misc/arm_l2x0.c +++ b/hw/misc/arm_l2x0.c @@ -159,14 +159,14 @@ static const MemoryRegionOps l2x0_mem_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static int l2x0_priv_init(SysBusDevice *dev) +static void l2x0_priv_init(Object *obj) { -L2x0State *s = ARM_L2X0(dev); +L2x0State *s = ARM_L2X0(obj); +SysBusDevice *dev = SYS_BUS_DEVICE(obj); -memory_region_init_io(&s->iomem, OBJECT(dev), &l2x0_mem_ops, s, +memory_region_init_io(&s->iomem, obj, &l2x0_mem_ops, s, "l2x0_cc", 0x1000); sysbus_init_mmio(dev, &s->iomem); -return 0; } static Property l2x0_properties[] = { @@ -176,10 +176,8 @@ static Property l2x0_properties[] = { static void l2x0_class_init(ObjectClass *klass, void *data) { -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass); -k->init = l2x0_priv_init; dc->vmsd = &vmstate_l2x0; dc->props = l2x0_properties; dc->reset = l2x0_priv_reset; @@ -189,6 +187,7 @@ static const TypeInfo l2x0_info = { .name = TYPE_ARM_L2X0, .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(L2x0State), +.instance_init = l2x0_priv_init, .class_init = l2x0_class_init, }; -- 2.1.4
[Qemu-devel] [PATCH 09/12] hw/misc: QOM'ify exynos4210_pmu.c
Drop the old SysBus init function and use instance_init Signed-off-by: xiaoqiang zhao --- hw/misc/exynos4210_pmu.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hw/misc/exynos4210_pmu.c b/hw/misc/exynos4210_pmu.c index 889abad..e30dbc7 100644 --- a/hw/misc/exynos4210_pmu.c +++ b/hw/misc/exynos4210_pmu.c @@ -457,15 +457,15 @@ static void exynos4210_pmu_reset(DeviceState *dev) } } -static int exynos4210_pmu_init(SysBusDevice *dev) +static void exynos4210_pmu_init(Object *obj) { -Exynos4210PmuState *s = EXYNOS4210_PMU(dev); +Exynos4210PmuState *s = EXYNOS4210_PMU(obj); +SysBusDevice *dev = SYS_BUS_DEVICE(obj); /* memory mapping */ -memory_region_init_io(&s->iomem, OBJECT(dev), &exynos4210_pmu_ops, s, +memory_region_init_io(&s->iomem, obj, &exynos4210_pmu_ops, s, "exynos4210.pmu", EXYNOS4210_PMU_REGS_MEM_SIZE); sysbus_init_mmio(dev, &s->iomem); -return 0; } static const VMStateDescription exynos4210_pmu_vmstate = { @@ -481,9 +481,7 @@ static const VMStateDescription exynos4210_pmu_vmstate = { static void exynos4210_pmu_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); -k->init = exynos4210_pmu_init; dc->reset = exynos4210_pmu_reset; dc->vmsd = &exynos4210_pmu_vmstate; } @@ -492,6 +490,7 @@ static const TypeInfo exynos4210_pmu_info = { .name = TYPE_EXYNOS4210_PMU, .parent= TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(Exynos4210PmuState), +.instance_init = exynos4210_pmu_init, .class_init= exynos4210_pmu_class_init, }; -- 2.1.4
[Qemu-devel] [PATCH 05/12] hw/gpio: QOM'ify omap_gpio.c
* Split the old SysBus init into an instance_init and DeviceClass::realize function * Drop the SysBus init function Signed-off-by: xiaoqiang zhao --- hw/gpio/omap_gpio.c | 61 +++-- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/hw/gpio/omap_gpio.c b/hw/gpio/omap_gpio.c index 9b1b004..dabef4a 100644 --- a/hw/gpio/omap_gpio.c +++ b/hw/gpio/omap_gpio.c @@ -23,6 +23,7 @@ #include "hw/arm/omap.h" #include "hw/sysbus.h" #include "qemu/error-report.h" +#include "qapi/error.h" struct omap_gpio_s { qemu_irq irq; @@ -678,48 +679,46 @@ static const MemoryRegionOps omap2_gpif_top_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static int omap_gpio_init(SysBusDevice *sbd) +static void omap_gpio_init(Object *obj) { -DeviceState *dev = DEVICE(sbd); -struct omap_gpif_s *s = OMAP1_GPIO(dev); +DeviceState *dev = DEVICE(obj); +struct omap_gpif_s *s = OMAP1_GPIO(obj); +SysBusDevice *sbd = SYS_BUS_DEVICE(obj); -if (!s->clk) { -error_report("omap-gpio: clk not connected"); -return -1; -} qdev_init_gpio_in(dev, omap_gpio_set, 16); qdev_init_gpio_out(dev, s->omap1.handler, 16); sysbus_init_irq(sbd, &s->omap1.irq); -memory_region_init_io(&s->iomem, OBJECT(s), &omap_gpio_ops, &s->omap1, +memory_region_init_io(&s->iomem, obj, &omap_gpio_ops, &s->omap1, "omap.gpio", 0x1000); sysbus_init_mmio(sbd, &s->iomem); -return 0; } -static int omap2_gpio_init(SysBusDevice *sbd) +static void omap_gpio_realize(DeviceState *dev, Error **errp) +{ +struct omap_gpif_s *s = OMAP1_GPIO(dev); + +if (!s->clk) { +error_setg(errp, "omap-gpio: clk not connected"); +} +} + +static void omap2_gpio_realize(DeviceState *dev, Error **errp) { -DeviceState *dev = DEVICE(sbd); struct omap2_gpif_s *s = OMAP2_GPIO(dev); +SysBusDevice *sbd = SYS_BUS_DEVICE(dev); int i; if (!s->iclk) { -error_report("omap2-gpio: iclk not connected"); -return -1; +error_setg(errp, "omap2-gpio: iclk not connected"); +return; } s->modulecount = s->mpu_model < omap2430 ? 4 - : s->mpu_model < omap3430 ? 5 - : 6; - -for (i = 0; i < s->modulecount; i++) { -if (!s->fclk[i]) { -error_report("omap2-gpio: fclk%d not connected", i); -return -1; -} -} +: s->mpu_model < omap3430 ? 5 +: 6; if (s->mpu_model < omap3430) { -memory_region_init_io(&s->iomem, OBJECT(s), &omap2_gpif_top_ops, s, +memory_region_init_io(&s->iomem, OBJECT(dev), &omap2_gpif_top_ops, s, "omap2.gpio", 0x1000); sysbus_init_mmio(sbd, &s->iomem); } @@ -732,17 +731,20 @@ static int omap2_gpio_init(SysBusDevice *sbd) for (i = 0; i < s->modulecount; i++) { struct omap2_gpio_s *m = &s->modules[i]; +if (!s->fclk[i]) { +error_setg(errp, "omap2-gpio: fclk%d not connected", i); +return; +} + m->revision = (s->mpu_model < omap3430) ? 0x18 : 0x25; m->handler = &s->handler[i * 32]; sysbus_init_irq(sbd, &m->irq[0]); /* mpu irq */ sysbus_init_irq(sbd, &m->irq[1]); /* dsp irq */ sysbus_init_irq(sbd, &m->wkup); -memory_region_init_io(&m->iomem, OBJECT(s), &omap2_gpio_module_ops, m, +memory_region_init_io(&m->iomem, OBJECT(dev), &omap2_gpio_module_ops, m, "omap.gpio-module", 0x1000); sysbus_init_mmio(sbd, &m->iomem); } - -return 0; } /* Using qdev pointer properties for the clocks is not ideal. @@ -766,9 +768,8 @@ static Property omap_gpio_properties[] = { static void omap_gpio_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); -k->init = omap_gpio_init; +dc->realize = omap_gpio_realize; dc->reset = omap_gpif_reset; dc->props = omap_gpio_properties; /* Reason: pointer property "clk" */ @@ -779,6 +780,7 @@ static const TypeInfo omap_gpio_info = { .name = TYPE_OMAP1_GPIO, .parent= TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(struct omap_gpif_s), +.instance_init = omap_gpio_init, .class_init= omap_gpio_class_init, }; @@ -797,9 +799,8 @@ static Property omap2_gpio_properties[] = { static void omap2_gpio_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); -k->init = omap2_gpio_init; +dc->realize = omap2_gpio_realize; dc->reset = omap2_gpif_reset; dc->props = omap2_gpio_properties; /* Reason: pointer properties "iclk", "fclk0", ..., "fclk5" */ -- 2.1.4
[Qemu-devel] [PATCH 10/12] hw/misc: QOM'ify mst_fpga.c
Drop the old SysBus init function and use instance_init Signed-off-by: xiaoqiang zhao --- hw/misc/mst_fpga.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/hw/misc/mst_fpga.c b/hw/misc/mst_fpga.c index 48d7dfb..a10f049 100644 --- a/hw/misc/mst_fpga.c +++ b/hw/misc/mst_fpga.c @@ -200,10 +200,11 @@ static int mst_fpga_post_load(void *opaque, int version_id) return 0; } -static int mst_fpga_init(SysBusDevice *sbd) +static void mst_fpga_init(Object *obj) { -DeviceState *dev = DEVICE(sbd); -mst_irq_state *s = MAINSTONE_FPGA(dev); +DeviceState *dev = DEVICE(obj); +mst_irq_state *s = MAINSTONE_FPGA(obj); +SysBusDevice *sbd = SYS_BUS_DEVICE(obj); s->pcmcia0 = MST_PCMCIAx_READY | MST_PCMCIAx_nCD; s->pcmcia1 = MST_PCMCIAx_READY | MST_PCMCIAx_nCD; @@ -213,10 +214,9 @@ static int mst_fpga_init(SysBusDevice *sbd) /* alloc the external 16 irqs */ qdev_init_gpio_in(dev, mst_fpga_set_irq, MST_NUM_IRQS); -memory_region_init_io(&s->iomem, OBJECT(s), &mst_fpga_ops, s, +memory_region_init_io(&s->iomem, obj, &mst_fpga_ops, s, "fpga", 0x0010); sysbus_init_mmio(sbd, &s->iomem); -return 0; } static VMStateDescription vmstate_mst_fpga_regs = { @@ -245,9 +245,7 @@ static VMStateDescription vmstate_mst_fpga_regs = { static void mst_fpga_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); -k->init = mst_fpga_init; dc->desc = "Mainstone II FPGA"; dc->vmsd = &vmstate_mst_fpga_regs; } @@ -256,6 +254,7 @@ static const TypeInfo mst_fpga_info = { .name = TYPE_MAINSTONE_FPGA, .parent= TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(mst_irq_state), +.instance_init = mst_fpga_init, .class_init= mst_fpga_class_init, }; -- 2.1.4
Re: [Qemu-devel] [PATCH v2 2/9] migration: Fix a potential issue
On (Mon) 13 Jun 2016 [05:07:39], Li, Liang Z wrote: > > > > > +static void wait_for_decompress_done(void) { > > > > > +int idx, thread_count; > > > > > + > > > > > +if (!migrate_use_compression()) { > > > > > +return; > > > > > +} > > > > > + > > > > > +thread_count = migrate_decompress_threads(); > > > > > +qemu_mutex_lock(&decomp_done_lock); > > > > > +for (idx = 0; idx < thread_count; idx++) { > > > > > +while (!decomp_param[idx].done) { > > > > > +qemu_cond_wait(&decomp_done_cond, > > &decomp_done_lock); > > > > > +} > > > > > +} > > > > > +qemu_mutex_unlock(&decomp_done_lock); > > > > > > > > Not sure how this works: in the previous patch, done is set to false > > > > under the decomp_done_lock. Here, we take the lock, and wait for done > > to turn false. > > > > That can't happen because this thread holds the lock. > > > > My reading is this is going to lead to a deadlock. What am I missing? > > > > > > > > > > This is the typical usage of the QemuCond, actually, in > > > qemu_cond_wait() , decomp_done_lock will be unlocked at first and then > > > locked again before > > > qemu_cond_wait() return. So deadlock won't happen. > > > > In qemu-thread-posix.c, I don't see such unlock/lock. > > > > > > Amit > > I mean in the 'pthread_cond_wait()' which called by qemu_cond_wait(). Yes, OK - makes sense now. Thanks, I'll continue the review. Amit
Re: [Qemu-devel] [PATCH v6 1/1] Introduce "xen-load-devices-state"
* Stefano Stabellini (sstabell...@kernel.org) wrote: > On Fri, 3 Jun 2016, Anthony PERARD wrote: > > On Fri, Jun 03, 2016 at 05:58:34PM +0800, Changlong Xie wrote: > > > From: Wen Congyang > > > > > > Introduce a "xen-load-devices-state" QAPI command that can be used to > > > load the state of all devices, but not the RAM or the block devices of > > > the VM. > > > > > > We only have hmp commands savevm/loadvm, and qmp commands > > > xen-save-devices-state. > > > > > > We use this new command for COLO: > > > 1. suspend both primary vm and secondary vm > > > 2. sync the state > > > 3. resume both primary vm and secondary vm > > > > > > In such case, we need to update all devices' state in any time. > > > > > > Signed-off-by: Wen Congyang > > > Signed-off-by: Changlong Xie > > > > Reviewed-by: Anthony PERARD > > David, > > can we still have your Ack on this patch? Yes, Reviewed-by: Dr. David Alan Gilbert Dave > > > > > --- > > > migration/savevm.c | 37 + > > > qapi-schema.json | 14 ++ > > > qmp-commands.hx| 27 +++ > > > 3 files changed, 78 insertions(+) > > > > > > diff --git a/migration/savevm.c b/migration/savevm.c > > > index 6c21231..ae2ef8b 100644 > > > --- a/migration/savevm.c > > > +++ b/migration/savevm.c > > > @@ -31,6 +31,7 @@ > > > #include "hw/boards.h" > > > #include "hw/hw.h" > > > #include "hw/qdev.h" > > > +#include "hw/xen/xen.h" > > > #include "net/net.h" > > > #include "monitor/monitor.h" > > > #include "sysemu/sysemu.h" > > > @@ -1754,6 +1755,12 @@ qemu_loadvm_section_start_full(QEMUFile *f, > > > MigrationIncomingState *mis) > > > return -EINVAL; > > > } > > > > > > +/* Validate if it is a device's state */ > > > +if (xen_enabled() && se->is_ram) { > > > +error_report("loadvm: %s RAM loading not allowed on Xen", idstr); > > > +return -EINVAL; > > > +} > > > + > > > /* Add entry */ > > > le = g_malloc0(sizeof(*le)); > > > > > > @@ -2064,6 +2071,36 @@ void qmp_xen_save_devices_state(const char > > > *filename, Error **errp) > > > } > > > } > > > > > > +void qmp_xen_load_devices_state(const char *filename, Error **errp) > > > +{ > > > +QEMUFile *f; > > > +QIOChannelFile *ioc; > > > +int ret; > > > + > > > +/* Guest must be paused before loading the device state; the RAM > > > state > > > + * will already have been loaded by xc > > > + */ > > > +if (runstate_is_running()) { > > > +error_setg(errp, "Cannot update device state while vm is > > > running"); > > > +return; > > > +} > > > +vm_stop(RUN_STATE_RESTORE_VM); > > > + > > > +ioc = qio_channel_file_new_path(filename, O_RDONLY | O_BINARY, 0, > > > errp); > > > +if (!ioc) { > > > +return; > > > +} > > > +f = qemu_fopen_channel_input(QIO_CHANNEL(ioc)); > > > + > > > +migration_incoming_state_new(f); > > > +ret = qemu_loadvm_state(f); > > > +qemu_fclose(f); > > > +if (ret < 0) { > > > +error_setg(errp, QERR_IO_ERROR); > > > +} > > > +migration_incoming_state_destroy(); > > > +} > > > + > > > int load_vmstate(const char *name) > > > { > > > BlockDriverState *bs, *bs_vm_state; > > > diff --git a/qapi-schema.json b/qapi-schema.json > > > index 8483bdf..48c3a6f 100644 > > > --- a/qapi-schema.json > > > +++ b/qapi-schema.json > > > @@ -4201,6 +4201,20 @@ > > >'data': [ 'none', 'record', 'play' ] } > > > > > > ## > > > +# @xen-load-devices-state: > > > +# > > > +# Load the state of all devices from file. The RAM and the block devices > > > +# of the VM are not loaded by this command. > > > +# > > > +# @filename: the file to load the state of the devices from as binary > > > +# data. See xen-save-devices-state.txt for a description of the binary > > > +# format. > > > +# > > > +# Since: 2.7 > > > +## > > > +{ 'command': 'xen-load-devices-state', 'data': {'filename': 'str'} } > > > + > > > +## > > > # @GICCapability: > > > # > > > # The struct describes capability for a specific GIC (Generic > > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > > index 28801a2..780e7f2 100644 > > > --- a/qmp-commands.hx > > > +++ b/qmp-commands.hx > > > @@ -587,6 +587,33 @@ Example: > > > EQMP > > > > > > { > > > +.name = "xen-load-devices-state", > > > +.args_type = "filename:F", > > > +.mhandler.cmd_new = qmp_marshal_xen_load_devices_state, > > > +}, > > > + > > > +SQMP > > > +xen-load-devices-state > > > +-- > > > + > > > +Load the state of all devices from file. The RAM and the block devices > > > +of the VM are not loaded by this command. > > > + > > > +Arguments: > > > + > > > +- "filename": the file to load the state of the devices from as binary > > > +data. See xen-save-devices-state.txt for a description of the binary > > > +format. > > > + > > > +Example: > > > + > > > +-> { "execute": "xen-load
[Qemu-devel] [PATCH 07/12] hw/gpio: QOM'ify zaurus.c
Drop the old SysBus init function and use instance_init Signed-off-by: xiaoqiang zhao --- hw/gpio/zaurus.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/hw/gpio/zaurus.c b/hw/gpio/zaurus.c index 555da28..15865e1 100644 --- a/hw/gpio/zaurus.c +++ b/hw/gpio/zaurus.c @@ -167,19 +167,18 @@ static void scoop_gpio_set(void *opaque, int line, int level) s->gpio_level &= ~(1 << line); } -static int scoop_init(SysBusDevice *sbd) +static void scoop_init(Object *obj) { -DeviceState *dev = DEVICE(sbd); -ScoopInfo *s = SCOOP(dev); +DeviceState *dev = DEVICE(obj); +ScoopInfo *s = SCOOP(obj); +SysBusDevice *sbd = SYS_BUS_DEVICE(obj); s->status = 0x02; qdev_init_gpio_out(dev, s->handler, 16); qdev_init_gpio_in(dev, scoop_gpio_set, 16); -memory_region_init_io(&s->iomem, OBJECT(s), &scoop_ops, s, "scoop", 0x1000); +memory_region_init_io(&s->iomem, obj, &scoop_ops, s, "scoop", 0x1000); sysbus_init_mmio(sbd, &s->iomem); - -return 0; } static int scoop_post_load(void *opaque, int version_id) @@ -239,9 +238,7 @@ static const VMStateDescription vmstate_scoop_regs = { static void scoop_sysbus_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); -k->init = scoop_init; dc->desc = "Scoop2 Sharp custom ASIC"; dc->vmsd = &vmstate_scoop_regs; } @@ -250,6 +247,7 @@ static const TypeInfo scoop_sysbus_info = { .name = TYPE_SCOOP, .parent= TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(ScoopInfo), +.instance_init = scoop_init, .class_init= scoop_sysbus_class_init, }; -- 2.1.4
[Qemu-devel] [PATCH 00/12] some ARM platform QOM'ify work (update)
This patch series QOM'ify ARM platform related devices. Where we drop the sysbus init function if possible and use instance_init and DeviceClass::realize function. xiaoqiang zhao (12): hw/i2c: QOM'ify bitbang_i2c.c hw/i2c: QOM'ify exynos4210_i2c.c hw/i2c: QOM'ify omap_i2c.c hw/i2c: QOM'ify versatile_i2c.c hw/gpio: QOM'ify omap_gpio.c hw/gpio: QOM'ify pl061.c hw/gpio: QOM'ify zaurus.c hw/misc: QOM'ify arm_l2x0.c hw/misc: QOM'ify exynos4210_pmu.c hw/misc: QOM'ify mst_fpga.c hw/dma: QOM'ify pxa2xx_dma.c hw/sd: QOM'ify pl181.c hw/dma/pxa2xx_dma.c | 38 +- hw/gpio/omap_gpio.c | 61 hw/gpio/pl061.c | 24 +++ hw/gpio/zaurus.c | 14 +-- hw/i2c/bitbang_i2c.c | 14 +-- hw/i2c/exynos4210_i2c.c | 13 +-- hw/i2c/omap_i2c.c| 44 -- hw/i2c/versatile_i2c.c | 19 +-- hw/misc/arm_l2x0.c | 11 - hw/misc/exynos4210_pmu.c | 11 - hw/misc/mst_fpga.c | 13 +-- hw/sd/pl181.c| 26 + 12 files changed, 143 insertions(+), 145 deletions(-) -- 2.1.4
[Qemu-devel] [RFC PATCH v1 1/4] migration: Remove static allocation of xzblre cache buffer
From: Vijaya Kumar K Allocate xzblre zero page cache buffer dynamically. Remove dependency on TARGET_PAGE_SIZE to make run-time page size detection for arm platforms. Signed-off-by: Vijaya Kumar K --- migration/ram.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/migration/ram.c b/migration/ram.c index 844ea46..ba3b352 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -69,7 +69,7 @@ static uint64_t bitmap_sync_count; /* 0x80 is reserved in migration.h start with 0x100 next */ #define RAM_SAVE_FLAG_COMPRESS_PAGE0x100 -static const uint8_t ZERO_TARGET_PAGE[TARGET_PAGE_SIZE]; +static uint8_t *ZERO_TARGET_PAGE; static inline bool is_zero_range(uint8_t *p, uint64_t size) { @@ -1437,6 +1437,7 @@ static void ram_migration_cleanup(void *opaque) cache_fini(XBZRLE.cache); g_free(XBZRLE.encoded_buf); g_free(XBZRLE.current_buf); +g_free(ZERO_TARGET_PAGE); XBZRLE.cache = NULL; XBZRLE.encoded_buf = NULL; XBZRLE.current_buf = NULL; @@ -1893,6 +1894,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque) if (migrate_use_xbzrle()) { XBZRLE_cache_lock(); +ZERO_TARGET_PAGE = g_malloc0(TARGET_PAGE_SIZE); XBZRLE.cache = cache_init(migrate_xbzrle_cache_size() / TARGET_PAGE_SIZE, TARGET_PAGE_SIZE); -- 1.7.9.5