[Qemu-devel] [PATCH RFC] hw/pc: set q35 as the default x86 machine
Moving to QEMU 3.0 seems like a good opportunity for such a change. I440FX is really old and does not support modern features like IOMMU. Q35's SATA emulation is faster than pc's IDE, native PCI express hotplug is cleaner than ACPI based one and so on... Also the libvirt guys added very good support for the Q35 machine (thanks!). Management software should always specify the machine type and for the current setups, adding '-machine pc' to the command line is not such a big deal. In time the pc machine will fade out and we will probably stop adding new versions at some point. Signed-off-by: Marcel Apfelbaum --- hw/i386/pc_piix.c | 2 -- hw/i386/pc_q35.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index b4c5b03274..16dd65198f 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -429,7 +429,6 @@ static void pc_i440fx_3_0_machine_options(MachineClass *m) { pc_i440fx_machine_options(m); m->alias = "pc"; -m->is_default = 1; } DEFINE_I440FX_MACHINE(v3_0, "pc-i440fx-3.0", NULL, @@ -438,7 +437,6 @@ DEFINE_I440FX_MACHINE(v3_0, "pc-i440fx-3.0", NULL, static void pc_i440fx_2_12_machine_options(MachineClass *m) { pc_i440fx_3_0_machine_options(m); -m->is_default = 0; m->alias = NULL; SET_MACHINE_COMPAT(m, PC_COMPAT_2_12); } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 83d6d75efa..b33c235d49 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -312,6 +312,7 @@ static void pc_q35_3_0_machine_options(MachineClass *m) { pc_q35_machine_options(m); m->alias = "q35"; +m->is_default = 1; } DEFINE_Q35_MACHINE(v3_0, "pc-q35-3.0", NULL, @@ -321,6 +322,7 @@ static void pc_q35_2_12_machine_options(MachineClass *m) { pc_q35_3_0_machine_options(m); m->alias = NULL; +m->is_default = 0; SET_MACHINE_COMPAT(m, PC_COMPAT_2_12); } -- 2.13.6
[Qemu-devel] [Bug 1774853] [NEW] claims temp file is used by another process
Public bug reported: QEMU emulator version 2.12.50 (v2.12.0-12378-g99a34dc4d2-dirty) "c:\Program Files\qemu\qemu-system-x86_64.exe" -net none -parallel none -bios OVMF.fd -L . -hda fat:rw:image vvfat image chs 1024,16,63 c:\Program Files\qemu\qemu-system-x86_64.exe: -hda fat:rw:image: Could not open 'C:\Users\tsiros\AppData\Local\Temp\qem5B92.tmp': The process cannot access the file because it is being used by another process. ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1774853 Title: claims temp file is used by another process Status in QEMU: New Bug description: QEMU emulator version 2.12.50 (v2.12.0-12378-g99a34dc4d2-dirty) "c:\Program Files\qemu\qemu-system-x86_64.exe" -net none -parallel none -bios OVMF.fd -L . -hda fat:rw:image vvfat image chs 1024,16,63 c:\Program Files\qemu\qemu-system-x86_64.exe: -hda fat:rw:image: Could not open 'C:\Users\tsiros\AppData\Local\Temp\qem5B92.tmp': The process cannot access the file because it is being used by another process. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1774853/+subscriptions
[Qemu-devel] [Bug 1774853] Re: claims temp file is used by another process
someone please delete this ** Changed in: qemu Status: New => Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1774853 Title: claims temp file is used by another process Status in QEMU: Invalid Bug description: QEMU emulator version 2.12.50 (v2.12.0-12378-g99a34dc4d2-dirty) "c:\Program Files\qemu\qemu-system-x86_64.exe" -net none -parallel none -bios OVMF.fd -L . -hda fat:rw:image vvfat image chs 1024,16,63 c:\Program Files\qemu\qemu-system-x86_64.exe: -hda fat:rw:image: Could not open 'C:\Users\tsiros\AppData\Local\Temp\qem5B92.tmp': The process cannot access the file because it is being used by another process. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1774853/+subscriptions
Re: [Qemu-devel] Mistake on man page
Ok, thanks for the clarification. On Fri, Jun 1, 2018 at 7:17 PM Daniel P. Berrangé wrote: > On Fri, Jun 01, 2018 at 04:47:55PM +0200, kvaps wrote: > > Hi, here is mistake in man page for qemu-nbd: > > > > --- a/qemu-nbd.texi > > +++ b/qemu-nbd.texi > > @@ -1,2 +1,2 @@ > > --- a/qemu-nbd.texi > > -Don't exit on the last connection > > +++ b/qemu-nbd.texi > > +Don't exit on the lost connection > > It isn't a mistake actually. qemu-nbd can allow multiple concurrent > client connections, and will exit once the last client closes. > > Regards, > Daniel > -- > |: https://berrange.com -o- > https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- > https://fstop138.berrange.com :| > |: https://entangle-photo.org-o- > https://www.instagram.com/dberrange :| >
Re: [Qemu-devel] [PATCH 2/2] hw/arm/armv7m: Remove unused armv7m_init() function
On 3 June 2018 at 01:48, Joel Stanley wrote: > Hi Peter, > > On 2 June 2018 at 00:13, Peter Maydell wrote: >> Remove the now-unused armv7m_init() function. This was a legacy from >> before we properly QOMified ARMv7M, and it has some flaws: >> >> * it combines work that needs to be done by an SoC object (creating >>and initializing the TYPE_ARMV7M object) with work that needs to >>be done by the board model (setting the system up to load the ELF >>file specified with -kernel) >> * TYPE_ARMV7M creation failure is fatal, but an SoC object wants to >>arrange to propagate the failure outward >> * it uses allocate-and-create via qdev_create() whereas the current >>preferred style for SoC objects is to do creation in-place >> >> Board and SoC models can instead do the two jobs this function >> was doing themselves, in the right places and with whatever their >> preferred style/error handling is. >> -DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int >> num_irq, >> - const char *kernel_filename, const char *cpu_type) >> -{ >> -DeviceState *armv7m; >> - >> -armv7m = qdev_create(NULL, TYPE_ARMV7M); >> -qdev_prop_set_uint32(armv7m, "num-irq", num_irq); >> -qdev_prop_set_string(armv7m, "cpu-type", cpu_type); >> -object_property_set_link(OBJECT(armv7m), OBJECT(get_system_memory()), >> - "memory", &error_abort); > > It looks like the snippet above is going to be a cut/paste for all v7m > machines. If you look at pretty much all the v7m machines except stellaris, they don't use this code fragment. (Instead you have an object_initialize()/qdev_set_parent_bus() in the SoC container's init function, calls to set properties and realize in the container's realize, and a call to armv7m_load_kernel() in the board code.) I agree that what you might call the "modern style" of writing SoC containers like that has a lot of boilerplate, but that's not specific to the armv7m object, and if we want to reduce boilerplate we should look at how we can do it consistently across devices. thanks -- PMM
Re: [Qemu-devel] Cortex M0 emulation tasks
On 2 June 2018 at 17:15:15, Stefan Hajnoczi (stefa...@gmail.com) wrote: > > I have put together a basic Cortex M0 ARMv6-M CPU that can serve > as > the basis for this work. Please see the RFC patches that I've sent > separately. you can also take a look at the Cortex-M implementation in the GNU MCU Eclipse QEMU. https://gnu-mcu-eclipse.github.io/qemu/ it is a bit old now; a refresh is planned, but no date is set yet. regards, Liviu
Re: [Qemu-devel] [PATCH v4 04/12] migration: avoid concurrent invoke channel_close by different threads
On Thu, May 31, 2018 at 6:52 PM, Dr. David Alan Gilbert wrote: > * 858585 jemmy (jemmy858...@gmail.com) wrote: >> On Wed, May 30, 2018 at 10:45 PM, Dr. David Alan Gilbert >> wrote: >> > * Lidong Chen (jemmy858...@gmail.com) wrote: >> >> From: Lidong Chen >> >> >> >> The channel_close maybe invoked by different threads. For example, source >> >> qemu invokes qemu_fclose in main thread, migration thread and return path >> >> thread. Destination qemu invokes qemu_fclose in main thread, listen thread >> >> and COLO incoming thread. >> >> >> >> Add a mutex in QEMUFile struct to avoid concurrent invoke channel_close. >> >> >> >> Signed-off-by: Lidong Chen >> >> --- >> >> migration/qemu-file.c | 5 + >> >> 1 file changed, 5 insertions(+) >> >> >> >> diff --git a/migration/qemu-file.c b/migration/qemu-file.c >> >> index 977b9ae..87d0f05 100644 >> >> --- a/migration/qemu-file.c >> >> +++ b/migration/qemu-file.c >> >> @@ -52,6 +52,7 @@ struct QEMUFile { >> >> unsigned int iovcnt; >> >> >> >> int last_error; >> >> +QemuMutex lock; >> > >> > That could do with a comment saying what you're protecting >> > >> >> }; >> >> >> >> /* >> >> @@ -96,6 +97,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, const >> >> QEMUFileOps *ops) >> >> >> >> f = g_new0(QEMUFile, 1); >> >> >> >> +qemu_mutex_init(&f->lock); >> >> f->opaque = opaque; >> >> f->ops = ops; >> >> return f; >> >> @@ -328,7 +330,9 @@ int qemu_fclose(QEMUFile *f) >> >> ret = qemu_file_get_error(f); >> >> >> >> if (f->ops->close) { >> >> +qemu_mutex_lock(&f->lock); >> >> int ret2 = f->ops->close(f->opaque); >> >> +qemu_mutex_unlock(&f->lock); >> > >> > OK, and at least for the RDMA code, if it calls >> > close a 2nd time, rioc->rdma is checked so it wont actually free stuff a >> > 2nd time. >> > >> >> if (ret >= 0) { >> >> ret = ret2; >> >> } >> >> @@ -339,6 +343,7 @@ int qemu_fclose(QEMUFile *f) >> >> if (f->last_error) { >> >> ret = f->last_error; >> >> } >> >> +qemu_mutex_destroy(&f->lock); >> >> g_free(f); >> > >> > Hmm but that's not safe; if two things really do call qemu_fclose() >> > on the same structure they race here and can end up destroying the lock >> > twice, or doing f->lock after the 1st one has already g_free(f). >> >> > >> > >> > So lets go back a step. >> > I think: >> > a) There should always be a separate QEMUFile* for >> > to_src_file and from_src_file - I don't see where you open >> > the 2nd one; I don't see your implementation of >> > f->ops->get_return_path. >> >> yes, current qemu version use a separate QEMUFile* for to_src_file and >> from_src_file. >> and the two QEMUFile point to one QIOChannelRDMA. >> >> the f->ops->get_return_path is implemented by channel_output_ops or >> channel_input_ops. > > Ah OK, yes that makes sense. > >> > b) I *think* that while the different threads might all call >> > fclose(), I think there should only ever be one qemu_fclose >> > call for each direction on the QEMUFile. >> > >> > But now we have two problems: >> > If (a) is true then f->lock is separate on each one so >> >doesn't really protect if the two directions are closed >> >at once. (Assuming (b) is true) >> >> yes, you are right. so I should add a QemuMutex in QIOChannel structure, not >> QEMUFile structure. and qemu_mutex_destroy the QemuMutex in >> qio_channel_finalize. > > OK, that sounds better. > > Dave > Hi Dave: Another way is protect channel_close in migration part, like QemuMutex rp_mutex. As Daniel mentioned, QIOChannel impls are only intended to a single thread. https://www.mail-archive.com/qemu-devel@nongnu.org/msg530100.html which way is better? Does QIOChannel have the plan to support multi thread? Not only channel_close need lock between different threads, writev_buffer write also need. thanks. >> Thank you. >> >> > >> > If (a) is false and we actually share a single QEMUFile then >> > that race at the end happens. >> > >> > Dave >> > >> > >> >> trace_qemu_file_fclose(); >> >> return ret; >> >> -- >> >> 1.8.3.1 >> >> >> > -- >> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v4 04/12] migration: avoid concurrent invoke channel_close by different threads
On Sun, Jun 3, 2018 at 9:50 PM, 858585 jemmy wrote: > On Thu, May 31, 2018 at 6:52 PM, Dr. David Alan Gilbert > wrote: >> * 858585 jemmy (jemmy858...@gmail.com) wrote: >>> On Wed, May 30, 2018 at 10:45 PM, Dr. David Alan Gilbert >>> wrote: >>> > * Lidong Chen (jemmy858...@gmail.com) wrote: >>> >> From: Lidong Chen >>> >> >>> >> The channel_close maybe invoked by different threads. For example, source >>> >> qemu invokes qemu_fclose in main thread, migration thread and return path >>> >> thread. Destination qemu invokes qemu_fclose in main thread, listen >>> >> thread >>> >> and COLO incoming thread. >>> >> >>> >> Add a mutex in QEMUFile struct to avoid concurrent invoke channel_close. >>> >> >>> >> Signed-off-by: Lidong Chen >>> >> --- >>> >> migration/qemu-file.c | 5 + >>> >> 1 file changed, 5 insertions(+) >>> >> >>> >> diff --git a/migration/qemu-file.c b/migration/qemu-file.c >>> >> index 977b9ae..87d0f05 100644 >>> >> --- a/migration/qemu-file.c >>> >> +++ b/migration/qemu-file.c >>> >> @@ -52,6 +52,7 @@ struct QEMUFile { >>> >> unsigned int iovcnt; >>> >> >>> >> int last_error; >>> >> +QemuMutex lock; >>> > >>> > That could do with a comment saying what you're protecting >>> > >>> >> }; >>> >> >>> >> /* >>> >> @@ -96,6 +97,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, const >>> >> QEMUFileOps *ops) >>> >> >>> >> f = g_new0(QEMUFile, 1); >>> >> >>> >> +qemu_mutex_init(&f->lock); >>> >> f->opaque = opaque; >>> >> f->ops = ops; >>> >> return f; >>> >> @@ -328,7 +330,9 @@ int qemu_fclose(QEMUFile *f) >>> >> ret = qemu_file_get_error(f); >>> >> >>> >> if (f->ops->close) { >>> >> +qemu_mutex_lock(&f->lock); >>> >> int ret2 = f->ops->close(f->opaque); >>> >> +qemu_mutex_unlock(&f->lock); >>> > >>> > OK, and at least for the RDMA code, if it calls >>> > close a 2nd time, rioc->rdma is checked so it wont actually free stuff a >>> > 2nd time. >>> > >>> >> if (ret >= 0) { >>> >> ret = ret2; >>> >> } >>> >> @@ -339,6 +343,7 @@ int qemu_fclose(QEMUFile *f) >>> >> if (f->last_error) { >>> >> ret = f->last_error; >>> >> } >>> >> +qemu_mutex_destroy(&f->lock); >>> >> g_free(f); >>> > >>> > Hmm but that's not safe; if two things really do call qemu_fclose() >>> > on the same structure they race here and can end up destroying the lock >>> > twice, or doing f->lock after the 1st one has already g_free(f). >>> >>> > >>> > >>> > So lets go back a step. >>> > I think: >>> > a) There should always be a separate QEMUFile* for >>> > to_src_file and from_src_file - I don't see where you open >>> > the 2nd one; I don't see your implementation of >>> > f->ops->get_return_path. >>> >>> yes, current qemu version use a separate QEMUFile* for to_src_file and >>> from_src_file. >>> and the two QEMUFile point to one QIOChannelRDMA. >>> >>> the f->ops->get_return_path is implemented by channel_output_ops or >>> channel_input_ops. >> >> Ah OK, yes that makes sense. >> >>> > b) I *think* that while the different threads might all call >>> > fclose(), I think there should only ever be one qemu_fclose >>> > call for each direction on the QEMUFile. >>> > >>> > But now we have two problems: >>> > If (a) is true then f->lock is separate on each one so >>> >doesn't really protect if the two directions are closed >>> >at once. (Assuming (b) is true) >>> >>> yes, you are right. so I should add a QemuMutex in QIOChannel structure, >>> not >>> QEMUFile structure. and qemu_mutex_destroy the QemuMutex in >>> qio_channel_finalize. >> >> OK, that sounds better. >> >> Dave >> > > Hi Dave: > Another way is protect channel_close in migration part, like > QemuMutex rp_mutex. > As Daniel mentioned, QIOChannel impls are only intended to a single > thread. > https://www.mail-archive.com/qemu-devel@nongnu.org/msg530100.html > > which way is better? Does QIOChannel have the plan to support multi > thread? > Not only channel_close need lock between different threads, > writev_buffer write also > need. > > thanks. > > I find qemu not call qemu_mutex_destroy to release rp_mutex in migration_instance_finalize:( although qemu_mutex_destroy is not necceesary, but it is a good practice to do. it's better we fixed it. >>> Thank you. >>> >>> > >>> > If (a) is false and we actually share a single QEMUFile then >>> > that race at the end happens. >>> > >>> > Dave >>> > >>> > >>> >> trace_qemu_file_fclose(); >>> >> return ret; >>> >> -- >>> >> 1.8.3.1 >>> >> >>> > -- >>> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK >> -- >> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v4 11/12] migration: poll the cm event while wait RDMA work request completion
+Gal Gal, please comment with our findings. Thanks! On 5/31/2018 10:36 AM, 858585 jemmy wrote: On Thu, May 31, 2018 at 1:33 AM, Dr. David Alan Gilbert wrote: * Lidong Chen (jemmy858...@gmail.com) wrote: If the peer qemu is crashed, the qemu_rdma_wait_comp_channel function maybe loop forever. so we should also poll the cm event fd, and when receive any cm event, we consider some error happened. Signed-off-by: Lidong Chen I don't understand enough about the way the infiniband fd's work to fully review this; so I'd appreciate if some one who does could comment/add their review. Hi Avaid: we need your help. I also not find any document about the cq channel event fd and cm channel event f. Should we set the events to G_IO_IN | G_IO_HUP | G_IO_ERR? or G_IO_IN is enough? pfds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR; pfds[1].events = G_IO_IN | G_IO_HUP | G_IO_ERR; Thanks. --- migration/rdma.c | 35 --- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/migration/rdma.c b/migration/rdma.c index 1b9e261..d611a06 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -1489,6 +1489,9 @@ static uint64_t qemu_rdma_poll(RDMAContext *rdma, uint64_t *wr_id_out, */ static int qemu_rdma_wait_comp_channel(RDMAContext *rdma) { +struct rdma_cm_event *cm_event; +int ret = -1; + /* * Coroutine doesn't start until migration_fd_process_incoming() * so don't yield unless we know we're running inside of a coroutine. @@ -1504,25 +1507,35 @@ static int qemu_rdma_wait_comp_channel(RDMAContext *rdma) * But we need to be able to handle 'cancel' or an error * without hanging forever. */ -while (!rdma->error_state && !rdma->received_error) { -GPollFD pfds[1]; +while (!rdma->error_state && !rdma->received_error) { +GPollFD pfds[2]; pfds[0].fd = rdma->comp_channel->fd; pfds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR; +pfds[0].revents = 0; + +pfds[1].fd = rdma->channel->fd; +pfds[1].events = G_IO_IN | G_IO_HUP | G_IO_ERR; +pfds[1].revents = 0; + /* 0.1s timeout, should be fine for a 'cancel' */ -switch (qemu_poll_ns(pfds, 1, 100 * 1000 * 1000)) { -case 1: /* fd active */ -return 0; +qemu_poll_ns(pfds, 2, 100 * 1000 * 1000); Shouldn't we still check the return value of this; if it's negative something has gone wrong. I will fix this. Thanks. Dave -case 0: /* Timeout, go around again */ -break; +if (pfds[1].revents) { +ret = rdma_get_cm_event(rdma->channel, &cm_event); +if (!ret) { +rdma_ack_cm_event(cm_event); +} +error_report("receive cm event while wait comp channel," + "cm event is %d", cm_event->event); -default: /* Error of some type - - * I don't trust errno from qemu_poll_ns - */ -error_report("%s: poll failed", __func__); +/* consider any rdma communication event as an error */ return -EPIPE; } +if (pfds[0].revents) { +return 0; +} + if (migrate_get_current()->state == MIGRATION_STATUS_CANCELLING) { /* Bail out and let the cancellation happen */ return -EPIPE; -- 1.8.3.1 -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] Virtio-net drivers immune to Nethammer?
On Sat, Jun 02, 2018 at 03:08:54AM +, procmem wrote: > > > Michael S. Tsirkin: > > On Fri, Jun 01, 2018 at 01:15:44PM +, procmem wrote: > >> > >> > >> Stefan Hajnoczi: > >>> On Mon, May 21, 2018 at 11:24:43PM +, procmem wrote: > Hi I'm a privacy distro maintainer investigating the implications of the > newly published nethammer attack [0] on KVM guests particularly the > virtio-net drivers. The summary of the paper is that rowhammer can be > remotely triggered by feeding susceptible* network driver crafted > traffic. This attack can do all kinds of nasty things such as modifying > SSL certs on the victim system. > > * Susceptible drivers are those relying on Intel CAT, uncached memory or > the clflush instruction. > > My question is, do virtio-net drivers do any of these things? > >>> I have CCed Michael Tsirkin and Jason Wang, the virtio maintainers. > >>> > *** > > [0] https://arxiv.org/abs/1805.04956 > > > > >> > >> Thanks :) I thought my message was forgotten > > > > > > I don't think virtio is using either of these. > > > > Linux does support CAT AFAIK but it has nothing to do with virtio. > > > > Thanks for confirming. This is good news indeed. I am considering > posting about this to kernel-hardening so it's on the sec team's radar > when considering upstream network drivers. What do you think? It's up to you but the usefulness of reposting like that will be limited IMHO, unless you have something specific to add. I think everyone saw the nethammer paper by now, and kernel hardening team doesn't review network driver patches. -- MST
Re: [Qemu-devel] [PATCH 1/8] virtio: feature bit, data structure for packed ring
On Tue, Apr 10, 2018 at 03:05:24PM +0800, Jason Wang wrote: > > > On 2018年04月04日 20:53, w...@redhat.com wrote: > >From: Wei Xu > > > >Only minimum definitions from the spec are included > >for prototype. > > > >Signed-off-by: Wei Xu > >--- > > hw/virtio/virtio.c | 47 > > +++--- > > include/hw/virtio/virtio.h | 12 ++- > > include/standard-headers/linux/virtio_config.h | 2 ++ > > 3 files changed, 56 insertions(+), 5 deletions(-) > > > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > >index 006d3d1..9a6bfe7 100644 > >--- a/hw/virtio/virtio.c > >+++ b/hw/virtio/virtio.c > >@@ -39,6 +39,14 @@ typedef struct VRingDesc > > uint16_t next; > > } VRingDesc; > >+typedef struct VRingDescPacked > >+{ > >+uint64_t addr; > >+uint32_t len; > >+uint16_t id; > >+uint16_t flags; > >+} VRingDescPacked; > >+ > > typedef struct VRingAvail > > { > > uint16_t flags; > >@@ -61,9 +69,18 @@ typedef struct VRingUsed > > typedef struct VRingMemoryRegionCaches { > > struct rcu_head rcu; > >-MemoryRegionCache desc; > >-MemoryRegionCache avail; > >-MemoryRegionCache used; > >+union { > >+struct { > >+MemoryRegionCache desc; > >+MemoryRegionCache avail; > >+MemoryRegionCache used; > >+}; > >+struct { > >+MemoryRegionCache desc_packed; > >+MemoryRegionCache driver; > >+MemoryRegionCache device; > >+}; > >+}; > > I think we can reuse exist memory region caches? Especially consider > device/driver area could be treated as a renaming of avail/used area. > > E.g desc for desc_packed, avail for driver area and used for device area. Yes, I will take it. > > > } VRingMemoryRegionCaches; > > typedef struct VRing > >@@ -77,10 +94,31 @@ typedef struct VRing > > VRingMemoryRegionCaches *caches; > > } VRing; > >+typedef struct VRingPackedDescEvent { > >+uint16_t desc_event_off:15, > >+ desc_event_wrap:1; > >+uint16_t desc_event_flags:2; > >+} VRingPackedDescEvent ; > >+ > >+typedef struct VRingPacked > >+{ > >+unsigned int num; > >+unsigned int num_default; > >+unsigned int align; > >+hwaddr desc; > >+hwaddr driver; > >+hwaddr device; > >+VRingMemoryRegionCaches *caches; > >+} VRingPacked; > > Same here, can we reuse VRing here? Yes. > > >+ > > struct VirtQueue > > { > >-VRing vring; > >+union { > >+struct VRing vring; > >+struct VRingPacked packed; > >+}; > >+uint8_t wrap_counter:1; > > /* Next head to pop */ > > uint16_t last_avail_idx; > >@@ -1220,6 +1258,7 @@ void virtio_reset(void *opaque) > > vdev->vq[i].vring.num = vdev->vq[i].vring.num_default; > > vdev->vq[i].inuse = 0; > > virtio_virtqueue_reset_region_cache(&vdev->vq[i]); > >+vdev->vq[i].wrap_counter = 1; > > } > > } > >diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > >index 098bdaa..563e88e 100644 > >--- a/include/hw/virtio/virtio.h > >+++ b/include/hw/virtio/virtio.h > >@@ -46,6 +46,14 @@ typedef struct VirtQueueElement > > unsigned int index; > > unsigned int out_num; > > unsigned int in_num; > >+ > >+/* Number of descriptors used by packed ring */ > > Do you mean the number of chained descriptors? These has been removed. > > >+uint16_t count; > >+uint8_t wrap_counter:1; > > What's the use of this bit? If you refer to my v1 vhost code, I used to have > this, but it won't work for OOO completion e.g when zerocopy is disabled. > I've dropped it now. > > This is tricky and can only work when device complete descriptors in order. Same here. > > >+/* FIXME: Length of every used buffer for a descriptor, > >+ move to dynamical allocating due to out/in sgs numbers */ > >+uint32_t len[VIRTQUEUE_MAX_SIZE]; > > Can you explain more about this? Also here. > > >+ > > hwaddr *in_addr; > > hwaddr *out_addr; > > struct iovec *in_sg; > >@@ -262,7 +270,9 @@ typedef struct VirtIORNGConf VirtIORNGConf; > > DEFINE_PROP_BIT64("any_layout", _state, _field, \ > >VIRTIO_F_ANY_LAYOUT, true), \ > > DEFINE_PROP_BIT64("iommu_platform", _state, _field, \ > >- VIRTIO_F_IOMMU_PLATFORM, false) > >+ VIRTIO_F_IOMMU_PLATFORM, false), \ > >+DEFINE_PROP_BIT64("ring_packed", _state, _field, \ > >+ VIRTIO_F_RING_PACKED, true) > > Remember to disable this for old machine types in next version. Sure, will do. > > Thanks > > > hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n); > > hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n); > >diff --git a/include/standard-headers/linux/virtio_config.h > >b/include/standard-headers/linux/virtio_config.h > >index b777069..6ee5529 100644 > >--- a/include/standard-headers/linux/virti
[Qemu-devel] [Bug 1754542] Re: colo: vm crash with segmentation fault
Hi Suiheng, This bug have been fixed in my latest patch. Please retest it. https://www.mail-archive.com/qemu-devel@nongnu.org/msg538383.html github: https://github.com/zhangckid/qemu/tree/qemu-colo-18jun1 Thanks Zhang Chen -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1754542 Title: colo: vm crash with segmentation fault Status in QEMU: New Bug description: I use Arch Linux x86_64 Zhang Chen's(https://github.com/zhangckid/qemu/tree/qemu-colo-18mar10) Following document 'COLO-FT.txt', I test colo feature on my hosts I run this command Primary: sudo /usr/local/bin/qemu-system-x86_64 -enable-kvm -m 2048 -smp 2 -qmp stdio -name primary \ -device piix3-usb-uhci \ -device usb-tablet -netdev tap,id=hn0,vhost=off \ -device virtio-net-pci,id=net-pci0,netdev=hn0 \ -drive if=virtio,id=primary-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\ children.0.file.filename=/var/lib/libvirt/images/1.raw,\ children.0.driver=raw -S Secondary: sudo /usr/local/bin/qemu-system-x86_64 -enable-kvm -m 2048 -smp 2 -qmp stdio -name secondary \ -device piix3-usb-uhci \ -device usb-tablet -netdev tap,id=hn0,vhost=off \ -device virtio-net-pci,id=net-pci0,netdev=hn0 \ -drive if=none,id=secondary-disk0,file.filename=/var/lib/libvirt/images/2.raw,driver=raw,node-name=node0 \ -drive if=virtio,id=active-disk0,driver=replication,mode=secondary,\ file.driver=qcow2,top-id=active-disk0,\ file.file.filename=/mnt/ramfs/active_disk.img,\ file.backing.driver=qcow2,\ file.backing.file.filename=/mnt/ramfs/hidden_disk.img,\ file.backing.backing=secondary-disk0 \ -incoming tcp:0: Secondary: {'execute':'qmp_capabilities'} { 'execute': 'nbd-server-start', 'arguments': {'addr': {'type': 'inet', 'data': {'host': '192.168.0.34', 'port': '8889'} } } } {'execute': 'nbd-server-add', 'arguments': {'device': 'secondary-disk0', 'writable': true } } Primary: {'execute':'qmp_capabilities'} { 'execute': 'human-monitor-command', 'arguments': {'command-line': 'drive_add -n buddy driver=replication,mode=primary,file.driver=nbd,file.host=192.168.0.34,file.port=8889,file.export=secondary-disk0,node-name=nbd_client0'}} { 'execute':'x-blockdev-change', 'arguments':{'parent': 'primary-disk0', 'node': 'nbd_client0' } } { 'execute': 'migrate-set-capabilities', 'arguments': {'capabilities': [ {'capability': 'x-colo', 'state': true } ] } } { 'execute': 'migrate', 'arguments': {'uri': 'tcp:192.168.0.34:' } } And two VM with cash Primary: {"timestamp": {"seconds": 1520763655, "microseconds": 511415}, "event": "RESUME"} [1]329 segmentation fault sudo /usr/local/bin/qemu-system-x86_64 -boot c -enable-kvm -m 2048 -smp 2 -qm Secondary: {"timestamp": {"seconds": 1520763655, "microseconds": 510907}, "event": "RESUME"} [1]367 segmentation fault sudo /usr/local/bin/qemu-system-x86_64 -boot c -enable-kvm -m 2048 -smp 2 -qm To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1754542/+subscriptions
Re: [Qemu-devel] [PATCH 3/8] virtio: add empty check for packed ring
On Tue, Apr 10, 2018 at 03:23:03PM +0800, Jason Wang wrote: > > > On 2018年04月04日 20:53, w...@redhat.com wrote: > >From: Wei Xu > > > >helper for ring empty check. > > And descriptor read. OK. > > > > >Signed-off-by: Wei Xu > >--- > > hw/virtio/virtio.c | 62 > > +++--- > > 1 file changed, 59 insertions(+), 3 deletions(-) > > > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > >index 73a35a4..478df3d 100644 > >--- a/hw/virtio/virtio.c > >+++ b/hw/virtio/virtio.c > >@@ -24,6 +24,9 @@ > > #include "hw/virtio/virtio-access.h" > > #include "sysemu/dma.h" > >+#define AVAIL_DESC_PACKED(b) ((b) << 7) > >+#define USED_DESC_PACKED(b) ((b) << 15) > > Can we pass value other than 1 to this macro? Yes, '0' is also provided for some clear/reset cases. > > >+ > > /* > > * The alignment to use between consumer and producer parts of vring. > > * x86 pagesize again. This is the default, used by transports like PCI > >@@ -446,10 +449,27 @@ int virtio_queue_ready(VirtQueue *vq) > > return vq->vring.avail != 0; > > } > >+static void vring_desc_read_packed(VirtIODevice *vdev, VRingDescPacked > >*desc, > >+MemoryRegionCache *cache, int i) > >+{ > >+address_space_read_cached(cache, i * sizeof(VRingDescPacked), > >+ desc, sizeof(VRingDescPacked)); > >+virtio_tswap64s(vdev, &desc->addr); > >+virtio_tswap32s(vdev, &desc->len); > >+virtio_tswap16s(vdev, &desc->id); > >+virtio_tswap16s(vdev, &desc->flags); > >+} > >+ > >+static inline bool is_desc_avail(struct VRingDescPacked* desc) > >+{ > >+return (!!(desc->flags & AVAIL_DESC_PACKED(1)) != > >+!!(desc->flags & USED_DESC_PACKED(1))); > > Don't we need to care about endian here? Usually we don't since endian has been converted during reading, will double check it. > > >+} > >+ > > /* Fetch avail_idx from VQ memory only when we really need to know if > > * guest has added some buffers. > > * Called within rcu_read_lock(). */ > >-static int virtio_queue_empty_rcu(VirtQueue *vq) > >+static int virtio_queue_empty_split_rcu(VirtQueue *vq) > > { > > if (unlikely(!vq->vring.avail)) { > > return 1; > >@@ -462,7 +482,7 @@ static int virtio_queue_empty_rcu(VirtQueue *vq) > > return vring_avail_idx(vq) == vq->last_avail_idx; > > } > >-int virtio_queue_empty(VirtQueue *vq) > >+static int virtio_queue_empty_split(VirtQueue *vq) > > { > > bool empty; > >@@ -480,6 +500,42 @@ int virtio_queue_empty(VirtQueue *vq) > > return empty; > > } > >+static int virtio_queue_empty_packed_rcu(VirtQueue *vq) > >+{ > >+struct VRingDescPacked desc; > >+VRingMemoryRegionCaches *cache; > >+ > >+if (unlikely(!vq->packed.desc)) { > >+return 1; > >+} > >+ > >+cache = vring_get_region_caches(vq); > >+vring_desc_read_packed(vq->vdev, &desc, &cache->desc_packed, > >vq->last_avail_idx); > >+ > >+/* Make sure we see the updated flag */ > >+smp_mb(); > > What we need here is to make sure flag is read before all other fields, > looks like this barrier can't. Isn't flag updated yet if it has been read? > > >+return !is_desc_avail(&desc); > >+} > >+ > >+static int virtio_queue_empty_packed(VirtQueue *vq) > >+{ > >+bool empty; > >+ > >+rcu_read_lock(); > >+empty = virtio_queue_empty_packed_rcu(vq); > >+rcu_read_unlock(); > >+return empty; > >+} > >+ > >+int virtio_queue_empty(VirtQueue *vq) > >+{ > >+if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) { > >+return virtio_queue_empty_packed(vq); > >+} else { > >+return virtio_queue_empty_split(vq); > >+} > >+} > >+ > > static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem, > > unsigned int len) > > { > >@@ -951,7 +1007,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) > > return NULL; > > } > > rcu_read_lock(); > >-if (virtio_queue_empty_rcu(vq)) { > >+if (virtio_queue_empty_split_rcu(vq)) { > > I think you'd better have a switch inside virtio_queue_empty_rcu() like > virtio_queue_empty() here. OK. > > Thanks > > > goto done; > > } > > /* Needed after virtio_queue_empty(), see comment in >
[Qemu-devel] [RFC v2] qapi: command category to stimulate high-level machine devices
Changes since v1: - Fix coding style issues Add a new category "stimulate" to host commands that act upon high-level devices associated with machines/boards. This is patch is part of an effort to add support for BBC:microbit educational computer to QEMU. The microbit board, as many other microcontroller boards, contains typical trivial (digital) input and output options such as buttons and LEDs, but also more sophiscated possibilities such as analog inputs and complex dedicated sensor ICs that are connected to the microbit machine by means of inter-ic bus (i2c). These devices interact with peripheral devices of the microcontroller in use, for example with the GPIO peripheral of the used NRF51. One aim of our efforts is to create a user interface that models the look and feel of the physical microbit board and lets the user interact with its inputs and provide feedback on its outsputs. For this it is necessary to transmit user inputs such as the pressing of a button to the machine. In return, when the state of an output is changed on the machine, this change needs to be reflected in the user interface. At the moment, there are only a few QMP commands that provide user input to the machine, eg. send-keys, input-button. These commands require very high-level concepts, which are not really suitable for applications that microcontrollers are typically used in in. This RFC is meant to start a discussion on how to provide stimuli to low-complex inputs and outputs typically found in embedded microcontroller applications. To the best of my knowledge, there are currently no examples of how to provide such stimuli to either SoC device peripheral or machine level concepts like pushbuttons and LEDs. To my understanding, the following concepts/apis are needed: - QMP commands to send stimuli to machine level concepts like pushbuttons - Machine level devices that receive these stimuli and act as an adapter to manipulate the associated SoC peripheral devices - For outputs, machine level devices are needed that observe changes in SoC peripherals and emit QMP events that clients can receive This patch adds a new qmp command "buttons-set-state" to update the push-down state of arbitrarily identified buttons (string identifier). The handler of this command is mocked to set the machines SoC gpio/irq lines associated with these buttons on the machine by means of object property aliases. This is just a mock until a proper concept/api is agreed upon. As i recently joined the QEMU community as part of GSoC, i am still a greenhorn to the codebase and i am really excited to discuss potential concepts and APIs to realize the described features. Steffen Based-on: <20180503090532.3113-1-j...@jms.id.au> Signed-off-by: Steffen Goertz --- Makefile | 8 +++ Makefile.objs | 4 Makefile.target | 1 + hw/arm/microbit.c | 7 ++ qapi/qapi-schema.json | 1 + qapi/stimulate.json | 24 stimulate.c | 52 +++ 7 files changed, 97 insertions(+) create mode 100644 qapi/stimulate.json create mode 100644 stimulate.c diff --git a/Makefile b/Makefile index d71dd5bea4..d9319efa68 100644 --- a/Makefile +++ b/Makefile @@ -108,6 +108,7 @@ GENERATED_FILES += qapi/qapi-types-tpm.h qapi/qapi-types-tpm.c GENERATED_FILES += qapi/qapi-types-trace.h qapi/qapi-types-trace.c GENERATED_FILES += qapi/qapi-types-transaction.h qapi/qapi-types-transaction.c GENERATED_FILES += qapi/qapi-types-ui.h qapi/qapi-types-ui.c +GENERATED_FILES += qapi/qapi-types-stimulate.h qapi/qapi-types-stimulate.c GENERATED_FILES += qapi/qapi-builtin-visit.h qapi/qapi-builtin-visit.c GENERATED_FILES += qapi/qapi-visit.h qapi/qapi-visit.c GENERATED_FILES += qapi/qapi-visit-block-core.h qapi/qapi-visit-block-core.c @@ -126,6 +127,7 @@ GENERATED_FILES += qapi/qapi-visit-tpm.h qapi/qapi-visit-tpm.c GENERATED_FILES += qapi/qapi-visit-trace.h qapi/qapi-visit-trace.c GENERATED_FILES += qapi/qapi-visit-transaction.h qapi/qapi-visit-transaction.c GENERATED_FILES += qapi/qapi-visit-ui.h qapi/qapi-visit-ui.c +GENERATED_FILES += qapi/qapi-visit-stimulate.h qapi/qapi-visit-stimulate.c GENERATED_FILES += qapi/qapi-commands.h qapi/qapi-commands.c GENERATED_FILES += qapi/qapi-commands-block-core.h qapi/qapi-commands-block-core.c GENERATED_FILES += qapi/qapi-commands-block.h qapi/qapi-commands-block.c @@ -143,6 +145,7 @@ GENERATED_FILES += qapi/qapi-commands-tpm.h qapi/qapi-commands-tpm.c GENERATED_FILES += qapi/qapi-commands-trace.h qapi/qapi-commands-trace.c GENERATED_FILES += qapi/qapi-commands-transaction.h qapi/qapi-commands-transaction.c GENERATED_FILES += qapi/qapi-commands-ui.h qapi/qapi-commands-ui.c +GENERATED_FILES += qapi/qapi-commands-stimulate.h qapi/qapi-commands-stimulate.c GENERATED_FILES += qapi/qapi-events.h qapi/qapi-events.c GENERATED_FILES += qapi/qapi-events-block-core.h qapi/qapi-events-block-core.c GENERATED_FILES += qapi/qap
Re: [Qemu-devel] [RFC v2] qapi: command category to stimulate high-level machine devices
Hi, This series failed docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. Type: series Message-id: 20180603192242.17675-1-cont...@steffen-goertz.de Subject: [Qemu-devel] [RFC v2] qapi: command category to stimulate high-level machine devices === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc # Let docker tests dump environment info export SHOW_ENV=1 export J=8 time make docker-test-mingw@fedora === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 148ce700a2 qapi: command category to stimulate high-level machine devices === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-nuhs55kf/src/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' BUILD fedora make[1]: Entering directory '/var/tmp/patchew-tester-tmp-nuhs55kf/src' GEN /var/tmp/patchew-tester-tmp-nuhs55kf/src/docker-src.2018-06-03-15.27.49.30327/qemu.tar Cloning into '/var/tmp/patchew-tester-tmp-nuhs55kf/src/docker-src.2018-06-03-15.27.49.30327/qemu.tar.vroot'... done. Checking out files: 49% (3040/6116) Checking out files: 50% (3058/6116) Checking out files: 51% (3120/6116) Checking out files: 52% (3181/6116) Checking out files: 53% (3242/6116) Checking out files: 54% (3303/6116) Checking out files: 55% (3364/6116) Checking out files: 56% (3425/6116) Checking out files: 57% (3487/6116) Checking out files: 58% (3548/6116) Checking out files: 59% (3609/6116) Checking out files: 60% (3670/6116) Checking out files: 61% (3731/6116) Checking out files: 62% (3792/6116) Checking out files: 63% (3854/6116) Checking out files: 64% (3915/6116) Checking out files: 65% (3976/6116) Checking out files: 66% (4037/6116) Checking out files: 67% (4098/6116) Checking out files: 68% (4159/6116) Checking out files: 69% (4221/6116) Checking out files: 70% (4282/6116) Checking out files: 71% (4343/6116) Checking out files: 72% (4404/6116) Checking out files: 73% (4465/6116) Checking out files: 74% (4526/6116) Checking out files: 75% (4587/6116) Checking out files: 76% (4649/6116) Checking out files: 77% (4710/6116) Checking out files: 78% (4771/6116) Checking out files: 79% (4832/6116) Checking out files: 80% (4893/6116) Checking out files: 81% (4954/6116) Checking out files: 82% (5016/6116) Checking out files: 83% (5077/6116) Checking out files: 84% (5138/6116) Checking out files: 85% (5199/6116) Checking out files: 86% (5260/6116) Checking out files: 87% (5321/6116) Checking out files: 88% (5383/6116) Checking out files: 89% (5444/6116) Checking out files: 90% (5505/6116) Checking out files: 91% (5566/6116) Checking out files: 92% (5627/6116) Checking out files: 93% (5688/6116) Checking out files: 94% (5750/6116) Checking out files: 95% (5811/6116) Checking out files: 96% (5872/6116) Checking out files: 97% (5933/6116) Checking out files: 98% (5994/6116) Checking out files: 98% (6036/6116) Checking out files: 99% (6055/6116) Checking out files: 100% (6116/6116) Checking out files: 100% (6116/6116), done. Your branch is up-to-date with 'origin/test'. Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-nuhs55kf/src/docker-src.2018-06-03-15.27.49.30327/qemu.tar.vroot/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-nuhs55kf/src/docker-src.2018-06-03-15.27.49.30327/qemu.tar.vroot/ui/keycodemapdb'... Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce' COPYRUNNER RUN test-mingw in qemu:fedora Packages installed: PyYAML-3.12-5.fc27.x86_64 SDL2-devel-2.0.7-2.fc27.x86_64 bc-1.07.1-3.fc27.x86_64 bison-3.0.4-8.fc27.x86_64 bluez-libs-devel-5.48-3.fc27.x86_64 brlapi-devel-0.6.6-8.fc27.x86_64 bzip2-1.0.6-24.fc27.x86_64 bzip2-devel-1.0.6-24.fc27.x86_64 ccache-3.3.6-1.fc27.x86_64 clang-5.0.1-5.fc27.x86_64 device-mapper-multipath-devel-0.7.1-9.git847cc43.fc27.x86_64 findutils-4.6.0-16.fc27.x86_64 flex-2.6.1-5.fc27.x86_64 gcc-7.3.1-5.fc27.x86_64 gcc-c++-7.3.1-5.fc27.x86_64 gettext-0.19.8.1-12.fc27.x86_64 git-2.14.3-3.fc27.x86_64 glib2-devel-2.54.3-2.fc27.x86_64 glusterfs-api-devel-3.12.7-1.fc27.x86_64 gnutls-devel-3.5.18-2.fc27.x86_64 gtk3-devel-3.22.26-2.fc27.x86_64 hostname-3.18-4.fc27.x86_64 libaio-devel-0.3.110-9.fc27.x86_64 libasan-7.3.1-5.fc27.x86_64 libattr-devel-2.4.47-21.fc27.x86_64 libcap-devel-2.25-7.fc27.x86_64 libcap-ng-devel-0.7.8-5.fc27.x86_64 libcurl-d
Re: [Qemu-devel] [RFC v2] qapi: command category to stimulate high-level machine devices
Hi, This series failed build test on s390x host. Please find the details below. Type: series Message-id: 20180603192242.17675-1-cont...@steffen-goertz.de Subject: [Qemu-devel] [RFC v2] qapi: command category to stimulate high-level machine devices === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch set -e echo "=== ENV ===" env echo "=== PACKAGES ===" rpm -qa echo "=== TEST BEGIN ===" CC=$HOME/bin/cc INSTALL=$PWD/install BUILD=$PWD/build echo -n "Using CC: " realpath $CC mkdir -p $BUILD $INSTALL SRC=$PWD cd $BUILD $SRC/configure --cc=$CC --prefix=$INSTALL make -j4 # XXX: we need reliable clean up # make check -j4 V=1 make install === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20180603192242.17675-1-cont...@steffen-goertz.de -> patchew/20180603192242.17675-1-cont...@steffen-goertz.de Switched to a new branch 'test' 148ce700a2 qapi: command category to stimulate high-level machine devices === OUTPUT BEGIN === === ENV === LANG=en_US.UTF-8 XDG_SESSION_ID=214891 USER=fam PWD=/var/tmp/patchew-tester-tmp-un92ne0p/src HOME=/home/fam SHELL=/bin/sh SHLVL=2 PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug LOGNAME=fam DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus XDG_RUNTIME_DIR=/run/user/1012 PATH=/usr/bin:/bin _=/usr/bin/env === PACKAGES === gpg-pubkey-873529b8-54e386ff glibc-debuginfo-common-2.24-10.fc25.s390x fedora-release-26-1.noarch dejavu-sans-mono-fonts-2.35-4.fc26.noarch xemacs-filesystem-21.5.34-22.20170124hgf412e9f093d4.fc26.noarch bash-4.4.12-7.fc26.s390x libSM-1.2.2-5.fc26.s390x libmpc-1.0.2-6.fc26.s390x libaio-0.3.110-7.fc26.s390x libverto-0.2.6-7.fc26.s390x perl-Scalar-List-Utils-1.48-1.fc26.s390x iptables-libs-1.6.1-2.fc26.s390x tcl-8.6.6-2.fc26.s390x libxshmfence-1.2-4.fc26.s390x expect-5.45-23.fc26.s390x perl-Thread-Queue-3.12-1.fc26.noarch perl-encoding-2.19-6.fc26.s390x keyutils-1.5.10-1.fc26.s390x gmp-devel-6.1.2-4.fc26.s390x enchant-1.6.0-16.fc26.s390x python-gobject-base-3.24.1-1.fc26.s390x python3-enchant-1.6.10-1.fc26.noarch python-lockfile-0.11.0-6.fc26.noarch python2-pyparsing-2.1.10-3.fc26.noarch python2-lxml-4.1.1-1.fc26.s390x librados2-10.2.7-2.fc26.s390x trousers-lib-0.3.13-7.fc26.s390x libdatrie-0.2.9-4.fc26.s390x libsoup-2.58.2-1.fc26.s390x passwd-0.79-9.fc26.s390x bind99-libs-9.9.10-3.P3.fc26.s390x python3-rpm-4.13.0.2-1.fc26.s390x systemd-233-7.fc26.s390x virglrenderer-0.6.0-1.20170210git76b3da97b.fc26.s390x s390utils-ziomon-1.36.1-3.fc26.s390x s390utils-osasnmpd-1.36.1-3.fc26.s390x libXrandr-1.5.1-2.fc26.s390x libglvnd-glx-1.0.0-1.fc26.s390x texlive-ifxetex-svn19685.0.5-33.fc26.2.noarch texlive-psnfss-svn33946.9.2a-33.fc26.2.noarch texlive-dvipdfmx-def-svn40328-33.fc26.2.noarch texlive-natbib-svn20668.8.31b-33.fc26.2.noarch texlive-xdvi-bin-svn40750-33.20160520.fc26.2.s390x texlive-cm-svn32865.0-33.fc26.2.noarch texlive-beton-svn15878.0-33.fc26.2.noarch texlive-fpl-svn15878.1.002-33.fc26.2.noarch texlive-mflogo-svn38628-33.fc26.2.noarch texlive-texlive-docindex-svn41430-33.fc26.2.noarch texlive-luaotfload-bin-svn34647.0-33.20160520.fc26.2.noarch texlive-koma-script-svn41508-33.fc26.2.noarch texlive-pst-tree-svn24142.1.12-33.fc26.2.noarch texlive-breqn-svn38099.0.98d-33.fc26.2.noarch texlive-xetex-svn41438-33.fc26.2.noarch gstreamer1-plugins-bad-free-1.12.3-1.fc26.s390x xorg-x11-font-utils-7.5-33.fc26.s390x ghostscript-fonts-5.50-36.fc26.noarch libXext-devel-1.3.3-5.fc26.s390x libusbx-devel-1.0.21-2.fc26.s390x libglvnd-devel-1.0.0-1.fc26.s390x emacs-25.3-3.fc26.s390x alsa-lib-devel-1.1.4.1-1.fc26.s390x kbd-2.0.4-2.fc26.s390x dconf-0.26.0-2.fc26.s390x mc-4.8.19-5.fc26.s390x doxygen-1.8.13-9.fc26.s390x dpkg-1.18.24-1.fc26.s390x libtdb-1.3.13-1.fc26.s390x python2-pynacl-1.1.1-1.fc26.s390x perl-Filter-1.58-1.fc26.s390x python2-pip-9.0.1-11.fc26.noarch dnf-2.7.5-2.fc26.noarch bind-license-9.11.2-1.P1.fc26.noarch libtasn1-4.13-1.fc26.s390x cpp-7.3.1-2.fc26.s390x pkgconf-1.3.12-2.fc26.s390x python2-fedora-0.10.0-1.fc26.noarch cmake-filesystem-3.10.1-11.fc26.s390x python3-requests-kerberos-0.12.0-1.fc26.noarch libmicrohttpd-0.9.59-1.fc26.s390x GeoIP-GeoLite-data-2018.01-1.fc26.noarch python2-libs-2.7.14-7.fc26.s390x libidn2-2.0.4-3.fc26.s390x p11-kit-devel-0.23.10-1.fc26.s390x perl-Errno-1.25-396.fc26.s390x libdrm-2.4.90-2.fc26.s390x sssd-common-1.16.1-1.fc26.s390x boost-random-1.63.0-11.fc26.s390x urw-fonts-2.4-24.fc26.noarch ccache-3.3.6-1.fc26.s390x glibc-debuginfo-2.24-10.fc25.s390x dejavu-fonts-common-2.35-4.fc26.noarch bind99-license-9.9.10-3.P3.fc26.noarch ncurses-libs-6.0-8.20170212.fc26.s390x libpng-1.6.28-2.fc26.s390x libICE-1.0.9-9.fc26.s390x perl-Text-ParseWords-3.30-366.fc26.noarch libtool-ltdl-2.4.6-17.fc26.s390x libselinux-utils-2.6-7.fc26.s390x userspace-rcu-0.9.3-2.fc26.s390x perl-Class-Inspector-1.31-3.f
[Qemu-devel] [RFC v3] qapi: command category to stimulate high-level machine devices
Changes in v2: - Remove stray 1 Changes in v1: - Fix coding style issues Add a new category "stimulate" to host commands that act upon high-level devices associated with machines/boards. This is patch is part of an effort to add support for BBC:microbit educational computer to QEMU. The microbit board, as many other microcontroller boards, contains typical trivial (digital) input and output options such as buttons and LEDs, but also more sophiscated possibilities such as analog inputs and complex dedicated sensor ICs that are connected to the microbit machine by means of inter-ic bus (i2c). These devices interact with peripheral devices of the microcontroller in use, for example with the GPIO peripheral of the used NRF51. One aim of our efforts is to create a user interface that models the look and feel of the physical microbit board and lets the user interact with its inputs and provide feedback on its outsputs. For this it is necessary to transmit user inputs such as the pressing of a button to the machine. In return, when the state of an output is changed on the machine, this change needs to be reflected in the user interface. At the moment, there are only a few QMP commands that provide user input to the machine, eg. send-keys, input-button. These commands require very high-level concepts, which are not really suitable for applications that microcontrollers are typically used in in. This RFC is meant to start a discussion on how to provide stimuli to low-complex inputs and outputs typically found in embedded microcontroller applications. To the best of my knowledge, there are currently no examples of how to provide such stimuli to either SoC device peripheral or machine level concepts like pushbuttons and LEDs. To my understanding, the following concepts/apis are needed: - QMP commands to send stimuli to machine level concepts like pushbuttons - Machine level devices that receive these stimuli and act as an adapter to manipulate the associated SoC peripheral devices - For outputs, machine level devices are needed that observe changes in SoC peripherals and emit QMP events that clients can receive This patch adds a new qmp command "buttons-set-state" to update the push-down state of arbitrarily identified buttons (string identifier). The handler of this command is mocked to set the machines SoC gpio/irq lines associated with these buttons on the machine by means of object property aliases. This is just a mock until a proper concept/api is agreed upon. As i recently joined the QEMU community as part of GSoC, i am still a greenhorn to the codebase and i am really excited to discuss potential concepts and APIs to realize the described features. Steffen Based-on: <20180503090532.3113-1-j...@jms.id.au> Signed-off-by: Steffen Goertz --- Makefile | 8 +++ Makefile.objs | 4 Makefile.target | 1 + hw/arm/microbit.c | 7 ++ qapi/qapi-schema.json | 1 + qapi/stimulate.json | 22 ++ stimulate.c | 52 +++ 7 files changed, 95 insertions(+) create mode 100644 qapi/stimulate.json create mode 100644 stimulate.c diff --git a/Makefile b/Makefile index d71dd5bea4..d9319efa68 100644 --- a/Makefile +++ b/Makefile @@ -108,6 +108,7 @@ GENERATED_FILES += qapi/qapi-types-tpm.h qapi/qapi-types-tpm.c GENERATED_FILES += qapi/qapi-types-trace.h qapi/qapi-types-trace.c GENERATED_FILES += qapi/qapi-types-transaction.h qapi/qapi-types-transaction.c GENERATED_FILES += qapi/qapi-types-ui.h qapi/qapi-types-ui.c +GENERATED_FILES += qapi/qapi-types-stimulate.h qapi/qapi-types-stimulate.c GENERATED_FILES += qapi/qapi-builtin-visit.h qapi/qapi-builtin-visit.c GENERATED_FILES += qapi/qapi-visit.h qapi/qapi-visit.c GENERATED_FILES += qapi/qapi-visit-block-core.h qapi/qapi-visit-block-core.c @@ -126,6 +127,7 @@ GENERATED_FILES += qapi/qapi-visit-tpm.h qapi/qapi-visit-tpm.c GENERATED_FILES += qapi/qapi-visit-trace.h qapi/qapi-visit-trace.c GENERATED_FILES += qapi/qapi-visit-transaction.h qapi/qapi-visit-transaction.c GENERATED_FILES += qapi/qapi-visit-ui.h qapi/qapi-visit-ui.c +GENERATED_FILES += qapi/qapi-visit-stimulate.h qapi/qapi-visit-stimulate.c GENERATED_FILES += qapi/qapi-commands.h qapi/qapi-commands.c GENERATED_FILES += qapi/qapi-commands-block-core.h qapi/qapi-commands-block-core.c GENERATED_FILES += qapi/qapi-commands-block.h qapi/qapi-commands-block.c @@ -143,6 +145,7 @@ GENERATED_FILES += qapi/qapi-commands-tpm.h qapi/qapi-commands-tpm.c GENERATED_FILES += qapi/qapi-commands-trace.h qapi/qapi-commands-trace.c GENERATED_FILES += qapi/qapi-commands-transaction.h qapi/qapi-commands-transaction.c GENERATED_FILES += qapi/qapi-commands-ui.h qapi/qapi-commands-ui.c +GENERATED_FILES += qapi/qapi-commands-stimulate.h qapi/qapi-commands-stimulate.c GENERATED_FILES += qapi/qapi-events.h qapi/qapi-events.c GENERATED_FILES += qapi/qapi-events-block-core.h qapi/qapi-events-block-core.
Re: [Qemu-devel] [RFC v3] qapi: command category to stimulate high-level machine devices
Hi, This series failed docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. Type: series Message-id: 20180603203412.11033-1-cont...@steffen-goertz.de Subject: [Qemu-devel] [RFC v3] qapi: command category to stimulate high-level machine devices === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc # Let docker tests dump environment info export SHOW_ENV=1 export J=8 time make docker-test-mingw@fedora === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 423a4be2c3 qapi: command category to stimulate high-level machine devices === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-_5yx3xb5/src/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' BUILD fedora make[1]: Entering directory '/var/tmp/patchew-tester-tmp-_5yx3xb5/src' GEN /var/tmp/patchew-tester-tmp-_5yx3xb5/src/docker-src.2018-06-03-16.39.13.7960/qemu.tar Cloning into '/var/tmp/patchew-tester-tmp-_5yx3xb5/src/docker-src.2018-06-03-16.39.13.7960/qemu.tar.vroot'... done. Your branch is up-to-date with 'origin/test'. Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-_5yx3xb5/src/docker-src.2018-06-03-16.39.13.7960/qemu.tar.vroot/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-_5yx3xb5/src/docker-src.2018-06-03-16.39.13.7960/qemu.tar.vroot/ui/keycodemapdb'... Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce' COPYRUNNER RUN test-mingw in qemu:fedora Packages installed: PyYAML-3.12-5.fc27.x86_64 SDL2-devel-2.0.7-2.fc27.x86_64 bc-1.07.1-3.fc27.x86_64 bison-3.0.4-8.fc27.x86_64 bluez-libs-devel-5.48-3.fc27.x86_64 brlapi-devel-0.6.6-8.fc27.x86_64 bzip2-1.0.6-24.fc27.x86_64 bzip2-devel-1.0.6-24.fc27.x86_64 ccache-3.3.6-1.fc27.x86_64 clang-5.0.1-5.fc27.x86_64 device-mapper-multipath-devel-0.7.1-9.git847cc43.fc27.x86_64 findutils-4.6.0-16.fc27.x86_64 flex-2.6.1-5.fc27.x86_64 gcc-7.3.1-5.fc27.x86_64 gcc-c++-7.3.1-5.fc27.x86_64 gettext-0.19.8.1-12.fc27.x86_64 git-2.14.3-3.fc27.x86_64 glib2-devel-2.54.3-2.fc27.x86_64 glusterfs-api-devel-3.12.7-1.fc27.x86_64 gnutls-devel-3.5.18-2.fc27.x86_64 gtk3-devel-3.22.26-2.fc27.x86_64 hostname-3.18-4.fc27.x86_64 libaio-devel-0.3.110-9.fc27.x86_64 libasan-7.3.1-5.fc27.x86_64 libattr-devel-2.4.47-21.fc27.x86_64 libcap-devel-2.25-7.fc27.x86_64 libcap-ng-devel-0.7.8-5.fc27.x86_64 libcurl-devel-7.55.1-10.fc27.x86_64 libfdt-devel-1.4.6-1.fc27.x86_64 libpng-devel-1.6.31-1.fc27.x86_64 librbd-devel-12.2.4-1.fc27.x86_64 libssh2-devel-1.8.0-5.fc27.x86_64 libubsan-7.3.1-5.fc27.x86_64 libusbx-devel-1.0.21-4.fc27.x86_64 libxml2-devel-2.9.7-1.fc27.x86_64 llvm-5.0.1-6.fc27.x86_64 lzo-devel-2.08-11.fc27.x86_64 make-4.2.1-4.fc27.x86_64 mingw32-SDL-1.2.15-9.fc27.noarch mingw32-bzip2-1.0.6-9.fc27.noarch mingw32-curl-7.54.1-2.fc27.noarch mingw32-glib2-2.54.1-1.fc27.noarch mingw32-gmp-6.1.2-2.fc27.noarch mingw32-gnutls-3.5.13-2.fc27.noarch mingw32-gtk2-2.24.31-4.fc27.noarch mingw32-gtk3-3.22.16-1.fc27.noarch mingw32-libjpeg-turbo-1.5.1-3.fc27.noarch mingw32-libpng-1.6.29-2.fc27.noarch mingw32-libssh2-1.8.0-3.fc27.noarch mingw32-libtasn1-4.13-1.fc27.noarch mingw32-nettle-3.3-3.fc27.noarch mingw32-pixman-0.34.0-3.fc27.noarch mingw32-pkg-config-0.28-9.fc27.x86_64 mingw64-SDL-1.2.15-9.fc27.noarch mingw64-bzip2-1.0.6-9.fc27.noarch mingw64-curl-7.54.1-2.fc27.noarch mingw64-glib2-2.54.1-1.fc27.noarch mingw64-gmp-6.1.2-2.fc27.noarch mingw64-gnutls-3.5.13-2.fc27.noarch mingw64-gtk2-2.24.31-4.fc27.noarch mingw64-gtk3-3.22.16-1.fc27.noarch mingw64-libjpeg-turbo-1.5.1-3.fc27.noarch mingw64-libpng-1.6.29-2.fc27.noarch mingw64-libssh2-1.8.0-3.fc27.noarch mingw64-libtasn1-4.13-1.fc27.noarch mingw64-nettle-3.3-3.fc27.noarch mingw64-pixman-0.34.0-3.fc27.noarch mingw64-pkg-config-0.28-9.fc27.x86_64 ncurses-devel-6.0-13.20170722.fc27.x86_64 nettle-devel-3.4-1.fc27.x86_64 nss-devel-3.36.0-1.0.fc27.x86_64 numactl-devel-2.0.11-5.fc27.x86_64 package libjpeg-devel is not installed perl-5.26.1-403.fc27.x86_64 pixman-devel-0.34.0-4.fc27.x86_64 python3-3.6.2-13.fc27.x86_64 snappy-devel-1.1.4-5.fc27.x86_64 sparse-0.5.1-2.fc27.x86_64 spice-server-devel-0.14.0-1.fc27.x86_64 systemtap-sdt-devel-3.2-3.fc27.x86_64 tar-1.29-7.fc27.x86_64 usbredir-devel-0.7.1-5.fc27.x86_64 virglrenderer-devel-0.6.0-3.20170210git76b3da97b.fc27.x86_64 vte3-devel-0.36.5-5.fc27.x86_64 which-2.21-4.fc27.x86_64 xen-devel-4.9.1-5.fc27.x86_64 zlib-devel-1.2.11-4.fc27.x86_64 Environment variables: TARGET_LIST= PACKAGES=ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname
Re: [Qemu-devel] [PATCH for 2.10 26/35] linux-user: use is_error() to avoid warnings and make the code clearer
Le 29/05/2018 à 17:19, Laurent Vivier a écrit : > Le 29/05/2018 à 16:25, Philippe Mathieu-Daudé a écrit : >> Hi Laurent, > > Hi Philippe, > >> On 07/24/2017 04:16 PM, Laurent Vivier wrote: >>> Le 24/07/2017 à 20:27, Philippe Mathieu-Daudé a écrit : linux-user/flatload.c:740:9: warning: Loss of sign in implicit conversion if (res > (unsigned long)-4096) ^~~ Reported-by: Clang Static Analyzer Signed-off-by: Philippe Mathieu-Daudé >>> >>> Reviewed-by: Laurent Vivier >>> --- linux-user/flatload.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/linux-user/flatload.c b/linux-user/flatload.c index a35a560904..10c529910f 100644 --- a/linux-user/flatload.c +++ b/linux-user/flatload.c @@ -224,8 +224,9 @@ static int decompress_exec( ret = bprm->file->f_op->read(bprm->file, buf, LBUFSIZE, &fpos); if (ret <= 0) break; - if (ret >= (unsigned long) -4096) +if (is_error(ret)) { break; +} len -= ret; strm.next_in = buf; @@ -283,8 +284,7 @@ calc_reloc(abi_ulong r, struct lib_info *p, int curid, int internalp) "in same module (%d != %d)\n", (unsigned) r, curid, id); goto failed; -} else if ( ! p[id].loaded && -load_flat_shared_library(id, p) > (unsigned long) -4096) { +} else if (!p[id].loaded && is_error(load_flat_shared_library(id, p))) { fprintf(stderr, "BINFMT_FLAT: failed to load library %d\n", id); goto failed; } @@ -523,9 +523,10 @@ static int load_flat_file(struct linux_binprm * bprm, fpos = 0; result = bprm->file->f_op->read(bprm->file, (char *) textpos, text_len, &fpos); -if (result < (unsigned long) -4096) +if (!is_error(result)) { result = decompress_exec(bprm, text_len, (char *) datapos, data_len + (relocs * sizeof(unsigned long)), 0); +} } else #endif @@ -693,8 +694,9 @@ static int load_flat_shared_library(int id, struct lib_info *libs) res = prepare_binprm(&bprm); - if (res <= (unsigned long)-4096) +if (!is_error(res)) { res = load_flat_file(&bprm, libs, id, NULL); +} if (bprm.file) { allow_write_access(bprm.file); fput(bprm.file); @@ -737,8 +739,9 @@ int load_flt_binary(struct linux_binprm *bprm, struct image_info *info) res = load_flat_file(bprm, libinfo, 0, &stack_len); -if (res > (unsigned long)-4096) +if (is_error(res)) { return res; +} /* Update data segment pointers for all libraries */ for (i=0; i>>> >> >> Can you take this via your linux-user tree? >> > > Applied, thanks. Unapplied, it needs a rebase: qemu/linux-user/flatload.c: In function 'load_flt_binary': qemu/linux-user/flatload.c:742:9: error: implicit declaration of function 'is_error'; did you mean 'g_error'? [-Werror=implicit-function-declaration] if (is_error(res)) { ^~~~ g_error qemu/linux-user/flatload.c:742:9: error: nested extern declaration of 'is_error' [-Werror=nested-externs] Thanks, Laurent
[Qemu-devel] [PATCH 0/4] Misc sam460ex improvements
Some more improvements to sam460ex emulation: - Implemented RTC (which also needed fixing I2C to work) - Fix a problem in sm501 which led to AmigaOS slow down BALATON Zoltan (4): ppc4xx_i2c: Rewrite PPC4xx I2C emulation hw/timer: Add basic M41T80 emulation sam460ex: Add RTC device sm501: Do not clear read only bits when writing register MAINTAINERS | 1 + default-configs/ppc-softmmu.mak | 2 + hw/display/sm501.c | 6 +- hw/i2c/ppc4xx_i2c.c | 366 +--- hw/ppc/sam460ex.c | 1 + hw/timer/Makefile.objs | 1 + hw/timer/m41t80.c | 117 + include/hw/i2c/ppc4xx_i2c.h | 19 +-- 8 files changed, 321 insertions(+), 192 deletions(-) create mode 100644 hw/timer/m41t80.c -- 2.7.6
[Qemu-devel] [PATCH 3/4] sam460ex: Add RTC device
The Sam460ex has an M41T80 serial RTC chip on I2C bus 0 at address 0x68. Signed-off-by: BALATON Zoltan --- hw/ppc/sam460ex.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c index a48e6e6..3848885 100644 --- a/hw/ppc/sam460ex.c +++ b/hw/ppc/sam460ex.c @@ -458,6 +458,7 @@ static void sam460ex_init(MachineState *machine) object_property_set_bool(OBJECT(dev), true, "realized", NULL); smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, smbus_eeprom_size); g_free(smbus_eeprom_buf); +i2c_create_slave(i2c[0]->bus, "m41t80", 0x68); dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]); i2c[1] = PPC4xx_I2C(dev); -- 2.7.6
[Qemu-devel] [PATCH 1/4] ppc4xx_i2c: Rewrite PPC4xx I2C emulation
I2C emulation currently is just enough for U-Boot to access SPD EEPROMs but features that guests use to access I2C devices are not correctly emulated. Rewrite to implement missing features to make it work with all clients. Signed-off-by: BALATON Zoltan --- Maybe this could be split up into more patches but because the previous implementation was wrong only allowing U-Boot to pass and no clients could access I2C devices before this rewrite it probably does not worth to try to make it a lot of small changes instead of dropping the previous hack and rewrite following features of real hardware more closely. (It turns out that each client driver accesses I2C in a different way so we need to implement almost all features of the hardware to please everyone.) default-configs/ppc-softmmu.mak | 1 + hw/i2c/ppc4xx_i2c.c | 366 +--- include/hw/i2c/ppc4xx_i2c.h | 19 +-- 3 files changed, 197 insertions(+), 189 deletions(-) diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak index 4d7be45..7d0dc2f 100644 --- a/default-configs/ppc-softmmu.mak +++ b/default-configs/ppc-softmmu.mak @@ -26,6 +26,7 @@ CONFIG_USB_EHCI_SYSBUS=y CONFIG_SM501=y CONFIG_IDE_SII3112=y CONFIG_I2C=y +CONFIG_BITBANG_I2C=y # For Macs CONFIG_MAC=y diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c index ab64d19..638bb01 100644 --- a/hw/i2c/ppc4xx_i2c.c +++ b/hw/i2c/ppc4xx_i2c.c @@ -3,7 +3,7 @@ * * Copyright (c) 2007 Jocelyn Mayer * Copyright (c) 2012 François Revol - * Copyright (c) 2016 BALATON Zoltan + * Copyright (c) 2016-2018 BALATON Zoltan * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -30,281 +30,300 @@ #include "cpu.h" #include "hw/hw.h" #include "hw/i2c/ppc4xx_i2c.h" +#include "bitbang_i2c.h" -#define PPC4xx_I2C_MEM_SIZE 0x12 +#define PPC4xx_I2C_MEM_SIZE 18 #define IIC_CNTL_PT (1 << 0) #define IIC_CNTL_READ (1 << 1) #define IIC_CNTL_CHT(1 << 2) #define IIC_CNTL_RPST (1 << 3) +#define IIC_CNTL_AMD(1 << 6) +#define IIC_CNTL_HMT(1 << 7) + +#define IIC_MDCNTL_EINT (1 << 2) +#define IIC_MDCNTL_ESM (1 << 3) +#define IIC_MDCNTL_FMDB (1 << 6) #define IIC_STS_PT (1 << 0) +#define IIC_STS_IRQA(1 << 1) #define IIC_STS_ERR (1 << 2) +#define IIC_STS_MDBF(1 << 4) #define IIC_STS_MDBS(1 << 5) #define IIC_EXTSTS_XFRA (1 << 0) +#define IIC_INTRMSK_EIMTC (1 << 0) +#define IIC_INTRMSK_EITA(1 << 1) +#define IIC_INTRMSK_EIIC(1 << 2) +#define IIC_INTRMSK_EIHE(1 << 3) + #define IIC_XTCNTLSS_SRST (1 << 0) +#define IIC_DIRECTCNTL_SDAC (1 << 3) +#define IIC_DIRECTCNTL_SCLC (1 << 2) +#define IIC_DIRECTCNTL_MSDA (1 << 1) +#define IIC_DIRECTCNTL_MSCL (1 << 0) + +typedef struct { +bitbang_i2c_interface *bitbang; +int mdidx; +uint8_t mdata[4]; +uint8_t lmadr; +uint8_t hmadr; +uint8_t cntl; +uint8_t mdcntl; +uint8_t sts; +uint8_t extsts; +uint8_t lsadr; +uint8_t hsadr; +uint8_t clkdiv; +uint8_t intrmsk; +uint8_t xfrcnt; +uint8_t xtcntlss; +uint8_t directcntl; +} PPC4xxI2CRegs; + static void ppc4xx_i2c_reset(DeviceState *s) { -PPC4xxI2CState *i2c = PPC4xx_I2C(s); +PPC4xxI2CRegs *i2c = PPC4xx_I2C(s)->regs; -/* FIXME: Should also reset bus? - *if (s->address != ADDR_RESET) { - *i2c_end_transfer(s->bus); - *} - */ - -i2c->mdata = 0; -i2c->lmadr = 0; -i2c->hmadr = 0; +i2c->mdidx = -1; +memset(i2c->mdata, 0, ARRAY_SIZE(i2c->mdata)); +/* [hl][ms]addr are not affected by reset */ i2c->cntl = 0; i2c->mdcntl = 0; i2c->sts = 0; -i2c->extsts = 0x8f; -i2c->sdata = 0; -i2c->lsadr = 0; -i2c->hsadr = 0; +i2c->extsts = (1 << 6); i2c->clkdiv = 0; i2c->intrmsk = 0; i2c->xfrcnt = 0; i2c->xtcntlss = 0; -i2c->directcntl = 0x0f; -i2c->intr = 0; -} - -static inline bool ppc4xx_i2c_is_master(PPC4xxI2CState *i2c) -{ -return true; +i2c->directcntl = 0xf; } static uint64_t ppc4xx_i2c_readb(void *opaque, hwaddr addr, unsigned int size) { -PPC4xxI2CState *i2c = PPC4xx_I2C(opaque); +PPC4xxI2CState *s = PPC4xx_I2C(opaque); +PPC4xxI2CRegs *i2c = s->regs; uint64_t ret; +int i; switch (addr) { -case 0x00: -ret = i2c->mdata; -if (ppc4xx_i2c_is_master(i2c)) { +case 0: +if (i2c->mdidx < 0) { ret = 0xff; - -if (!(i2c->sts & IIC_STS_MDBS)) { -qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Trying to read " - "without starting transfer\n", - TYPE_PPC4xx_I2C, __func__); -} else { -int pending = (i2c->cntl >> 4) & 3; - -/* get the next byte */ -int byte
[Qemu-devel] [PATCH 4/4] sm501: Do not clear read only bits when writing register
When writing a register that has read only bits besides reserved bits we have to avoid changing read only bits that may have non zero default values. While at it, fix a reserved bit mask and a white space error. Signed-off-by: BALATON Zoltan --- hw/display/sm501.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/display/sm501.c b/hw/display/sm501.c index f4bb33c..51b2bb8 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -837,10 +837,10 @@ static void sm501_system_config_write(void *opaque, hwaddr addr, switch (addr) { case SM501_SYSTEM_CONTROL: -s->system_control = value & 0xE300B8F7; +s->system_control |= value & 0xEF00B8F7; break; case SM501_MISC_CONTROL: -s->misc_control = value & 0xFF7FFF20; +s->misc_control |= value & 0xFF7FFF10; break; case SM501_GPIO31_0_CONTROL: s->gpio_31_0_control = value; @@ -854,7 +854,7 @@ static void sm501_system_config_write(void *opaque, hwaddr addr, s->dram_control |= value & 0x7FC3; break; case SM501_ARBTRTN_CONTROL: -s->arbitration_control = value & 0x3777; +s->arbitration_control = value & 0x3777; break; case SM501_IRQ_MASK: s->irq_mask = value; -- 2.7.6
[Qemu-devel] [PATCH 2/4] hw/timer: Add basic M41T80 emulation
Basic emulation of the M41T80 serial (I2C) RTC chip. Only getting time of day is implemented. Setting time and RTC alarm are not supported. Signed-off-by: BALATON Zoltan --- MAINTAINERS | 1 + default-configs/ppc-softmmu.mak | 1 + hw/timer/Makefile.objs | 1 + hw/timer/m41t80.c | 117 4 files changed, 120 insertions(+) create mode 100644 hw/timer/m41t80.c diff --git a/MAINTAINERS b/MAINTAINERS index 41cd373..9e13bc1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -826,6 +826,7 @@ M: BALATON Zoltan L: qemu-...@nongnu.org S: Maintained F: hw/ide/sii3112.c +F: hw/timer/m41t80.c SH4 Machines diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak index 7d0dc2f..9fbaadc 100644 --- a/default-configs/ppc-softmmu.mak +++ b/default-configs/ppc-softmmu.mak @@ -27,6 +27,7 @@ CONFIG_SM501=y CONFIG_IDE_SII3112=y CONFIG_I2C=y CONFIG_BITBANG_I2C=y +CONFIG_M41T80=y # For Macs CONFIG_MAC=y diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs index 8b27a4b..e16b2b9 100644 --- a/hw/timer/Makefile.objs +++ b/hw/timer/Makefile.objs @@ -6,6 +6,7 @@ common-obj-$(CONFIG_CADENCE) += cadence_ttc.o common-obj-$(CONFIG_DS1338) += ds1338.o common-obj-$(CONFIG_HPET) += hpet.o common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o +common-obj-$(CONFIG_M41T80) += m41t80.o common-obj-$(CONFIG_M48T59) += m48t59.o ifeq ($(CONFIG_ISA_BUS),y) common-obj-$(CONFIG_M48T59) += m48t59-isa.o diff --git a/hw/timer/m41t80.c b/hw/timer/m41t80.c new file mode 100644 index 000..9dbdb1b --- /dev/null +++ b/hw/timer/m41t80.c @@ -0,0 +1,117 @@ +/* + * M41T80 serial rtc emulation + * + * Copyright (c) 2018 BALATON Zoltan + * + * This work is licensed under the GNU GPL license version 2 or later. + * + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "qemu/timer.h" +#include "qemu/bcd.h" +#include "hw/i2c/i2c.h" + +#define TYPE_M41T80 "m41t80" +#define M41T80(obj) OBJECT_CHECK(M41t80State, (obj), TYPE_M41T80) + +typedef struct M41t80State { +I2CSlave parent_obj; +int8_t addr; +} M41t80State; + +static void m41t80_realize(DeviceState *dev, Error **errp) +{ +M41t80State *s = M41T80(dev); + +s->addr = -1; +} + +static int m41t80_send(I2CSlave *i2c, uint8_t data) +{ +M41t80State *s = M41T80(i2c); + +if (s->addr < 0) { +s->addr = data; +} else { +s->addr++; +} +return 0; +} + +static int m41t80_recv(I2CSlave *i2c) +{ +M41t80State *s = M41T80(i2c); +struct tm now; +qemu_timeval tv; + +if (s->addr < 0) { +s->addr = 0; +} +if (s->addr >= 1 && s->addr <= 7) { +qemu_get_timedate(&now, -1); +} +switch (s->addr++) { +case 0: +qemu_gettimeofday(&tv); +return to_bcd(tv.tv_usec / 1); +case 1: +return to_bcd(now.tm_sec); +case 2: +return to_bcd(now.tm_min); +case 3: +return to_bcd(now.tm_hour); +case 4: +return to_bcd(now.tm_wday); +case 5: +return to_bcd(now.tm_mday); +case 6: +return to_bcd(now.tm_mon + 1); +case 7: +return to_bcd(now.tm_year % 100); +case 8 ... 19: +qemu_log_mask(LOG_UNIMP, "\n%s: unimplemented register: %d\n", + __func__, s->addr - 1); +return 0; +default: +qemu_log_mask(LOG_GUEST_ERROR, "\n%s: invalid register: %d", + __func__, s->addr - 1); +return 0; +} +} + +static int m41t80_event(I2CSlave *i2c, enum i2c_event event) +{ +M41t80State *s = M41T80(i2c); + +if (event == I2C_START_SEND) { +s->addr = -1; +} +return 0; +} + +static void m41t80_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); + +dc->realize = m41t80_realize; +sc->send = m41t80_send; +sc->recv = m41t80_recv; +sc->event = m41t80_event; +} + +static const TypeInfo m41t80_info = { +.name = TYPE_M41T80, +.parent= TYPE_I2C_SLAVE, +.instance_size = sizeof(M41t80State), +.class_init= m41t80_class_init, +}; + +static void m41t80_register_types(void) +{ +type_register_static(&m41t80_info); +} + +type_init(m41t80_register_types) -- 2.7.6
Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix
On 01/06/2018 00:49, Richard Henderson wrote: > If the interp_prefix is a complete chroot, it may have a *lot* of files. > Setting up the cache for this is quite expensive. > > For the most part, we can use the *at versions of various syscalls to > attempt the operation in the prefix. For the few cases that remain, > attempt the operation in the prefix via concatenation and then retry > if that fails. > I like the idea, but it breaks real chroot. You can test it with: wget https://github.com/vivier/linux-user-test-scrips/raw/master/create_chroot.sh then sudo sh ./create_chroot.sh /path/to/static/qemu-s390x stretch It fails with: Reading package lists... Done E: Method /usr/lib/apt/methods/http did not start correctly E: Failed to fetch http://ftp.us.debian.org/debian/dists/stretch/InRelease E: Some index files failed to download. They have been ignored, or old ones used instead. stretch s390x FAILED Thanks, Laurent
Re: [Qemu-devel] [PATCH 4/4] uninorth: remove token register from uninorth device
On Sun, May 06, 2018 at 03:20:05PM +0100, Mark Cave-Ayland wrote: > >From observation of various OS sources it can be seen that the token register > introduced in 4e46dcdbd3 "PPC: Newworld: Add uninorth token register" is not > required, since the only register currently implemented is the uninorth > hardware > version which is read-only. > > Remove the token register implementation and instead return the uninorth > version corresponding to the hardware. > > Signed-off-by: Mark Cave-Ayland Applied, thanks. > --- > hw/pci-host/uninorth.c | 11 +-- > include/hw/pci-host/uninorth.h | 4 +++- > 2 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c > index a658f9230a..abebfaf755 100644 > --- a/hw/pci-host/uninorth.c > +++ b/hw/pci-host/uninorth.c > @@ -524,19 +524,18 @@ static void unin_write(void *opaque, hwaddr addr, > uint64_t value, > unsigned size) > { > trace_unin_write(addr, value); > -if (addr == 0x0) { > -*(int *)opaque = value; > -} > } > > static uint64_t unin_read(void *opaque, hwaddr addr, unsigned size) > { > uint32_t value; > > -value = 0; > switch (addr) { > case 0: > -value = *(int *)opaque; > +value = UNINORTH_VERSION_10A; > +break; > +default: > +value = 0; > } > > trace_unin_read(addr, value); > @@ -559,7 +558,7 @@ static void unin_init(Object *obj) > UNINState *s = UNI_NORTH(obj); > SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > > -memory_region_init_io(&s->mem, obj, &unin_ops, &s->token, "unin", > 0x1000); > +memory_region_init_io(&s->mem, obj, &unin_ops, s, "unin", 0x1000); > > sysbus_init_mmio(sbd, &s->mem); > } > diff --git a/include/hw/pci-host/uninorth.h b/include/hw/pci-host/uninorth.h > index f6654bad9b..2a1cf9f284 100644 > --- a/include/hw/pci-host/uninorth.h > +++ b/include/hw/pci-host/uninorth.h > @@ -29,6 +29,9 @@ > > #include "hw/ppc/openpic.h" > > +/* UniNorth version */ > +#define UNINORTH_VERSION_10A0x7 > + > #define TYPE_UNI_NORTH_PCI_HOST_BRIDGE "uni-north-pci-pcihost" > #define TYPE_UNI_NORTH_AGP_HOST_BRIDGE "uni-north-agp-pcihost" > #define TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE > "uni-north-internal-pci-pcihost" > @@ -57,7 +60,6 @@ typedef struct UNINState { > SysBusDevice parent_obj; > > MemoryRegion mem; > -int token[1]; > } UNINState; > > #define TYPE_UNI_NORTH "uni-north" -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 1/4] uninorth: add impl min_access_size and max_access_size to unin_ops
On Sun, May 06, 2018 at 03:20:02PM +0100, Mark Cave-Ayland wrote: > >From testing all my local images the uninorth registers are only ever > read or written with 32-bit accesses. If that's the case, shouldn't you be setting the range of valid accesses, rather than the range of implemented accesses? > > Signed-off-by: Mark Cave-Ayland > --- > hw/pci-host/uninorth.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c > index ba76b84dbc..a658f9230a 100644 > --- a/hw/pci-host/uninorth.c > +++ b/hw/pci-host/uninorth.c > @@ -548,6 +548,10 @@ static const MemoryRegionOps unin_ops = { > .read = unin_read, > .write = unin_write, > .endianness = DEVICE_BIG_ENDIAN, > +.impl = { > +.min_access_size = 4, > +.max_access_size = 4, > +}, > }; > > static void unin_init(Object *obj) -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v3] target/ppc: Allow PIR read in privileged mode
On Mon, May 07, 2018 at 01:52:42PM -0300, luporl wrote: > According to PowerISA, the PIR register should be readable in privileged > mode also, not only in hypervisor privileged mode. > > PowerISA 3.0 - 4.3.3 Processor Identification Register > > "Read access to the PIR is privileged; write access is not > provided." Yes... but a little further down it says "The PIR is a hypervisor resource". Looking at the older 2.07 ISA, it says that guest-supervisor mode reads to the PIR should be redirected to the GPIR register, which this change won't accomplish. So, I'm not sure what to make of this. > > Cc: David Gibson > Cc: Alexander Graf > Cc: qemu-...@nongnu.org > Signed-off-by: Leandro Lupori > Reviewed-by: Jose Ricardo Ziviani > Reviewed-by: Greg Kurz > --- > Changes in v2: > - added my Signed-off-by, maintainers CC and Jose's Reviewed-by tags > > Changes in v3: > - added subsystem name, version tag and summary of changes > - added the section of PowerISA that describes PIR access privileges > > target/ppc/translate_init.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c > index a72be6d121..7b56e3ffb9 100644 > --- a/target/ppc/translate_init.c > +++ b/target/ppc/translate_init.c > @@ -7816,7 +7816,7 @@ static void gen_spr_book3s_ids(CPUPPCState *env) > /* Processor identification */ > spr_register_hv(env, SPR_PIR, "PIR", > SPR_NOACCESS, SPR_NOACCESS, > - SPR_NOACCESS, SPR_NOACCESS, > + &spr_read_generic, SPR_NOACCESS, > &spr_read_generic, NULL, > 0x); > spr_register_hv(env, SPR_HID0, "HID0", -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 2/4] macio: add trace-events to timer device
On Sun, May 06, 2018 at 03:20:03PM +0100, Mark Cave-Ayland wrote: > Signed-off-by: Mark Cave-Ayland Applied, thanks. > --- > hw/misc/macio/macio.c | 3 +++ > hw/misc/macio/trace-events | 4 > 2 files changed, 7 insertions(+) > > diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c > index 79621eb879..f9a40eea81 100644 > --- a/hw/misc/macio/macio.c > +++ b/hw/misc/macio/macio.c > @@ -32,6 +32,7 @@ > #include "hw/char/escc.h" > #include "hw/misc/macio/macio.h" > #include "hw/intc/heathrow_pic.h" > +#include "trace.h" > > /* Note: this code is strongly inspirated from the corresponding code > * in PearPC */ > @@ -246,6 +247,7 @@ static void macio_oldworld_init(Object *obj) > static void timer_write(void *opaque, hwaddr addr, uint64_t value, > unsigned size) > { > +trace_macio_timer_write(addr, size, value); > } > > static uint64_t timer_read(void *opaque, hwaddr addr, unsigned size) > @@ -266,6 +268,7 @@ static uint64_t timer_read(void *opaque, hwaddr addr, > unsigned size) > break; > } > > +trace_macio_timer_read(addr, size, value); > return value; > } > > diff --git a/hw/misc/macio/trace-events b/hw/misc/macio/trace-events > index 24c0a36824..d499d78c99 100644 > --- a/hw/misc/macio/trace-events > +++ b/hw/misc/macio/trace-events > @@ -9,3 +9,7 @@ cuda_packet_receive(int len) "length %d" > cuda_packet_receive_data(int i, const uint8_t data) "[%d] 0x%02x" > cuda_packet_send(int len) "length %d" > cuda_packet_send_data(int i, const uint8_t data) "[%d] 0x%02x" > + > +# hw/misc/macio/macio.c > +macio_timer_write(uint64_t addr, unsigned len, uint64_t val) "write addr > 0x%"PRIx64 " len %d val 0x%"PRIx64 > +macio_timer_read(uint64_t addr, unsigned len, uint32_t val) "read addr > 0x%"PRIx64 " len %d val 0x%"PRIx32 -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] ppc: e500: use g_strdup_printf() instead of snprintf()
On Tue, May 08, 2018 at 12:01:59PM +0100, Peter Maydell wrote: > On 8 May 2018 at 10:34, Greg Kurz wrote: > > On Mon, 7 May 2018 12:53:45 -0500 > > Eric Blake wrote: > > > >> On 05/07/2018 04:02 AM, Greg Kurz wrote: > >> > qemu-system-ppc fails to build with GCC 8.0.1: > >> > > >> > /home/hsp/src/qemu-master/hw/ppc/e500.c: In function > >> > ‘ppce500_load_device_tree’: > >> > /home/hsp/src/qemu-master/hw/ppc/e500.c:442:37: error: ‘/pic@’ > >> > directive output may be truncated writing 5 bytes into a region of > >> > size between 1 and 128 [-Werror=format-truncation=] > >> > snprintf(mpic, sizeof(mpic), "%s/pic@%llx", soc, > >> > MPC8544_MPIC_REGS_OFFSET); > >> > ^ > >> > >> > > >> > Fix this by converting e500 to use g_strdup_printf()+g_free() instead > >> > of snprintf(). This is done globally, even for call sites that don't > >> > break build, since this is the preferred practice in QEMU. > >> > > >> > Reported-by: Howard Spoelstra > >> > Signed-off-by: Greg Kurz > >> > --- > >> > hw/ppc/e500.c | 39 +++ > >> > 1 file changed, 23 insertions(+), 16 deletions(-) > >> > > >> > >> Reviewed-by: Eric Blake > >> > > > > Hi Peter, > > > > David said the next pull request for ppc would happen in a month. This > > patch fixes an annoying build break with recent GCC, and it already > > got two positive reviews, is it possible to have it merged upstream ? > > Sure; applied to master. Thanks! -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 4/8] virtio: add detach element for packed ring(1.1)
On Tue, Apr 10, 2018 at 03:32:53PM +0800, Jason Wang wrote: > > > On 2018年04月04日 20:54, w...@redhat.com wrote: > >From: Wei Xu > > > >helper for packed ring > > It's odd and hard to review if you put detach patch first. I think this > patch needs to be reordered after the implementation of pop/map. This patch is not necessary after sync to tiwei's v5, so we can skip it. Wei > > Thanks > > >Signed-off-by: Wei Xu > >--- > > hw/virtio/virtio.c | 21 +++-- > > 1 file changed, 19 insertions(+), 2 deletions(-) > > > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > >index 478df3d..fdee40f 100644 > >--- a/hw/virtio/virtio.c > >+++ b/hw/virtio/virtio.c > >@@ -561,6 +561,20 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const > >VirtQueueElement *elem, > > elem->out_sg[i].iov_len); > > } > >+static void virtqueue_detach_element_split(VirtQueue *vq, > >+const VirtQueueElement *elem, unsigned int len) > >+{ > >+vq->inuse--; > >+virtqueue_unmap_sg(vq, elem, len); > >+} > >+ > >+static void virtqueue_detach_element_packed(VirtQueue *vq, > >+const VirtQueueElement *elem, unsigned int len) > >+{ > >+vq->inuse -= elem->count; > >+virtqueue_unmap_sg(vq, elem, len); > >+} > >+ > > /* virtqueue_detach_element: > > * @vq: The #VirtQueue > > * @elem: The #VirtQueueElement > >@@ -573,8 +587,11 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const > >VirtQueueElement *elem, > > void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem, > >unsigned int len) > > { > >-vq->inuse--; > >-virtqueue_unmap_sg(vq, elem, len); > >+if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) { > >+virtqueue_detach_element_packed(vq, elem, len); > >+} else { > >+virtqueue_detach_element_split(vq, elem, len); > >+} > > } > > /* virtqueue_unpop: >
Re: [Qemu-devel] [PATCH RFC] hw/pc: set q35 as the default x86 machine
On Sun, Jun 03, 2018 at 12:27:49PM +0300, Marcel Apfelbaum wrote: > Moving to QEMU 3.0 seems like a good opportunity for such a change. > > I440FX is really old and does not support modern features like IOMMU. > Q35's SATA emulation is faster than pc's IDE, native PCI express hotplug > is cleaner than ACPI based one and so on... > > Also the libvirt guys added very good support for the Q35 machine (thanks!). > > Management software should always specify the machine type and for the > current setups, adding '-machine pc' to the command line is not such a > big deal. > > In time the pc machine will fade out and we will probably stop adding > new versions at some point. > > Signed-off-by: Marcel Apfelbaum For command line users, I think changing the default isn't nice. Yes it's easy to add -machine pc but there's no documentation that tells you to do so. Add to that shortcuts like -cdrom stop working, hotplug needs extra bridges to work, and one can see that while management tool users benefit from q35, command line users will suffer. Can't we add a tag for management without changing the command line default? How about "management-default"? "recommended"? "latest"? -- MST
Re: [Qemu-devel] [PATCH 4/8] virtio: add detach element for packed ring(1.1)
On Mon, Jun 04, 2018 at 09:34:35AM +0800, Wei Xu wrote: > On Tue, Apr 10, 2018 at 03:32:53PM +0800, Jason Wang wrote: > > > > > > On 2018年04月04日 20:54, w...@redhat.com wrote: > > >From: Wei Xu > > > > > >helper for packed ring > > > > It's odd and hard to review if you put detach patch first. I think this > > patch needs to be reordered after the implementation of pop/map. > > This patch is not necessary after sync to tiwei's v5, so we can skip it. > > Wei I suspect we will need to bring detach back eventually but yes, it can wait. > > > > Thanks > > > > >Signed-off-by: Wei Xu > > >--- > > > hw/virtio/virtio.c | 21 +++-- > > > 1 file changed, 19 insertions(+), 2 deletions(-) > > > > > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > >index 478df3d..fdee40f 100644 > > >--- a/hw/virtio/virtio.c > > >+++ b/hw/virtio/virtio.c > > >@@ -561,6 +561,20 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const > > >VirtQueueElement *elem, > > > elem->out_sg[i].iov_len); > > > } > > >+static void virtqueue_detach_element_split(VirtQueue *vq, > > >+const VirtQueueElement *elem, unsigned int > > >len) > > >+{ > > >+vq->inuse--; > > >+virtqueue_unmap_sg(vq, elem, len); > > >+} > > >+ > > >+static void virtqueue_detach_element_packed(VirtQueue *vq, > > >+const VirtQueueElement *elem, unsigned int > > >len) > > >+{ > > >+vq->inuse -= elem->count; > > >+virtqueue_unmap_sg(vq, elem, len); > > >+} > > >+ > > > /* virtqueue_detach_element: > > > * @vq: The #VirtQueue > > > * @elem: The #VirtQueueElement > > >@@ -573,8 +587,11 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const > > >VirtQueueElement *elem, > > > void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement > > > *elem, > > >unsigned int len) > > > { > > >-vq->inuse--; > > >-virtqueue_unmap_sg(vq, elem, len); > > >+if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) { > > >+virtqueue_detach_element_packed(vq, elem, len); > > >+} else { > > >+virtqueue_detach_element_split(vq, elem, len); > > >+} > > > } > > > /* virtqueue_unpop: > >
Re: [Qemu-devel] [PATCH v7 3/5] migration: API to clear bits of guest free pages from the dirty bitmap
On Fri, Jun 01, 2018 at 08:32:27PM +0800, Wei Wang wrote: > On 06/01/2018 06:06 PM, Peter Xu wrote: > > On Fri, Jun 01, 2018 at 03:36:01PM +0800, Wei Wang wrote: > > > On 06/01/2018 12:00 PM, Peter Xu wrote: > > > > On Tue, Apr 24, 2018 at 02:13:46PM +0800, Wei Wang wrote: > > > > >/* > > > > > + * This function clears bits of the free pages reported by the > > > > > caller from the > > > > > + * migration dirty bitmap. @addr is the host address corresponding > > > > > to the > > > > > + * start of the continuous guest free pages, and @len is the total > > > > > bytes of > > > > > + * those pages. > > > > > + */ > > > > > +void qemu_guest_free_page_hint(void *addr, size_t len) > > > > > +{ > > > > > +RAMBlock *block; > > > > > +ram_addr_t offset; > > > > > +size_t used_len, start, npages; > > > > Do we need to check here on whether a migration is in progress? Since > > > > if not I'm not sure whether this hint still makes any sense any more, > > > > and more importantly it seems to me that block->bmap below at [1] is > > > > only valid during a migration. So I'm not sure whether QEMU will > > > > crash if this function is called without a running migration. > > > OK. How about just adding comments above to have users noted that this > > > function should be used during migration? > > > > > > If we want to do a sanity check here, I think it would be easier to just > > > check !block->bmap here. > > I think the faster way might be that we check against the migration > > state. > > > > Sounds good. We can do a sanity check: > > MigrationState *s = migrate_get_current(); > if (!migration_is_setup_or_active(s->state)) > return; Yes. > > > > > > > > > > > + > > > > > +for (; len > 0; len -= used_len) { > > > > > +block = qemu_ram_block_from_host(addr, false, &offset); > > > > > +if (unlikely(!block)) { > > > > > +return; > > > > We should never reach here, should we? Assuming the callers of this > > > > function should always pass in a correct host address. If we are very > > > > sure that the host addr should be valid, could we just assert? > > > Probably not the case, because of the corner case that the memory would be > > > hot unplugged after the free page is reported to QEMU. > > Question: Do we allow to do hot plug/unplug for memory during > > migration? > > I think so. From the code, I don't find where it forbids memory hotplug > during migration. I don't play with that much; do we need to do "device_add" after all? (qemu) object_add memory-backend-file,id=mem1,size=1G,mem-path=/mnt/hugepages-1GB (qemu) device_add pc-dimm,id=dimm1,memdev=mem1 If so, we may not allow that since in qdev_device_add() we don't allow that: if (!migration_is_idle()) { error_setg(errp, "device_add not allowed while migrating"); return NULL; } > > > > > > > > > > > > +} > > > > > + > > > > > +/* > > > > > + * This handles the case that the RAMBlock is resized after > > > > > the free > > > > > + * page hint is reported. > > > > > + */ > > > > > +if (unlikely(offset > block->used_length)) { > > > > > +return; > > > > > +} > > > > > + > > > > > +if (len <= block->used_length - offset) { > > > > > +used_len = len; > > > > > +} else { > > > > > +used_len = block->used_length - offset; > > > > > +addr += used_len; > > > > > +} > > > > > + > > > > > +start = offset >> TARGET_PAGE_BITS; > > > > > +npages = used_len >> TARGET_PAGE_BITS; > > > > > + > > > > > +qemu_mutex_lock(&ram_state->bitmap_mutex); > > > > So now I think I understand the lock can still be meaningful since > > > > this function now can be called outside the migration thread (e.g., in > > > > vcpu thread). But still it would be nice to mention it somewhere on > > (Actually after read the next patch I think it's in iothread, so I'd > > better reply with all the series read over next time :) > > That's fine actually :) Whether it is called by an iothread or a vcpu thread > doesn't affect our discussion here. > > I think we could just focus on the interfaces here and the usage in live > migration. I can explain more when needed. Ok. Thanks! -- Peter Xu
[Qemu-devel] [PATCH qemu v4] memory/hmp: Print owners/parents in "info mtree"
This adds owners/parents (which are the same, just occasionally owner==NULL) printing for memory regions; a new '-o' flag enabled new output. Signed-off-by: Alexey Kardashevskiy --- The only orphan object I know is printed now as: - (prio 0, i/o): mem-container-smram owner:{obj type=kvm-accel} Unrelated observation: QEMU in x86_64 constantly hits breakpoints in unassigned_mem_accepts and unassigned_mem_read (when set), in early boot stage before SeaBIOS even shows up, the address is 0xfed40f00 and that is not anything assigned. "info mtree" shows that it is surrounded by: fec0-fec00fff (prio 0, i/o): kvm-ioapic fed0-fed003ff (prio 0, i/o): hpet fee0-feef (prio 4096, i/o): kvm-apic-msi Is it something to worry about? --- Changes: v4: * prints object typename if canonical path could not be resolved v3: * removed QOM's "id" property as there are no objects left which would have this property and own an MR v2: * cleanups --- include/exec/memory.h | 2 +- memory.c | 72 --- monitor.c | 4 ++- hmp-commands-info.hx | 7 ++--- 4 files changed, 70 insertions(+), 15 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 67ea7fe..95340b8 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -1627,7 +1627,7 @@ void memory_global_dirty_log_start(void); void memory_global_dirty_log_stop(void); void mtree_info(fprintf_function mon_printf, void *f, bool flatview, -bool dispatch_tree); +bool dispatch_tree, bool owner); /** * memory_region_request_mmio_ptr: request a pointer to an mmio diff --git a/memory.c b/memory.c index 10fa2dd..bf0a9f2 100644 --- a/memory.c +++ b/memory.c @@ -2832,10 +2832,49 @@ typedef QTAILQ_HEAD(mrqueue, MemoryRegionList) MemoryRegionListHead; int128_sub((size), int128_one())) : 0) #define MTREE_INDENT " " +static void mtree_expand_owner(fprintf_function mon_printf, void *f, + const char *label, Object *obj) +{ +DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE); + +mon_printf(f, " %s:{%s", label, dev ? "dev" : "obj"); +if (dev && dev->id) { +mon_printf(f, " id=%s", dev->id); +} else { +gchar *canonical_path = object_get_canonical_path(obj); +if (canonical_path) { +mon_printf(f, " path=%s", canonical_path); +g_free(canonical_path); +} else { +mon_printf(f, " type=%s", object_get_typename(obj)); +} +} +mon_printf(f, "}"); +} + +static void mtree_print_mr_owner(fprintf_function mon_printf, void *f, + const MemoryRegion *mr) +{ +Object *owner = mr->owner; +Object *parent = memory_region_owner((MemoryRegion *)mr); + +if (!owner && !parent) { +mon_printf(f, " orphan"); +return; +} +if (owner) { +mtree_expand_owner(mon_printf, f, "owner", owner); +} +if (parent && parent != owner) { +mtree_expand_owner(mon_printf, f, "parent", parent); +} +} + static void mtree_print_mr(fprintf_function mon_printf, void *f, const MemoryRegion *mr, unsigned int level, hwaddr base, - MemoryRegionListHead *alias_print_queue) + MemoryRegionListHead *alias_print_queue, + bool owner) { MemoryRegionList *new_ml, *ml, *next_ml; MemoryRegionListHead submr_print_queue; @@ -2881,7 +2920,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, } mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): alias %s @%s " TARGET_FMT_plx - "-" TARGET_FMT_plx "%s\n", + "-" TARGET_FMT_plx "%s", cur_start, cur_end, mr->priority, memory_region_type((MemoryRegion *)mr), @@ -2890,15 +2929,22 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, mr->alias_offset, mr->alias_offset + MR_SIZE(mr->size), mr->enabled ? "" : " [disabled]"); +if (owner) { +mtree_print_mr_owner(mon_printf, f, mr); +} } else { mon_printf(f, - TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): %s%s\n", + TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): %s%s", cur_start, cur_end, mr->priority, memory_region_type((MemoryRegion *)mr), memory_region_name(mr), mr->enabled ? "" : " [disabled]"); +if (owner) { +mtree_print_mr_owner(mon_printf, f,
Re: [Qemu-devel] [PATCH v4] migration: discard non-migratable RAMBlocks
Cédric Le Goater wrote: > On the POWER9 processor, the XIVE interrupt controller can control > interrupt sources using MMIO to trigger events, to EOI or to turn off > the sources. Priority management and interrupt acknowledgment is also > controlled by MMIO in the presenter sub-engine. > > These MMIO regions are exposed to guests in QEMU with a set of 'ram > device' memory mappings, similarly to VFIO, and the VMAs are populated > dynamically with the appropriate pages using a fault handler. > > But, these regions are an issue for migration. We need to discard the > associated RAMBlocks from the RAM state on the source VM and let the > destination VM rebuild the memory mappings on the new host in the > post_load() operation just before resuming the system. > > To achieve this goal, the following introduces a new RAMBlock flag > RAM_MIGRATABLE which is updated in the vmstate_register_ram() and > vmstate_unregister_ram() routines. This flag is then used by the > migration to identify RAMBlocks to discard on the source. Some checks > are also performed on the destination to make sure nothing invalid was > sent. > > This change impacts the boston, malta and jazz mips boards for which > migration compatibility is broken. > > Signed-off-by: Cédric Le Goater Reviewed-by: Juan Quintela queued
Re: [Qemu-devel] [PATCH v2 for 2.13] migration: Don't activate block devices if using -S
"Dr. David Alan Gilbert (git)" wrote: > From: "Dr. David Alan Gilbert" > > Activating the block devices causes the locks to be taken on > the backing file. If we're running with -S and the destination libvirt > hasn't started the destination with 'cont', it's expecting the locks are > still untaken. > > Don't activate the block devices if we're not going to autostart the VM; > 'cont' already will do that anyway. This change is tied to the new > migration capability 'late-block-activate' that defaults to off, keeping > the old behaviour by default. > > bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854 > Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Juan Quintela
[Qemu-devel] [PULL 1/5] migration: introduce decompress-error-check
From: Xiao Guangrong QEMU 3.0 enables strict check for compression & decompression to make the migration more robust, that depends on the source to fix the internal design which triggers the unexpected error conditions To make it work for migrating old version QEMU to 2.13 QEMU, we introduce this parameter to disable the error check on the destination which is the default behavior of the machine type which is older than 2.13, alternately, the strict check can be enabled explicitly as followings: -M pc-q35-2.11 -global migration.decompress-error-check=true Signed-off-by: Xiao Guangrong Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela --- hw/arm/virt.c | 4 hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + include/hw/compat.h | 7 ++- migration/migration.c | 4 migration/migration.h | 7 +++ migration/ram.c | 2 +- 7 files changed, 24 insertions(+), 2 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index a3a28e20e8..b2a67a4c00 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1693,6 +1693,9 @@ static void machvirt_machine_init(void) } type_init(machvirt_machine_init); +#define VIRT_COMPAT_2_12 \ +HW_COMPAT_2_12 + static void virt_2_12_instance_init(Object *obj) { VirtMachineState *vms = VIRT_MACHINE(obj); @@ -1763,6 +1766,7 @@ static void virt_2_12_instance_init(Object *obj) static void virt_machine_2_12_options(MachineClass *mc) { +SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_12); } DEFINE_VIRT_MACHINE_AS_LATEST(2, 12) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index b4c5b03274..3d81136065 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -430,6 +430,7 @@ static void pc_i440fx_3_0_machine_options(MachineClass *m) pc_i440fx_machine_options(m); m->alias = "pc"; m->is_default = 1; +SET_MACHINE_COMPAT(m, PC_COMPAT_2_12); } DEFINE_I440FX_MACHINE(v3_0, "pc-i440fx-3.0", NULL, diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 83d6d75efa..b60cbb9266 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -312,6 +312,7 @@ static void pc_q35_3_0_machine_options(MachineClass *m) { pc_q35_machine_options(m); m->alias = "q35"; +SET_MACHINE_COMPAT(m, PC_COMPAT_2_12); } DEFINE_Q35_MACHINE(v3_0, "pc-q35-3.0", NULL, diff --git a/include/hw/compat.h b/include/hw/compat.h index 4681c2719a..563908b874 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -1,7 +1,12 @@ #ifndef HW_COMPAT_H #define HW_COMPAT_H -#define HW_COMPAT_2_12 +#define HW_COMPAT_2_12 \ +{\ +.driver = "migration",\ +.property = "decompress-error-check",\ +.value= "off",\ +}, #define HW_COMPAT_2_11 \ {\ diff --git a/migration/migration.c b/migration/migration.c index 05aec2c905..a5384865ff 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2971,6 +2971,8 @@ void migration_global_dump(Monitor *mon) ms->send_configuration ? "on" : "off"); monitor_printf(mon, "send-section-footer: %s\n", ms->send_section_footer ? "on" : "off"); +monitor_printf(mon, "decompress-error-check: %s\n", + ms->decompress_error_check ? "on" : "off"); } #define DEFINE_PROP_MIG_CAP(name, x) \ @@ -2984,6 +2986,8 @@ static Property migration_properties[] = { send_configuration, true), DEFINE_PROP_BOOL("send-section-footer", MigrationState, send_section_footer, true), +DEFINE_PROP_BOOL("decompress-error-check", MigrationState, + decompress_error_check, true), /* Migration parameters */ DEFINE_PROP_UINT8("x-compress-level", MigrationState, diff --git a/migration/migration.h b/migration/migration.h index 8f0c82159b..5af57d616c 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -212,6 +212,13 @@ struct MigrationState /* Needed by postcopy-pause state */ QemuSemaphore postcopy_pause_sem; QemuSemaphore postcopy_pause_rp_sem; +/* + * Whether we abort the migration if decompression errors are + * detected at the destination. It is left at false for qemu + * older than 3.0, since only newer qemu sends streams that + * do not trigger spurious decompression errors. + */ +bool decompress_error_check; }; void migrate_set_state(int *state, int old_state, int new_state); diff --git a/migration/ram.c b/migration/ram.c index c53e8369a3..090187ca04 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -2881,7 +2881,7 @@ static void *do_data_decompress(void *opaque) ret = qemu_uncompress_data(¶m->stream, des, pagesize, param->compbuf, len); -if (ret < 0) { +if (ret < 0 && migrate_get_current()->decompress_error_check) { error_report("decompress data failed"); qemu_file_set_error(decomp_file, ret); } -- 2.17.0
[Qemu-devel] [PULL 0/5] Migration pull request
The following changes since commit 392fba9f583223786f844dce9b2e7f9a0ce0147a: Merge remote-tracking branch 'remotes/stsquad/tags/pull-travis-updates-010618-1' into staging (2018-06-01 17:32:30 +0100) are available in the Git repository at: git://github.com/juanquintela/qemu.git tags/migration/20180604 for you to fetch changes up to c5e76115ccb4979cec795a8ae38becd07c2fde9f: migration: not wait RDMA_CM_EVENT_DISCONNECTED event after rdma_disconnect (2018-06-04 05:46:15 +0200) migration/next for 20180604 - RDMA fixes from (lidong) - Fix docempress-error-check (Xiao) - make -S and block work better (dave) - don't migrate "bad" ramblocks (Cédric) Please apply, Juan. Cédric Le Goater (1): migration: discard non-migratable RAMBlocks Dr. David Alan Gilbert (1): migration: Don't activate block devices if using -S Lidong Chen (2): migration: remove unnecessary variables len in QIOChannelRDMA migration: not wait RDMA_CM_EVENT_DISCONNECTED event after rdma_disconnect Xiao Guangrong (1): migration: introduce decompress-error-check exec.c| 38 + hw/arm/virt.c | 4 hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + include/exec/cpu-common.h | 4 include/hw/compat.h | 7 ++- migration/migration.c | 38 ++--- migration/migration.h | 7 +++ migration/postcopy-ram.c | 12 ++-- migration/ram.c | 48 ++- migration/rdma.c | 27 +- migration/savevm.c| 2 ++ migration/trace-events| 1 - qapi/migration.json | 6 +- 14 files changed, 149 insertions(+), 47 deletions(-)
[Qemu-devel] [PULL 3/5] migration: Don't activate block devices if using -S
From: "Dr. David Alan Gilbert" Activating the block devices causes the locks to be taken on the backing file. If we're running with -S and the destination libvirt hasn't started the destination with 'cont', it's expecting the locks are still untaken. Don't activate the block devices if we're not going to autostart the VM; 'cont' already will do that anyway. This change is tied to the new migration capability 'late-block-activate' that defaults to off, keeping the old behaviour by default. bz: https://bugzilla.redhat.com/show_bug.cgi?id=1560854 Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela --- migration/migration.c | 34 +++--- qapi/migration.json | 6 +- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index a5384865ff..1e99ec9b7e 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -202,6 +202,16 @@ static void migrate_generate_event(int new_state) } } +static bool migrate_late_block_activate(void) +{ +MigrationState *s; + +s = migrate_get_current(); + +return s->enabled_capabilities[ +MIGRATION_CAPABILITY_LATE_BLOCK_ACTIVATE]; +} + /* * Called on -incoming with a defer: uri. * The migration can be started later after any parameters have been @@ -311,13 +321,23 @@ static void process_incoming_migration_bh(void *opaque) Error *local_err = NULL; MigrationIncomingState *mis = opaque; -/* Make sure all file formats flush their mutable metadata. - * If we get an error here, just don't restart the VM yet. */ -bdrv_invalidate_cache_all(&local_err); -if (local_err) { -error_report_err(local_err); -local_err = NULL; -autostart = false; +/* If capability late_block_activate is set: + * Only fire up the block code now if we're going to restart the + * VM, else 'cont' will do it. + * This causes file locking to happen; so we don't want it to happen + * unless we really are starting the VM. + */ +if (!migrate_late_block_activate() || + (autostart && (!global_state_received() || +global_state_get_runstate() == RUN_STATE_RUNNING))) { +/* Make sure all file formats flush their mutable metadata. + * If we get an error here, just don't restart the VM yet. */ +bdrv_invalidate_cache_all(&local_err); +if (local_err) { +error_report_err(local_err); +local_err = NULL; +autostart = false; +} } /* diff --git a/qapi/migration.json b/qapi/migration.json index dc9cc85545..f7e10ee90f 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -376,13 +376,17 @@ # @postcopy-blocktime: Calculate downtime for postcopy live migration # (since 3.0) # +# @late-block-activate: If enabled, the destination will not activate block +# devices (and thus take locks) immediately at the end of migration. +# (since 3.0) +# # Since: 1.2 ## { 'enum': 'MigrationCapability', 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks', 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram', 'block', 'return-path', 'pause-before-switchover', 'x-multifd', - 'dirty-bitmaps', 'postcopy-blocktime' ] } + 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate' ] } ## # @MigrationCapabilityStatus: -- 2.17.0
[Qemu-devel] [PULL 2/5] migration: discard non-migratable RAMBlocks
From: Cédric Le Goater On the POWER9 processor, the XIVE interrupt controller can control interrupt sources using MMIO to trigger events, to EOI or to turn off the sources. Priority management and interrupt acknowledgment is also controlled by MMIO in the presenter sub-engine. These MMIO regions are exposed to guests in QEMU with a set of 'ram device' memory mappings, similarly to VFIO, and the VMAs are populated dynamically with the appropriate pages using a fault handler. But, these regions are an issue for migration. We need to discard the associated RAMBlocks from the RAM state on the source VM and let the destination VM rebuild the memory mappings on the new host in the post_load() operation just before resuming the system. To achieve this goal, the following introduces a new RAMBlock flag RAM_MIGRATABLE which is updated in the vmstate_register_ram() and vmstate_unregister_ram() routines. This flag is then used by the migration to identify RAMBlocks to discard on the source. Some checks are also performed on the destination to make sure nothing invalid was sent. This change impacts the boston, malta and jazz mips boards for which migration compatibility is broken. Signed-off-by: Cédric Le Goater Reviewed-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Juan Quintela --- exec.c| 38 include/exec/cpu-common.h | 4 migration/postcopy-ram.c | 12 +- migration/ram.c | 46 +-- migration/savevm.c| 2 ++ 5 files changed, 84 insertions(+), 18 deletions(-) diff --git a/exec.c b/exec.c index c30f905598..af90e914cf 100644 --- a/exec.c +++ b/exec.c @@ -104,6 +104,9 @@ static MemoryRegion io_mem_unassigned; * (Set during postcopy) */ #define RAM_UF_ZEROPAGE (1 << 3) + +/* RAM can be migrated */ +#define RAM_MIGRATABLE (1 << 4) #endif #ifdef TARGET_PAGE_BITS_VARY @@ -1838,6 +1841,21 @@ void qemu_ram_set_uf_zeroable(RAMBlock *rb) rb->flags |= RAM_UF_ZEROPAGE; } +bool qemu_ram_is_migratable(RAMBlock *rb) +{ +return rb->flags & RAM_MIGRATABLE; +} + +void qemu_ram_set_migratable(RAMBlock *rb) +{ +rb->flags |= RAM_MIGRATABLE; +} + +void qemu_ram_unset_migratable(RAMBlock *rb) +{ +rb->flags &= ~RAM_MIGRATABLE; +} + /* Called with iothread lock held. */ void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState *dev) { @@ -3893,6 +3911,26 @@ int qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque) return ret; } +int qemu_ram_foreach_migratable_block(RAMBlockIterFunc func, void *opaque) +{ +RAMBlock *block; +int ret = 0; + +rcu_read_lock(); +RAMBLOCK_FOREACH(block) { +if (!qemu_ram_is_migratable(block)) { +continue; +} +ret = func(block->idstr, block->host, block->offset, + block->used_length, opaque); +if (ret) { +break; +} +} +rcu_read_unlock(); +return ret; +} + /* * Unmap pages of memory from start to start+length such that * they a) read as 0, b) Trigger whatever fault mechanism diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h index 24d335f95d..0b58e262f3 100644 --- a/include/exec/cpu-common.h +++ b/include/exec/cpu-common.h @@ -75,6 +75,9 @@ const char *qemu_ram_get_idstr(RAMBlock *rb); bool qemu_ram_is_shared(RAMBlock *rb); bool qemu_ram_is_uf_zeroable(RAMBlock *rb); void qemu_ram_set_uf_zeroable(RAMBlock *rb); +bool qemu_ram_is_migratable(RAMBlock *rb); +void qemu_ram_set_migratable(RAMBlock *rb); +void qemu_ram_unset_migratable(RAMBlock *rb); size_t qemu_ram_pagesize(RAMBlock *block); size_t qemu_ram_pagesize_largest(void); @@ -119,6 +122,7 @@ typedef int (RAMBlockIterFunc)(const char *block_name, void *host_addr, ram_addr_t offset, ram_addr_t length, void *opaque); int qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque); +int qemu_ram_foreach_migratable_block(RAMBlockIterFunc func, void *opaque); int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length); #endif diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index 658b750a8e..48e51556a7 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -374,7 +374,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis) } /* We don't support postcopy with shared RAM yet */ -if (qemu_ram_foreach_block(test_ramblock_postcopiable, NULL)) { +if (qemu_ram_foreach_migratable_block(test_ramblock_postcopiable, NULL)) { goto out; } @@ -502,7 +502,7 @@ static int cleanup_range(const char *block_name, void *host_addr, */ int postcopy_ram_incoming_init(MigrationIncomingState *mis, size_t ram_pages) { -if (qemu_ram_foreach_block(init_range, NULL)) { +if (qemu_ram_foreach_migratable_block(init_range, NULL)) { return -1; } @@ -524,7 +524,7 @@ int postcopy_ram_incoming_cleanup(MigrationInco
[Qemu-devel] [PULL 4/5] migration: remove unnecessary variables len in QIOChannelRDMA
From: Lidong Chen Because qio_channel_rdma_writev and qio_channel_rdma_readv maybe invoked by different threads concurrently, this patch removes unnecessary variables len in QIOChannelRDMA and use local variable instead. Signed-off-by: Lidong Chen Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Daniel P. Berrangé Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela Signed-off-by: Lidong Chen --- migration/rdma.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/migration/rdma.c b/migration/rdma.c index 7d233b0820..60779221b1 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -400,7 +400,6 @@ struct QIOChannelRDMA { QIOChannel parent; RDMAContext *rdma; QEMUFile *file; -size_t len; bool blocking; /* XXX we don't actually honour this yet */ }; @@ -2608,6 +2607,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, int ret; ssize_t done = 0; size_t i; +size_t len = 0; CHECK_ERROR_STATE(); @@ -2627,10 +2627,10 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, while (remaining) { RDMAControlHeader head; -rioc->len = MIN(remaining, RDMA_SEND_INCREMENT); -remaining -= rioc->len; +len = MIN(remaining, RDMA_SEND_INCREMENT); +remaining -= len; -head.len = rioc->len; +head.len = len; head.type = RDMA_CONTROL_QEMU_FILE; ret = qemu_rdma_exchange_send(rdma, &head, data, NULL, NULL, NULL); @@ -2640,8 +2640,8 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc, return ret; } -data += rioc->len; -done += rioc->len; +data += len; +done += len; } } @@ -2736,8 +2736,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, } } } -rioc->len = done; -return rioc->len; +return done; } /* -- 2.17.0
[Qemu-devel] [PULL 5/5] migration: not wait RDMA_CM_EVENT_DISCONNECTED event after rdma_disconnect
From: Lidong Chen When cancel migration during RDMA precopy, the source qemu main thread hangs sometime. The backtrace is: (gdb) bt #0 0x7f249eabd43d in write () from /lib64/libpthread.so.0 #1 0x7f24a1ce98e4 in rdma_get_cm_event (channel=0x4675d10, event=0x7ffe2f643dd0) at src/cma.c:2189 #2 0x007b6166 in qemu_rdma_cleanup (rdma=0x6784000) at migration/rdma.c:2296 #3 0x007b7cae in qio_channel_rdma_close (ioc=0x3bfcc30, errp=0x0) at migration/rdma.c:2999 #4 0x008db60e in qio_channel_close (ioc=0x3bfcc30, errp=0x0) at io/channel.c:273 #5 0x007a8765 in channel_close (opaque=0x3bfcc30) at migration/qemu-file-channel.c:98 #6 0x007a71f9 in qemu_fclose (f=0x527c000) at migration/qemu-file.c:334 #7 0x00795b96 in migrate_fd_cleanup (opaque=0x3b46280) at migration/migration.c:1162 #8 0x0093a71b in aio_bh_call (bh=0x3db7a20) at util/async.c:90 #9 0x0093a7b2 in aio_bh_poll (ctx=0x3b121c0) at util/async.c:118 #10 0x0093f2ad in aio_dispatch (ctx=0x3b121c0) at util/aio-posix.c:436 #11 0x0093ab41 in aio_ctx_dispatch (source=0x3b121c0, callback=0x0, user_data=0x0) at util/async.c:261 #12 0x7f249f73c7aa in g_main_context_dispatch () from /lib64/libglib-2.0.so.0 #13 0x0093dc5e in glib_pollfds_poll () at util/main-loop.c:215 #14 0x0093dd4e in os_host_main_loop_wait (timeout=2800) at util/main-loop.c:263 #15 0x0093de05 in main_loop_wait (nonblocking=0) at util/main-loop.c:522 #16 0x005bc6a5 in main_loop () at vl.c:1944 #17 0x005c39b5 in main (argc=56, argv=0x7ffe2f6443f8, envp=0x3ad0030) at vl.c:4752 It does not get the RDMA_CM_EVENT_DISCONNECTED event after rdma_disconnect sometime. According to IB Spec once active side send DREQ message, it should wait for DREP message and only once it arrived it should trigger a DISCONNECT event. DREP message can be dropped due to network issues. For that case the spec defines a DREP_timeout state in the CM state machine, if the DREP is dropped we should get a timeout and a TIMEWAIT_EXIT event will be trigger. Unfortunately the current kernel CM implementation doesn't include the DREP_timeout state and in above scenario we will not get DISCONNECT or TIMEWAIT_EXIT events. So it should not invoke rdma_get_cm_event which may hang forever, and the event channel is also destroyed in qemu_rdma_cleanup. Signed-off-by: Lidong Chen Reviewed-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Juan Quintela --- migration/rdma.c | 12 ++-- migration/trace-events | 1 - 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/migration/rdma.c b/migration/rdma.c index 60779221b1..05aee3d591 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -2267,8 +2267,7 @@ static int qemu_rdma_write(QEMUFile *f, RDMAContext *rdma, static void qemu_rdma_cleanup(RDMAContext *rdma) { -struct rdma_cm_event *cm_event; -int ret, idx; +int idx; if (rdma->cm_id && rdma->connected) { if ((rdma->error_state || @@ -2282,14 +2281,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma) qemu_rdma_post_send_control(rdma, NULL, &head); } -ret = rdma_disconnect(rdma->cm_id); -if (!ret) { -trace_qemu_rdma_cleanup_waiting_for_disconnect(); -ret = rdma_get_cm_event(rdma->channel, &cm_event); -if (!ret) { -rdma_ack_cm_event(cm_event); -} -} +rdma_disconnect(rdma->cm_id); trace_qemu_rdma_cleanup_disconnect(); rdma->connected = false; } diff --git a/migration/trace-events b/migration/trace-events index 3c798ddd11..4a768eaaeb 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -146,7 +146,6 @@ qemu_rdma_accept_pin_state(bool pin) "%d" qemu_rdma_accept_pin_verbsc(void *verbs) "Verbs context after listen: %p" qemu_rdma_block_for_wrid_miss(const char *wcompstr, int wcomp, const char *gcompstr, uint64_t req) "A Wanted wrid %s (%d) but got %s (%" PRIu64 ")" qemu_rdma_cleanup_disconnect(void) "" -qemu_rdma_cleanup_waiting_for_disconnect(void) "" qemu_rdma_close(void) "" qemu_rdma_connect_pin_all_requested(void) "" qemu_rdma_connect_pin_all_outcome(bool pin) "%d" -- 2.17.0
Re: [Qemu-devel] [PULL 0/5] Migration pull request
Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20180604042156.13812-1-quint...@redhat.com Subject: [Qemu-devel] [PULL 0/5] Migration pull request === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu t [tag update]patchew/20180514065700.22202-1-...@kaod.org -> patchew/20180514065700.22202-1-...@kaod.org * [new tag] patchew/20180604042156.13812-1-quint...@redhat.com -> patchew/20180604042156.13812-1-quint...@redhat.com Switched to a new branch 'test' 99ac0fb20d migration: not wait RDMA_CM_EVENT_DISCONNECTED event after rdma_disconnect 7c902253bb migration: remove unnecessary variables len in QIOChannelRDMA 3c1203cb38 migration: Don't activate block devices if using -S 2ecc458546 migration: discard non-migratable RAMBlocks 718b388d4e migration: introduce decompress-error-check === OUTPUT BEGIN === Checking PATCH 1/5: migration: introduce decompress-error-check... Checking PATCH 2/5: migration: discard non-migratable RAMBlocks... ERROR: Macros with multiple statements should be enclosed in a do - while loop #191: FILE: migration/ram.c:161: +#define RAMBLOCK_FOREACH_MIGRATABLE(block) \ +RAMBLOCK_FOREACH(block)\ +if (!qemu_ram_is_migratable(block)) {} else ERROR: trailing statements should be on next line #193: FILE: migration/ram.c:163: +if (!qemu_ram_is_migratable(block)) {} else total: 2 errors, 0 warnings, 273 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Checking PATCH 3/5: migration: Don't activate block devices if using -S... Checking PATCH 4/5: migration: remove unnecessary variables len in QIOChannelRDMA... Checking PATCH 5/5: migration: not wait RDMA_CM_EVENT_DISCONNECTED event after rdma_disconnect... === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[Qemu-devel] [Bug 1426092] Re: virtio-balloon-device locks up Guest
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1426092 Title: virtio-balloon-device locks up Guest Status in QEMU: Expired Bug description: Setting arbitrary balloon target values locks up the guest in some cases crashes it, if the target memory is < used +~5% free. Found while testing aggressive memory over-commit, scenarios. You get messages like: [ 155.827448] [] (show_stack) from [] (dump_stack+0x6c/0x88) [ 155.837076] [] (dump_stack) from [] (warn_alloc_failed+0xe0/0x120) [ 155.847075] [] (warn_alloc_failed) from [] (__alloc_pages_nodemask+0x600/0x91c) [ 155.859039] [] (__alloc_pages_nodemask) from [] (balloon_page_enqueue+0x20/0xbc) [ 155.870556] [] (balloon_page_enqueue) from [] (balloon+0x140/0x2cc) [ 155.881377] [] (balloon) from [] (kthread+0xd8/0xf4) page dumped bacause: nonzero _count BUG: BAad page state in process Xorg pfn:396e5 Test Environment: x86_64 Ubuntu 13.10, Guest Linux Kernel 3.19, qemu 2.2.0 with following patches applied - balloon OOM enhancement commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5 Author: Raushaniya Maksudova Boot guest with '-balloon virtio', -qmp -hmp access 1. sudo info balloon | socat - tcp4-connect:127.0.0.1: - or over qmp interface {"execute":"qom-get", "arguments": { "path": "/machine/peripheral-anon/device[1]","property": "guest-stats" }} {"return": {"stats": {"stat-swap-out": 0, "stat-free-memory": 513454080, "stat-minor-faults": 1261, "stat-major-faults": 0, "stat-total-memory": 526073856, "stat-swap-in": 0}, "last-update": 11109}} 2. Take memory down check free memory using (1) 3. Issue "sudo echo balloon XXX | socat - tcp4-connect:127.0.0.1:" - XXX is value in threshold mentioned above and you get Guest lockup ARM - samething except '-device virtio-balloon-device' used Libvirt - virsh setmem causes same issue. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1426092/+subscriptions
[Qemu-devel] [Bug 1025244] Re: qcow2 image increasing disk size above the virtual limit
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1025244 Title: qcow2 image increasing disk size above the virtual limit Status in QEMU: Expired Status in qemu-kvm package in Ubuntu: Expired Bug description: Using qemu/kvm, qcow2 images, ext4 file systems on both guest and host Host and Guest: Ubuntu server 12.04 64bit To create an image I did this: qemu-img create -f qcow2 -o preallocation=metadata ubuntu-pdc-vda.img 10737418240 (not sure about the exact bytes, but around this) ls -l ubuntu-pdc-vda.img fallocate -l theSizeInBytesFromAbove ubuntu-pdc-vda.img The problem is that the image is growing progressively and has obviously no limit, although I gave it one. The root filesystem's image is the same case: qemu-img info ubuntu-pdc-vda.img image: ubuntu-pdc-vda.img file format: qcow2 virtual size: 10G (10737418240 bytes) disk size: 14G cluster_size: 65536 and for confirmation: du -sh ubuntu-pdc-vda.img 15G ubuntu-pdc-vda.img I made a test and saw that when I delete something from the guest, the real size of the image is not decreasing (I read it is normal). OK, but when I write something again, it doesn't use the freed space, but instead grows the image. So for example: 1. The initial physical size of the image is 1GB. 2. I copy 1GB of data in the guest. It's physical size becomes 2GB. 3. I delete this data (1GB). The physical size of the image remains 2GB. 4. I copy another 1GB of data to the guest. 5. The physical size of the image becomes 3GB. 6. And so on with no limit. It doesn't care if the virtual size is less. Is this normal - the real/physical size of the image to be larger than the virtual limit??? To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1025244/+subscriptions
[Qemu-devel] [Bug 1025244] Re: qcow2 image increasing disk size above the virtual limit
[Expired for qemu-kvm (Ubuntu) because there has been no activity for 60 days.] ** Changed in: qemu-kvm (Ubuntu) Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1025244 Title: qcow2 image increasing disk size above the virtual limit Status in QEMU: Expired Status in qemu-kvm package in Ubuntu: Expired Bug description: Using qemu/kvm, qcow2 images, ext4 file systems on both guest and host Host and Guest: Ubuntu server 12.04 64bit To create an image I did this: qemu-img create -f qcow2 -o preallocation=metadata ubuntu-pdc-vda.img 10737418240 (not sure about the exact bytes, but around this) ls -l ubuntu-pdc-vda.img fallocate -l theSizeInBytesFromAbove ubuntu-pdc-vda.img The problem is that the image is growing progressively and has obviously no limit, although I gave it one. The root filesystem's image is the same case: qemu-img info ubuntu-pdc-vda.img image: ubuntu-pdc-vda.img file format: qcow2 virtual size: 10G (10737418240 bytes) disk size: 14G cluster_size: 65536 and for confirmation: du -sh ubuntu-pdc-vda.img 15G ubuntu-pdc-vda.img I made a test and saw that when I delete something from the guest, the real size of the image is not decreasing (I read it is normal). OK, but when I write something again, it doesn't use the freed space, but instead grows the image. So for example: 1. The initial physical size of the image is 1GB. 2. I copy 1GB of data in the guest. It's physical size becomes 2GB. 3. I delete this data (1GB). The physical size of the image remains 2GB. 4. I copy another 1GB of data to the guest. 5. The physical size of the image becomes 3GB. 6. And so on with no limit. It doesn't care if the virtual size is less. Is this normal - the real/physical size of the image to be larger than the virtual limit??? To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1025244/+subscriptions
Re: [Qemu-devel] [RFC v2 2/4] tests: iotests: don't compare SHUTDOWN event
On Thu, May 31, 2018 at 09:42:23AM -0500, Eric Blake wrote: > On 05/31/2018 12:16 AM, Peter Xu wrote: > > This event is not really necessary. After OOB series it might affect > > the timing of the script so this event may or may not be there comparing > > to the old *.out results. Let's just filter it out. > > This is worrying. Are you stating that the SHUTDOWN event can occur in a > different order than it used to, or is it even worse that the SHUTDOWN event > disappears altogether? If enabling OOB makes the SHUTDOWN event sometimes > disappear, that's a regression that we should fix. If it just makes things > occur in a different order, we need an explanation why that is okay. The event might conditionally disappear in two of the 100+ qcow2 tests. And when it happens, it's not disappearing in all the testcases in the test but only some. For example, 087 might conditionally fail with this: 087 8s ... - output mismatch (see 087.out.bad) --- /home/peterx/git/qemu/tests/qemu-iotests/087.out2018-06-01 18:44:22.378982462 +0800 +++ /home/peterx/git/qemu/bin/tests/qemu-iotests/087.out.bad2018-06-01 18:53:44.267840928 +0800 @@ -8,7 +8,6 @@ {"return": {}} {"error": {"class": "GenericError", "desc": "'node-name' must be specified for the root node"}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} === Duplicate ID === @@ -53,7 +52,6 @@ {"return": {}} {"return": {}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} === Missing driver === Firstly, it does not fail every time I run "./check -qcow2 087", but it might fail like 1 out of 5. Then, it's not failing all the testcases in 087. For above example, it's failing "Missing ID and node-name" and "Encrypted image LUKS", and it can change too. > > > > > Since some of the scripts are using qmp-pretty, we need some trick in > > the filtering script to make sure sed works for multiple lines to > > explicitly mask out this event. > > > > CC: John Snow > > CC: Max Reitz > > CC: Kevin Wolf > > Signed-off-by: Peter Xu > > --- > > > +++ b/tests/qemu-iotests/067.out > > @@ -70,6 +70,7 @@ Testing: -drive > > file=TEST_DIR/t.IMGFMT,format=IMGFMT,if=none,id=disk -device vir > > } > > } > > + > > === -drive/device_add and device_del === > > Why is this blank line being added (multiple times in this file)? Is there > something about the new filter that isn't quite stripping all the newlines > when encountering the SHUTDOWN event in pretty form? > > Aha - it's because test 067 is already doing a very similar filtering of > events. Can we reduce the code duplication by promoting _filter_qmp_events > from there into common.filter (as a separate patch)? Yeah, can do. > > > +++ b/tests/qemu-iotests/common.filter > > @@ -88,7 +88,10 @@ _filter_qmp() > > sed -e 's#\("\(micro\)\?seconds": \)[0-9]\+#\1 TIMESTAMP#g' \ > > -e 's#^{"QMP":.*}$#QMP_VERSION#' \ > > -e '/^"QMP": {\s*$/, /^}\s*$/ c\' \ > > --e 'QMP_VERSION' > > +-e 'QMP_VERSION' | \ > > +tr '\n' '\r' | \ > > +sed -e > > 's/{\s*"timestamp":\s*{\s*"seconds":\s*TIMESTAMP,\s*"microseconds":\s*TIMESTAMP\s*},\s*"event":\s*"SHUTDOWN",\s*"data":\s*{\s*"guest":\s*false\s*}\s*}\s//' > > | \ > > Really long line. This should do the same: > > sed -e 's/\r{\(\r[^}]\|[^\r]\)*SHUTDOWN\(\r[^}]\|[^\r]\)*\r}//' > > where the \(\r[^}]\|[^\r]\)* subpattern picks up all line breaks that do not > end the current top-level {}, as well as any non-line breaks. > > In fact, if you like my suggestion about promoting the filter from 67 into > common.filter, we have two use cases: filter a single pretty-printed filter, > and filter ANY pretty-printed filter. Maybe we do that as follows: > > # $1 is a regex of event names to filter; leave empty to filter all > _filter_qmp_events() > { > fluff='\(\r[^}]\|[^\r]\)*' > tr \\n \\r | \ > sed -e "s/$fluff"'"event": "'"$1$fluff\\r}//" \ > | tr \\r \\n > } > > at which point '_filter_qmp_events SHUTDOWN' works in this patch, and > '_filter_qmp_events' works for 067. [Untested, but hopefully that gives you > some ideas to play with] Regex magic. Thanks for the lesson. :) -- Peter Xu
Re: [Qemu-devel] [PATCH V8 01/17] filter-rewriter: fix memory leak for connection in connection_track_table
On 2018年06月03日 13:05, Zhang Chen wrote: After a net connection is closed, we didn't clear its releated resources in connection_track_table, which will lead to memory leak. Let't track the state of net connection, if it is closed, its related resources will be cleared up. Signed-off-by: zhanghailiang Signed-off-by: Zhang Chen --- net/colo.h| 4 +++ net/filter-rewriter.c | 69 ++- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/net/colo.h b/net/colo.h index da6c36dcf7..cd118510c5 100644 --- a/net/colo.h +++ b/net/colo.h @@ -18,6 +18,7 @@ #include "slirp/slirp.h" #include "qemu/jhash.h" #include "qemu/timer.h" +#include "slirp/tcp.h" #define HASHTABLE_MAX_SIZE 16384 @@ -86,6 +87,9 @@ typedef struct Connection { * run once in independent tcp connection */ int syn_flag; + +int tcp_state; /* TCP FSM state */ +tcp_seq fin_ack_seq; /* the seq of 'fin=1,ack=1' */ So the question is, the state machine is not complete. I suspect there will be corner cases that will be left because of lacking sufficient states. LAST_ACK happens only for passive close. How about active close? So I think we need either maintain a full state machine or not instead of a partial one. We don't want endless bugs. Thanks } Connection; uint32_t connection_key_hash(const void *opaque); diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c index 62dad2d773..0909a9a8af 100644 --- a/net/filter-rewriter.c +++ b/net/filter-rewriter.c @@ -59,9 +59,9 @@ static int is_tcp_packet(Packet *pkt) } /* handle tcp packet from primary guest */ -static int handle_primary_tcp_pkt(NetFilterState *nf, +static int handle_primary_tcp_pkt(RewriterState *rf, Connection *conn, - Packet *pkt) + Packet *pkt, ConnectionKey *key) { struct tcphdr *tcp_pkt; @@ -99,15 +99,44 @@ static int handle_primary_tcp_pkt(NetFilterState *nf, net_checksum_calculate((uint8_t *)pkt->data + pkt->vnet_hdr_len, pkt->size - pkt->vnet_hdr_len); } +/* + * Case 1: + * The *server* side of this connect is VM, *client* tries to close + * the connection. + * + * We got 'ack=1' packets from client side, it acks 'fin=1, ack=1' + * packet from server side. From this point, we can ensure that there + * will be no packets in the connection, except that, some errors + * happen between the path of 'filter object' and vNIC, if this rare + * case really happen, we can still create a new connection, + * So it is safe to remove the connection from connection_track_table. + * + */ +if ((conn->tcp_state == TCPS_LAST_ACK) && +(ntohl(tcp_pkt->th_ack) == (conn->fin_ack_seq + 1))) { +g_hash_table_remove(rf->connection_track_table, key); +} +} +/* + * Case 2: + * The *server* side of this connect is VM, *server* tries to close + * the connection. + * + * We got 'fin=1, ack=1' packet from client side, we need to + * record the seq of 'fin=1, ack=1' packet. + */ +if ((tcp_pkt->th_flags & (TH_ACK | TH_FIN)) == (TH_ACK | TH_FIN)) { +conn->fin_ack_seq = htonl(tcp_pkt->th_seq); +conn->tcp_state = TCPS_LAST_ACK; } return 0; } /* handle tcp packet from secondary guest */ -static int handle_secondary_tcp_pkt(NetFilterState *nf, +static int handle_secondary_tcp_pkt(RewriterState *rf, Connection *conn, -Packet *pkt) +Packet *pkt, ConnectionKey *key) { struct tcphdr *tcp_pkt; @@ -139,8 +168,34 @@ static int handle_secondary_tcp_pkt(NetFilterState *nf, net_checksum_calculate((uint8_t *)pkt->data + pkt->vnet_hdr_len, pkt->size - pkt->vnet_hdr_len); } +/* + * Case 2: + * The *server* side of this connect is VM, *server* tries to close + * the connection. + * + * We got 'ack=1' packets from server side, it acks 'fin=1, ack=1' + * packet from client side. Like Case 1, there should be no packets + * in the connection from now know, But the difference here is + * if the packet is lost, We will get the resent 'fin=1,ack=1' packet. + * TODO: Fix above case. + */ +if ((conn->tcp_state == TCPS_LAST_ACK) && +(ntohl(tcp_pkt->th_ack) == (conn->fin_ack_seq + 1))) { +g_hash_table_remove(rf->connection_track_table, key); +} +} +/* + * Case 1: + * The *server* side of this connect is VM, *client* tries to close + * the connection. + * + * We got 'fin=1, ack=1' packet f
Re: [Qemu-devel] [RFC v2 0/2] kvm "fake DAX" device flushing
Hi Igor, > > [...] > > - Qemu virtio-pmem device > > It exposes a persistent memory range to KVM guest which > > at host side is file backed memory and works as persistent > > memory device. In addition to this it provides virtio > > device handling of flushing interface. KVM guest performs > > Qemu side asynchronous sync using this interface. > a random high level question, > Have you considered using a separate (from memory itself) > virtio device as controller for exposing some memory, async flushing. > And then just slaving pc-dimm devices to it with notification/ACPI > code suppressed so that guest won't touch them? No. > > That way it might be more scale-able, you consume only 1 PCI slot > for controller vs multiple for virtio-pmem devices. That sounds like a good suggestion. I will note it as an enhancement once we have other concerns related to basic working of 'flush' interface addressed. Then probably we can work on things 'need to optimize' with robust core flush functionality. BTW any sample code doing this right now in Qemu? > > > > Changes from previous RFC[1]: > > > > - Reuse existing 'pmem' code for registering persistent > > memory and other operations instead of creating an entirely > > new block driver. > > - Use VIRTIO driver to register memory information with > > nvdimm_bus and create region_type accordingly. > > - Call VIRTIO flush from existing pmem driver. > > > > Details of project idea for 'fake DAX' flushing interface is > > shared [2] & [3]. > > > > Pankaj Gupta (2): > >Add virtio-pmem guest driver > >pmem: device flush over VIRTIO > > > > [1] https://marc.info/?l=linux-mm&m=150782346802290&w=2 > > [2] https://www.spinics.net/lists/kvm/msg149761.html > > [3] https://www.spinics.net/lists/kvm/msg153095.html > > > > drivers/nvdimm/region_devs.c |7 ++ > > drivers/virtio/Kconfig | 12 +++ > > drivers/virtio/Makefile |1 > > drivers/virtio/virtio_pmem.c | 118 > > +++ > > include/linux/libnvdimm.h|4 + > > include/uapi/linux/virtio_ids.h |1 > > include/uapi/linux/virtio_pmem.h | 58 +++ > > 7 files changed, 201 insertions(+) > > > > >
Re: [Qemu-devel] [PATCH 1/4] spapr: remove irq_hint parameter from spapr_irq_alloc()
>> The property is a public interface. Just because it's not used by >> libvirt does not mean that nobody is using it. So yes, please follow the >> rules and mark it as deprecated first for two release, before you really >> remove it. > > I don't see a section for 'Deprecated' properties in the qemu-doc files. So I added a new one, which gives us : B.7 Device options B.7.1 Block device options B.7.1.1 "backing": "" (since 2.12.0) B.7.2 vio-spapr-device device options B.7.2.1 "irq": "" (since 2.13.0) Cheers, C.
Re: [Qemu-devel] [PATCH 7/8] virtio: get avail bytes check for packed ring
On Wed, Apr 11, 2018 at 11:03:24AM +0800, Jason Wang wrote: > > > On 2018年04月04日 20:54, w...@redhat.com wrote: > >From: Wei Xu > > > >mostly as same as 1.0, copy it separately for > >prototype, need a refactoring. > > > >Signed-off-by: Wei Xu > >--- > > hw/virtio/virtio.c | 142 > > +++-- > > 1 file changed, 139 insertions(+), 3 deletions(-) > > > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > >index def07c6..cf726f3 100644 > >--- a/hw/virtio/virtio.c > >+++ b/hw/virtio/virtio.c > >@@ -836,9 +836,9 @@ static int virtqueue_read_next_desc(VirtIODevice *vdev, > >VRingDesc *desc, > > return VIRTQUEUE_READ_DESC_MORE; > > } > >-void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes, > >- unsigned int *out_bytes, > >- unsigned max_in_bytes, unsigned > >max_out_bytes) > >+static void virtqueue_get_avail_bytes_split(VirtQueue *vq, > >+unsigned int *in_bytes, unsigned int *out_bytes, > >+unsigned max_in_bytes, unsigned max_out_bytes) > > { > > VirtIODevice *vdev = vq->vdev; > > unsigned int max, idx; > >@@ -961,6 +961,142 @@ err: > > goto done; > > } > >+static void virtqueue_get_avail_bytes_packed(VirtQueue *vq, > >+unsigned int *in_bytes, unsigned int *out_bytes, > >+unsigned max_in_bytes, unsigned max_out_bytes) > >+{ > >+VirtIODevice *vdev = vq->vdev; > >+unsigned int max, idx; > >+unsigned int total_bufs, in_total, out_total; > >+MemoryRegionCache *desc_cache; > >+VRingMemoryRegionCaches *caches; > >+MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID; > >+int64_t len = 0; > >+VRingDescPacked desc; > >+ > >+if (unlikely(!vq->packed.desc)) { > >+if (in_bytes) { > >+*in_bytes = 0; > >+} > >+if (out_bytes) { > >+*out_bytes = 0; > >+} > >+return; > >+} > >+ > >+rcu_read_lock(); > >+idx = vq->last_avail_idx; > >+total_bufs = in_total = out_total = 0; > >+ > >+max = vq->packed.num; > >+caches = vring_get_region_caches(vq); > >+if (caches->desc.len < max * sizeof(VRingDescPacked)) { > >+virtio_error(vdev, "Cannot map descriptor ring"); > >+goto err; > >+} > >+ > >+desc_cache = &caches->desc; > >+vring_desc_read_packed(vdev, &desc, desc_cache, idx); > >+while (is_desc_avail(&desc)) { > >+unsigned int num_bufs; > >+unsigned int i; > >+ > >+num_bufs = total_bufs; > >+ > >+if (desc.flags & VRING_DESC_F_INDIRECT) { > >+if (desc.len % sizeof(VRingDescPacked)) { > >+virtio_error(vdev, "Invalid size for indirect buffer > >table"); > >+goto err; > >+} > >+ > >+/* If we've got too many, that implies a descriptor loop. */ > >+if (num_bufs >= max) { > >+virtio_error(vdev, "Looped descriptor"); > >+goto err; > >+} > >+ > >+/* loop over the indirect descriptor table */ > >+len = address_space_cache_init(&indirect_desc_cache, > >+ vdev->dma_as, > >+ desc.addr, desc.len, false); > >+desc_cache = &indirect_desc_cache; > >+if (len < desc.len) { > >+virtio_error(vdev, "Cannot map indirect buffer"); > >+goto err; > >+} > >+ > >+max = desc.len / sizeof(VRingDescPacked); > >+num_bufs = i = 0; > >+vring_desc_read_packed(vdev, &desc, desc_cache, i); > >+} > >+ > >+do { > >+/* If we've got too many, that implies a descriptor loop. */ > >+if (++num_bufs > max) { > >+virtio_error(vdev, "Looped descriptor"); > >+goto err; > >+} > >+ > >+if (desc.flags & VRING_DESC_F_WRITE) { > >+in_total += desc.len; > >+} else { > >+out_total += desc.len; > >+} > >+if (in_total >= max_in_bytes && out_total >= max_out_bytes) { > >+goto done; > >+} > >+ > >+if (desc_cache == &indirect_desc_cache) { > >+vring_desc_read_packed(vdev, &desc, desc_cache, > >+ ++i % vq->packed.num); > >+} else { > >+vring_desc_read_packed(vdev, &desc, desc_cache, > >+ ++idx % vq->packed.num); > >+} > > Need to make sure desc.flags was read before other fields, otherwise we may > get stale address. OK, need a barrier here, thanks. Wei > > Thanks > > >+} while (desc.flags & VRING_DESC_F_NEXT); > >+ > >+if (desc_cache == &indirect_des
Re: [Qemu-devel] [PATCH for-2.11.2] spapr: make pseries-2.11 the default machine type
On Tue, May 22, 2018 at 07:17:28PM +0200, Greg Kurz wrote: > The spapr capability framework was introduced in QEMU 2.12. It allows > to have an explicit control on how host features are exposed to the > guest. This is especially needed to handle migration between hetero- > geneous hosts (eg, POWER8 to POWER9). It is also used to expose fixes/ > workarounds against speculative execution vulnerabilities to guests. > The framework was hence backported to QEMU 2.11.1, especially these > commits: > > 0fac4aa93074 spapr: Add pseries-2.12 machine type > 9070f408f491 spapr: Treat Hardware Transactional Memory (HTM) as an > optional capability > > 0fac4aa93074 has the confusing effect of making pseries-2.12 the default > machine type for QEMU 2.11.1, instead of the expected pseries-2.11. This > patch changes the default machine back to pseries-2.11. > > Unfortunately, 9070f408f491 enforces the HTM capability for pseries-2.11. > This isn't supported by TCG and breaks 'make check'. So this patch also > adds a hack to turn HTM off when using TCG. > > Signed-off-by: Greg Kurz Acked-by: David Gibson > --- > hw/ppc/spapr.c |4 ++-- > hw/ppc/spapr_caps.c |5 + > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 1a2dd1f597d9..6499a867520f 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -3820,7 +3820,7 @@ static void > spapr_machine_2_12_class_options(MachineClass *mc) > /* Defaults for the latest behaviour inherited from the base class */ > } > > -DEFINE_SPAPR_MACHINE(2_12, "2.12", true); > +DEFINE_SPAPR_MACHINE(2_12, "2.12", false); > > /* > * pseries-2.11 > @@ -3842,7 +3842,7 @@ static void > spapr_machine_2_11_class_options(MachineClass *mc) > SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_11); > } > > -DEFINE_SPAPR_MACHINE(2_11, "2.11", false); > +DEFINE_SPAPR_MACHINE(2_11, "2.11", true); > > /* > * pseries-2.10 > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > index 7b229517be38..82043e60e78b 100644 > --- a/hw/ppc/spapr_caps.c > +++ b/hw/ppc/spapr_caps.c > @@ -285,6 +285,11 @@ static sPAPRCapabilities > default_caps_with_cpu(sPAPRMachineState *spapr, > > caps = smc->default_caps; > > +/* HACK for 2.11.2: fix make check */ > +if (tcg_enabled()) { > +caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF; > +} > + > if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_07, >0, spapr->max_compat_pvr)) { > caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF; > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 2/4] hw/ppc/spapr_drc: Replace error_setg(&error_abort) by error_report() + abort()
On Tue, May 29, 2018 at 02:48:19PM -0300, Philippe Mathieu-Daudé wrote: > Use error_report() + abort() instead of error_setg(&error_abort), > as suggested by the "qapi/error.h" documentation: > > Please don't error_setg(&error_fatal, ...), use error_report() and > exit(), because that's more obvious. > Likewise, don't error_setg(&error_abort, ...), use assert(). > > Use abort() instead of the suggested assert() because the error message > already got displayed. > > Suggested-by: Eric Blake > Signed-off-by: Philippe Mathieu-Daudé Applied to ppc-for-2.13, thanks. > --- > hw/ppc/spapr_drc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c > index 8a045d6b93..2edb7d1e9c 100644 > --- a/hw/ppc/spapr_drc.c > +++ b/hw/ppc/spapr_drc.c > @@ -366,7 +366,8 @@ static void prop_get_fdt(Object *obj, Visitor *v, const > char *name, > break; > } > default: > -error_setg(&error_abort, "device FDT in unexpected state: %d", > tag); > +error_report("device FDT in unexpected state: %d", tag); > +abort(); > } > fdt_offset = fdt_offset_next; > } while (fdt_depth != 0); -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] target/ppc: Use proper logging function for possible guest errors
On Mon, May 28, 2018 at 08:11:19PM +0200, Thomas Huth wrote: > fprintf() and qemu_log_separate() are frowned upon these days for printing > logging information in QEMU. Accessing the wrong SPRs indicates wrong guest > behaviour in most cases, and we've got a proper way to log such situations, > which is the qemu_log_mask(LOG_GUEST_ERROR, ...) function. So use this > function now for logging the bad SPR accesses instead. > > Signed-off-by: Thomas Huth Applied to ppc-for-2.13, thanks. > --- > target/ppc/translate.c | 37 - > 1 file changed, 12 insertions(+), 25 deletions(-) > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index e30d99f..0806ee0 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -3933,13 +3933,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > * allowing userland application to read the PVR > */ > if (sprn != SPR_PVR) { > -fprintf(stderr, "Trying to read privileged spr %d (0x%03x) > at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - > 4); > -if (qemu_log_separate()) { > -qemu_log("Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, > - ctx->base.pc_next - 4); > -} > +qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged > spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, > sprn, > + ctx->base.pc_next - 4); > } > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > @@ -3951,12 +3947,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > return; > } > /* Not defined */ > -fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -if (qemu_log_separate()) { > -qemu_log("Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -} > +qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to read invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > /* The behaviour depends on MSR:PR and SPR# bit 0x10, > * it can generate a priv, a hv emu or a no-op > @@ -4097,12 +4090,9 @@ static void gen_mtspr(DisasContext *ctx) > (*write_cb)(ctx, sprn, rS(ctx->opcode)); > } else { > /* Privilege exception */ > -fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -if (qemu_log_separate()) { > -qemu_log("Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - > 4); > -} > +qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, > + ctx->base.pc_next - 4); > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > } else { > @@ -4114,12 +4104,9 @@ static void gen_mtspr(DisasContext *ctx) > } > > /* Not defined */ > -if (qemu_log_separate()) { > -qemu_log("Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > -} > -fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " > -TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > +qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to write invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > > /* The behaviour depends on MSR:PR and SPR# bit 0x10, -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] 40p: remove pci_allow_0_address = true from 40p machine class
On Fri, May 25, 2018 at 10:15:23PM +0100, Mark Cave-Ayland wrote: > The Linux sandalfoot zImage has an initialisation process which resets the > VGA controller by setting all the BAR addresses to zero to access the VGA > ioports at their legacy addresses. > > Unfortunately setting the framebuffer BAR to address 0 makes the framebuffer > memory overlap the internal VGA memory causing accesses to fail, and so > prevents the kernel from switching successfully to text mode. > > Since OpenHackWare configures the framebuffer BAR address outside of the > legacy > VGA internal memory space, remove pci_allow_0_address from the 40p machine > class > which causes the BAR reprogramming to zero to fail and so the VGA internal > memory can be accessed correctly again. > > Signed-off-by: Mark Cave-Ayland Applied, thanks. > --- > hw/ppc/prep.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c > index be4db6a687..5ed0bcd862 100644 > --- a/hw/ppc/prep.c > +++ b/hw/ppc/prep.c > @@ -884,7 +884,6 @@ static void ibm_40p_machine_init(MachineClass *mc) > mc->desc = "IBM RS/6000 7020 (40p)", > mc->init = ibm_40p_init; > mc->max_cpus = 1; > -mc->pci_allow_0_address = true; > mc->default_ram_size = 128 * M_BYTE; > mc->block_default_type = IF_SCSI; > mc->default_boot_order = "c"; -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] prep: fix keyboard for the 40p machine
On Thu, May 24, 2018 at 06:39:58AM +0100, Mark Cave-Ayland wrote: > Commit 72d3d8f052 "hw/isa/superio: Add a keyboard/mouse controller (8042)" > added an 8042 keyboard device to the PC87312 superio device to replace that > being used by the prep machine. > > Unfortunately this commit didn't do the same for the 40p machine which broke > the keyboard by registering two 8042 keyboard devices at the same address. > > Resolve this by similarly removing the 8042 keyboard from the 40p machine as > done for the prep machine in commit 72d3d8f052. > > Signed-off-by: Mark Cave-Ayland Applied to ppc-for-3.0. > --- > hw/ppc/prep.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c > index a1e7219db6..be4db6a687 100644 > --- a/hw/ppc/prep.c > +++ b/hw/ppc/prep.c > @@ -770,7 +770,6 @@ static void ibm_40p_init(MachineState *machine) > > /* add some more devices */ > if (defaults_enabled()) { > -isa_create_simple(isa_bus, TYPE_I8042); > m48t59 = NVRAM(isa_create_simple(isa_bus, "isa-m48t59")); > > dev = DEVICE(isa_create(isa_bus, "cs4231a")); -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH V8 02/17] colo-compare: implement the process of checkpoint
On 2018年06月03日 13:05, Zhang Chen wrote: While do checkpoint, we need to flush all the unhandled packets, By using the filter notifier mechanism, we can easily to notify every compare object to do this process, which runs inside of compare threads as a coroutine. Signed-off-by: zhanghailiang Signed-off-by: Zhang Chen --- include/migration/colo.h | 6 net/colo-compare.c | 76 net/colo-compare.h | 22 3 files changed, 104 insertions(+) create mode 100644 net/colo-compare.h diff --git a/include/migration/colo.h b/include/migration/colo.h index 2fe48ad353..fefb2fcf4c 100644 --- a/include/migration/colo.h +++ b/include/migration/colo.h @@ -16,6 +16,12 @@ #include "qemu-common.h" #include "qapi/qapi-types-migration.h" +enum colo_event { +COLO_EVENT_NONE, +COLO_EVENT_CHECKPOINT, +COLO_EVENT_FAILOVER, +}; + void colo_info_init(void); void migrate_start_colo_process(MigrationState *s); diff --git a/net/colo-compare.c b/net/colo-compare.c index 23b2d2c4cc..7ff3ae8904 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -27,11 +27,16 @@ #include "qemu/sockets.h" #include "net/colo.h" #include "sysemu/iothread.h" +#include "net/colo-compare.h" +#include "migration/colo.h" #define TYPE_COLO_COMPARE "colo-compare" #define COLO_COMPARE(obj) \ OBJECT_CHECK(CompareState, (obj), TYPE_COLO_COMPARE) +static QTAILQ_HEAD(, CompareState) net_compares = + QTAILQ_HEAD_INITIALIZER(net_compares); + #define COMPARE_READ_LEN_MAX NET_BUFSIZE #define MAX_QUEUE_SIZE 1024 @@ -41,6 +46,10 @@ /* TODO: Should be configurable */ #define REGULAR_PACKET_CHECK_MS 3000 +static QemuMutex event_mtx; +static QemuCond event_complete_cond; +static int event_unhandled_count; + /* * + CompareState ++ * | | @@ -87,6 +96,11 @@ typedef struct CompareState { IOThread *iothread; GMainContext *worker_context; QEMUTimer *packet_check_timer; + +QEMUBH *event_bh; +enum colo_event event; + +QTAILQ_ENTRY(CompareState) next; } CompareState; typedef struct CompareClass { @@ -736,6 +750,25 @@ static void check_old_packet_regular(void *opaque) REGULAR_PACKET_CHECK_MS); } +/* Public API, Used for COLO frame to notify compare event */ +void colo_notify_compares_event(void *opaque, int event, Error **errp) +{ +CompareState *s; + +qemu_mutex_lock(&event_mtx); +QTAILQ_FOREACH(s, &net_compares, next) { +s->event = event; +qemu_bh_schedule(s->event_bh); +event_unhandled_count++; +} +/* Wait all compare threads to finish handling this event */ +while (event_unhandled_count > 0) { +qemu_cond_wait(&event_complete_cond, &event_mtx); +} + +qemu_mutex_unlock(&event_mtx); +} + static void colo_compare_timer_init(CompareState *s) { AioContext *ctx = iothread_get_aio_context(s->iothread); @@ -756,6 +789,28 @@ static void colo_compare_timer_del(CompareState *s) } } +static void colo_flush_packets(void *opaque, void *user_data); + +static void colo_compare_handle_event(void *opaque) +{ +CompareState *s = opaque; + +switch (s->event) { +case COLO_EVENT_CHECKPOINT: +g_queue_foreach(&s->conn_list, colo_flush_packets, s); +break; +case COLO_EVENT_FAILOVER: +break; +default: +break; +} +qemu_mutex_lock(&event_mtx); Isn't this a deadlock? Since colo_notify_compares_event() won't release event_mtx until event_unhandled_count reaches zero. +assert(event_unhandled_count > 0); +event_unhandled_count--; +qemu_cond_broadcast(&event_complete_cond); +qemu_mutex_unlock(&event_mtx); +} + static void colo_compare_iothread(CompareState *s) { object_ref(OBJECT(s->iothread)); @@ -769,6 +824,7 @@ static void colo_compare_iothread(CompareState *s) s, s->worker_context, true); colo_compare_timer_init(s); +s->event_bh = qemu_bh_new(colo_compare_handle_event, s); } static char *compare_get_pri_indev(Object *obj, Error **errp) @@ -926,8 +982,13 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp) net_socket_rs_init(&s->pri_rs, compare_pri_rs_finalize, s->vnet_hdr); net_socket_rs_init(&s->sec_rs, compare_sec_rs_finalize, s->vnet_hdr); +QTAILQ_INSERT_TAIL(&net_compares, s, next); + g_queue_init(&s->conn_list); +qemu_mutex_init(&event_mtx); +qemu_cond_init(&event_complete_cond); + s->connection_track_table = g_hash_table_new_full(connection_key_hash, connection_key_equal, g_free, @@ -990,6 +1051,7 @@ static void colo_compare_init(Object *obj) static void colo_compare_finalize(Object *obj) { CompareState *s = COLO_COMPARE(obj); +CompareState *tmp = NULL;
Re: [Qemu-devel] [PATCH V8 03/17] colo-compare: use notifier to notify packets comparing result
On 2018年06月03日 13:05, Zhang Chen wrote: It's a good idea to use notifier to notify COLO frame of inconsistent packets comparing. Signed-off-by: Zhang Chen Signed-off-by: zhanghailiang --- net/colo-compare.c | 32 +--- net/colo-compare.h | 2 ++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 7ff3ae8904..05061cd1c4 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -29,6 +29,7 @@ #include "sysemu/iothread.h" #include "net/colo-compare.h" #include "migration/colo.h" +#include "migration/migration.h" #define TYPE_COLO_COMPARE "colo-compare" #define COLO_COMPARE(obj) \ @@ -37,6 +38,9 @@ static QTAILQ_HEAD(, CompareState) net_compares = QTAILQ_HEAD_INITIALIZER(net_compares); +static NotifierList colo_compare_notifiers = +NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers); + #define COMPARE_READ_LEN_MAX NET_BUFSIZE #define MAX_QUEUE_SIZE 1024 @@ -561,8 +565,24 @@ static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time) } } +static void colo_compare_inconsistent_notify(void) +{ Not good at English but inconsistency sounds better here. Thanks +notifier_list_notify(&colo_compare_notifiers, +migrate_get_current()); +} + +void colo_compare_register_notifier(Notifier *notify) +{ +notifier_list_add(&colo_compare_notifiers, notify); +} + +void colo_compare_unregister_notifier(Notifier *notify) +{ +notifier_remove(notify); +} + static int colo_old_packet_check_one_conn(Connection *conn, - void *user_data) + void *user_data) { GList *result = NULL; int64_t check_time = REGULAR_PACKET_CHECK_MS; @@ -573,10 +593,7 @@ static int colo_old_packet_check_one_conn(Connection *conn, if (result) { /* Do checkpoint will flush old packet */ -/* - * TODO: Notify colo frame to do checkpoint. - * colo_compare_inconsistent_notify(); - */ +colo_compare_inconsistent_notify(); return 0; } @@ -620,11 +637,12 @@ static void colo_compare_packet(CompareState *s, Connection *conn, /* * If one packet arrive late, the secondary_list or * primary_list will be empty, so we can't compare it - * until next comparison. + * until next comparison. If the packets in the list are + * timeout, it will trigger a checkpoint request. */ trace_colo_compare_main("packet different"); g_queue_push_head(&conn->primary_list, pkt); -/* TODO: colo_notify_checkpoint();*/ +colo_compare_inconsistent_notify(); break; } } diff --git a/net/colo-compare.h b/net/colo-compare.h index 1b1ce76aea..22ddd512e2 100644 --- a/net/colo-compare.h +++ b/net/colo-compare.h @@ -18,5 +18,7 @@ #define QEMU_COLO_COMPARE_H void colo_notify_compares_event(void *opaque, int event, Error **errp); +void colo_compare_register_notifier(Notifier *notify); +void colo_compare_unregister_notifier(Notifier *notify); #endif /* QEMU_COLO_COMPARE_H */
Re: [Qemu-devel] [PATCH v4 00/19] reverse debugging
Ping? Отправлено с помощью BlueMail На 28 Май 2018 г., 10:13, в 10:13, Pavel Dovgalyuk написал:п>GDB remote protocol supports reverse debugging of the targets. >It includes 'reverse step' and 'reverse continue' operations. >The first one finds the previous step of the execution, >and the second one is intended to stop at the last breakpoint that >would happen when the program is executed normally. > >Reverse debugging is possible in the replay mode, when at least >one snapshot was created at the record or replay phase. >QEMU can use these snapshots for travelling back in time with GDB. > >Running the execution in replay mode allows using GDB reverse debugging >commands: > - reverse-stepi (or rsi): Steps one instruction to the past. > QEMU loads on of the prior snapshots and proceeds to the desired > instruction forward. When that step is reaches, execution stops. > - reverse-continue (or rc): Runs execution "backwards". > QEMU tries to find breakpoint or watchpoint by loaded prior snapshot > and replaying the execution. Then QEMU loads snapshots again and > replays to the latest breakpoint. When there are no breakpoints in > the examined section of the execution, QEMU finds one more snapshot > and tries again. After the first snapshot is processed, execution > stops at this snapshot. > >The set of patches include the following modifications: > - gdbstub update for reverse debugging support > - functions that automatically perform reverse step and reverse > continue operations > - hmp/qmp commands for manipulating the replay process > - improvement of the snapshotting for saving the execution step > in the snapshot parameters > - other record/replay fixes > >The patches are available in the repository: >https://github.com/ispras/qemu/tree/rr-180524 > >v4 changes: >- changed 'since 2.13' to 'since 3.0' in json (as suggested by Eric >Blake) > >v3 changes: >- Fixed PS/2 bug with save/load vm, which caused failures of the >replay. > The patch was sent separately. > - Rebased to the new code base. > - Minor fixes. > >v2 changes: > - documented reverse debugging > - fixed start vmstate loading in record mode > - documented qcow2 changes (as suggested by Eric Blake) > - made icount SnapshotInfo field optional (as suggested by Eric Blake) > - renamed qmp commands (as suggested by Eric Blake) > - minor changes > >--- > >Pavel Dovgalyuk (19): > block: implement bdrv_snapshot_goto for blkreplay > replay: disable default snapshot for record/replay > replay: update docs for record/replay with block devices > replay: don't drain/flush bdrv queue while RR is working > replay: finish record/replay before closing the disks > qcow2: introduce icount field for snapshots > migration: introduce icount field for snapshots > replay: introduce info hmp/qmp command > replay: introduce breakpoint at the specified step > replay: implement replay-seek command to proceed to the desired step > replay: flush events when exiting > timer: remove replay clock probe in deadline calculation > replay: refine replay-time module > translator: fix breakpoint processing > replay: flush rr queue before loading the vmstate > gdbstub: add reverse step support in replay mode > gdbstub: add reverse continue support in replay mode > replay: describe reverse debugging in docs/replay.txt > replay: allow loading any snapshots before recording > > > accel/tcg/translator.c|9 + > block/blkreplay.c |8 + > block/io.c| 22 +++ > block/qapi.c | 17 ++- > block/qcow2-snapshot.c|9 + > block/qcow2.h |2 > blockdev.c| 10 ++ > cpus.c| 19 ++- > docs/interop/qcow2.txt|4 + > docs/replay.txt | 45 +++ > exec.c|6 + > gdbstub.c | 50 +++- > hmp-commands-info.hx | 14 ++ > hmp-commands.hx | 30 + > hmp.h |3 > include/block/snapshot.h |1 > include/sysemu/replay.h | 18 +++ > migration/savevm.c| 15 +- > qapi/block-core.json |5 + > qapi/block.json |3 > qapi/misc.json| 68 +++ > replay/Makefile.objs |3 >replay/replay-debugging.c | 287 >+ > replay/replay-events.c| 14 -- > replay/replay-internal.h | 10 +- > replay/replay-snapshot.c | 17 ++- > replay/replay-time.c | 27 ++-- > replay/replay.c | 22 +++ > stubs/replay.c| 10 ++ > util/qemu-timer.c | 11 -- > vl.c | 18 ++- > 31 files changed, 696 insertions(+), 81 deletions(-) > create mode 100644 replay/replay-debugging.c > >-- >Pavel Dovgalyuk
Re: [Qemu-devel] [PATCH V8 14/17] filter: Add handle_event method for NetFilterClass
On 2018年06月03日 13:05, Zhang Chen wrote: Filter needs to process the event of checkpoint/failover or other event passed by COLO frame. Signed-off-by: zhanghailiang --- include/net/filter.h | 5 + net/filter.c | 17 + net/net.c| 28 3 files changed, 50 insertions(+) diff --git a/include/net/filter.h b/include/net/filter.h index 435acd6f82..49da666ac0 100644 --- a/include/net/filter.h +++ b/include/net/filter.h @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc, typedef void (FilterStatusChanged) (NetFilterState *nf, Error **errp); +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error **errp); + typedef struct NetFilterClass { ObjectClass parent_class; @@ -45,6 +47,7 @@ typedef struct NetFilterClass { FilterSetup *setup; FilterCleanup *cleanup; FilterStatusChanged *status_changed; +FilterHandleEvent *handle_event; /* mandatory */ FilterReceiveIOV *receive_iov; } NetFilterClass; @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, int iovcnt, void *opaque); +void colo_notify_filters_event(int event, Error **errp); + #endif /* QEMU_NET_FILTER_H */ diff --git a/net/filter.c b/net/filter.c index 2fd7d7d663..0f17eba143 100644 --- a/net/filter.c +++ b/net/filter.c @@ -17,6 +17,8 @@ #include "net/vhost_net.h" #include "qom/object_interfaces.h" #include "qemu/iov.h" +#include "net/colo.h" +#include "migration/colo.h" static inline bool qemu_can_skip_netfilter(NetFilterState *nf) { @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj) g_free(nf->netdev_id); } +static void dummy_handle_event(NetFilterState *nf, int event, Error **errp) +{ +switch (event) { +case COLO_EVENT_CHECKPOINT: +break; +case COLO_EVENT_FAILOVER: +object_property_set_str(OBJECT(nf), "off", "status", errp); +break; +default: +break; +} +} + static void netfilter_class_init(ObjectClass *oc, void *data) { UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); +NetFilterClass *nfc = NETFILTER_CLASS(oc); ucc->complete = netfilter_complete; +nfc->handle_event = dummy_handle_event; } static const TypeInfo netfilter_info = { diff --git a/net/net.c b/net/net.c index efb9eaf779..378cd2f9ec 100644 --- a/net/net.c +++ b/net/net.c @@ -1329,6 +1329,34 @@ void hmp_info_network(Monitor *mon, const QDict *qdict) } } +void colo_notify_filters_event(int event, Error **errp) +{ +NetClientState *nc, *peer; +NetClientDriver type; +NetFilterState *nf; +NetFilterClass *nfc = NULL; +Error *local_err = NULL; + +QTAILQ_FOREACH(nc, &net_clients, next) { +peer = nc->peer; +type = nc->info->type; +if (!peer || type != NET_CLIENT_DRIVER_TAP) { +continue; I think when COLO is enabled, qemu should reject all other types of backends. +} +QTAILQ_FOREACH(nf, &nc->filters, next) { +nfc = NETFILTER_GET_CLASS(OBJECT(nf)); +if (!nfc->handle_event) { +continue; +} Is this still necessary? Thanks +nfc->handle_event(nf, event, &local_err); +if (local_err) { +error_propagate(errp, local_err); +return; +} +} +} +} + void qmp_set_link(const char *name, bool up, Error **errp) { NetClientState *ncs[MAX_QUEUE_NUM];