Re: [Qemu-devel] [PATCH RFC 0/2] Fix migration issues
On 10/26/2018 11:24 PM, Dr. David Alan Gilbert wrote: * Peter Xu (pet...@redhat.com) wrote: On Fri, Oct 26, 2018 at 09:10:19PM +0800, Fei Li wrote: On 10/25/2018 08:58 PM, Peter Xu wrote: On Thu, Oct 25, 2018 at 05:04:00PM +0800, Fei Li wrote: [...] @@ -1325,22 +1325,24 @@ bool multifd_recv_all_channels_created(void) /* Return true if multifd is ready for the migration, otherwise false */ bool multifd_recv_new_channel(QIOChannel *ioc) { + MigrationIncomingState *mis = migration_incoming_get_current(); MultiFDRecvParams *p; Error *local_err = NULL; int id; id = multifd_recv_initial_packet(ioc, &local_err); if (id < 0) { - multifd_recv_terminate_threads(local_err); - return false; + error_reportf_err(local_err, + "failed to receive packet via multifd channel %x: ", + multifd_recv_state->count); + goto fail; } p = &multifd_recv_state->params[id]; if (p->c != NULL) { error_setg(&local_err, "multifd: received id '%d' already setup'", id); - multifd_recv_terminate_threads(local_err); - return false; + goto fail; } p->c = ioc; object_ref(OBJECT(ioc)); @@ -1352,6 +1354,11 @@ bool multifd_recv_new_channel(QIOChannel *ioc) QEMU_THREAD_JOINABLE); atomic_inc(&multifd_recv_state->count); return multifd_recv_state->count == migrate_multifd_channels(); +fail: + multifd_recv_terminate_threads(local_err); + qemu_fclose(mis->from_src_file); + mis->from_src_file = NULL; + exit(EXIT_FAILURE); } Yeah I think it makes sense to at least report some details when error happens, but I'm not sure whether it's good to explicitly exit() here. IMHO you can add an Error** in multifd_recv_new_channel() parameter list to do that, and even through migration_ioc_process_incoming(). What do you think? Regards, You mean exit() in migration_ioc_process_incoming(), or further caller migration_channel_process_incoming()? Actually either is ok for me. :) But today I find if using postcopy and multifd together to do live migration, it seems the hang still occurs even with the above codes, so sad about that. I will keep debugging and see how to fix this. Maybe you can move the error_report_err() in migration_channel_process_incoming() out of the TLS path so we can report the error if either TLS or non-TLS case got something wrong. Thanks for the advice. I will do the update in the next version. :) And I don't even know whether multifd could work with postcopy... Nope, it's not expected to work yet. Dave Thanks for the helpful information. :) BTW, in the next version, I'd like to merge these three migration patches into the "[PATCH RFC v6 ] qemu_thread_create: propagate the error to callers to handle", and cc you inside the patches. Please help to review. Have a nice day, thanks again Fei Regards, -- Peter Xu -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [RFC PATCH 00/21] Trace updates and plugin RFC
> From: Alex Bennée [mailto:alex.ben...@linaro.org] > Pavel Dovgalyuk writes: > > >> From: Alex Bennée [mailto:alex.ben...@linaro.org] > >> Pavel Dovgalyuk writes: > >> > >> >> From: Alex Bennée [mailto:alex.ben...@linaro.org] > >> >> Any serious analysis tool should allow for us to track all memory > >> >> accesses so I think the guest_mem_before trace point should probably > >> >> be split into guest_mem_before_store and guest_mem_after_load. We > >> >> could go the whole hog and add potential trace points for start/end of > >> >> all memory operations. > >> > > >> > I wanted to ask about memory tracing and found this one. > >> > Is it possible to use tracepoints for capturing all memory accesses? > >> > In our implementation we insert helpers before and after tcg > >> > read/write operations. > >> > >> The current tracepoint isn't enough but yes I think we could. The first > >> thing I need to do is de-macrofy the atomic helpers a little just to > >> make it a bit simpler to add the before/after tracepoints. > > > > But memory accesses can use 'fast path' without the helpers. > > Thus you still need inserting the new helper for that case. > > trace_guest_mem_before_tcg in tcg-op.c already does this but currently > only before operations. That's why I want to split it and pass the > load/store value register values into the helpers. One more question about your trace points. In case of using trace point on every instruction execution, we may need accessing vCPU registers (including the flags). Are they valid in such cases? I'm asking, because at least i386 translation optimizes writebacks. Pavel Dovgalyuk
[Qemu-devel] KVM Forum 2018 BoF Meeting minutes - architecture common code
Participants: Christopher Dall, Marc Zyngier, Paolo Bonzini, Suraj Jitindar Singh, Frediano Ziglio, Sean Christopherson, Radim Krcmar, Janosch Frank, Christian Borntraeger +some more - Discussion about common code for different architectures - initiated by my talk https://schd.ws/hosted_files/kvmforum2018/a0/Christian-Borntraegercrossarch.pdf - We agreed that we as architecture maintainers should teach people during review about making things generic when applicable - Problem: we do not understand "the other side" at all, e.g. one example in x86 code was: does spte means shadow page table or also nested pages (EPT) - maybe we need a hack-a-thon to sit together and talk about this - other idea: To see if we can factor out things, just provide a header file with "needed" interfaces and the pass that along to the other side Things to look at: - dirty log, now 2 common code variante (and arch specific ones) - ordering of ioctl (e.g. qemu vs native kvm tool) triggered sometimes bugs on ARM - can we test this better and maybe also refactor some of the common checks - api tests should be extended to all architectures - rmap code for nested virtualization -> x86, power and now arm. All private copies (s390 uses radix to make it a 4th variant) - can we avoid compiling virt/kvm/ from arch folders? - Christopher also wants to look at nested page table faults (stage 2 faults on Arm), where on the Arm side we have a lot of code which is not arch-specific, but related to finding a page to back the fault, figuring out if the page is hugetlbfs, thp, page size, permission of page, and so on. This could expand to functionality for shadow page tables which we'll use for nested virt on Arm as well. Things such as managing caches of shadow page tables for certain guest contexts, handling emulated TLB invalidations, etc. should have some commonality at least between x86 and Arm. we agreed to have a meeting of arch maintainers either before or after next KVM forum or independently. This will be discussed separately. Christian Borntraeger will kick of that discussion.
Re: [Qemu-devel] [PATCH] hw/pvrdma: Check the correct return value
On 10/25/18 9:17 AM, Yuval Shaia wrote: Return value of 0 means ok, we want to free the memory only in case of error. Signed-off-by: Yuval Shaia --- hw/rdma/vmw/pvrdma_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c index 4faeb21631..57d6f41ae6 100644 --- a/hw/rdma/vmw/pvrdma_cmd.c +++ b/hw/rdma/vmw/pvrdma_cmd.c @@ -232,7 +232,7 @@ static int create_mr(PVRDMADev *dev, union pvrdma_cmd_req *req, cmd->start, cmd->length, host_virt, cmd->access_flags, &resp->mr_handle, &resp->lkey, &resp->rkey); -if (host_virt && !resp->hdr.err) { +if (resp->hdr.err && host_virt) { munmap(host_virt, cmd->length); } Reviewed-by: Marcel Apfelbaum Thanks, Marcel
Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
Marc-André Lureau writes: > Hi > > On Tue, Oct 9, 2018 at 8:41 PM Markus Armbruster wrote: >> >> Markus Armbruster writes: >> >> > Thomas Huth writes: >> > >> >> On 2018-08-31 15:24, Marc-André Lureau wrote: [...] >> >>> Tbh, I don't care so much about the naming of the test, but (for once) >> >>> I respectfully don't think Markus suggestion is better. >> >>> >> >>> The function checks "qom-set" without 'value' argument: >> >>> "qom-set-without-value", no brainer.. >> > >> > Nope, that's not what it tests. It tests the visitor, the marshalling >> > code generator, and the QMP core handle a certain kind of invalid >> > arguments correctly. It does not test qom-set. I explained all that >> > already. >> > >> >>> Naming it "invalid-arg" is so generic that I wouldn't be able what it >> >>> does. >> > >> > I can accept "missing-any" or "missing-any-arg". I object to any name >> > involving qom-set, because the test is not about qom-set at all. >> > >> > If it was, then putting it in qmp-test.c would be wrong. >> > >> >> Ok, then let's keep it this way. As I said, IMHO the current naming is >> >> not really bad, and I also don't have any suggestions for a perfect name >> >> right now. >> > >> > We don't need a perfect name. We need one that's not actively >> > misleading. >> >> Marc-André, would "qmp/missing-any-arg" and test_missing_any_arg() work >> for you? > > Yes, do you want me to update the patch? Yes, please.
[Qemu-devel] [PATCH v2 0/1] Add PKU on Skylake-Server CPU model
This patch adds PKU on Skylake-Server CPU model. Changelog: v2 Remove OSPKE which is not needed. Add Skylake-Server-*-cpu.pku=off entries on PC_COMPAT_3_0 to keep PKU disabled on pc-*-3.0 and older. Tao Xu (1): i386: Add PKU on Skylake-Server CPU model include/hw/i386/pc.h | 10 +- target/i386/cpu.c| 4 2 files changed, 13 insertions(+), 1 deletion(-) -- 2.17.1
[Qemu-devel] [PATCH v2 1/1] i386: Add PKU on Skylake-Server CPU model
As the release document ref below link (page 13): https://software.intel.com/sites/default/files/managed/c5/15/\ architecture-instruction-set-extensions-programming-reference.pdf PKU is supported in Skylake Server (Only Server) and later, and on Intel(R) Xeon(R) Processor Scalable Family. So PKU is supposed to be in Skylake-Server CPU model. And PKU's CPUID has been exposed to QEMU. But PKU can't be find in Skylake-Server CPU model in the code. So this patch will fix this issue in Skylake-Server CPU model. Signed-off-by: Tao Xu --- include/hw/i386/pc.h | 10 +- target/i386/cpu.c| 4 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index dfe6746692..136fe497b6 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -300,7 +300,15 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); .driver = TYPE_X86_CPU,\ .property = "x-hv-synic-kvm-only",\ .value= "on",\ -} +},{\ +.driver = "Skylake-Server" "-" TYPE_X86_CPU,\ +.property = "pku",\ +.value= "off",\ +},{\ +.driver = "Skylake-Server-IBRS" "-" TYPE_X86_CPU,\ +.property = "pku",\ +.value= "off",\ +}, #define PC_COMPAT_2_12 \ HW_COMPAT_2_12 \ diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 1469a1be01..6fb7bee0f2 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -2322,6 +2322,8 @@ static X86CPUDefinition builtin_x86_defs[] = { CPUID_7_0_EBX_AVX512F | CPUID_7_0_EBX_AVX512DQ | CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512CD | CPUID_7_0_EBX_AVX512VL | CPUID_7_0_EBX_CLFLUSHOPT, +.features[FEAT_7_0_ECX] = +CPUID_7_0_ECX_PKU, /* Missing: XSAVES (not supported by some Linux versions, * including v4.1 to v4.12). * KVM doesn't yet expose any XSAVES state save component, @@ -2372,6 +2374,8 @@ static X86CPUDefinition builtin_x86_defs[] = { CPUID_7_0_EBX_AVX512F | CPUID_7_0_EBX_AVX512DQ | CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512CD | CPUID_7_0_EBX_AVX512VL, +.features[FEAT_7_0_ECX] = +CPUID_7_0_ECX_PKU, /* Missing: XSAVES (not supported by some Linux versions, * including v4.1 to v4.12). * KVM doesn't yet expose any XSAVES state save component, -- 2.17.1
Re: [Qemu-devel] [PATCH 0/3] Mark non-volatile memory regions
Hi On Wed, Oct 3, 2018 at 3:46 PM Marc-André Lureau wrote: > > Hi, > > As discussed in the "[PATCH v10 6/6] tpm: add ACPI memory clear > interface" thread, and proposed in the "[PATCH] RFC: mark non-volatile > memory region", here is a small series to mark non-volvatile memory > regions and skip them on dump. Upcoming TCG "Platform Reset Attack > Mitigation" will also use this information to eventually skip > non-volatile memory. > > Thanks > > Marc-André Lureau (3): > memory: learn about non-volatile memory region > nvdimm: set non-volatile on the memory region > memory-mapping: skip non-volatile memory regions in GuestPhysBlockList Only last patch got reviews, but that means the first are probably ok as well. Is anybody reviewing & picking the series? thanks > > include/exec/memory.h| 25 ++ > hw/mem/nvdimm.c | 1 + > memory.c | 45 +++- > memory_mapping.c | 3 ++- > docs/devel/migration.rst | 1 + > 5 files changed, 64 insertions(+), 11 deletions(-) > > -- > 2.19.0.271.gfe8321ec05 > > -- Marc-André Lureau
Re: [Qemu-devel] [PATCH 2/3] adlib: mark as insecure and deprecated.
On Fri, Oct 26, 2018 at 05:23:37PM +0530, P J P wrote: > +-- On Fri, 26 Oct 2018, Paolo Bonzini wrote --+ > | Oh, thanks! I said I was dumb. :) So the fix is just this: > | > | diff --git a/hw/audio/fmopl.h b/hw/audio/fmopl.h > | index e7e578a48e..7199afaa3c 100644 > | --- a/hw/audio/fmopl.h > | +++ b/hw/audio/fmopl.h > | @@ -72,8 +72,8 @@ typedef struct fm_opl_f { > | /* Rhythm sention */ > | uint8_t rhythm; /* Rhythm mode , key flag */ > | /* time tables */ > | - int32_t AR_TABLE[75]; /* atttack rate tables */ > | - int32_t DR_TABLE[75]; /* decay rate tables */ > | + int32_t AR_TABLE[76]; /* atttack rate tables */ > | + int32_t DR_TABLE[76]; /* decay rate tables */ > | uint32_t FN_TABLE[1024]; /* fnumber -> increment counter */ > | /* LFO */ > | int32_t *ams_table; > | > | and init_timetables will just fill it with the right value? (I checked > | against another implementation at http://opl3.cozendey.com/). > > Gerd has proposed to a patch to deprecate adlib, as it's not used as much. > IMO > deprecation is better option. But if that is not happening, above seems good. I think we can actually do both. cheers, Gerd
Re: [Qemu-devel] [PATCH 3/3] cirrus: mark as deprecated
On Thu, Oct 25, 2018 at 09:37:58PM +0100, Daniel P. Berrangé wrote: > On Thu, Oct 25, 2018 at 10:52:56AM +0200, Gerd Hoffmann wrote: > > While being at it deprecate cirrus too. > > > > Reason (short version): use stdvga instead. > > Verbose version: > > https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful > > Every single one of my guests is using cirrus. This wasn't an explicit > choice on my part, so I believe it is being used as the default in > virt-install. > > I don't debate the points in the blog post above that stdvga is a > better choice, but I don't think that's enough to justify deprecating > cirrus at this point in time, because when it then gets deleted it > will break way too many existing deployments. > > We need to socialize info in that blog post above more widely and > especially ensure that apps are not using that by default. I don't > see it being viable to formally deprecate it in QEMU any time soon > though given existing usage. Well, getting that message to the users is the point of deprecating it. I think in this specific case we'll need a longer (maybe much longer) deprecation period than only two releases. But I think it still makes sense to deprecate it now. In qemu it isn't the default any more since release 2.2. libvirt switched the default not too long after that. Not fully sure how things look like higher up the stack. cheers, Gerd
Re: [Qemu-devel] [PATCH 1/3] qdev: add deprecation_reason to DeviceClass
On Thu, Oct 25, 2018 at 01:12:15PM +0200, Philippe Mathieu-Daudé wrote: > Hi Gerd, > > On 25/10/18 10:52, Gerd Hoffmann wrote: > > Simliar to deprecated machine types. > > "Similar" > > > Print a warning when creating a deprecated device. > > Add deprecation notice to -device help. > > > > TODO: add to intospection. > > "introspection" > > Do we want the TODO in the git history? No. It's RfC because of the missing introspection bits ;) cheers, Gerd
Re: [Qemu-devel] [PATCH] SDL: set a hint to not bypass the window compositor
On Wed, Oct 24, 2018 at 04:37:48PM +0200, Sebastian Krzyszkowiak wrote: > Without that, window effects in KWin get suspended as soon as any > qemu-sdl window becomes visible. While the SDL default makes sense > for games, it's not really suitable for QEMU. > > Signed-off-by: Sebastian Krzyszkowiak Added to UI patch queue. thanks, Gerd
Re: [Qemu-devel] [PATCH] usb: ohci: make num_ports to an unsinged integer
On Mon, Oct 22, 2018 at 08:00:18PM -0700, Li Qiang wrote: > This can avoid setting OCHIState.num_ports to a negative num. Added to usb queue. thanks, Gerd
Re: [Qemu-devel] [PATCH] vga_int: remove unused function protype
On Mon, Oct 22, 2018 at 04:00:53PM +0800, yuchen...@synology.com wrote: > From: yuchenlin > > Signed-off-by: yuchenlin > --- > hw/display/vga_int.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h > index 6e4fa48a79..55c418eab5 100644 > --- a/hw/display/vga_int.h > +++ b/hw/display/vga_int.h > @@ -166,7 +166,6 @@ MemoryRegion *vga_init_io(VGACommonState *s, Object *obj, >const MemoryRegionPortio **vbe_ports); > void vga_common_reset(VGACommonState *s); > > -void vga_sync_dirty_bitmap(VGACommonState *s); > void vga_dirty_log_start(VGACommonState *s); > void vga_dirty_log_stop(VGACommonState *s); Added to vga queue. thanks, Gerd
Re: [Qemu-devel] [PATCH 0/2] hw: ccid-card-emulated: fix some resources leak
On Fri, Oct 19, 2018 at 03:50:34AM -0700, Li Qiang wrote: > > Li Qiang (2): > hw: ccid-card-emulated: introduce clean_event_notifier > hw: ccid-card-emulated: cleanup resource when realize in error path Added to usb queue. thanks, Gerd
Re: [Qemu-devel] [RFC 00/48] Plugin support
> From: Emilio G. Cota [mailto:c...@braap.org] > - 2-pass translation. Once a "TB translation" callback is called, > the plugin must know the span of the TB. We should not > force plugins to guess where the TB will end; that is strictly > QEMU's job, and can change any time. A TB is thus a sequence > of instructions of whatever length the particular QEMU > implementation decides. Thus, for each TB, a 3-step process > is followed: (1) the plugin layer keeps a copy of the contents > of the current TB, (2) once the TB is well-defined, its > descriptor and contents are passed to plugins, which then > register their desired instrumentation (e.g. "call me back > on this particular instruction", or "call me back when > the whole TB executes"); note that plugins can use a disassembler > like capstone to decide what to do with each instruction; they > can also allocate memory and then get a pointer to it passed > back from the callbacks. And finally, (3) the target translator > is called again to generate the final instrumented translated TB. > This is what I called the "2-pass translation", since we go > twice over the translation loop in translator.c. Note that the > 2-pass approach has virtually no overhead (0.40% for SPEC06int); > translation is much cheaper than execution. But anyway, if no > plugins have subscribed to TB translation, we only do one pass. Can plugin affect the translation somehow to force flushing cached registers? E.g. callback may need correct EFLAGS which usually does not updated until the end of the block. > - Support for inlining instrumentation. This is done via an > explicit API, i.e. we do not export TCG ops, which are internal > to QEMU. For now, I just have support for incrementing a u64 > with an immediate, e.g. to increment a counter. It means that we'll have "yet another one TCG"? Pavel Dovgalyuk
Re: [Qemu-devel] [PATCH 3/3] memory-mapping: skip non-volatile memory regions in GuestPhysBlockList
On 03/10/2018 13:44, Marc-André Lureau wrote: > diff --git a/memory_mapping.c b/memory_mapping.c > index 775466f3a8..724dd0b417 100644 > --- a/memory_mapping.c > +++ b/memory_mapping.c > @@ -206,7 +206,8 @@ static void guest_phys_blocks_region_add(MemoryListener > *listener, > > /* we only care about RAM */ > if (!memory_region_is_ram(section->mr) || > -memory_region_is_ram_device(section->mr)) { > +memory_region_is_ram_device(section->mr) || > +memory_region_is_nonvolatile(section->mr)) { > return; > } > We should also have diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py index 5a857cebcf..dd180b531c 100644 --- a/scripts/dump-guest-memory.py +++ b/scripts/dump-guest-memory.py @@ -417,7 +417,9 @@ def get_guest_phys_blocks(): memory_region = flat_range["mr"].dereference() # we only care about RAM -if not memory_region["ram"]: +if not memory_region["ram"] \ + or memory_region["ram_device"] \ + or memory_region["nonvolatile"]: continue section_size = int128_get64(flat_range["addr"]["size"]) here. I queued the patches and will post this soon as a separate patch. Paolo
Re: [Qemu-devel] [PATCH 1/9] qom/user-creatable: add a few helper macros
On Fri, 26 Oct 2018 12:13:21 -0300 Eduardo Habkost wrote: > On Mon, Oct 22, 2018 at 03:33:30PM +0100, Igor Mammedov wrote: > > On Wed, 12 Sep 2018 16:55:23 +0400 > > Marc-André Lureau wrote: > > > > > Improve a bit code readability. > > > > > > Signed-off-by: Marc-André Lureau > > > --- > > > include/qom/object_interfaces.h | 4 > > > qom/object.c| 4 ++-- > > > qom/object_interfaces.c | 9 +++-- > > > 3 files changed, 9 insertions(+), 8 deletions(-) > > > > > > diff --git a/include/qom/object_interfaces.h > > > b/include/qom/object_interfaces.h > > > index 4d513fb329..46b0861457 100644 > > > --- a/include/qom/object_interfaces.h > > > +++ b/include/qom/object_interfaces.h > > > @@ -9,9 +9,13 @@ > > > #define USER_CREATABLE_CLASS(klass) \ > > > OBJECT_CLASS_CHECK(UserCreatableClass, (klass), \ > > > TYPE_USER_CREATABLE) > > > +#define IS_USER_CREATABLE_CLASS(klass) \ > > > +object_class_dynamic_cast(OBJECT_CLASS(oc), TYPE_USER_CREATABLE) > > > #define USER_CREATABLE_GET_CLASS(obj) \ > > > OBJECT_GET_CLASS(UserCreatableClass, (obj), \ > > >TYPE_USER_CREATABLE) > > > +#define IS_USER_CREATABLE(obj) \ > > > +object_dynamic_cast(OBJECT(obj), TYPE_USER_CREATABLE) > > > #define USER_CREATABLE(obj) \ > > > INTERFACE_CHECK(UserCreatable, (obj), \ > > > TYPE_USER_CREATABLE) > > > diff --git a/qom/object.c b/qom/object.c > > > index 75d1d48944..0703e8e4ff 100644 > > > --- a/qom/object.c > > > +++ b/qom/object.c > > > @@ -424,7 +424,7 @@ void object_initialize_childv(Object *parentobj, > > > const char *propname, > > > goto out; > > > } > > > > > > -if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) { > > > +if (IS_USER_CREATABLE(obj)) { > > > user_creatable_complete(obj, &local_err); > > > if (local_err) { > > > object_unparent(obj); > > > @@ -605,7 +605,7 @@ Object *object_new_with_propv(const char *typename, > > > goto error; > > > } > > > > > > -if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) { > > > +if (IS_USER_CREATABLE(obj)) { > > > user_creatable_complete(obj, &local_err); > > > if (local_err) { > > > object_unparent(obj); > > > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c > > > index 72b97a8bed..e3084bc04a 100644 > > > --- a/qom/object_interfaces.c > > > +++ b/qom/object_interfaces.c > > > @@ -10,18 +10,15 @@ > > > > > > void user_creatable_complete(Object *obj, Error **errp) > > > { > > > - > > > UserCreatableClass *ucc; > > > -UserCreatable *uc = > > > -(UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE); > > > > > > -if (!uc) { > > > +if (!IS_USER_CREATABLE(obj)) { > > > return; > > > } > > > > > > -ucc = USER_CREATABLE_GET_CLASS(uc); > > > +ucc = USER_CREATABLE_GET_CLASS(obj); > > > if (ucc->complete) { > > > -ucc->complete(uc, errp); > > > +ucc->complete(USER_CREATABLE(obj), errp); > > ^^^ > > even though function becomes more concise, > > this will call expensive dynamic cast 2nd time (IS_USER_CREATABLE was the > > 1st and discarded) > > so I'm not sure is a good idea to regress startup time for readability. > > (INTERFACE_CHECK is a nop if CONFIG_QOM_CAST_DEBUG is not > enabled, so I don't understand how it would regress startup time. Isn't it enabled by default though? Maybe we should flip default to disabled then cast should be ok and enable it when generic '--debug' is enabled.
Re: [Qemu-devel] [PATCH] vga_int: remove unused function protype
On 2018-10-29 17:44, Gerd Hoffmann wrote: On Mon, Oct 22, 2018 at 04:00:53PM +0800, yuchen...@synology.com wrote: From: yuchenlin Signed-off-by: yuchenlin --- hw/display/vga_int.h | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 6e4fa48a79..55c418eab5 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -166,7 +166,6 @@ MemoryRegion *vga_init_io(VGACommonState *s, Object *obj, const MemoryRegionPortio **vbe_ports); void vga_common_reset(VGACommonState *s); -void vga_sync_dirty_bitmap(VGACommonState *s); void vga_dirty_log_start(VGACommonState *s); void vga_dirty_log_stop(VGACommonState *s); Added to vga queue. thanks, Gerd Hi, Gerd Laurent has sent a pull request for this trivial commit. See: http://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05896.html Thanks, yuchenlin
[Qemu-devel] [PATCH] migration: fix concurrent call of multifd_save_cleanup
Concurrent call of multifd_save_cleanup() is unsafe, it will lead to null pointer dereference. 'multifd_save_cleanup()' should not be called in multifd_new_send_channel_async(), move it to ram_save_cleanup() like other features do. Signed-off-by: Liang Li --- migration/migration.c | 5 - migration/ram.c | 7 +++ migration/ram.h | 2 +- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 8b36e7f..f422218 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1372,7 +1372,6 @@ static void migrate_fd_cleanup(void *opaque) qemu_savevm_state_cleanup(); if (s->to_dst_file) { -Error *local_err = NULL; QEMUFile *tmp; trace_migrate_fd_cleanup(); @@ -1382,10 +1381,6 @@ static void migrate_fd_cleanup(void *opaque) s->migration_thread_running = false; } qemu_mutex_lock_iothread(); - -if (multifd_save_cleanup(&local_err) != 0) { -error_report_err(local_err); -} qemu_mutex_lock(&s->qemu_file_lock); tmp = s->to_dst_file; s->to_dst_file = NULL; diff --git a/migration/ram.c b/migration/ram.c index 7e7deec..a232b9c 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -917,7 +917,7 @@ static void multifd_send_terminate_threads(Error *err) } } -int multifd_save_cleanup(Error **errp) +int multifd_save_cleanup(void) { int i; int ret = 0; @@ -1071,9 +1071,7 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque) Error *local_err = NULL; if (qio_task_propagate_error(task, &local_err)) { -if (multifd_save_cleanup(&local_err) != 0) { -migrate_set_error(migrate_get_current(), local_err); -} +migrate_set_error(migrate_get_current(), local_err); } else { p->c = QIO_CHANNEL(sioc); qio_channel_set_delay(p->c, false); @@ -2542,6 +2540,7 @@ static void ram_save_cleanup(void *opaque) xbzrle_cleanup(); compress_threads_save_cleanup(); +multifd_save_cleanup(); ram_state_cleanup(rsp); } diff --git a/migration/ram.h b/migration/ram.h index 83ff1bc..c4fafea 100644 --- a/migration/ram.h +++ b/migration/ram.h @@ -43,7 +43,7 @@ uint64_t ram_bytes_remaining(void); uint64_t ram_bytes_total(void); int multifd_save_setup(void); -int multifd_save_cleanup(Error **errp); +int multifd_save_cleanup(void); int multifd_load_setup(void); int multifd_load_cleanup(Error **errp); bool multifd_recv_all_channels_created(void); -- 1.8.3.1
Re: [Qemu-devel] [PATCH v7 04/20] target/mips: Add and integrate MXU decoding engine placeholder
In that case, I guess this should be OK for now, as MXU support is initiated by Craig and this will be an easy add-on when he provide necessary information. Reviewed-by: Stefan Markovic On 28.10.18. 19:39, Aleksandar Markovic wrote: >> Subject: Re: [PATCH v7 04/20] target/mips: Add and integrate MXU decoding >> engine > placeholder >> >>> Is the best way to implement this to include processing of MUL, CLZ, >>> CLO, SDBBP instructions into decode_opc_mxu as their encodings aren't >>> overlaid by MXU instructions considering MIPS SPECIAL2 instruction >>> pool and MXU Instruction Set? >> The problem is that we don't have the documentation for Ingenic's base >> instruction set. My understanding is that Craig established necessity of >> including non-MXU MUL into decode_opc_mxu() by experimentation, >> or by looking at Ingenic's toolchain source code. >> >> Note that CLZ, CLO, SDBBP are moved from SPECIAL2 to another >> place in opcode space in MIPS R6. >> >> Craig, can you offer any insight on CLZ, CLO, SDBBP in Ingenic's base >> instruction set? They are in SPECIAL2 opcode space for MIPS pre-R6. >> >> Worse come to worst, I recommend adding "TODO" comment to an >> appropriate place in decode_opc_mxu(), and go forward without handling >> CLZ, CLO, SDBBP - given that all changes in this series are just the first >> phase of implementing MXU support - they won't affect any production >> code at this moment. >> > I think this comment should be added to the decode_opc_mxu(), within patch 11: > > /* > * TODO: Investigate necessity of including handling of > * CLZ, CLO, SDBB in this function, as they belong to > * SPECIAL2 opcode space for regular pre-R6 MIPS ISAs. > */ > > Thanks, > Aleksandar
Re: [Qemu-devel] [PATCH v7 13/20] target/mips: Move MUL, S32M2I, S32I2M handling out of main MXU switch
Following the patch 04/20 discussion: Reviewed-by: Stefan Markovic On 26.10.18. 11:45, Stefan Markovic wrote: > > On 24.10.18. 14:18, Aleksandar Markovic wrote: >> From: Aleksandar Markovic >> >> Move MUL, S32M2I, S32I2M handling out of switch. These are all >> instructions that do not depend on MXU_EN flag of MXU_CR. >> >> Signed-off-by: Aleksandar Markovic >> --- >> target/mips/translate.c | 41 +++-- >> 1 file changed, 23 insertions(+), 18 deletions(-) > > > See my comment for patch 04/20. > > CLZ, CLO, SDDP are missing? > > >> diff --git a/target/mips/translate.c b/target/mips/translate.c >> index c8c71c4..111affb 100644 >> --- a/target/mips/translate.c >> +++ b/target/mips/translate.c >> @@ -24859,6 +24859,29 @@ static void decode_opc_mxu(CPUMIPSState >> *env, DisasContext *ctx) >> { >> uint32_t opcode = extract32(ctx->opcode, 0, 6); >> + if (opcode == OPC__MXU_MUL) { >> + uint32_t rs, rt, rd, op1; >> + >> + rs = extract32(ctx->opcode, 21, 5); >> + rt = extract32(ctx->opcode, 16, 5); >> + rd = extract32(ctx->opcode, 11, 5); >> + op1 = MASK_SPECIAL2(ctx->opcode); >> + >> + gen_arith(ctx, op1, rd, rs, rt); >> + >> + return; >> + } >> + >> + if (opcode == OPC_MXU_S32M2I) { >> + gen_mxu_s32m2i(ctx); >> + return; >> + } >> + >> + if (opcode == OPC_MXU_S32I2M) { >> + gen_mxu_s32i2m(ctx); >> + return; >> + } >> + >> switch (opcode) { >> case OPC_MXU_S32MADD: >> /* TODO: Implement emulation of S32MADD instruction. */ >> @@ -24870,18 +24893,6 @@ static void decode_opc_mxu(CPUMIPSState >> *env, DisasContext *ctx) >> MIPS_INVAL("OPC_MXU_S32MADDU"); >> generate_exception_end(ctx, EXCP_RI); >> break; >> - case OPC__MXU_MUL: /* 0x2 - unused in MXU specs */ >> - { >> - uint32_t rs, rt, rd, op1; >> - >> - rs = extract32(ctx->opcode, 21, 5); >> - rt = extract32(ctx->opcode, 16, 5); >> - rd = extract32(ctx->opcode, 11, 5); >> - op1 = MASK_SPECIAL2(ctx->opcode); >> - >> - gen_arith(ctx, op1, rd, rs, rt); >> - } >> - break; >> case OPC_MXU__POOL00: >> decode_opc_mxu__pool00(env, ctx); >> break; >> @@ -25033,12 +25044,6 @@ static void decode_opc_mxu(CPUMIPSState >> *env, DisasContext *ctx) >> MIPS_INVAL("OPC_MXU_S16SDI"); >> generate_exception_end(ctx, EXCP_RI); >> break; >> - case OPC_MXU_S32M2I: >> - gen_mxu_s32m2i(ctx); >> - break; >> - case OPC_MXU_S32I2M: >> - gen_mxu_s32i2m(ctx); >> - break; >> case OPC_MXU_D32SLL: >> /* TODO: Implement emulation of D32SLL instruction. */ >> MIPS_INVAL("OPC_MXU_D32SLL");
Re: [Qemu-devel] [PULL 0/9] Testing patches
On 26 October 2018 at 15:15, Fam Zheng wrote: > The following changes since commit 808ebd66e467f77c0d1f8c6346235f81e9c99cf2: > > Merge remote-tracking branch 'remotes/riscv/tags/riscv-for-master-3.1-sf0' > into staging (2018-10-25 17:41:03 +0100) > > are available in the Git repository at: > > git://github.com/famz/qemu.git tags/testing-pull-request > > for you to fetch changes up to 63a24c5e2354833a84f18bdf0e857fad8812f65b: > > tests/vm: Do not abuse parallelism when HOST != TARGET architecture > (2018-10-26 22:03:21 +0800) > > > Testing patches > > One fix for mingw build and some improvements in VM based testing, many thanks > to Paolo and Phil. Applied, thanks. -- PMM
[Qemu-devel] could somebody who understands the block refcounting look at CID 1395870, CID 1395871?
Hi; could somebody who understands the block layer refcounting have a look at Coverity issues CID 1395870 and 1395871, please? In both cases, Coverity reports a use-after-free because it thinks that a sequence where a code path might (conditionally) end up calling blk_deref() twice could be freeing the memory in the first call and using it after. I'm not sure whether these are false positives because the refcounting has confused Coverity, or genuine issues where we have got refcounting logic wrong, so I don't know if we need a fix or if we should squash the coverity bug as a false-positive... thanks -- PMM
Re: [Qemu-devel] [PULL 2/3] target/mips: Implement emulation of nanoMIPS EVA instructions
On 25 October 2018 at 21:19, Aleksandar Markovic wrote: > From: Dimitrije Nikolic > > Implement emulation of nanoMIPS EVA instructions. They are all > part of P.LS.E0 instruction pool, or one of its subpools. > > Reviewed-by: Stefan Markovic > Signed-off-by: Dimitrije Nikolic > Signed-off-by: Aleksandar Markovic > -- Hi; Coverity points out (CID 1396475) that the switch cases for NM_LLWPE and NM_SCWPE fall through without either a 'break' statement or a '/* fall through */' comment: > +case NM_P_LLE: > +switch (extract32(ctx->opcode, 2, 2)) { > +case NM_LLE: > +check_xnp(ctx); > +check_eva(ctx); > +check_cp0_enabled(ctx); > +gen_ld(ctx, OPC_LLE, rt, rs, s); > +break; > +case NM_LLWPE: > +check_xnp(ctx); > +check_eva(ctx); > +check_cp0_enabled(ctx); > +gen_llwp(ctx, rs, 0, rt, extract32(ctx->opcode, 3, > 5)); > +default: > +generate_exception_end(ctx, EXCP_RI); > +break; > +} > +break; > +case NM_P_SCE: > +switch (extract32(ctx->opcode, 2, 2)) { > +case NM_SCE: > +check_xnp(ctx); > +check_eva(ctx); > +check_cp0_enabled(ctx); > +gen_st_cond(ctx, OPC_SCE, rt, rs, s); > +break; > +case NM_SCWPE: > +check_xnp(ctx); > +check_eva(ctx); > +check_cp0_enabled(ctx); > +gen_scwp(ctx, rs, 0, rt, extract32(ctx->opcode, 3, > 5)); > +default: > +generate_exception_end(ctx, EXCP_RI); > +break; > +} > +break; > +} > +break; > case NM_P_LS_WM: > case NM_P_LS_UAWM: > check_nms(ctx); Could you send a patch which adds whichever of the two is correct, please? thanks -- PMM
Re: [Qemu-devel] [RFC 48/48] plugin: add a couple of very simple examples
> From: Emilio G. Cota [mailto:c...@braap.org] > Signed-off-by: Emilio G. Cota > --- > plugin-examples/bbcount_avgsize_racy.c | 50 ++ > plugin-examples/mem_count_racy_both.c | 58 ++ > plugin-examples/Makefile | 31 ++ > 3 files changed, 139 insertions(+) > create mode 100644 plugin-examples/bbcount_avgsize_racy.c > create mode 100644 plugin-examples/mem_count_racy_both.c > create mode 100644 plugin-examples/Makefile > > diff --git a/plugin-examples/mem_count_racy_both.c > b/plugin-examples/mem_count_racy_both.c > new file mode 100644 > index 00..a47f2025bf > --- /dev/null > +++ b/plugin-examples/mem_count_racy_both.c > @@ -0,0 +1,58 @@ > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +static uint64_t mem_count; > +static int stdout_fd; > +static bool do_inline; > + > +static void plugin_exit(qemu_plugin_id_t id, void *p) > +{ > +dprintf(stdout_fd, "accesses: %" PRIu64 "\n", mem_count); > +} > + > +static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo, > + uint64_t vaddr, void *udata) > +{ > +mem_count++; > +} > + > +static void vcpu_tb_trans(qemu_plugin_id_t id, unsigned int cpu_index, > + struct qemu_plugin_tb *tb) > +{ > +size_t n = qemu_plugin_tb_n_insns(tb); > +size_t i; > + > +for (i = 0; i < n; i++) { > +struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i); > + > +if (do_inline) { > +qemu_plugin_register_vcpu_mem_inline(insn, > + QEMU_PLUGIN_INLINE_ADD_U64, > + &mem_count, 1); > +} else { > +qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem, > + QEMU_PLUGIN_CB_NO_REGS, NULL); > +} > +} > +} > + > +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, int argc, > + char **argv) > +{ > +if (argc && strcmp(argv[0], "inline") == 0) { > +do_inline = true; > +} > +/* plugin_exit might write to stdout after stdout has been closed */ > +stdout_fd = dup(STDOUT_FILENO); > +assert(stdout_fd); > + > +qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans); > +qemu_plugin_register_atexit_cb(id, plugin_exit, NULL); > +return 0; > +} Thanks for the series. Can you provide more plugin examples for better understanding of double-translate idea? E.g., plugins that hook specific instructions or addresses. Pavel Dovgalyuk
Re: [Qemu-devel] bochs-display and the future of VGA support
On Sun, Oct 28, 2018 at 01:40:29PM +0100, John Paul Adrian Glaubitz wrote: > Hi! > > I just happened to read Gerd Hoffmann's post on the bochs-display [1] driver > and was wondering what the future plans for VGA support are. > > Phoronix makes it sound like [2] that VGA support for guests is supposed to > be removed which I find hard to believe. No plans to remove vga support. > Since once VGA support is gone, QEMU would no longer be able to run > older operating systems which is one of the key features of an > emulator in my opinion. > > Can anyone tell me what the plans are? Well, the plan is to be able to run guests which wouldn't use the VGA anyway without the rather complex VGA emulation enabled. cheers, Gerd
Re: [Qemu-devel] [PULL V2 01/26] filter-rewriter: Add TCP state machine and fix memory leak in connection_track_table
On 19 October 2018 at 04:22, Jason Wang wrote: > From: Zhang Chen > > We add almost full TCP state machine in filter-rewriter, except > TCPS_LISTEN and some simplify in VM active close FIN states. > The reason for this simplify job is because guest kernel will track > the TCP status and wait 2MSL time too, if client resend the FIN packet, > guest will resend the last ACK, so we needn't wait 2MSL time in > filter-rewriter. > > After a net connection is closed, we didn't clear its related resources > in connection_track_table, which will lead to memory leak. > > Let's track the state of net connection, if it is closed, its related > resources will be cleared up. Hi. Coverity (CID 1396477) points out that here: > +/* > + * Active close step 2. > + */ > +if (conn->tcp_state == TCPS_FIN_WAIT_1) { > +conn->tcp_state = TCPS_TIME_WAIT; ...this assignment to conn->tcp_state has no effect, because... > +/* > + * For simplify implementation, we needn't wait 2MSL time > + * in filter rewriter. Because guest kernel will track the > + * TCP status and wait 2MSL time, if client resend the FIN > + * packet, guest will apply the last ACK too. > + */ > +conn->tcp_state = TCPS_CLOSED; ...we immediately overwrite it with a different value. > +g_hash_table_remove(rf->connection_track_table, key); > +} > } What was the intention of the code here? thanks -- PMM
Re: [Qemu-devel] [PATCH v9 2/6] monitor: resume the monitor earlier if needed
Hi On Wed, Oct 10, 2018 at 7:22 AM Peter Xu wrote: > > On Tue, Oct 09, 2018 at 12:54:37PM +0400, Marc-André Lureau wrote: > > Hi > > On Tue, Oct 9, 2018 at 10:28 AM Peter Xu wrote: > > > > > > Currently when QMP request queue full we won't resume the monitor until > > > we have completely handled the current command. It's not necessary > > > since even before it's handled the queue is already non-full. Moving > > > the resume logic earlier before the command execution. > > > > > > Note that now monitor_resume() is heavy weighted after 8af6bb14a3a8 and > > > it's even possible (as pointed out by Marc-André) that the function > > > itself may try to take the monitor lock again, so let's do the resume > > > after the monitor lock is released to avoid possible dead lock. > > > > > > Signed-off-by: Peter Xu > > > --- > > > monitor.c | 10 ++ > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/monitor.c b/monitor.c > > > index 1f83775fff..f5911399d8 100644 > > > --- a/monitor.c > > > +++ b/monitor.c > > > @@ -4149,6 +4149,12 @@ static void monitor_qmp_bh_dispatcher(void *data) > > > need_resume = !qmp_oob_enabled(mon) || > > > mon->qmp.qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1; > > > qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); > > > + > > > +if (need_resume) { > > > +/* Pairs with the monitor_suspend() in handle_qmp_command() */ > > > +monitor_resume(mon); > > > +} > > > > "need_resume" is only set on non-oob enabled monitors. > > ... or on oob-enabled monitors when queue full? yes, after patch 1 > > > > > On monitor_resume(), monitor_qmp_read() may end up being called, which > > may call handle_qmp_command(). > > > > With regular commands, a new command is queued. But if the command is > > "exec-oob", it will dispatch immediately, thus not following the QMP > > reply ordering constrain. > > > > Shouldn't it be an error to call exec-oob on non-oob enabled monitors? > > Do you mean a "qmp_capabilities" command to enable oob, and a > continuous "exec-oob" command? No I meant calling "exec-oob" on a monitor without oob capability. I checked again, and it does already fail with "QMP input member 'exec-oob' is unexpected". So nevermind. > > I can't say I fully understand the scenario you mentioned, but I think > it does violate the rule if we resume the monitor before we finish > executing the "qmp_capabilities" command since that command should > still be run with "non-oob" context. So in that case we should do the > resume after dispatching. For the other queue-full case we shouldn't > need to, as Markus suggested. > > Instead of bothering with all these, how about I drop this patch? We > might resume the monitor a little bit later when queue full, but I > don't think that's a big deal for now. Yes, if it's a minor optimization, we can postpone it for now. thanks > > Regards, > > -- > Peter Xu -- Marc-André Lureau
Re: [Qemu-devel] [PULL 2/3] target/mips: Implement emulation of nanoMIPS EVA instructions
> From: Peter Maydell > Sent: Monday, October 29, 2018 11:57 AM > Subject: Re: [PULL 2/3] target/mips: Implement emulation of nanoMIPS EVA > instructions > > On 25 October 2018 at 21:19, Aleksandar Markovic > wrote: > > From: Dimitrije Nikolic > > > > Implement emulation of nanoMIPS EVA instructions. They are all > > part of P.LS.E0 instruction pool, or one of its subpools. > > > > Reviewed-by: Stefan Markovic > > Signed-off-by: Dimitrije Nikolic > > Signed-off-by: Aleksandar Markovic > > -- > > Hi; Coverity points out (CID 1396475) that the switch > cases for NM_LLWPE and NM_SCWPE fall through without > either a 'break' statement or a '/* fall through */' comment: > > > +case NM_P_LLE: > > +switch (extract32(ctx->opcode, 2, 2)) { > > +case NM_LLE: > > +check_xnp(ctx); > > +check_eva(ctx); > > +check_cp0_enabled(ctx); > > +gen_ld(ctx, OPC_LLE, rt, rs, s); > > +break; > > +case NM_LLWPE: > > +check_xnp(ctx); > > +check_eva(ctx); > > +check_cp0_enabled(ctx); > > +gen_llwp(ctx, rs, 0, rt, extract32(ctx->opcode, 3, > > 5)); > > +default: > > +generate_exception_end(ctx, EXCP_RI); > > +break; > > +} > > +break; > > +case NM_P_SCE: > > +switch (extract32(ctx->opcode, 2, 2)) { > > +case NM_SCE: > > +check_xnp(ctx); > > +check_eva(ctx); > > +check_cp0_enabled(ctx); > > +gen_st_cond(ctx, OPC_SCE, rt, rs, s); > > +break; > > +case NM_SCWPE: > > +check_xnp(ctx); > > +check_eva(ctx); > > +check_cp0_enabled(ctx); > > +gen_scwp(ctx, rs, 0, rt, extract32(ctx->opcode, 3, > > 5)); > > +default: > > +generate_exception_end(ctx, EXCP_RI); > > +break; > > +} > > +break; > > +} > > +break; > > case NM_P_LS_WM: > > case NM_P_LS_UAWM: > > check_nms(ctx); > > Could you send a patch which adds whichever of the two is correct, please? > Sure. 'break' is missing in both cases. Thanks, Aleksandar > thanks > -- PMM
[Qemu-devel] [PATCH] target/mips: Add two missing breaks for NM_LLWPE and NM_SCWPE decoder cases
From: Aleksandar Markovic Coverity found two fallthroughs that lack break statements. Fix them. Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/mips/translate.c b/target/mips/translate.c index b8ace0b..813ad19 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -21402,6 +21402,7 @@ static int decode_nanomips_32_48_opc(CPUMIPSState *env, DisasContext *ctx) check_eva(ctx); check_cp0_enabled(ctx); gen_llwp(ctx, rs, 0, rt, extract32(ctx->opcode, 3, 5)); +break; default: generate_exception_end(ctx, EXCP_RI); break; @@ -21420,6 +21421,7 @@ static int decode_nanomips_32_48_opc(CPUMIPSState *env, DisasContext *ctx) check_eva(ctx); check_cp0_enabled(ctx); gen_scwp(ctx, rs, 0, rt, extract32(ctx->opcode, 3, 5)); +break; default: generate_exception_end(ctx, EXCP_RI); break; -- 2.7.4
Re: [Qemu-devel] [PATCH] vhost-user: fix qemu crash caused by failed backend
On Tue, Oct 02, 2018 at 01:54:25PM +0400, Marc-André Lureau wrote: > Hi > > On Thu, Sep 27, 2018 at 7:37 PM Liang Li wrote: > > > > During live migration, when stopping vhost-user device, 'vhost_dev_stop' > > will be called, 'vhost_dev_stop' will call a batch of 'vhost_user_read' > > and 'vhost_user_write'. If a previous 'vhost_user_read' or > > 'vhost_user_write' > > failed because the vhost user backend failed, the 'CHR_EVENT_CLOSED' event > > will be triggerd, followed by the call chain > > chr_closed_bh()->vhost_user_stop()-> > > vhost_net_cleanup()->vhost_dev_cleanup() > > > > vhost_dev_cleanup will clear vhost_dev struct, so the later > > 'vhost_user_read' > > or 'vhost_user_read' will reference null pointer and cause qemu crash > > Do you have a backtrace to help understand the issue? > thanks > sorry for late response. Yes, I have. But it's the backtrace for qemu-kvm-2.10. and we found this issue when doing pressure test, it was triggered by a buggy ovs-dpdk backend, an ovs-dpdk coredump followed by a qemu coredump. the backtrace is like bellow: == 0 0x7f0af85ea069 in vhost_user_read (msg=msg@entry=0x7f0a2a4b5300, dev=0x7f0afaee0340) at /usr/src/debug/qemu-2.10.0/hw/virtio/vhost-user.c:139 1 0x7f0af85ea2df in vhost_user_get_vring_base (dev=0x7f0afaee0340, ring=0x7f0a2a4b5450) at /usr/src/debug/qemu-2.10.0/hw/virtio/vhost-user.c:458 2 0x7f0af85e715e in vhost_virtqueue_stop (dev=dev@entry=0x7f0afaee0340, vdev=vdev@entry=0x7f0afcba0170, vq=0x7f0afaee05d0, idx=1) at /usr/src/debug/qemu-2.10.0/hw/virtio/vhost.c:1138 3 0x7f0af85e8e24 in vhost_dev_stop (hdev=hdev@entry=0x7f0afaee0340, vdev=vdev@entry=0x7f0afcba0170) at /usr/src/debug/qemu-2.10.0/hw/virtio/vhost.c:1601 4 0x7f0af85d1418 in vhost_net_stop_one (net=0x7f0afaee0340, dev=0x7f0afcba0170) at /usr/src/debug/qemu-2.10.0/hw/net/vhost_net.c:289 5 0x7f0af85d191b in vhost_net_stop (dev=dev@entry=0x7f0afcba0170, ncs=, total_queues=total_queues@entry=1) at /usr/src/debug/qemu-2.10.0/hw/net/vhost_net.c:3 6 0x7f0af85ceba6 in virtio_net_set_status (status=, n=0x7f0afcba0170) at /usr/src/debug/qemu-2.10.0/hw/net/virtio-net.c:180 7 0x7f0af85ceba6 in virtio_net_set_status (vdev=0x7f0afcba0170, status=15 '\017') at /usr/src/debug/qemu-2.10.0/hw/net/virtio-net.c:254 8 0x7f0af85e0f2c in virtio_set_status (vdev=0x7f0afcba0170, val=) at /usr/src/debug/qemu-2.10.0/hw/virtio/virtio.c:1147 9 0x7f0af866dce2 in vm_state_notify (running=running@entry=0, state=state@entry=RUN_STATE_FINISH_MIGRATE) at vl.c:1623 10 0x7f0af858f11a in do_vm_stop (state=RUN_STATE_FINISH_MIGRATE, send_stop=send_stop@entry=true) at /usr/src/debug/qemu-2.10.0/cpus.c:941 11 0x7f0af858f159 in vm_stop (state=) at /usr/src/debug/qemu-2.10.0/cpus.c:1818 12 0x7f0af858f296 in vm_stop_force_state (state=state@entry=RUN_STATE_FINISH_MIGRATE) at /usr/src/debug/qemu-2.10.0/cpus.c:1868 13 0x7f0af87551d7 in migration_thread (start_time=, old_vm_running=, current_active_state=4, s=0x7f0afaf00500) at migration/migration.c:1956 14 0x7f0af87551d7 in migration_thread (opaque=0x7f0afaf00500) at migration/migration.c:2129 15 0x7f0af217fdc5 in start_thread () at /lib64/libpthread.so.0 16 0x7f0af1eae73d in clone () at /lib64/libc.so.6 (gdb) l 134 } 135 136 static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg) 137 { 138 struct vhost_user *u = dev->opaque; 139 CharBackend *chr = u->chr; 140 uint8_t *p = (uint8_t *) msg; 141 int r, size = VHOST_USER_HDR_SIZE; 142 143 r = qemu_chr_fe_read_all(chr, p, size); 144 if (r != size) { 145 error_report("Failed to read msg header. Read %d instead of %d." 146 " Original request %d.", r, size, msg->request); 147 goto fail; 148 } 149 150 /* validate received flags */ 151 if (msg->flags != (VHOST_USER_REPLY_MASK | VHOST_USER_VERSION)) { 152 error_report("Failed to read msg header." 153 " Flags 0x%x instead of 0x%x.", msg->flags, (gdb) p u $1 = (struct vhost_user *) 0x0 (gdb) p dev $2 = (struct vhost_dev *) 0x7f0afaee0340 (gdb) p *dev $3 = {vdev = 0x0, memory_listener = {begin = 0x0, commit = 0x0, region_add = 0x0, region_del = 0x0, region_nop = 0x0, log_start = 0x0, log_stop = 0x0, log_sync = 0x0, log_global_start = 0x0, log_global_stop = 0x0, eventfd_add = 0x0, eventfd_del = 0x0, coalesced_mmio_add = 0x0, coalesced_mmio_del = 0x0, priority = 0, address_space = 0x0, link = {tqe_next = 0x0, tqe_prev = 0x0}, link_as = {tqe_next = 0x0, tqe_prev = 0x0}}, iommu_listener = {begin = 0x0, commit = 0x0, region_add = 0x0, region_del = 0x0, region_nop = 0x0, log_start = 0x0, log_stop = 0x0
[Qemu-devel] [PATCH v8 04/20] target/mips: Add and integrate MXU decoding engine placeholder
From: Aleksandar Markovic Provide the placeholder and add the invocation logic for MXU decoding engine. Reviewed-by: Stefan Markovic Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 8 1 file changed, 8 insertions(+) diff --git a/target/mips/translate.c b/target/mips/translate.c index 9f6200a..265fac6 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -23976,6 +23976,12 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx) } } +static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx) +{ +MIPS_INVAL("decode_opc_mxu"); +generate_exception_end(ctx, EXCP_RI); +} + static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx) { int rs, rt, rd; @@ -26219,6 +26225,8 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) case OPC_SPECIAL2: if ((ctx->insn_flags & INSN_R5900) && (ctx->insn_flags & ASE_MMI)) { decode_tx79_mmi(env, ctx); +} else if (ctx->insn_flags & ASE_MXU) { +decode_opc_mxu(env, ctx); } else { decode_opc_special2_legacy(env, ctx); } -- 2.7.4
[Qemu-devel] [PATCH v8 00/20] target/mips: Add limited support for Ingenic's MXU ASE
From: Aleksandar Markovic This patch set begins to add MXU ASE instruction support. v7->v8: - corrected several mistakes in MXU ASE overview note - add several clarifying comments - rebased to the latest code v6->v7: - move MXU_EN check to the main MXU decoding function - amend MXU ASE overview note v5->v6: - added bit definitions for 'aptn1' and 'eptn2'. - pool04 eliminated, since it is covered by a single instruction. - moved MUL, S32M2I, S32I2M handling out of main MXU switch. - rebased to the latest code (this series applies on top of the current MIPS pull request) v4->v5: - added full decoding engine for MXU ASE - changes on aptn2, optn2, optn3 are now stand-alone patches - all patches on individual instructions are reworked to fit new decoding engine, and also cosmetically improved - rebased to the latest code Aleksandar Markovic (8): target/mips: Amend MXU instruction opcodes target/mips: Add and integrate MXU decoding engine placeholder target/mips: Add MXU decoding engine target/mips: Add bit encoding for MXU accumulate add/sub 1-bit pattern 'aptn1' target/mips: Add bit encoding for MXU execute add/sub pattern 'eptn2' target/mips: Move MUL, S32M2I, S32I2M handling out of main MXU switch target/mips: Move MXU_EN check one level higher target/mips: Amend MXU ASE overview note Craig Janeczek (12): target/mips: Introduce MXU registers target/mips: Define a bit for MXU in insn_flags target/mips: Add bit encoding for MXU accumulate add/sub 2-bit pattern 'aptn2' target/mips: Add bit encoding for MXU operand getting pattern 'optn2' target/mips: Add bit encoding for MXU operand getting pattern 'optn3' target/mips: Add emulation of non-MXU MULL within MXU decoding engine target/mips: Add emulation of MXU instructions S32I2M and S32M2I target/mips: Add emulation of MXU instruction S8LDD target/mips: Add emulation of MXU instruction D16MUL target/mips: Add emulation of MXU instruction D16MAC target/mips: Add emulation of MXU instructions Q8MUL and Q8MULSU target/mips: Add emulation of MXU instructions S32LDD and S32LDDR target/mips/cpu.h | 10 + target/mips/mips-defs.h |1 + target/mips/translate.c | 2104 ++- 3 files changed, 1896 insertions(+), 219 deletions(-) -- 2.7.4
[Qemu-devel] [PATCH v8 01/20] target/mips: Introduce MXU registers
From: Craig Janeczek Define and initialize the 16 MXU registers - 15 general computational register, and 1 control register). There is also a zero register, but it does not have any corresponding variable. Reviewed-by: Richard Henderson Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/cpu.h | 10 ++ target/mips/translate.c | 20 2 files changed, 30 insertions(+) diff --git a/target/mips/cpu.h b/target/mips/cpu.h index e48be4b..03c03fd 100644 --- a/target/mips/cpu.h +++ b/target/mips/cpu.h @@ -170,6 +170,16 @@ struct TCState { MSACSR_FS_MASK) float_status msa_fp_status; + +#define NUMBER_OF_MXU_REGISTERS 16 +target_ulong mxu_gpr[NUMBER_OF_MXU_REGISTERS - 1]; +target_ulong mxu_cr; +#define MXU_CR_LC 31 +#define MXU_CR_RC 30 +#define MXU_CR_BIAS 2 +#define MXU_CR_RD_EN1 +#define MXU_CR_MXU_EN 0 + }; typedef struct CPUMIPSState CPUMIPSState; diff --git a/target/mips/translate.c b/target/mips/translate.c index 51a5488..36727c3 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -2379,6 +2379,10 @@ static TCGv_i32 fpu_fcr0, fpu_fcr31; static TCGv_i64 fpu_f64[32]; static TCGv_i64 msa_wr_d[64]; +/* MXU registers */ +static TCGv mxu_gpr[NUMBER_OF_MXU_REGISTERS - 1]; +static TCGv mxu_CR; + #include "exec/gen-icount.h" #define gen_helper_0e0i(name, arg) do { \ @@ -2501,6 +2505,11 @@ static const char * const msaregnames[] = { "w30.d0", "w30.d1", "w31.d0", "w31.d1", }; +static const char * const mxuregnames[] = { +"XR1", "XR2", "XR3", "XR4", "XR5", "XR6", "XR7", "XR8", +"XR9", "XR10", "XR11", "XR12", "XR13", "XR14", "XR15", "MXU_CR", +}; + #define LOG_DISAS(...)\ do { \ if (MIPS_DEBUG_DISAS) { \ @@ -27229,6 +27238,17 @@ void mips_tcg_init(void) fpu_fcr31 = tcg_global_mem_new_i32(cpu_env, offsetof(CPUMIPSState, active_fpu.fcr31), "fcr31"); + +for (i = 0; i < NUMBER_OF_MXU_REGISTERS - 1; i++) { +mxu_gpr[i] = tcg_global_mem_new(cpu_env, +offsetof(CPUMIPSState, + active_tc.mxu_gpr[i]), +mxuregnames[i]); +} + +mxu_CR = tcg_global_mem_new(cpu_env, +offsetof(CPUMIPSState, active_tc.mxu_cr), +mxuregnames[NUMBER_OF_MXU_REGISTERS - 1]); } #include "translate_init.inc.c" -- 2.7.4
[Qemu-devel] [PATCH v8 05/20] target/mips: Add MXU decoding engine
From: Aleksandar Markovic Add MXU decoding engine: add handlers for all instruction pools, and main decode handler. The handlers, for now, for the purpose of this patch, contain only sceleton in the form of a single switch statement. Reviewed-by: Stefan Markovic Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 1143 ++- 1 file changed, 1141 insertions(+), 2 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index 265fac6..699a7e7 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -23976,12 +23976,1151 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx) } } +/* + * + * Decode MXU pool00 + * + * 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 + * +---+-+-+---+---+---+---+ + * | SPECIAL2 |0 0 0 0 0|x x x| XRc | XRb | XRa |MXU__POOL00| + * +---+-+-+---+---+---+---+ + * + */ +static void decode_opc_mxu__pool00(CPUMIPSState *env, DisasContext *ctx) +{ +uint32_t opcode = extract32(ctx->opcode, 18, 3); + +switch (opcode) { +case OPC_MXU_S32MAX: +/* TODO: Implement emulation of S32MAX instruction. */ +MIPS_INVAL("OPC_MXU_S32MAX"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_S32MIN: +/* TODO: Implement emulation of S32MIN instruction. */ +MIPS_INVAL("OPC_MXU_S32MIN"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_D16MAX: +/* TODO: Implement emulation of D16MAX instruction. */ +MIPS_INVAL("OPC_MXU_D16MAX"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_D16MIN: +/* TODO: Implement emulation of D16MIN instruction. */ +MIPS_INVAL("OPC_MXU_D16MIN"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_Q8MAX: +/* TODO: Implement emulation of Q8MAX instruction. */ +MIPS_INVAL("OPC_MXU_Q8MAX"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_Q8MIN: +/* TODO: Implement emulation of Q8MIN instruction. */ +MIPS_INVAL("OPC_MXU_Q8MIN"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_Q8SLT: +/* TODO: Implement emulation of Q8SLT instruction. */ +MIPS_INVAL("OPC_MXU_Q8SLT"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_Q8SLTU: +/* TODO: Implement emulation of Q8SLTU instruction. */ +MIPS_INVAL("OPC_MXU_Q8SLTU"); +generate_exception_end(ctx, EXCP_RI); +break; +default: +MIPS_INVAL("decode_opc_mxu"); +generate_exception_end(ctx, EXCP_RI); +break; +} +} + +/* + * + * Decode MXU pool01 + * + * S32SLT, D16SLT, D16AVG, D16AVGR, Q8AVG, Q8AVGR: + * 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 + * +---+-+-+---+---+---+---+ + * | SPECIAL2 |0 0 0 0 0|x x x| XRc | XRb | XRa |MXU__POOL01| + * +---+-+-+---+---+---+---+ + * + * Q8ADD: + * 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 + * +---+---+-+-+---+---+---+---+ + * | SPECIAL2 |en2|0 0 0|x x x| XRc | XRb | XRa |MXU__POOL01| + * +---+---+-+-+---+---+---+---+ + * + */ +static void decode_opc_mxu__pool01(CPUMIPSState *env, DisasContext *ctx) +{ +uint32_t opcode = extract32(ctx->opcode, 18, 3); + +switch (opcode) { +case OPC_MXU_S32SLT: +/* TODO: Implement emulation of S32SLT instruction. */ +MIPS_INVAL("OPC_MXU_S32SLT"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_D16SLT: +/* TODO: Implement emulation of D16SLT instruction. */ +MIPS_INVAL("OPC_MXU_D16SLT"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_D16AVG: +/* TODO: Implement emulation of D16AVG instruction. */ +MIPS_INVAL("OPC_MXU_D16AVG"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_D16AVGR: +/* TODO: Implement emulation of D16AVGR instruction. */ +MIPS_INVAL("OPC_MXU_D16AVGR"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_Q8AVG: +/* TODO: Implement emulation of Q8AVG instruction. */ +MIPS_INVAL("OPC_MXU_Q8AVG"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_Q8AVGR: +/* TODO: Implement emulation of Q8AVGR instruction. */ +MIPS_INVAL("OPC_MXU_Q8AVGR"); +generate_exception_end(ctx, EXCP_RI); +break; +case OPC_MXU_Q8ADD: +/* TODO: Implement emulation of Q8ADD instruction. */ +MIPS_INVAL("OPC_MXU_Q8ADD"); +generate_exception_end(ctx, EXCP_RI);
[Qemu-devel] [PATCH v8 06/20] target/mips: Add bit encoding for MXU accumulate add/sub 1-bit pattern 'aptn1'
From: Aleksandar Markovic Add bit encoding for MXU accumulate add/subtract 1-bit pattern 'aptn1'. Reviewed-by: Stefan Markovic Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/target/mips/translate.c b/target/mips/translate.c index 699a7e7..7130729 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -23976,6 +23976,12 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx) } } + +/* MXU accumulate add/subtract 1-bit pattern 'aptn1' */ +#define MXU_APTN1_A0 +#define MXU_APTN1_S1 + + /* * * Decode MXU pool00 -- 2.7.4
[Qemu-devel] [PATCH v8 02/20] target/mips: Define a bit for MXU in insn_flags
From: Craig Janeczek Define a bit for MXU in insn_flags. This is the first non-MIPS (third party) ASE supported in QEMU for MIPS, so it is placed in the section "bits 56-63: vendor-specific ASEs". Reviewed-by: Aleksandar Markovic Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/mips-defs.h | 1 + 1 file changed, 1 insertion(+) diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h index 5177618..dbdb4b2 100644 --- a/target/mips/mips-defs.h +++ b/target/mips/mips-defs.h @@ -69,6 +69,7 @@ * bits 56-63: vendor-specific ASEs */ #define ASE_MMI 0x0100ULL +#define ASE_MXU 0x0200ULL /* MIPS CPU defines. */ #defineCPU_MIPS1 (ISA_MIPS1) -- 2.7.4
[Qemu-devel] [PATCH v8 10/20] target/mips: Add bit encoding for MXU operand getting pattern 'optn3'
From: Craig Janeczek Add bit encoding for MXU operand getting pattern 'optn3'. Reviewed-by: Stefan Markovic Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/target/mips/translate.c b/target/mips/translate.c index 2cf0ba5..13e158a 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -23999,6 +23999,16 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx) #define MXU_OPTN2_HW2 #define MXU_OPTN2_XW3 +/* MXU operand getting pattern 'optn3' */ +#define MXU_OPTN3_PTN0 0 +#define MXU_OPTN3_PTN1 1 +#define MXU_OPTN3_PTN2 2 +#define MXU_OPTN3_PTN3 3 +#define MXU_OPTN3_PTN4 4 +#define MXU_OPTN3_PTN5 5 +#define MXU_OPTN3_PTN6 6 +#define MXU_OPTN3_PTN7 7 + /* * -- 2.7.4
[Qemu-devel] [PATCH v8 07/20] target/mips: Add bit encoding for MXU accumulate add/sub 2-bit pattern 'aptn2'
From: Craig Janeczek Add bit encoding for MXU accumulate add/subtract 2-bit pattern 'aptn2'. Reviewed-by: Stefan Markovic Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/target/mips/translate.c b/target/mips/translate.c index 7130729..7116697 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -23981,6 +23981,12 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx) #define MXU_APTN1_A0 #define MXU_APTN1_S1 +/* MXU accumulate add/subtract 2-bit pattern 'aptn2' */ +#define MXU_APTN2_AA0 +#define MXU_APTN2_AS1 +#define MXU_APTN2_SA2 +#define MXU_APTN2_SS3 + /* * -- 2.7.4
[Qemu-devel] [PATCH v8 03/20] target/mips: Amend MXU instruction opcodes
From: Aleksandar Markovic Amend MXU instruction opcodes. Pool04 is actually only instruction OPC_MXU_S16MAD. Two cases within S16MAD are recognized by 1-bit subfield 'aptn1'. Reviewed-by: Stefan Markovic Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 160 +--- 1 file changed, 69 insertions(+), 91 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index 36727c3..9f6200a 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -1486,7 +1486,7 @@ enum { * S32OR XRa, XRb, XRc D32SARW XRa, XRb, XRc, Rb *Q16SLL XRa, XRb, XRc, XRd, sft4 *Q16SLR XRa, XRb, XRc, XRd, sft4 - * Miscelaneous instructions Q16SAR XRa, XRb, XRc, XRd, sft4 + * Miscellaneous instructions Q16SAR XRa, XRb, XRc, XRd, sft4 * - Q16SLLV XRa, XRb, Rb *Q16SLRV XRa, XRb, Rb * S32SFL XRa, XRb, XRc, XRd, optn2 Q16SARV XRa, XRb, Rb @@ -1504,7 +1504,8 @@ enum { * * ┌─ 00 ─ OPC_MXU_S32MADD * ├─ 01 ─ OPC_MXU_S32MADDU - * ├─ 10 ─ + * ├─ 10 ─(non-MXU OPC_MUL) + * │ * │ 20..18 * ├─ 11 ─ OPC_MXU__POOL00 ─┬─ 000 ─ OPC_MXU_S32MAX * │├─ 001 ─ OPC_MXU_S32MIN @@ -1536,73 +1537,67 @@ enum { * ├─ 001010 ─ OPC_MXU_D16MAC * ├─ 001011 ─ OPC_MXU_D16MACF * ├─ 001100 ─ OPC_MXU_D16MADL - * │ 25..24 - * ├─ 001101 ─ OPC_MXU__POOL04 ─┬─ 00 ─ OPC_MXU_S16MAD - * │└─ 01 ─ OPC_MXU_S16MAD_1 + * ├─ 001101 ─ OPC_MXU_S16MAD * ├─ 001110 ─ OPC_MXU_Q16ADD - * ├─ 00 ─ OPC_MXU_D16MACE - * │ 23 - * ├─ 01 ─ OPC_MXU__POOL05 ─┬─ 0 ─ OPC_MXU_S32LDD - * │└─ 1 ─ OPC_MXU_S32LDDR + * ├─ 00 ─ OPC_MXU_D16MACE 23 + * │┌─ 0 ─ OPC_MXU_S32LDD + * ├─ 01 ─ OPC_MXU__POOL04 ─┴─ 1 ─ OPC_MXU_S32LDDR * │ * │ 23 - * ├─ 010001 ─ OPC_MXU__POOL06 ─┬─ 0 ─ OPC_MXU_S32STD + * ├─ 010001 ─ OPC_MXU__POOL05 ─┬─ 0 ─ OPC_MXU_S32STD * │└─ 1 ─ OPC_MXU_S32STDR * │ * │ 13..10 - * ├─ 010010 ─ OPC_MXU__POOL07 ─┬─ ─ OPC_MXU_S32LDDV + * ├─ 010010 ─ OPC_MXU__POOL06 ─┬─ ─ OPC_MXU_S32LDDV * │└─ 0001 ─ OPC_MXU_S32LDDVR * │ * │ 13..10 - * ├─ 010011 ─ OPC_MXU__POOL08 ─┬─ ─ OPC_MXU_S32STDV + * ├─ 010011 ─ OPC_MXU__POOL07 ─┬─ ─ OPC_MXU_S32STDV * │└─ 0001 ─ OPC_MXU_S32STDVR * │ * │ 23 - * ├─ 010100 ─ OPC_MXU__POOL09 ─┬─ 0 ─ OPC_MXU_S32LDI + * ├─ 010100 ─ OPC_MXU__POOL08 ─┬─ 0 ─ OPC_MXU_S32LDI * │└─ 1 ─ OPC_MXU_S32LDIR * │ * │ 23 - * ├─ 010101 ─ OPC_MXU__POOL10 ─┬─ 0 ─ OPC_MXU_S32SDI + * ├─ 010101 ─ OPC_MXU__POOL09 ─┬─ 0 ─ OPC_MXU_S32SDI * │└─ 1 ─ OPC_MXU_S32SDIR * │ * │ 13..10 - * ├─ 010110 ─ OPC_MXU__POOL11 ─┬─ ─ OPC_MXU_S32LDIV + * ├─ 010110 ─ OPC_MXU__POOL10 ─┬─ ─ OPC_MXU_S32LDIV * │└─ 0001 ─ OPC_MXU_S32LDIVR * │ * │ 13..10 - * ├─ 010111 ─ OPC_MXU__POOL12 ─┬─ ─ OPC_MXU_S32SDIV + * ├─ 010111 ─ OPC_MXU__POOL11 ─┬─ ─ OPC_MXU_S32SDIV * │└─ 0001 ─ OPC_MXU_S32SDIVR * ├─ 011000 ─ OPC_MXU_D32ADD * │ 23..22 - * MXU├─ 011001 ─ OPC_MXU__POOL13 ─┬─ 00 ─ OPC_MXU_D32ACC + * MXU├─ 011001 ─ OPC_MXU__POOL12 ─┬─ 00 ─ OPC_MXU_D32ACC * opcodes ─┤├─ 01 ─ OPC_MXU_D32ACCM * │└─ 10 ─ OPC_MXU_D32ASUM * ├─ 011010 ─ * │ 23..22 - * ├─ 011011 ─ OPC_MXU__POOL14 ─┬─ 00 ─ OPC_MXU_Q16ACC + * ├─ 011011 ─ OPC_MXU__POOL13 ─┬─ 00 ─ OPC_MXU_Q16ACC * │├─ 01 ─ OPC_MXU_Q16ACCM * │└─ 10 ─ OPC_MXU_Q16ASUM * │ * │ 23..22 - * ├─ 011100 ─ OPC_MXU__POOL15 ─┬─ 00 ─ OPC_MXU_Q8ADDE + * ├─ 011100 ─ OP
[Qemu-devel] [PATCH v8 09/20] target/mips: Add bit encoding for MXU operand getting pattern 'optn2'
From: Craig Janeczek Add bit encoding for MXU operand getting pattern 'optn2'. Reviewed-by: Stefan Markovic Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/target/mips/translate.c b/target/mips/translate.c index 1935796..2cf0ba5 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -23993,6 +23993,12 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx) #define MXU_EPTN2_SA2 #define MXU_EPTN2_SS3 +/* MXU operand getting pattern 'optn2' */ +#define MXU_OPTN2_WW0 +#define MXU_OPTN2_LW1 +#define MXU_OPTN2_HW2 +#define MXU_OPTN2_XW3 + /* * -- 2.7.4
[Qemu-devel] [PATCH v8 11/20] target/mips: Add emulation of non-MXU MULL within MXU decoding engine
From: Craig Janeczek Add emulation of non-MXU MULL within MXU decoding engine. Reviewed-by: Aleksandar Markovic Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index 13e158a..a98de28 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -1648,7 +1648,7 @@ enum { enum { OPC_MXU_S32MADD = 0x00, OPC_MXU_S32MADDU = 0x01, -/* not assigned 0x02 */ +OPC__MXU_MUL = 0x02, OPC_MXU__POOL00 = 0x03, OPC_MXU_S32MSUB = 0x04, OPC_MXU_S32MSUBU = 0x05, @@ -24907,6 +24907,11 @@ static void decode_opc_mxu__pool20(CPUMIPSState *env, DisasContext *ctx) */ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx) { +/* + * TODO: Investigate necessity of including handling of + * CLZ, CLO, SDBB in this function, as they belong to + * SPECIAL2 opcode space for regular pre-R6 MIPS ISAs. + */ uint32_t opcode = extract32(ctx->opcode, 0, 6); switch (opcode) { @@ -24920,6 +24925,18 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx) MIPS_INVAL("OPC_MXU_S32MADDU"); generate_exception_end(ctx, EXCP_RI); break; +case OPC__MXU_MUL: /* 0x2 - unused in MXU specs */ +{ +uint32_t rs, rt, rd, op1; + +rs = extract32(ctx->opcode, 21, 5); +rt = extract32(ctx->opcode, 16, 5); +rd = extract32(ctx->opcode, 11, 5); +op1 = MASK_SPECIAL2(ctx->opcode); + +gen_arith(ctx, op1, rd, rs, rt); +} +break; case OPC_MXU__POOL00: decode_opc_mxu__pool00(env, ctx); break; -- 2.7.4
[Qemu-devel] [PATCH v8 12/20] target/mips: Add emulation of MXU instructions S32I2M and S32M2I
From: Craig Janeczek Add support for emulating the S32I2M and S32M2I MXU instructions. This commit also contains utility functions for reading/writing to MXU registers. This is required for overall MXU instruction support. Reviewed-by: Aleksandar Markovic Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 91 + 1 file changed, 85 insertions(+), 6 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index a98de28..8b91676 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -2569,6 +2569,36 @@ static inline void gen_store_srsgpr (int from, int to) } } +/* MXU General purpose registers moves. */ +static inline void gen_load_mxu_gpr(TCGv t, unsigned int reg) +{ +if (reg == 0) { +tcg_gen_movi_tl(t, 0); +} else if (reg <= 15) { +tcg_gen_mov_tl(t, mxu_gpr[reg - 1]); +} +} + +static inline void gen_store_mxu_gpr(TCGv t, unsigned int reg) +{ +if (reg > 0 && reg <= 15) { +tcg_gen_mov_tl(mxu_gpr[reg - 1], t); +} +} + +/* MXU control register moves. */ +static inline void gen_load_mxu_cr(TCGv t) +{ +tcg_gen_mov_tl(t, mxu_CR); +} + +static inline void gen_store_mxu_cr(TCGv t) +{ +/* TODO: Add handling of RW rules for MXU_CR. */ +tcg_gen_mov_tl(mxu_CR, t); +} + + /* Tests */ static inline void gen_save_pc(target_ulong pc) { @@ -24011,6 +24041,59 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx) /* + * S32I2M XRa, rb - Register move from GRF to XRF + */ +static void gen_mxu_s32i2m(DisasContext *ctx) +{ +TCGv t0; +uint32_t XRa, Rb; + +t0 = tcg_temp_new(); + +XRa = extract32(ctx->opcode, 6, 5); +Rb = extract32(ctx->opcode, 16, 5); + +gen_load_gpr(t0, Rb); +if (XRa <= 15) { +gen_store_mxu_gpr(t0, XRa); +} else if (XRa == 16) { +gen_store_mxu_cr(t0); +} + +tcg_temp_free(t0); +} + +/* + * S32M2I XRa, rb - Register move from XRF to GRF + */ +static void gen_mxu_s32m2i(DisasContext *ctx) +{ +TCGv t0; +uint32_t XRa, Rb; + +t0 = tcg_temp_new(); + +XRa = extract32(ctx->opcode, 6, 5); +Rb = extract32(ctx->opcode, 16, 5); + +if (XRa <= 15) { +gen_load_mxu_gpr(t0, XRa); +} else if (XRa == 16) { +gen_load_mxu_cr(t0); +} + +gen_store_gpr(t0, Rb); + +tcg_temp_free(t0); +} + + +/* + * Decoding engine for MXU + * === + */ + +/* * * Decode MXU pool00 * @@ -25089,14 +25172,10 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx) generate_exception_end(ctx, EXCP_RI); break; case OPC_MXU_S32M2I: -/* TODO: Implement emulation of S32M2I instruction. */ -MIPS_INVAL("OPC_MXU_S32M2I"); -generate_exception_end(ctx, EXCP_RI); +gen_mxu_s32m2i(ctx); break; case OPC_MXU_S32I2M: -/* TODO: Implement emulation of S32I2M instruction. */ -MIPS_INVAL("OPC_MXU_S32I2M"); -generate_exception_end(ctx, EXCP_RI); +gen_mxu_s32i2m(ctx); break; case OPC_MXU_D32SLL: /* TODO: Implement emulation of D32SLL instruction. */ -- 2.7.4
Re: [Qemu-devel] [RFC PATCH 00/21] Trace updates and plugin RFC
Pavel Dovgalyuk writes: >> From: Alex Bennée [mailto:alex.ben...@linaro.org] >> Pavel Dovgalyuk writes: >> >> >> From: Alex Bennée [mailto:alex.ben...@linaro.org] >> >> Pavel Dovgalyuk writes: >> >> >> >> >> From: Alex Bennée [mailto:alex.ben...@linaro.org] >> >> >> Any serious analysis tool should allow for us to track all memory >> >> >> accesses so I think the guest_mem_before trace point should probably >> >> >> be split into guest_mem_before_store and guest_mem_after_load. We >> >> >> could go the whole hog and add potential trace points for start/end of >> >> >> all memory operations. >> >> > >> >> > I wanted to ask about memory tracing and found this one. >> >> > Is it possible to use tracepoints for capturing all memory accesses? >> >> > In our implementation we insert helpers before and after tcg >> >> > read/write operations. >> >> >> >> The current tracepoint isn't enough but yes I think we could. The first >> >> thing I need to do is de-macrofy the atomic helpers a little just to >> >> make it a bit simpler to add the before/after tracepoints. >> > >> > But memory accesses can use 'fast path' without the helpers. >> > Thus you still need inserting the new helper for that case. >> >> trace_guest_mem_before_tcg in tcg-op.c already does this but currently >> only before operations. That's why I want to split it and pass the >> load/store value register values into the helpers. > > One more question about your trace points. > In case of using trace point on every instruction execution, we may need > accessing vCPU registers (including the flags). Are they valid in such > cases? They are probably valid but the tricky bit will be doing it in a way that doesn't expose the internals of the TCG. Maybe we could exploit the GDB interface for this or come up with a named referencex API. > I'm asking, because at least i386 translation optimizes writebacks. How so? I have to admit the i386 translation code is the most opaque to me but I wouldn't have thought changing the semantics of the guests load/store operations would be a sensible idea. Of course now you mention it my thoughts about memory tracing have been influenced by nice clean RISCy load/store architectures where it's rare to have calculation ops working directly with memory. > > Pavel Dovgalyuk -- Alex Bennée
[Qemu-devel] [PATCH v8 08/20] target/mips: Add bit encoding for MXU execute add/sub pattern 'eptn2'
From: Aleksandar Markovic Add bit encoding for MXU execute 2-bit add/subtract pattern 'eptn2'. Reviewed-by: Stefan Markovic Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/target/mips/translate.c b/target/mips/translate.c index 7116697..1935796 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -23987,6 +23987,12 @@ static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx) #define MXU_APTN2_SA2 #define MXU_APTN2_SS3 +/* MXU execute add/subtract 2-bit pattern 'eptn2' */ +#define MXU_EPTN2_AA0 +#define MXU_EPTN2_AS1 +#define MXU_EPTN2_SA2 +#define MXU_EPTN2_SS3 + /* * -- 2.7.4
[Qemu-devel] [PATCH v8 20/20] target/mips: Amend MXU ASE overview note
From: Aleksandar Markovic Add prefix, suffix, operation descriptions, and other corrections and amendments to the comment that describes MXU ASE. Reviewed-by: Stefan Markovic Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 84 +++-- 1 file changed, 74 insertions(+), 10 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index ce88039..6112e72 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -1410,25 +1410,89 @@ enum { * MXU unit contains 17 registers called X0-X16. X0 is always zero, and X16 is * the control register. * - * The notation used in MXU assembler mnemonics: + * The notation used in MXU assembler mnemonics + * + * + * Registers: * * XRa, XRb, XRc, XRd - MXU registers * Rb, Rc, Rd, Rs, Rt - general purpose MIPS registers - * s12- a subfield of an instruction code - * strd2 - a subfield of an instruction code - * eptn2 - a subfield of an instruction code - * eptn3 - a subfield of an instruction code - * optn2 - a subfield of an instruction code - * optn3 - a subfield of an instruction code - * sft4 - a subfield of an instruction code + * + * Subfields: + * + * aptn1 - 1-bit accumulate add/subtract pattern + * aptn2 - 2-bit accumulate add/subtract pattern + * eptn2 - 2-bit execute add/subtract pattern + * optn2 - 2-bit operand pattern + * optn3 - 3-bit operand pattern + * sft4 - 4-bit shift amount + * strd2 - 2-bit stride amount + * + * Prefixes: + * + * + * S 32 + * D 16 + * Q 8 + * + * Suffixes: + * + * E - Expand results + * F - Fixed point multiplication + * L - Low part result + * R - Doing rounding + * V - Variable instead of immediate + * W - Combine above L and V + * + * Operations: + * + * ADD - Add or subtract + * ADDC - Add with carry-in + * ACC - Accumulate + * ASUM - Sum together then accumulate (add or subtract) + * ASUMC - Sum together then accumulate (add or subtract) with carry-in + * AVG - Average between 2 operands + * ABD - Absolute difference + * ALN - Align data + * AND - Logical bitwise 'and' operation + * CPS - Copy sign + * EXTR - Extract bits + * I2M - Move from GPR register to MXU register + * LDD - Load data from memory to XRF + * LDI - Load data from memory to XRF (and increase the address base) + * LUI - Load unsigned immediate + * MUL - Multiply + * MULU - Unsigned multiply + * MADD - 64-bit operand add 32x32 product + * MSUB - 64-bit operand subtract 32x32 product + * MAC - Multiply and accumulate (add or subtract) + * MAD - Multiply and add or subtract + * MAX - Maximum between 2 operands + * MIN - Minimum between 2 operands + * M2I - Move from MXU register to GPR register + * MOVZ - Move if zero + * MOVN - Move if non-zero + * NOR - Logical bitwise 'nor' operation + * OR- Logical bitwise 'or' operation + * STD - Store data from XRF to memory + * SDI - Store data from XRF to memory (and increase the address base) + * SLT - Set of less than comparison + * SAD - Sum of absolute differences + * SLL - Logical shift left + * SLR - Logical shift right + * SAR - Arithmetic shift right + * SAT - Saturation + * SFL - Shuffle + * SCOP - Calculate x’s scope (-1, means x<0; 0, means x==0; 1, means x>0) + * XOR - Logical bitwise 'exclusive or' operation * * Load/Store instructions Multiplication instructions * --- --- * * S32LDD XRa, Rb, s12 S32MADD XRa, XRd, Rs, Rt * S32STD XRa, Rb, s12 S32MADDU XRa, XRd, Rs, Rt - * S32LDDV XRa, Rb, rc, strd2S32SUB XRa, XRd, Rs, Rt - * S32STDV XRa, Rb, rc, strd2S32SUBU XRa, XRd, Rs, Rt + * S32LDDV XRa, Rb, rc, strd2S32MSUB XRa, XRd, Rs, Rt + * S32STDV XRa, Rb, rc, strd2S32MSUBU XRa, XRd, Rs, Rt * S32LDI XRa, Rb, s12 S32MUL XRa, XRd, Rs, Rt * S32SDI XRa, Rb, s12 S32MULU XRa, XRd, Rs, Rt * S32LDIV XRa, Rb, rc, strd2D16MUL XRa, XRb, XRc, XRd, optn2 -- 2.7.4
[Qemu-devel] [PATCH v8 14/20] target/mips: Add emulation of MXU instruction S8LDD
From: Craig Janeczek Add support for emulating the S8LDD MXU instruction. Reviewed-by: Aleksandar Markovic Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 90 +++-- 1 file changed, 87 insertions(+), 3 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index fb3bfaf..b06571b 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -24087,6 +24087,92 @@ static void gen_mxu_s32m2i(DisasContext *ctx) tcg_temp_free(t0); } +/* + * S8LDD XRa, Rb, s8, optn3 - Load a byte from memory to XRF + */ +static void gen_mxu_s8ldd(DisasContext *ctx) +{ +TCGv t0, t1; +TCGLabel *l0; +uint32_t XRa, Rb, s8, optn3; + +t0 = tcg_temp_new(); +t1 = tcg_temp_new(); + +l0 = gen_new_label(); + +XRa = extract32(ctx->opcode, 6, 4); +s8 = extract32(ctx->opcode, 10, 8); +optn3 = extract32(ctx->opcode, 18, 3); +Rb = extract32(ctx->opcode, 21, 5); + +gen_load_mxu_cr(t0); +tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN); +tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0); + +gen_load_gpr(t0, Rb); +tcg_gen_addi_tl(t0, t0, (int8_t)s8); + +switch (optn3) { +/* XRa[7:0] = tmp8 */ +case MXU_OPTN3_PTN0: +tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB); +gen_load_mxu_gpr(t0, XRa); +tcg_gen_deposit_tl(t0, t0, t1, 0, 8); +break; +/* XRa[15:8] = tmp8 */ +case MXU_OPTN3_PTN1: +tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB); +gen_load_mxu_gpr(t0, XRa); +tcg_gen_deposit_tl(t0, t0, t1, 8, 8); +break; +/* XRa[23:16] = tmp8 */ +case MXU_OPTN3_PTN2: +tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB); +gen_load_mxu_gpr(t0, XRa); +tcg_gen_deposit_tl(t0, t0, t1, 16, 8); +break; +/* XRa[31:24] = tmp8 */ +case MXU_OPTN3_PTN3: +tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB); +gen_load_mxu_gpr(t0, XRa); +tcg_gen_deposit_tl(t0, t0, t1, 24, 8); +break; +/* XRa = {8'b0, tmp8, 8'b0, tmp8} */ +case MXU_OPTN3_PTN4: +tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB); +tcg_gen_deposit_tl(t0, t1, t1, 16, 16); +break; +/* XRa = {tmp8, 8'b0, tmp8, 8'b0} */ +case MXU_OPTN3_PTN5: +tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB); +tcg_gen_shli_tl(t1, t1, 8); +tcg_gen_deposit_tl(t0, t1, t1, 16, 16); +break; +/* XRa = {{8{sign of tmp8}}, tmp8, {8{sign of tmp8}}, tmp8} */ +case MXU_OPTN3_PTN6: +tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_SB); +tcg_gen_mov_tl(t0, t1); +tcg_gen_andi_tl(t0, t0, 0xFF00); +tcg_gen_shli_tl(t1, t1, 16); +tcg_gen_or_tl(t0, t0, t1); +break; +/* XRa = {tmp8, tmp8, tmp8, tmp8} */ +case MXU_OPTN3_PTN7: +tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB); +tcg_gen_deposit_tl(t1, t1, t1, 8, 8); +tcg_gen_deposit_tl(t0, t1, t1, 16, 16); +break; +} + +gen_store_mxu_gpr(t0, XRa); + +gen_set_label(l0); + +tcg_temp_free(t0); +tcg_temp_free(t1); +} + /* * Decoding engine for MXU @@ -25132,9 +25218,7 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx) generate_exception_end(ctx, EXCP_RI); break; case OPC_MXU_S8LDD: -/* TODO: Implement emulation of S8LDD instruction. */ -MIPS_INVAL("OPC_MXU_S8LDD"); -generate_exception_end(ctx, EXCP_RI); +gen_mxu_s8ldd(ctx); break; case OPC_MXU_S8STD: /* TODO: Implement emulation of S8STD instruction. */ -- 2.7.4
[Qemu-devel] [PATCH v8 13/20] target/mips: Move MUL, S32M2I, S32I2M handling out of main MXU switch
From: Aleksandar Markovic Move MUL, S32M2I, S32I2M handling out of switch. These are all instructions that do not depend on MXU_EN flag of MXU_CR. Reviewed-by: Stefan Markovic Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 41 +++-- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index 8b91676..fb3bfaf 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -24997,6 +24997,29 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx) */ uint32_t opcode = extract32(ctx->opcode, 0, 6); +if (opcode == OPC__MXU_MUL) { +uint32_t rs, rt, rd, op1; + +rs = extract32(ctx->opcode, 21, 5); +rt = extract32(ctx->opcode, 16, 5); +rd = extract32(ctx->opcode, 11, 5); +op1 = MASK_SPECIAL2(ctx->opcode); + +gen_arith(ctx, op1, rd, rs, rt); + +return; +} + +if (opcode == OPC_MXU_S32M2I) { +gen_mxu_s32m2i(ctx); +return; +} + +if (opcode == OPC_MXU_S32I2M) { +gen_mxu_s32i2m(ctx); +return; +} + switch (opcode) { case OPC_MXU_S32MADD: /* TODO: Implement emulation of S32MADD instruction. */ @@ -25008,18 +25031,6 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx) MIPS_INVAL("OPC_MXU_S32MADDU"); generate_exception_end(ctx, EXCP_RI); break; -case OPC__MXU_MUL: /* 0x2 - unused in MXU specs */ -{ -uint32_t rs, rt, rd, op1; - -rs = extract32(ctx->opcode, 21, 5); -rt = extract32(ctx->opcode, 16, 5); -rd = extract32(ctx->opcode, 11, 5); -op1 = MASK_SPECIAL2(ctx->opcode); - -gen_arith(ctx, op1, rd, rs, rt); -} -break; case OPC_MXU__POOL00: decode_opc_mxu__pool00(env, ctx); break; @@ -25171,12 +25182,6 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx) MIPS_INVAL("OPC_MXU_S16SDI"); generate_exception_end(ctx, EXCP_RI); break; -case OPC_MXU_S32M2I: -gen_mxu_s32m2i(ctx); -break; -case OPC_MXU_S32I2M: -gen_mxu_s32i2m(ctx); -break; case OPC_MXU_D32SLL: /* TODO: Implement emulation of D32SLL instruction. */ MIPS_INVAL("OPC_MXU_D32SLL"); -- 2.7.4
[Qemu-devel] [PATCH v8 19/20] target/mips: Move MXU_EN check one level higher
From: Aleksandar Markovic Move MXU_EN check to the main MXU decoding function, to avoid code repetition. Reviewed-by: Stefan Markovic Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 509 ++-- 1 file changed, 238 insertions(+), 271 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index 30c5721..ce88039 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -24093,23 +24093,16 @@ static void gen_mxu_s32m2i(DisasContext *ctx) static void gen_mxu_s8ldd(DisasContext *ctx) { TCGv t0, t1; -TCGLabel *l0; uint32_t XRa, Rb, s8, optn3; t0 = tcg_temp_new(); t1 = tcg_temp_new(); -l0 = gen_new_label(); - XRa = extract32(ctx->opcode, 6, 4); s8 = extract32(ctx->opcode, 10, 8); optn3 = extract32(ctx->opcode, 18, 3); Rb = extract32(ctx->opcode, 21, 5); -gen_load_mxu_cr(t0); -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN); -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0); - gen_load_gpr(t0, Rb); tcg_gen_addi_tl(t0, t0, (int8_t)s8); @@ -24167,8 +24160,6 @@ static void gen_mxu_s8ldd(DisasContext *ctx) gen_store_mxu_gpr(t0, XRa); -gen_set_label(l0); - tcg_temp_free(t0); tcg_temp_free(t1); } @@ -24179,7 +24170,6 @@ static void gen_mxu_s8ldd(DisasContext *ctx) static void gen_mxu_d16mul(DisasContext *ctx) { TCGv t0, t1, t2, t3; -TCGLabel *l0; uint32_t XRa, XRb, XRc, XRd, optn2; t0 = tcg_temp_new(); @@ -24187,18 +24177,12 @@ static void gen_mxu_d16mul(DisasContext *ctx) t2 = tcg_temp_new(); t3 = tcg_temp_new(); -l0 = gen_new_label(); - XRa = extract32(ctx->opcode, 6, 4); XRb = extract32(ctx->opcode, 10, 4); XRc = extract32(ctx->opcode, 14, 4); XRd = extract32(ctx->opcode, 18, 4); optn2 = extract32(ctx->opcode, 22, 2); -gen_load_mxu_cr(t0); -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN); -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0); - gen_load_mxu_gpr(t1, XRb); tcg_gen_sextract_tl(t0, t1, 0, 16); tcg_gen_sextract_tl(t1, t1, 16, 16); @@ -24227,8 +24211,6 @@ static void gen_mxu_d16mul(DisasContext *ctx) gen_store_mxu_gpr(t3, XRa); gen_store_mxu_gpr(t2, XRd); -gen_set_label(l0); - tcg_temp_free(t0); tcg_temp_free(t1); tcg_temp_free(t2); @@ -24242,7 +24224,6 @@ static void gen_mxu_d16mul(DisasContext *ctx) static void gen_mxu_d16mac(DisasContext *ctx) { TCGv t0, t1, t2, t3; -TCGLabel *l0; uint32_t XRa, XRb, XRc, XRd, optn2, aptn2; t0 = tcg_temp_new(); @@ -24250,8 +24231,6 @@ static void gen_mxu_d16mac(DisasContext *ctx) t2 = tcg_temp_new(); t3 = tcg_temp_new(); -l0 = gen_new_label(); - XRa = extract32(ctx->opcode, 6, 4); XRb = extract32(ctx->opcode, 10, 4); XRc = extract32(ctx->opcode, 14, 4); @@ -24259,10 +24238,6 @@ static void gen_mxu_d16mac(DisasContext *ctx) optn2 = extract32(ctx->opcode, 22, 2); aptn2 = extract32(ctx->opcode, 24, 2); -gen_load_mxu_cr(t0); -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN); -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0); - gen_load_mxu_gpr(t1, XRb); tcg_gen_sextract_tl(t0, t1, 0, 16); tcg_gen_sextract_tl(t1, t1, 16, 16); @@ -24313,8 +24288,6 @@ static void gen_mxu_d16mac(DisasContext *ctx) gen_store_mxu_gpr(t3, XRa); gen_store_mxu_gpr(t2, XRd); -gen_set_label(l0); - tcg_temp_free(t0); tcg_temp_free(t1); tcg_temp_free(t2); @@ -24328,7 +24301,6 @@ static void gen_mxu_d16mac(DisasContext *ctx) static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx) { TCGv t0, t1, t2, t3, t4, t5, t6, t7; -TCGLabel *l0; uint32_t XRa, XRb, XRc, XRd, sel; t0 = tcg_temp_new(); @@ -24340,18 +24312,12 @@ static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx) t6 = tcg_temp_new(); t7 = tcg_temp_new(); -l0 = gen_new_label(); - XRa = extract32(ctx->opcode, 6, 4); XRb = extract32(ctx->opcode, 10, 4); XRc = extract32(ctx->opcode, 14, 4); XRd = extract32(ctx->opcode, 18, 4); sel = extract32(ctx->opcode, 22, 2); -gen_load_mxu_cr(t0); -tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN); -tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0); - gen_load_mxu_gpr(t3, XRb); gen_load_mxu_gpr(t7, XRc); @@ -24402,8 +24368,6 @@ static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx) gen_store_mxu_gpr(t0, XRd); gen_store_mxu_gpr(t1, XRa); -gen_set_label(l0); - tcg_temp_free(t0); tcg_temp_free(t1); tcg_temp_free(t2); @@ -24421,23 +24385,16 @@ static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx) static void gen_mxu_s32ldd_s32lddr(DisasContext *ctx) { TCGv t0, t1; -TCGLabel *l0; uint32_t XRa, Rb, s12, sel; t0 = tcg_temp_new(); t1 = tcg_temp_new(); -l0 = gen_new_label(); - XRa = extract32(ctx->opcode, 6, 4); s12 = extract32(ctx->opcode, 10, 10);
[Qemu-devel] [PATCH v8 15/20] target/mips: Add emulation of MXU instruction D16MUL
From: Craig Janeczek Add support for emulating the D16MUL MXU instruction. Reviewed-by: Aleksandar Markovic Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 66 ++--- 1 file changed, 63 insertions(+), 3 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index b06571b..75607e1 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -24173,6 +24173,68 @@ static void gen_mxu_s8ldd(DisasContext *ctx) tcg_temp_free(t1); } +/* + * D16MUL XRa, XRb, XRc, XRd, optn2 - Signed 16 bit pattern multiplication + */ +static void gen_mxu_d16mul(DisasContext *ctx) +{ +TCGv t0, t1, t2, t3; +TCGLabel *l0; +uint32_t XRa, XRb, XRc, XRd, optn2; + +t0 = tcg_temp_new(); +t1 = tcg_temp_new(); +t2 = tcg_temp_new(); +t3 = tcg_temp_new(); + +l0 = gen_new_label(); + +XRa = extract32(ctx->opcode, 6, 4); +XRb = extract32(ctx->opcode, 10, 4); +XRc = extract32(ctx->opcode, 14, 4); +XRd = extract32(ctx->opcode, 18, 4); +optn2 = extract32(ctx->opcode, 22, 2); + +gen_load_mxu_cr(t0); +tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN); +tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0); + +gen_load_mxu_gpr(t1, XRb); +tcg_gen_sextract_tl(t0, t1, 0, 16); +tcg_gen_sextract_tl(t1, t1, 16, 16); +gen_load_mxu_gpr(t3, XRc); +tcg_gen_sextract_tl(t2, t3, 0, 16); +tcg_gen_sextract_tl(t3, t3, 16, 16); + +switch (optn2) { +case MXU_OPTN2_WW: /* XRB.H*XRC.H == lop, XRB.L*XRC.L == rop */ +tcg_gen_mul_tl(t3, t1, t3); +tcg_gen_mul_tl(t2, t0, t2); +break; +case MXU_OPTN2_LW: /* XRB.L*XRC.H == lop, XRB.L*XRC.L == rop */ +tcg_gen_mul_tl(t3, t0, t3); +tcg_gen_mul_tl(t2, t0, t2); +break; +case MXU_OPTN2_HW: /* XRB.H*XRC.H == lop, XRB.H*XRC.L == rop */ +tcg_gen_mul_tl(t3, t1, t3); +tcg_gen_mul_tl(t2, t1, t2); +break; +case MXU_OPTN2_XW: /* XRB.L*XRC.H == lop, XRB.H*XRC.L == rop */ +tcg_gen_mul_tl(t3, t0, t3); +tcg_gen_mul_tl(t2, t1, t2); +break; +} +gen_store_mxu_gpr(t3, XRa); +gen_store_mxu_gpr(t2, XRd); + +gen_set_label(l0); + +tcg_temp_free(t0); +tcg_temp_free(t1); +tcg_temp_free(t2); +tcg_temp_free(t3); +} + /* * Decoding engine for MXU @@ -25137,9 +25199,7 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx) decode_opc_mxu__pool02(env, ctx); break; case OPC_MXU_D16MUL: -/* TODO: Implement emulation of D16MUL instruction. */ -MIPS_INVAL("OPC_MXU_D16MUL"); -generate_exception_end(ctx, EXCP_RI); +gen_mxu_d16mul(ctx); break; case OPC_MXU__POOL03: decode_opc_mxu__pool03(env, ctx); -- 2.7.4
[Qemu-devel] [PATCH v8 17/20] target/mips: Add emulation of MXU instructions Q8MUL and Q8MULSU
From: Craig Janeczek Adds support for emulating the Q8MUL and Q8MULSU MXU instructions. Reviewed-by: Aleksandar Markovic Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 101 1 file changed, 94 insertions(+), 7 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index 031d4c2..14e19d8 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -24321,6 +24321,99 @@ static void gen_mxu_d16mac(DisasContext *ctx) tcg_temp_free(t3); } +/* + * Q8MUL XRa, XRb, XRc, XRd - Parallel unsigned 8 bit pattern multiply + * Q8MULSU XRa, XRb, XRc, XRd - Parallel signed 8 bit pattern multiply + */ +static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx) +{ +TCGv t0, t1, t2, t3, t4, t5, t6, t7; +TCGLabel *l0; +uint32_t XRa, XRb, XRc, XRd, sel; + +t0 = tcg_temp_new(); +t1 = tcg_temp_new(); +t2 = tcg_temp_new(); +t3 = tcg_temp_new(); +t4 = tcg_temp_new(); +t5 = tcg_temp_new(); +t6 = tcg_temp_new(); +t7 = tcg_temp_new(); + +l0 = gen_new_label(); + +XRa = extract32(ctx->opcode, 6, 4); +XRb = extract32(ctx->opcode, 10, 4); +XRc = extract32(ctx->opcode, 14, 4); +XRd = extract32(ctx->opcode, 18, 4); +sel = extract32(ctx->opcode, 22, 2); + +gen_load_mxu_cr(t0); +tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN); +tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0); + +gen_load_mxu_gpr(t3, XRb); +gen_load_mxu_gpr(t7, XRc); + +if (sel == 0x2) { +/* Q8MULSU */ +tcg_gen_ext8s_tl(t0, t3); +tcg_gen_shri_tl(t3, t3, 8); +tcg_gen_ext8s_tl(t1, t3); +tcg_gen_shri_tl(t3, t3, 8); +tcg_gen_ext8s_tl(t2, t3); +tcg_gen_shri_tl(t3, t3, 8); +tcg_gen_ext8s_tl(t3, t3); +} else { +/* Q8MUL */ +tcg_gen_ext8u_tl(t0, t3); +tcg_gen_shri_tl(t3, t3, 8); +tcg_gen_ext8u_tl(t1, t3); +tcg_gen_shri_tl(t3, t3, 8); +tcg_gen_ext8u_tl(t2, t3); +tcg_gen_shri_tl(t3, t3, 8); +tcg_gen_ext8u_tl(t3, t3); +} + +tcg_gen_ext8u_tl(t4, t7); +tcg_gen_shri_tl(t7, t7, 8); +tcg_gen_ext8u_tl(t5, t7); +tcg_gen_shri_tl(t7, t7, 8); +tcg_gen_ext8u_tl(t6, t7); +tcg_gen_shri_tl(t7, t7, 8); +tcg_gen_ext8u_tl(t7, t7); + +tcg_gen_mul_tl(t0, t0, t4); +tcg_gen_mul_tl(t1, t1, t5); +tcg_gen_mul_tl(t2, t2, t6); +tcg_gen_mul_tl(t3, t3, t7); + +tcg_gen_andi_tl(t0, t0, 0x); +tcg_gen_andi_tl(t1, t1, 0x); +tcg_gen_andi_tl(t2, t2, 0x); +tcg_gen_andi_tl(t3, t3, 0x); + +tcg_gen_shli_tl(t1, t1, 16); +tcg_gen_shli_tl(t3, t3, 16); + +tcg_gen_or_tl(t0, t0, t1); +tcg_gen_or_tl(t1, t2, t3); + +gen_store_mxu_gpr(t0, XRd); +gen_store_mxu_gpr(t1, XRa); + +gen_set_label(l0); + +tcg_temp_free(t0); +tcg_temp_free(t1); +tcg_temp_free(t2); +tcg_temp_free(t3); +tcg_temp_free(t4); +tcg_temp_free(t5); +tcg_temp_free(t6); +tcg_temp_free(t7); +} + /* * Decoding engine for MXU @@ -25112,14 +25205,8 @@ static void decode_opc_mxu__pool18(CPUMIPSState *env, DisasContext *ctx) switch (opcode) { case OPC_MXU_Q8MUL: -/* TODO: Implement emulation of Q8MUL instruction. */ -MIPS_INVAL("OPC_MXU_Q8MUL"); -generate_exception_end(ctx, EXCP_RI); -break; case OPC_MXU_Q8MULSU: -/* TODO: Implement emulation of Q8MULSU instruction. */ -MIPS_INVAL("OPC_MXU_Q8MULSU"); -generate_exception_end(ctx, EXCP_RI); +gen_mxu_q8mul_q8mulsu(ctx); break; default: MIPS_INVAL("decode_opc_mxu"); -- 2.7.4
[Qemu-devel] [PATCH v8 16/20] target/mips: Add emulation of MXU instruction D16MAC
From: Craig Janeczek Add support for emulating the D16MAC MXU instruction. Reviewed-by: Aleksandar Markovic Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 90 +++-- 1 file changed, 87 insertions(+), 3 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index 75607e1..031d4c2 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -24235,6 +24235,92 @@ static void gen_mxu_d16mul(DisasContext *ctx) tcg_temp_free(t3); } +/* + * D16MAC XRa, XRb, XRc, XRd, aptn2, optn2 - Signed 16 bit pattern multiply + * and accumulate + */ +static void gen_mxu_d16mac(DisasContext *ctx) +{ +TCGv t0, t1, t2, t3; +TCGLabel *l0; +uint32_t XRa, XRb, XRc, XRd, optn2, aptn2; + +t0 = tcg_temp_new(); +t1 = tcg_temp_new(); +t2 = tcg_temp_new(); +t3 = tcg_temp_new(); + +l0 = gen_new_label(); + +XRa = extract32(ctx->opcode, 6, 4); +XRb = extract32(ctx->opcode, 10, 4); +XRc = extract32(ctx->opcode, 14, 4); +XRd = extract32(ctx->opcode, 18, 4); +optn2 = extract32(ctx->opcode, 22, 2); +aptn2 = extract32(ctx->opcode, 24, 2); + +gen_load_mxu_cr(t0); +tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN); +tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0); + +gen_load_mxu_gpr(t1, XRb); +tcg_gen_sextract_tl(t0, t1, 0, 16); +tcg_gen_sextract_tl(t1, t1, 16, 16); + +gen_load_mxu_gpr(t3, XRc); +tcg_gen_sextract_tl(t2, t3, 0, 16); +tcg_gen_sextract_tl(t3, t3, 16, 16); + +switch (optn2) { +case MXU_OPTN2_WW: /* XRB.H*XRC.H == lop, XRB.L*XRC.L == rop */ +tcg_gen_mul_tl(t3, t1, t3); +tcg_gen_mul_tl(t2, t0, t2); +break; +case MXU_OPTN2_LW: /* XRB.L*XRC.H == lop, XRB.L*XRC.L == rop */ +tcg_gen_mul_tl(t3, t0, t3); +tcg_gen_mul_tl(t2, t0, t2); +break; +case MXU_OPTN2_HW: /* XRB.H*XRC.H == lop, XRB.H*XRC.L == rop */ +tcg_gen_mul_tl(t3, t1, t3); +tcg_gen_mul_tl(t2, t1, t2); +break; +case MXU_OPTN2_XW: /* XRB.L*XRC.H == lop, XRB.H*XRC.L == rop */ +tcg_gen_mul_tl(t3, t0, t3); +tcg_gen_mul_tl(t2, t1, t2); +break; +} +gen_load_mxu_gpr(t0, XRa); +gen_load_mxu_gpr(t1, XRd); + +switch (aptn2) { +case MXU_APTN2_AA: +tcg_gen_add_tl(t3, t0, t3); +tcg_gen_add_tl(t2, t1, t2); +break; +case MXU_APTN2_AS: +tcg_gen_add_tl(t3, t0, t3); +tcg_gen_sub_tl(t2, t1, t2); +break; +case MXU_APTN2_SA: +tcg_gen_sub_tl(t3, t0, t3); +tcg_gen_add_tl(t2, t1, t2); +break; +case MXU_APTN2_SS: +tcg_gen_sub_tl(t3, t0, t3); +tcg_gen_sub_tl(t2, t1, t2); +break; +} +gen_store_mxu_gpr(t3, XRa); +gen_store_mxu_gpr(t2, XRd); + +gen_set_label(l0); + +tcg_temp_free(t0); +tcg_temp_free(t1); +tcg_temp_free(t2); +tcg_temp_free(t3); +} + /* * Decoding engine for MXU @@ -25205,9 +25291,7 @@ static void decode_opc_mxu(CPUMIPSState *env, DisasContext *ctx) decode_opc_mxu__pool03(env, ctx); break; case OPC_MXU_D16MAC: -/* TODO: Implement emulation of D16MAC instruction. */ -MIPS_INVAL("OPC_MXU_D16MAC"); -generate_exception_end(ctx, EXCP_RI); +gen_mxu_d16mac(ctx); break; case OPC_MXU_D16MACF: /* TODO: Implement emulation of D16MACF instruction. */ -- 2.7.4
[Qemu-devel] [PATCH v8 18/20] target/mips: Add emulation of MXU instructions S32LDD and S32LDDR
From: Craig Janeczek Add support for emulating the S32LDD and S32LDDR MXU instructions. Reviewed-by: Aleksandar Markovic Signed-off-by: Craig Janeczek Signed-off-by: Aleksandar Markovic --- target/mips/translate.c | 54 ++--- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index 14e19d8..30c5721 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -24414,6 +24414,52 @@ static void gen_mxu_q8mul_q8mulsu(DisasContext *ctx) tcg_temp_free(t7); } +/* + * S32LDD XRa, Rb, S12 - Load a word from memory to XRF + * S32LDDR XRa, Rb, S12 - Load a word from memory to XRF, reversed byte seq. + */ +static void gen_mxu_s32ldd_s32lddr(DisasContext *ctx) +{ +TCGv t0, t1; +TCGLabel *l0; +uint32_t XRa, Rb, s12, sel; + +t0 = tcg_temp_new(); +t1 = tcg_temp_new(); + +l0 = gen_new_label(); + +XRa = extract32(ctx->opcode, 6, 4); +s12 = extract32(ctx->opcode, 10, 10); +sel = extract32(ctx->opcode, 20, 1); +Rb = extract32(ctx->opcode, 21, 5); + +gen_load_mxu_cr(t0); +tcg_gen_andi_tl(t0, t0, MXU_CR_MXU_EN); +tcg_gen_brcondi_tl(TCG_COND_NE, t0, MXU_CR_MXU_EN, l0); + +gen_load_gpr(t0, Rb); + +tcg_gen_movi_tl(t1, s12); +tcg_gen_shli_tl(t1, t1, 2); +if (s12 & 0x200) { +tcg_gen_ori_tl(t1, t1, 0xF000); +} +tcg_gen_add_tl(t1, t0, t1); +tcg_gen_qemu_ld_tl(t1, t1, ctx->mem_idx, MO_SL); + +if (sel == 1) { +/* S32LDDR */ +tcg_gen_bswap32_tl(t1, t1); +} +gen_store_mxu_gpr(t1, XRa); + +gen_set_label(l0); + +tcg_temp_free(t0); +tcg_temp_free(t1); +} + /* * Decoding engine for MXU @@ -24643,14 +24689,8 @@ static void decode_opc_mxu__pool04(CPUMIPSState *env, DisasContext *ctx) switch (opcode) { case OPC_MXU_S32LDD: -/* TODO: Implement emulation of S32LDD instruction. */ -MIPS_INVAL("OPC_MXU_S32LDD"); -generate_exception_end(ctx, EXCP_RI); -break; case OPC_MXU_S32LDDR: -/* TODO: Implement emulation of S32LDDR instruction. */ -MIPS_INVAL("OPC_MXU_S32LDDR"); -generate_exception_end(ctx, EXCP_RI); +gen_mxu_s32ldd_s32lddr(ctx); break; default: MIPS_INVAL("decode_opc_mxu"); -- 2.7.4
Re: [Qemu-devel] [PATCH 3/6] char: add a QEMU_CHAR_FEATURE_GCONTEXT flag
Hi On Wed, Oct 10, 2018 at 7:54 AM Peter Xu wrote: > > On Tue, Oct 09, 2018 at 05:12:48PM +0400, Marc-André Lureau wrote: > > The feature should be set if the chardev is able to switch > > GMainContext. Callers that want to put a chardev in a different thread > > context can/should check this capabilities. > > IIRC we've had some discussion about whether we should allow to > dynamically switch context for chardevs, and a (temporarily) > conclusion is that we'd prefer to forbidden it for simplicity > (although it's still allowed in current master). Does this patch mean > that we'd still want to allow that for some future scenarios? Currently, the API let you change context dynamically, although doing it safely is left for the caller to handle. And it's not guarantee to work with all backends: some chardev don't simply allow you to use a different gcontext, this is what this flag should be about. The flag may become obsolete if we change the API to restrict setting context at creation time. For now, I think we should consider this patch and the following one. thanks > > > > > Signed-off-by: Marc-André Lureau > > --- > > include/chardev/char.h | 3 +++ > > chardev/char.c | 11 +++ > > 2 files changed, 14 insertions(+) > > > > diff --git a/include/chardev/char.h b/include/chardev/char.h > > index 7becd8c80c..014566c3de 100644 > > --- a/include/chardev/char.h > > +++ b/include/chardev/char.h > > @@ -47,6 +47,9 @@ typedef enum { > > QEMU_CHAR_FEATURE_FD_PASS, > > /* Whether replay or record mode is enabled */ > > QEMU_CHAR_FEATURE_REPLAY, > > +/* Whether the gcontext can be changed after calling > > + * qemu_chr_be_update_read_handlers() */ > > +QEMU_CHAR_FEATURE_GCONTEXT, > > > > QEMU_CHAR_FEATURE_LAST, > > } ChardevFeature; > > diff --git a/chardev/char.c b/chardev/char.c > > index e115166995..fa1b74e0d9 100644 > > --- a/chardev/char.c > > +++ b/chardev/char.c > > @@ -196,6 +196,8 @@ void qemu_chr_be_update_read_handlers(Chardev *s, > > s->gcontext = context; > > if (cc->chr_update_read_handler) { > > cc->chr_update_read_handler(s); > > +} else if (s->gcontext) { > > +error_report("switching context isn't supported by this chardev"); > > } > > } > > > > @@ -240,6 +242,15 @@ static void char_init(Object *obj) > > > > chr->logfd = -1; > > qemu_mutex_init(&chr->chr_write_lock); > > + > > +/* > > + * Assume if chr_update_read_handler is implemented it will > > + * take the updated gcontext into account. > > + */ > > +if (CHARDEV_GET_CLASS(chr)->chr_update_read_handler) { > > +qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT); > > +} > > + > > } > > > > static int null_chr_write(Chardev *chr, const uint8_t *buf, int len) > > -- > > 2.19.0.271.gfe8321ec05 > > > > Regards, > > -- > Peter Xu > -- Marc-André Lureau
Re: [Qemu-devel] [PATCH v8 00/20] target/mips: Add limited support for Ingenic's MXU ASE
> From: Aleksandar Markovic > Subject: [PATCH v8 00/20] target/mips: Add limited support for Ingenic's MXU > ASE > > From: Aleksandar Markovic > > This patch set begins to add MXU ASE instruction support. > Craig, do you have any comments/suggestions on this series? Regards, Aleksandar > v7->v8: > > - corrected several mistakes in MXU ASE overview note > - add several clarifying comments > - rebased to the latest code > > v6->v7: > > - move MXU_EN check to the main MXU decoding function > - amend MXU ASE overview note > > v5->v6: > > - added bit definitions for 'aptn1' and 'eptn2'. > - pool04 eliminated, since it is covered by a single instruction. > - moved MUL, S32M2I, S32I2M handling out of main MXU switch. > - rebased to the latest code (this series applies on top of > the current MIPS pull request) > > v4->v5: > > - added full decoding engine for MXU ASE > - changes on aptn2, optn2, optn3 are now stand-alone patches > - all patches on individual instructions are reworked to fit > new decoding engine, and also cosmetically improved > - rebased to the latest code > > Aleksandar Markovic (8): > target/mips: Amend MXU instruction opcodes > target/mips: Add and integrate MXU decoding engine placeholder > target/mips: Add MXU decoding engine > target/mips: Add bit encoding for MXU accumulate add/sub 1-bit pattern > 'aptn1' > target/mips: Add bit encoding for MXU execute add/sub pattern 'eptn2' > target/mips: Move MUL, S32M2I, S32I2M handling out of main MXU switch > target/mips: Move MXU_EN check one level higher > target/mips: Amend MXU ASE overview note > > Craig Janeczek (12): > target/mips: Introduce MXU registers > target/mips: Define a bit for MXU in insn_flags > target/mips: Add bit encoding for MXU accumulate add/sub 2-bit pattern > 'aptn2' > target/mips: Add bit encoding for MXU operand getting pattern 'optn2' > target/mips: Add bit encoding for MXU operand getting pattern 'optn3' > target/mips: Add emulation of non-MXU MULL within MXU decoding engine > target/mips: Add emulation of MXU instructions S32I2M and S32M2I > target/mips: Add emulation of MXU instruction S8LDD > target/mips: Add emulation of MXU instruction D16MUL > target/mips: Add emulation of MXU instruction D16MAC > target/mips: Add emulation of MXU instructions Q8MUL and Q8MULSU > target/mips: Add emulation of MXU instructions S32LDD and S32LDDR > > target/mips/cpu.h | 10 + > target/mips/mips-defs.h |1 + > target/mips/translate.c | 2104 > ++- > 3 files changed, 1896 insertions(+), 219 deletions(-) > > -- > 2.7.4
[Qemu-devel] qemu3.0.0: Linux on non x86 CPUs run x86 executable
Hi, everyone. When I have installed the QEMU3.0.0 in the Linux on non X86 CPUS, I want to use user space emulator to run X86 executable. So I get the document from the QEMU web page (QEMU document) . I find the section 5.3 and read the content. But I have some questions in the following: (1)I can't understand the content: "On non x86 CPUs, you need first to download at least an x86 glibc (qemu-runtime-i386-XXX-.tar.gz on the QEMU web page). Ensure that LD_LIBRARY_PATH is not set: unset LD_LIBRARY_PATH". How I get the X86 glibc? And How to use it? Compiling and installing it in the Linux on non X86 CPUS or not? (2) I also haven't understood the content:"Then you can launch the precompiled ls x86 executable: qemu-i386 tests/i386/ls". The file of "ls x86 executable" is in the any Linux from the path "/bin/ls" on the X86 CPUS. Is that right? Please help me to resolve these questions. Thanks very much.
Re: [Qemu-devel] [PATCH 2/6] monitor: accept chardev input from iothread
Hi On Wed, Oct 10, 2018 at 7:45 AM Peter Xu wrote: > > On Tue, Oct 09, 2018 at 05:12:47PM +0400, Marc-André Lureau wrote: > > Chardev backends may not handle safely IO events from concurrent > > threads. Better to wake up the chardev from the monitor IO thread if > > it's being used as the chardev context. > > > > Signed-off-by: Marc-André Lureau > > --- > > monitor.c | 12 ++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/monitor.c b/monitor.c > > index ab60c9f87e..a25514490a 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -4297,6 +4297,13 @@ int monitor_suspend(Monitor *mon) > > return 0; > > } > > > > +static void monitor_accept_input(void *opaque) > > +{ > > +Monitor *mon = opaque; > > + > > +qemu_chr_fe_accept_input(&mon->chr); > > +} > > + > > void monitor_resume(Monitor *mon) > > { > > if (monitor_is_hmp_non_interactive(mon)) { > > @@ -4310,13 +4317,14 @@ void monitor_resume(Monitor *mon) > > * let's kick the thread in case it's sleeping. > > */ > > if (mon->use_io_thread) { > > -aio_notify(iothread_get_aio_context(mon_iothread)); > > + > > aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread), > > +monitor_accept_input, mon); > > Just to make sure: this will definitely cover the previous > aio_notify(), am I right? > > Imho some comment would always be nice here because QMPs with > use_io_thread=true seems special anyway. > > > } > > } else { > > assert(mon->rs); > > readline_show_prompt(mon->rs); > > +monitor_accept_input(mon); > > } > > -qemu_chr_fe_accept_input(&mon->chr); > > How about the QMP monitors with oob=off? Will it miss the call? good catch > > I would consider caching the aio context into Monitor struct when > monitor init, then call aio_bh_schedule_oneshot() always with the > per-monitor aio cache. This could unify the code paths too so we keep > the oob special path as less as possible. Saving the aio context isn't really necessary. However, I'll unify the code paths in v2. thanks > > > } > > trace_monitor_suspend(mon, -1); > > } > > -- > > 2.19.0.271.gfe8321ec05 > > > > Regards, > > -- > Peter Xu > -- Marc-André Lureau
Re: [Qemu-devel] [PATCH] target/mips: Support Toshiba specific three-operand MADD and MADDU
> > From: Aleksandar Markovic > Subject: Re: [PATCH] target/mips: Support Toshiba specific three-operand MADD > and MADDU > > > From: Richard Henderson > > Sent: Tuesday, October 16, 2018 8:37 PM > > Subject: Re: [PATCH] target/mips: Support Toshiba specific three-operand > > MADD and > MADDU > > > > On 10/16/18 11:19 AM, Fredrik Noring wrote: > > > /* global register indices */ > > > static TCGv cpu_gpr[32], cpu_PC; > > > static TCGv cpu_HI[MIPS_DSP_ACC], cpu_LO[MIPS_DSP_ACC]; > > > > > > One option is to create a new array such as > > > > > > static TCGv_i64 mmi_gpr[32]; > > > > > > that represents the upper 64 bits of each GPR. Then cpu_gpr must be of > > > a 64-bit type too, even when QEMU runs in 32-bit user mode. The R5900 > > > does not implement CP0.Status.UX in hardware, though, so system mode is > > > 64 bits, regardless. > > > > I would not implement r5900 for mips32 in that case, > > I would implement it only for TARGET_MIPS64. > > > > r~ > > Hello, Richard. > > I truly need your help here. As you can conclude from the discussion, R5900 > folks > (anybody correct me if I am wrong) have some problems using any ABI other > than O32. > (For example, the standard gcc switch are "-mabi=32 -march=r5900".) Other > similar CPUs, > for example R4000, are built with TARGET_MIPS64, both user and system mode. > R5900 > would not have TARGET_MIPS64 in such arrangement. This looks outlandish to > me. Given > that R5900 is a 64-bit MIPS III-like processor, is there any anomaly in this > arrangement, or this should work and is OK? > > Thanks, > Aleksandar > Guys, Without TARGET_MIPS64, we can't say we emulate R5900 - we are emulating some other CPU that never existed. Convince me that I am wrong. Aleksandar
Re: [Qemu-devel] [PULL 00/20] Trivial patches patches
On 26 October 2018 at 16:31, Laurent Vivier wrote: > The following changes since commit 808ebd66e467f77c0d1f8c6346235f81e9c99cf2: > > Merge remote-tracking branch 'remotes/riscv/tags/riscv-for-master-3.1-sf0' > into staging (2018-10-25 17:41:03 +0100) > > are available in the Git repository at: > > git://github.com/vivier/qemu.git tags/trivial-patches-pull-request > > for you to fetch changes up to 4b03da6e87c34793137a231b558231fd406c05e8: > > ppc: move at24c to its own CONFIG_ symbol (2018-10-26 17:17:32 +0200) > > > QEMU trivial patches collected between June and October 2018 > (Thank you to Thomas Huth) > > Hi; I get a compile error due to a format string issue: In file included from /home/petmay01/qemu-for-merges/hw/net/milkymist-minimac2.c:33:0: /home/petmay01/qemu-for-merges/hw/net/milkymist-minimac2.c: In function 'minimac2_write': /home/petmay01/qemu-for-merges/hw/net/milkymist-minimac2.c:420:23: error: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'uint64_t {aka long long unsign ed int}' [-Werror=format=] "milkymist_minimac2_wr%d: 0x%" HWADDR_PRIx " = 0x%lx\n", ^ /home/petmay01/qemu-for-merges/include/qemu/log.h:85:22: note: in definition of macro 'qemu_lo g_mask' qemu_log(FMT, ## __VA_ARGS__); \ ^ cc1: all warnings being treated as errors (affects windows crossbuilds, OSX and 32-bit hosts). thanks -- PMM
Re: [Qemu-devel] [PATCH 4/6] monitor: check if chardev can switch gcontext for OOB
Hi On Wed, Oct 10, 2018 at 8:38 AM Peter Xu wrote: > > On Tue, Oct 09, 2018 at 05:12:49PM +0400, Marc-André Lureau wrote: > > Note: this patch will conflict with Peter "[PATCH v9 3/6] monitor: > > remove "x-oob", turn oob on by default", but can be trivially updated. > > > > Signed-off-by: Marc-André Lureau > > --- > > monitor.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/monitor.c b/monitor.c > > index a25514490a..c175cf6f0d 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -4550,9 +4550,10 @@ void monitor_init(Chardev *chr, int flags) > > bool use_oob = flags & MONITOR_USE_OOB; > > > > if (use_oob) { > > -if (CHARDEV_IS_MUX(chr)) { > > +if (!qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT)) { > > error_report("Monitor out-of-band is not supported with " > > - "MUX typed chardev backend"); > > + "%s typed chardev backend", > > + object_get_typename(OBJECT(chr))); > > This seems a bit confusing to me at the first glance since we forbid > mux because not all frontends are ready to run outside main loop (and > now we have mon_iothread so it'll be odd too to run anything > non-monitor on that too...), rather than whether the backend can > dynamically switch its context. > > I'm not sure, but do you mean you want to disable oob for backends > like spice or braille? I just noticed that it seems even legal if we > pipe a qmp monitor with a windows mouse... yes, basically any chardev that doesn't handle the gcontext argument. They match those that don't implement chr_update_read_handler at this point. > I believe in all cases the commit message can be enhanced on > explaining "why" of this patch. :) ok, I'll keep the explicit mux case, even if it overlaps with gcontext feature, and update commit message. thanks > > Regards, > > -- > Peter Xu > -- Marc-André Lureau
[Qemu-devel] a64 simd decode in handle_vec_simd_shli()
Hi; Coverity is complaining (in CID 1396476) about a problem in the handle_vec_simd_shli() function, where we might dereference sli_op[] with a size that's greater than 3. It thinks size might be > 3 because we do a check if (size > 3 && !is_q) { unallocated_encoding(s); return; } suggesting that we could have is_q and size > 3. I'm having difficulty figuring out where this check has come from; it doesn't seem to match up with the pseudocode and in any case I don't think size can ever be > 3. We calculate: int size = 32 - clz32(immh) - 1; where immh is a 4 bit field which we know cannot be all-zeroes. So the clz32() return must be in {28,29,30,31} and the resulting size is in {0,1,2,3}, so the check above can't ever fire. Am I missing something? As far as I can see we should simply delete the can't-happen condition, which will probably satisfy coverity. thanks -- PMM
Re: [Qemu-devel] [PATCH 6/9] qom/object: set globals when initializing object
On Wed, 12 Sep 2018 16:55:28 +0400 Marc-André Lureau wrote: > Set globals for all objects, although only TYPE_DEVICE & > TYPE_USER_CREATABLE can have globals for now. > > Signed-off-by: Marc-André Lureau > --- > hw/core/qdev.c | 6 -- > qom/object.c | 2 ++ > 2 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 473060b551..28c6c8d7c9 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -974,11 +974,6 @@ static void device_initfn(Object *obj) > QLIST_INIT(&dev->gpios); > } > > -static void device_post_init(Object *obj) > -{ > -object_property_set_globals(obj); > -} > - > /* Unlink device from bus and free the structure. */ > static void device_finalize(Object *obj) > { > @@ -1103,7 +1098,6 @@ static const TypeInfo device_type_info = { > .parent = TYPE_OBJECT, > .instance_size = sizeof(DeviceState), > .instance_init = device_initfn, > -.instance_post_init = device_post_init, > .instance_finalize = device_finalize, > .class_base_init = device_class_base_init, > .class_init = device_class_init, > diff --git a/qom/object.c b/qom/object.c > index 0703e8e4ff..025ad0e191 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -12,6 +12,7 @@ > > #include "qemu/osdep.h" > #include "qapi/error.h" > +#include "qom/globals.h" > #include "qom/object.h" > #include "qom/object_interfaces.h" > #include "qemu/cutils.h" > @@ -382,6 +383,7 @@ static void object_initialize_with_type(void *data, > size_t size, TypeImpl *type) > obj->properties = g_hash_table_new_full(g_str_hash, g_str_equal, > NULL, object_property_free); > object_init_with_type(obj, type); > +object_property_set_globals(obj); > object_post_init_with_type(obj, type); this would somewhat inverse post_init call chain, if there weren't any other instance_post_init() users that would be fine but it potentially can break arm_cpu_post_init() since globals would be set before the later finishes creating per instance properties. arm cpu feature setting code is spread across initfn/post_init/realize and it's unreadable mess. Maybe we should get rid of instance_post_init() altogether and explicitly call arm_cpu_post_init() from each initfn(). > } >
Re: [Qemu-devel] [RFC PATCH 00/21] Trace updates and plugin RFC
> From: Alex Bennée [mailto:alex.ben...@linaro.org] > Pavel Dovgalyuk writes: > > One more question about your trace points. > > In case of using trace point on every instruction execution, we may need > > accessing vCPU registers (including the flags). Are they valid in such > > cases? > > They are probably valid but the tricky bit will be doing it in a way > that doesn't expose the internals of the TCG. Maybe we could exploit the > GDB interface for this or come up with a named referencex API. > > > I'm asking, because at least i386 translation optimizes writebacks. > > How so? I have to admit the i386 translation code is the most opaque to > me but I wouldn't have thought changing the semantics of the guests > load/store operations would be a sensible idea. Writeback to the registers (say EFLAGS), not to the memory. Pavel Dovgalyuk
Re: [Qemu-devel] qemu3.0.0: Linux on non x86 CPUs run x86 executable
wj193102 writes: > Hi, everyone. >When I have installed the QEMU3.0.0 in the Linux on non X86 CPUS, I > want to use user space emulator to run X86 executable. So I get the document > from the QEMU web page (QEMU document) . > I find the section 5.3 and read the content. But I have some questions in the > following: > (1)I can't understand the content: "On non x86 CPUs, you need first to > download at least an x86 glibc (qemu-runtime-i386-XXX-.tar.gz on the QEMU web > page). Ensure that LD_LIBRARY_PATH is not set: > unset LD_LIBRARY_PATH". How I get the X86 glibc? And How to use it? >Compiling and installing it in the Linux on non X86 CPUS or >not? The usual way is to create a chroot and install some x86 distro in there so that it can provide the requisite bits and pieces. The details vary depending on what chroot you want to setup. I personally tend to use Debian as it has excellent architecture support. If you have a working docker setup you can even host your x86 userspace in a docker container which simplifies some aspects. > (2) I also haven't understood the content:"Then you can launch the > precompiled ls x86 executable: qemu-i386 tests/i386/ls". The file of > "ls x86 executable" is in the any Linux from the path "/bin/ls" on > the X86 CPUS. Is that right? Yes. The very simplest case is a statically compiled binaries as you then don't need to worry about jumping hoops to get the dynamic linker working. > Please help me to resolve these questions. Thanks very much. If you just want to prove you have a working x86 emulation you can just build the tcg test cases: cd i386-linux-user make guest-tests ./qemu-i386 ./tests/linux-test -- Alex Bennée
[Qemu-devel] Transcript from KVM Forum Contributor Q&A Panel
The transcript of the KVM Forum Contributor Q&A Panel is available here: https://etherpad.net/p/KVMForum2018Panel The topics discussed included how to get big features upstream, email/patch workflow, how to discuss ideas with the community, Spectre/Meltdown, and the perspective of cloud providers who are investing in QEMU/KVM. The panelists included Christian Borntraeger (IBM), Cornelia Huck (Red Hat), Peter Feiner (Google), Peter Maydell (qemu.git maintainer, Linaro), and Wanpeng Li (Tencent). Video will be available soon. Enjoy, Stefan
Re: [Qemu-devel] [PATCH 2/2] nvme: free cmbuf in nvme_exit
On 29/10/18 7:29, Li Qiang wrote: This avoid a memory leak in unhotplug nvme device. Signed-off-by: Li Qiang Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 359a06d0ad..09d7c90259 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1332,6 +1332,9 @@ static void nvme_exit(PCIDevice *pci_dev) g_free(n->cq); g_free(n->sq); +if (n->cmb_size_mb) { +g_free(n->cmbuf); +} msix_uninit_exclusive_bar(pci_dev); }
Re: [Qemu-devel] a64 simd decode in handle_vec_simd_shli()
On 10/29/18 12:06 PM, Peter Maydell wrote: > I'm having difficulty figuring out where this check has come from; > it doesn't seem to match up with the pseudocode and in any case > I don't think size can ever be > 3. We calculate: > > int size = 32 - clz32(immh) - 1; > where immh is a 4 bit field which we know cannot be all-zeroes. > So the clz32() return must be in {28,29,30,31} and the resulting > size is in {0,1,2,3}, so the check above can't ever fire. Correct. The check appeared with the initial commit for aa64 support, so perhaps Alex just trying to be defensive in his coding? > Am I missing something? As far as I can see we should simply delete > the can't-happen condition, which will probably satisfy coverity. Agreed. r~
Re: [Qemu-devel] a64 simd decode in handle_vec_simd_shli()
On 29/10/18 13:06, Peter Maydell wrote: Hi; Coverity is complaining (in CID 1396476) about a problem in the handle_vec_simd_shli() function, where we might dereference sli_op[] with a size that's greater than 3. It thinks size might be > 3 because we do a check if (size > 3 && !is_q) { unallocated_encoding(s); return; } suggesting that we could have is_q and size > 3. I'm having difficulty figuring out where this check has come from; it doesn't seem to match up with the pseudocode and in any case I don't think size can ever be > 3. We calculate: int size = 32 - clz32(immh) - 1; where immh is a 4 bit field which we know cannot be all-zeroes. So the clz32() return must be in {28,29,30,31} and the resulting size is in {0,1,2,3}, so the check above can't ever fire. Clang was emitting the same warning in memory_region_oldmmio_read_accessor() before you remove it: tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr); Am I missing something? As far as I can see we should simply delete the can't-happen condition, which will probably satisfy coverity. thanks -- PMM
Re: [Qemu-devel] a64 simd decode in handle_vec_simd_shli()
On 29 October 2018 at 12:32, Richard Henderson wrote: > On 10/29/18 12:06 PM, Peter Maydell wrote: >> I'm having difficulty figuring out where this check has come from; >> it doesn't seem to match up with the pseudocode and in any case >> I don't think size can ever be > 3. We calculate: >> >> int size = 32 - clz32(immh) - 1; >> where immh is a 4 bit field which we know cannot be all-zeroes. >> So the clz32() return must be in {28,29,30,31} and the resulting >> size is in {0,1,2,3}, so the check above can't ever fire. > > Correct. > > The check appeared with the initial commit for aa64 support, so perhaps Alex > just trying to be defensive in his coding? > >> Am I missing something? As far as I can see we should simply delete >> the can't-happen condition, which will probably satisfy coverity. > > Agreed. As assert that size is in [0..3] would probably be reasonable too, since the thing that requires it (that the immh field is 4-bit non-zero) is something implicitly determined by the decode so it's a bit action-at-a-distance. I'll put together a patch. thanks -- PMM
[Qemu-devel] [PULL 0/2] Vga 20181029 patches
The following changes since commit b312532fd03413d0e6ae6767ec793a3e30f487b8: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-10-19 19:01:07 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/vga-20181029-pull-request for you to fetch changes up to e69a10f468d8f6aa6c00a4308f5a8e1f2fd6b3a1: vga_int: remove unused function protype (2018-10-29 10:43:48 +0100) vga: two fixes. Gerd Hoffmann (1): qxl: store channel id in qxl->id yuchenlin (1): vga_int: remove unused function protype hw/display/qxl.h | 1 + hw/display/vga_int.h | 1 - hw/display/qxl.c | 19 --- 3 files changed, 13 insertions(+), 8 deletions(-) -- 2.9.3
[Qemu-devel] [PULL 2/2] vga_int: remove unused function protype
From: yuchenlin Signed-off-by: yuchenlin Reviewed-by: Philippe Mathieu-Daudé Message-id: 20181022080053.9379-1-yuchen...@synology.com Signed-off-by: Gerd Hoffmann --- hw/display/vga_int.h | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 6e4fa48a79..55c418eab5 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -166,7 +166,6 @@ MemoryRegion *vga_init_io(VGACommonState *s, Object *obj, const MemoryRegionPortio **vbe_ports); void vga_common_reset(VGACommonState *s); -void vga_sync_dirty_bitmap(VGACommonState *s); void vga_dirty_log_start(VGACommonState *s); void vga_dirty_log_stop(VGACommonState *s); -- 2.9.3
[Qemu-devel] [PULL 1/2] qxl: store channel id in qxl->id
See qemu_spice_add_display_interface(), the console index is also used as channel id. So put that into the qxl->id field too. In typical use cases (one primary qxl-vga device, optionally one or more secondary qxl devices, no non-qxl display devices) this doesn't change anything. With this in place the qxl->id can not be used any more to figure whenever a given device is primary (with vga compat mode) or secondary. So add a bool to track this. Cc: spice-de...@lists.freedesktop.org Signed-off-by: Gerd Hoffmann Message-id: 20181012114540.27829-1-kra...@redhat.com --- hw/display/qxl.h | 1 + hw/display/qxl.c | 19 --- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/hw/display/qxl.h b/hw/display/qxl.h index dd9c0522b7..6f9d1f21fa 100644 --- a/hw/display/qxl.h +++ b/hw/display/qxl.h @@ -34,6 +34,7 @@ typedef struct PCIQXLDevice { PortioList vga_port_list; SimpleSpiceDisplay ssd; intid; +bool have_vga; uint32_t debug; uint32_t guestdebug; uint32_t cmdlog; diff --git a/hw/display/qxl.c b/hw/display/qxl.c index f608abc769..9087db5dee 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -848,7 +848,7 @@ static int interface_get_cursor_command(QXLInstance *sin, struct QXLCommandExt * qxl->guest_primary.commands++; qxl_track_command(qxl, ext); qxl_log_command(qxl, "csr", ext); -if (qxl->id == 0) { +if (qxl->have_vga) { qxl_render_cursor(qxl, ext); } trace_qxl_ring_cursor_get(qxl->id, qxl_mode_to_string(qxl->mode)); @@ -1255,7 +1255,7 @@ static void qxl_soft_reset(PCIQXLDevice *d) d->current_async = QXL_UNDEFINED_IO; qemu_mutex_unlock(&d->async_lock); -if (d->id == 0) { +if (d->have_vga) { qxl_enter_vga_mode(d); } else { d->mode = QXL_MODE_UNDEFINED; @@ -2139,7 +2139,7 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error **errp) memory_region_init_io(&qxl->io_bar, OBJECT(qxl), &qxl_io_ops, qxl, "qxl-ioports", io_size); -if (qxl->id == 0) { +if (qxl->have_vga) { vga_dirty_log_start(&qxl->vga); } memory_region_set_flush_coalesced(&qxl->io_bar); @@ -2171,7 +2171,7 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error **errp) /* print pci bar details */ dprint(qxl, 1, "ram/%s: %" PRId64 " MB [region 0]\n", - qxl->id == 0 ? "pri" : "sec", qxl->vga.vram_size / MiB); + qxl->have_vga ? "pri" : "sec", qxl->vga.vram_size / MiB); dprint(qxl, 1, "vram/32: %" PRIx64 " MB [region 1]\n", qxl->vram32_size / MiB); dprint(qxl, 1, "vram/64: %" PRIx64 " MB %s\n", @@ -2199,7 +2199,6 @@ static void qxl_realize_primary(PCIDevice *dev, Error **errp) VGACommonState *vga = &qxl->vga; Error *local_err = NULL; -qxl->id = 0; qxl_init_ramsize(qxl); vga->vbe_size = qxl->vgamem_size; vga->vram_size_mb = qxl->vga.vram_size / MiB; @@ -2210,8 +2209,15 @@ static void qxl_realize_primary(PCIDevice *dev, Error **errp) vga, "vga"); portio_list_set_flush_coalesced(&qxl->vga_port_list); portio_list_add(&qxl->vga_port_list, pci_address_space_io(dev), 0x3b0); +qxl->have_vga = true; vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl); +qxl->id = qemu_console_get_index(vga->con); /* == channel_id */ +if (qxl->id != 0) { +error_setg(errp, "primary qxl-vga device must be console 0 " + "(first display device on the command line)"); +return; +} qxl_realize_common(qxl, &local_err); if (local_err) { @@ -2226,15 +2232,14 @@ static void qxl_realize_primary(PCIDevice *dev, Error **errp) static void qxl_realize_secondary(PCIDevice *dev, Error **errp) { -static int device_id = 1; PCIQXLDevice *qxl = PCI_QXL(dev); -qxl->id = device_id++; qxl_init_ramsize(qxl); memory_region_init_ram(&qxl->vga.vram, OBJECT(dev), "qxl.vgavram", qxl->vga.vram_size, &error_fatal); qxl->vga.vram_ptr = memory_region_get_ram_ptr(&qxl->vga.vram); qxl->vga.con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl); +qxl->id = qemu_console_get_index(qxl->vga.con); /* == channel_id */ qxl_realize_common(qxl, errp); } -- 2.9.3
Re: [Qemu-devel] [PATCH] target/mips: Add two missing breaks for NM_LLWPE and NM_SCWPE decoder cases
On 29.10.18. 12:15, Aleksandar Markovic wrote: > From: Aleksandar Markovic > > Coverity found two fallthroughs that lack break statements. Fix them. > > Signed-off-by: Aleksandar Markovic > --- > target/mips/translate.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Stefan Markovic > diff --git a/target/mips/translate.c b/target/mips/translate.c > index b8ace0b..813ad19 100644 > --- a/target/mips/translate.c > +++ b/target/mips/translate.c > @@ -21402,6 +21402,7 @@ static int decode_nanomips_32_48_opc(CPUMIPSState > *env, DisasContext *ctx) > check_eva(ctx); > check_cp0_enabled(ctx); > gen_llwp(ctx, rs, 0, rt, extract32(ctx->opcode, 3, > 5)); > +break; > default: > generate_exception_end(ctx, EXCP_RI); > break; > @@ -21420,6 +21421,7 @@ static int decode_nanomips_32_48_opc(CPUMIPSState > *env, DisasContext *ctx) > check_eva(ctx); > check_cp0_enabled(ctx); > gen_scwp(ctx, rs, 0, rt, extract32(ctx->opcode, 3, > 5)); > +break; > default: > generate_exception_end(ctx, EXCP_RI); > break;
[Qemu-devel] [PATCH v2 0/6] monitor: misc fixes
Hi, Here is a small series of fixes for the monitor, mostly related to threading issues. v2: after Peter review - patch 2: fix resuming with oob=off - patch 4: keep MUX case explicit, improve commit message Marc-André Lureau (6): monitor: inline ambiguous helper functions monitor: accept chardev input from iothread char: add a QEMU_CHAR_FEATURE_GCONTEXT flag monitor: check if chardev can switch gcontext for OOB monitor: prevent inserting new monitors after cleanup monitor: avoid potential dead-lock when cleaning up include/chardev/char.h | 3 +++ chardev/char.c | 11 + monitor.c | 51 +++--- 3 files changed, 47 insertions(+), 18 deletions(-) -- 2.19.0.271.gfe8321ec05
[Qemu-devel] [PATCH v2 3/6] char: add a QEMU_CHAR_FEATURE_GCONTEXT flag
The feature should be set if the chardev is able to switch GMainContext. Callers that want to put a chardev in a different thread context can/should check this capabilities. Signed-off-by: Marc-André Lureau --- include/chardev/char.h | 3 +++ chardev/char.c | 11 +++ 2 files changed, 14 insertions(+) diff --git a/include/chardev/char.h b/include/chardev/char.h index 7becd8c80c..014566c3de 100644 --- a/include/chardev/char.h +++ b/include/chardev/char.h @@ -47,6 +47,9 @@ typedef enum { QEMU_CHAR_FEATURE_FD_PASS, /* Whether replay or record mode is enabled */ QEMU_CHAR_FEATURE_REPLAY, +/* Whether the gcontext can be changed after calling + * qemu_chr_be_update_read_handlers() */ +QEMU_CHAR_FEATURE_GCONTEXT, QEMU_CHAR_FEATURE_LAST, } ChardevFeature; diff --git a/chardev/char.c b/chardev/char.c index 7f07a1bfbd..b5ee58b7d2 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -196,6 +196,8 @@ void qemu_chr_be_update_read_handlers(Chardev *s, s->gcontext = context; if (cc->chr_update_read_handler) { cc->chr_update_read_handler(s); +} else if (s->gcontext) { +error_report("switching context isn't supported by this chardev"); } } @@ -240,6 +242,15 @@ static void char_init(Object *obj) chr->logfd = -1; qemu_mutex_init(&chr->chr_write_lock); + +/* + * Assume if chr_update_read_handler is implemented it will + * take the updated gcontext into account. + */ +if (CHARDEV_GET_CLASS(chr)->chr_update_read_handler) { +qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT); +} + } static int null_chr_write(Chardev *chr, const uint8_t *buf, int len) -- 2.19.0.271.gfe8321ec05
[Qemu-devel] [PATCH v2 2/6] monitor: accept chardev input from iothread
Chardev backends may not handle safely IO events from concurrent threads. Better to wake up the chardev from the monitor IO thread if it's being used as the chardev context. Unify code paths by using a BH in all cases. Drop the now redundant aio_notify() call. Signed-off-by: Marc-André Lureau --- monitor.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/monitor.c b/monitor.c index 07712d89f9..511dd11d1c 100644 --- a/monitor.c +++ b/monitor.c @@ -4304,6 +4304,13 @@ int monitor_suspend(Monitor *mon) return 0; } +static void monitor_accept_input(void *opaque) +{ +Monitor *mon = opaque; + +qemu_chr_fe_accept_input(&mon->chr); +} + void monitor_resume(Monitor *mon) { if (monitor_is_hmp_non_interactive(mon)) { @@ -4311,20 +4318,24 @@ void monitor_resume(Monitor *mon) } if (atomic_dec_fetch(&mon->suspend_cnt) == 0) { +AioContext *ctx = qemu_get_aio_context(); + if (monitor_is_qmp(mon)) { /* * For QMP monitors that are running in the I/O thread, * let's kick the thread in case it's sleeping. */ if (mon->use_io_thread) { -aio_notify(iothread_get_aio_context(mon_iothread)); +ctx = iothread_get_aio_context(mon_iothread); } } else { assert(mon->rs); readline_show_prompt(mon->rs); } -qemu_chr_fe_accept_input(&mon->chr); + +aio_bh_schedule_oneshot(ctx, monitor_accept_input, mon); } + trace_monitor_suspend(mon, -1); } -- 2.19.0.271.gfe8321ec05
[Qemu-devel] [PATCH v2 1/6] monitor: inline ambiguous helper functions
The function were not named with "mon_iothread", or following the AIO vs GMainContext distinction. Inline them instead. Signed-off-by: Marc-André Lureau --- monitor.c | 14 ++ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/monitor.c b/monitor.c index 823b5a1099..07712d89f9 100644 --- a/monitor.c +++ b/monitor.c @@ -4448,16 +4448,6 @@ static void sortcmdlist(void) qsort((void *)info_cmds, array_num, elem_size, compare_mon_cmd); } -static GMainContext *monitor_get_io_context(void) -{ -return iothread_get_g_main_context(mon_iothread); -} - -static AioContext *monitor_get_aio_context(void) -{ -return iothread_get_aio_context(mon_iothread); -} - static void monitor_iothread_init(void) { mon_iothread = iothread_create("mon_iothread", &error_abort); @@ -4545,7 +4535,7 @@ static void monitor_qmp_setup_handlers_bh(void *opaque) GMainContext *context; assert(mon->use_io_thread); -context = monitor_get_io_context(); +context = iothread_get_g_main_context(mon_iothread); assert(context); qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, monitor_qmp_read, monitor_qmp_event, NULL, mon, context, true); @@ -4597,7 +4587,7 @@ void monitor_init(Chardev *chr, int flags) * since chardev might be running in the monitor I/O * thread. Schedule a bottom half. */ -aio_bh_schedule_oneshot(monitor_get_aio_context(), +aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread), monitor_qmp_setup_handlers_bh, mon); /* The bottom half will add @mon to @mon_list */ return; -- 2.19.0.271.gfe8321ec05
[Qemu-devel] [PATCH v2 6/6] monitor: avoid potential dead-lock when cleaning up
When a monitor is connected to a Spice chardev, the monitor cleanup can dead-lock: #0 0x7f43446637fd in __lll_lock_wait () at /lib64/libpthread.so.0 #1 0x7f434465ccf4 in pthread_mutex_lock () at /lib64/libpthread.so.0 #2 0x556dd79f22ba in qemu_mutex_lock_impl (mutex=0x556dd81c9220 , file=0x556dd7ae3648 "/home/elmarco/src/qq/monitor.c", line=645) at /home/elmarco/src/qq/util/qemu-thread-posix.c:66 #3 0x556dd7431bd5 in monitor_qapi_event_queue (event=QAPI_EVENT_SPICE_DISCONNECTED, qdict=0x556dd9abc850, errp=0x7fffb7bbddd8) at /home/elmarco/src/qq/monitor.c:645 #4 0x556dd79d476b in qapi_event_send_spice_disconnected (server=0x556dd98ee760, client=0x556ddaaa8560, errp=0x556dd82180d0 ) at qapi/qapi-events-ui.c:149 #5 0x556dd7870fc1 in channel_event (event=3, info=0x556ddad1b590) at /home/elmarco/src/qq/ui/spice-core.c:235 #6 0x7f434560a6bb in reds_handle_channel_event (reds=, event=3, info=0x556ddad1b590) at reds.c:316 #7 0x7f43455f393b in main_dispatcher_self_handle_channel_event (info=0x556ddad1b590, event=3, self=0x556dd9a7d8c0) at main-dispatcher.c:197 #8 0x7f43455f393b in main_dispatcher_channel_event (self=0x556dd9a7d8c0, event=event@entry=3, info=0x556ddad1b590) at main-dispatcher.c:197 #9 0x7f4345612833 in red_stream_push_channel_event (s=s@entry=0x556ddae2ef40, event=event@entry=3) at red-stream.c:414 #10 0x7f434561286b in red_stream_free (s=0x556ddae2ef40) at red-stream.c:388 #11 0x7f43455f9ddc in red_channel_client_finalize (object=0x556dd9bb21a0) at red-channel-client.c:347 #12 0x7f434b5f9fb9 in g_object_unref () at /lib64/libgobject-2.0.so.0 #13 0x7f43455fc212 in red_channel_client_push (rcc=0x556dd9bb21a0) at red-channel-client.c:1341 #14 0x556dd76081ba in spice_port_set_fe_open (chr=0x556dd9925e20, fe_open=0) at /home/elmarco/src/qq/chardev/spice.c:241 #15 0x556dd796d74a in qemu_chr_fe_set_open (be=0x556dd9a37c00, fe_open=0) at /home/elmarco/src/qq/chardev/char-fe.c:340 #16 0x556dd796d4d9 in qemu_chr_fe_set_handlers (b=0x556dd9a37c00, fd_can_read=0x0, fd_read=0x0, fd_event=0x0, be_change=0x0, opaque=0x0, context=0x0, set_open=true) at /home/elmarco/src/qq/chardev/char-fe.c:280 #17 0x556dd796d359 in qemu_chr_fe_deinit (b=0x556dd9a37c00, del=false) at /home/elmarco/src/qq/chardev/char-fe.c:233 #18 0x556dd7432240 in monitor_data_destroy (mon=0x556dd9a37c00) at /home/elmarco/src/qq/monitor.c:786 #19 0x556dd743b968 in monitor_cleanup () at /home/elmarco/src/qq/monitor.c:4683 #20 0x556dd75ce776 in main (argc=3, argv=0x7fffb7bbe458, envp=0x7fffb7bbe478) at /home/elmarco/src/qq/vl.c:4660 Because spice code tries to emit a "disconnected" signal on the monitors. Fix this dead-lock by releasing the monitor lock for flush/destroy. Signed-off-by: Marc-André Lureau --- monitor.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/monitor.c b/monitor.c index 7fe89daa87..b55e604a98 100644 --- a/monitor.c +++ b/monitor.c @@ -4643,8 +4643,10 @@ void monitor_cleanup(void) monitor_destroyed = true; QTAILQ_FOREACH_SAFE(mon, &mon_list, entry, next) { QTAILQ_REMOVE(&mon_list, mon, entry); +qemu_mutex_unlock(&monitor_lock); monitor_flush(mon); monitor_data_destroy(mon); +qemu_mutex_lock(&monitor_lock); g_free(mon); } qemu_mutex_unlock(&monitor_lock); -- 2.19.0.271.gfe8321ec05
[Qemu-devel] [PATCH RFC v6 5/7] migration: fix the multifd code when receiving less channels
In our current code, when multifd is used during migration, if there is an error before the destination receives all new channels, the source keeps running, however the destination does not exit but keeps waiting until the source is killed deliberately. Fix this by simply killing the destination when it fails to receive packet via some channel. Cc: Dr. David Alan Gilbert Cc: Peter Xu Signed-off-by: Fei Li --- migration/channel.c | 7 ++- migration/migration.c | 9 +++-- migration/migration.h | 2 +- migration/ram.c | 17 ++--- migration/ram.h | 2 +- 5 files changed, 29 insertions(+), 8 deletions(-) diff --git a/migration/channel.c b/migration/channel.c index 33e0e9b82f..572be4245a 100644 --- a/migration/channel.c +++ b/migration/channel.c @@ -44,7 +44,12 @@ void migration_channel_process_incoming(QIOChannel *ioc) error_report_err(local_err); } } else { -migration_ioc_process_incoming(ioc); +Error *local_err = NULL; +migration_ioc_process_incoming(ioc, &local_err); +if (local_err) { +error_report_err(local_err); +exit(EXIT_FAILURE); +} } } diff --git a/migration/migration.c b/migration/migration.c index 8b36e7f184..87dfc7374f 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -541,7 +541,7 @@ void migration_fd_process_incoming(QEMUFile *f) migration_incoming_process(); } -void migration_ioc_process_incoming(QIOChannel *ioc) +void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp) { MigrationIncomingState *mis = migration_incoming_get_current(); bool start_migration; @@ -563,9 +563,14 @@ void migration_ioc_process_incoming(QIOChannel *ioc) */ start_migration = !migrate_use_multifd(); } else { +Error *local_err = NULL; /* Multiple connections */ assert(migrate_use_multifd()); -start_migration = multifd_recv_new_channel(ioc); +start_migration = multifd_recv_new_channel(ioc, &local_err); +if (local_err) { +error_propagate(errp, local_err); +return; +} } if (start_migration) { diff --git a/migration/migration.h b/migration/migration.h index f7813f8261..7df4d426d0 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -229,7 +229,7 @@ struct MigrationState void migrate_set_state(int *state, int old_state, int new_state); void migration_fd_process_incoming(QEMUFile *f); -void migration_ioc_process_incoming(QIOChannel *ioc); +void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp); void migration_incoming_process(void); bool migration_has_all_channels(void); diff --git a/migration/ram.c b/migration/ram.c index 4db3b3e8f4..8f03afe228 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1072,6 +1072,7 @@ out: static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque) { MultiFDSendParams *p = opaque; +MigrationState *s = migrate_get_current(); QIOChannel *sioc = QIO_CHANNEL(qio_task_get_source(task)); Error *local_err = NULL; @@ -1080,6 +1081,7 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque) } if (qio_task_propagate_error(task, &local_err)) { +migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED); if (multifd_save_cleanup(&local_err) != 0) { migrate_set_error(migrate_get_current(), local_err); } @@ -1337,16 +1339,20 @@ bool multifd_recv_all_channels_created(void) } /* Return true if multifd is ready for the migration, otherwise false */ -bool multifd_recv_new_channel(QIOChannel *ioc) +bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp) { +MigrationIncomingState *mis = migration_incoming_get_current(); MultiFDRecvParams *p; Error *local_err = NULL; int id; id = multifd_recv_initial_packet(ioc, &local_err); if (id < 0) { +error_propagate_prepend(errp, local_err, +"failed to receive packet via multifd channel %x: ", +multifd_recv_state->count); multifd_recv_terminate_threads(local_err, false); -return false; +goto fail; } p = &multifd_recv_state->params[id]; @@ -1354,7 +1360,8 @@ bool multifd_recv_new_channel(QIOChannel *ioc) error_setg(&local_err, "multifd: received id '%d' already setup'", id); multifd_recv_terminate_threads(local_err, true); -return false; +error_propagate(errp, local_err); +goto fail; } p->c = ioc; object_ref(OBJECT(ioc)); @@ -1366,6 +1373,10 @@ bool multifd_recv_new_channel(QIOChannel *ioc) QEMU_THREAD_JOINABLE); atomic_inc(&multifd_recv_state->count); return multifd_recv_state->count == migrate_multifd_channels(); +fail: +qemu_fclose(mis->from_src_file); +mis->from_src_file = NULL; +
[Qemu-devel] [PATCH RFC v6 1/7] Fix segmentation fault when qemu_signal_init fails
When qemu_signal_init() fails in qemu_init_main_loop(), we return without setting an error. Its callers crash then when they try to report the error with error_report_err(). To avoid such segmentation fault, add a new Error parameter to make the call trace to propagate the err to the final caller. Cc: Markus Armbruster Cc: Fam Zheng Signed-off-by: Fei Li Reviewed-by: Fam Zheng --- util/main-loop.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/util/main-loop.c b/util/main-loop.c index affe0403c5..443cb4cfe8 100644 --- a/util/main-loop.c +++ b/util/main-loop.c @@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque) } } -static int qemu_signal_init(void) +static int qemu_signal_init(Error **errp) { int sigfd; sigset_t set; @@ -96,7 +96,7 @@ static int qemu_signal_init(void) sigdelset(&set, SIG_IPI); sigfd = qemu_signalfd(&set); if (sigfd == -1) { -fprintf(stderr, "failed to create signalfd\n"); +error_setg_errno(errp, errno, "failed to create signalfd"); return -errno; } @@ -109,7 +109,7 @@ static int qemu_signal_init(void) #else /* _WIN32 */ -static int qemu_signal_init(void) +static int qemu_signal_init(Error **errp) { return 0; } @@ -148,7 +148,7 @@ int qemu_init_main_loop(Error **errp) init_clocks(qemu_timer_notify_cb); -ret = qemu_signal_init(); +ret = qemu_signal_init(errp); if (ret) { return ret; } -- 2.13.7
[Qemu-devel] [PATCH v3] milkymist-minimac2: Use qemu_log_mask(GUEST_ERROR) instead of error_report
qemu_log_mask(GUEST_ERROR) is more appropriate: $ qemu -d help Log items (comma separated): guest_errorslog when the guest OS does something invalid (eg accessing a non-existent register) Signed-off-by: Philippe Mathieu-Daudé Acked-by: Michael Walle --- v3: Fixed format string (Peter Maydell) hw/net/milkymist-minimac2.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/hw/net/milkymist-minimac2.c b/hw/net/milkymist-minimac2.c index 7ef1daee41..85c9fc0b65 100644 --- a/hw/net/milkymist-minimac2.c +++ b/hw/net/milkymist-minimac2.c @@ -30,6 +30,7 @@ #include "hw/sysbus.h" #include "trace.h" #include "net/net.h" +#include "qemu/log.h" #include "qemu/error-report.h" #include @@ -214,7 +215,8 @@ static size_t assemble_frame(uint8_t *buf, size_t size, uint32_t crc; if (size < payload_size + 12) { -error_report("milkymist_minimac2: received too big ethernet frame"); +qemu_log_mask(LOG_GUEST_ERROR, "milkymist_minimac2: frame too big " + "(%zd bytes)\n", payload_size); return 0; } @@ -347,8 +349,9 @@ minimac2_read(void *opaque, hwaddr addr, unsigned size) break; default: -error_report("milkymist_minimac2: read access to unknown register 0x" -TARGET_FMT_plx, addr << 2); +qemu_log_mask(LOG_GUEST_ERROR, + "milkymist_minimac2_rd%d: 0x%" HWADDR_PRIx "\n", + size, addr << 2); break; } @@ -413,8 +416,10 @@ minimac2_write(void *opaque, hwaddr addr, uint64_t value, break; default: -error_report("milkymist_minimac2: write access to unknown register 0x" -TARGET_FMT_plx, addr << 2); +qemu_log_mask(LOG_GUEST_ERROR, + "milkymist_minimac2_wr%d: 0x%" HWADDR_PRIx + " = 0x%" PRIx64 "\n", + size, addr << 2, value); break; } } -- 2.19.1
[Qemu-devel] [PATCH RFC v6 0/7] qemu_thread_create: propagate errors to callers to check
Hi, This idea comes from BiteSizedTasks, and this patch series implement the error checking of qemu_thread_create: make qemu_thread_create return a flag to indicate if it succeeded rather than failing with an error; make all callers check it. The first and the third patch fixes some segmentation faults occured during the debugging. The second patch paves the way for the last patch as that patch is too long. The last patch modifies the qemu_thread_create() and makes it return a bool to all direct callers to indicate if it succeeds. The middle three fixes some migration issues. Please help to review, thanks. :) v6: - Add a new migration-multifd related patch. BTW, delete the previous vnc related patch as it has been upstreamed. - Use error_setg_errno() to set the errno when qemu_thread_create() fails for both Linux and Windows implementation. - Optimize the first patch, less codes are needed v5: - Remove `errno = err` in qemu_thread_create() for Linux, and change `return errno` to `return -1` in qemu_signal_init() to indicate the error in case qemu_thread_create() fails. - Delete the v4-added qemu_cond/mutex_destroy() in iothread_complete() as the destroy() will be done by its callers' object_unref(). v4: - Separate the migration compression patch from this series - Add one more error handling patch related with migration - Add more cleaning up code for touched functions v3: - Add two migration related patches to fix the segmentaion fault - Extract the segmentation fault fix from v2's last patch to be a separate patch - Add cleaning up code for touched functions - Update some error messages v2: - Pass errp straightly instead of using a local_err & error_propagate - Return a bool: false/true to indicate if one function succeeds - Merge v1's last two patches into one to avoid the compile error - Fix one omitted error in patch1 and update some error messages Fei Li (7): Fix segmentation fault when qemu_signal_init fails qemu_init_vcpu: add a new Error parameter to propagate qemu_thread_join: fix segmentation fault migration: fix some segmentation faults when using multifd migration: fix the multifd code when receiving less channels migration: fix some error handling qemu_thread_create: propagate the error to callers to handle accel/tcg/user-exec-stub.c | 3 +- cpus.c | 79 +- dump.c | 6 ++- hw/misc/edu.c | 6 ++- hw/ppc/spapr_hcall.c| 10 - hw/rdma/rdma_backend.c | 4 +- hw/usb/ccid-card-emulated.c | 13 -- include/qemu/thread.h | 4 +- include/qom/cpu.h | 2 +- io/task.c | 3 +- iothread.c | 16 --- migration/channel.c | 7 +++- migration/migration.c | 66 ++--- migration/migration.h | 2 +- migration/postcopy-ram.c| 17 +++- migration/ram.c | 93 ++--- migration/ram.h | 4 +- migration/savevm.c | 11 +++-- target/alpha/cpu.c | 4 +- target/arm/cpu.c| 4 +- target/cris/cpu.c | 4 +- target/hppa/cpu.c | 4 +- target/i386/cpu.c | 4 +- target/lm32/cpu.c | 4 +- target/m68k/cpu.c | 4 +- target/microblaze/cpu.c | 4 +- target/mips/cpu.c | 4 +- target/moxie/cpu.c | 4 +- target/nios2/cpu.c | 4 +- target/openrisc/cpu.c | 4 +- target/ppc/translate_init.inc.c | 4 +- target/riscv/cpu.c | 4 +- target/s390x/cpu.c | 4 +- target/sh4/cpu.c| 4 +- target/sparc/cpu.c | 4 +- target/tilegx/cpu.c | 4 +- target/tricore/cpu.c| 4 +- target/unicore32/cpu.c | 4 +- target/xtensa/cpu.c | 4 +- tests/atomic_add-bench.c| 3 +- tests/iothread.c| 2 +- tests/qht-bench.c | 3 +- tests/rcutorture.c | 3 +- tests/test-aio.c| 2 +- tests/test-rcu-list.c | 3 +- ui/vnc-jobs.c | 17 +--- ui/vnc-jobs.h | 2 +- ui/vnc.c| 4 +- util/compatfd.c | 12 +- util/main-loop.c| 8 ++-- util/oslib-posix.c | 17 ++-- util/qemu-thread-posix.c| 27 util/qemu-thread-win32.c| 18 +--- util/rcu.c | 3 +- util/thread-pool.c | 4 +- 55 files changed, 393 insertions(+), 165 deletions(-) -- 2.13.7
[Qemu-devel] [PATCH v2 5/6] monitor: prevent inserting new monitors after cleanup
Add a monitor_destroyed global to check if monitor_cleanup() has been already called. In this case, don't insert the new monitor in the list, but free it instead. Signed-off-by: Marc-André Lureau --- monitor.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/monitor.c b/monitor.c index fffeb27ef9..7fe89daa87 100644 --- a/monitor.c +++ b/monitor.c @@ -263,10 +263,11 @@ typedef struct QMPRequest QMPRequest; /* QMP checker flags */ #define QMP_ACCEPT_UNKNOWNS 1 -/* Protects mon_list, monitor_qapi_event_state. */ +/* Protects mon_list, monitor_qapi_event_state, monitor_destroyed. */ static QemuMutex monitor_lock; static GHashTable *monitor_qapi_event_state; static QTAILQ_HEAD(mon_list, Monitor) mon_list; +static bool monitor_destroyed; /* Protects mon_fdsets */ static QemuMutex mon_fdsets_lock; @@ -4536,8 +4537,16 @@ void error_vprintf_unless_qmp(const char *fmt, va_list ap) static void monitor_list_append(Monitor *mon) { qemu_mutex_lock(&monitor_lock); -QTAILQ_INSERT_HEAD(&mon_list, mon, entry); +if (!monitor_destroyed) { +QTAILQ_INSERT_HEAD(&mon_list, mon, entry); +mon = NULL; +} qemu_mutex_unlock(&monitor_lock); + +if (mon) { +monitor_data_destroy(mon); +g_free(mon); +} } static void monitor_qmp_setup_handlers_bh(void *opaque) @@ -4631,6 +4640,7 @@ void monitor_cleanup(void) /* Flush output buffers and destroy monitors */ qemu_mutex_lock(&monitor_lock); +monitor_destroyed = true; QTAILQ_FOREACH_SAFE(mon, &mon_list, entry, next) { QTAILQ_REMOVE(&mon_list, mon, entry); monitor_flush(mon); -- 2.19.0.271.gfe8321ec05
[Qemu-devel] [PATCH RFC v6 4/7] migration: fix some segmentation faults when using multifd
When multifd is used during migration, a segmentaion fault will occur in the source when multifd_save_cleanup() is called again if the multifd_send_state has been freed in earlier error handling. This can happen when migrate_fd_connect() fails and multifd_fd_cleanup() is called, and then multifd_new_send_channel_async() fails and multifd_save_cleanup() is called again. If the QIOChannel *c of multifd_recv_state->params[i] (p->c) is not initialized, there is no need to close the channel. Or else a segmentation fault will occur in multifd_recv_terminate_threads() when multifd_recv_initial_packet() fails. Signed-off-by: Fei Li --- migration/ram.c | 28 +--- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index 7e7deec4d8..4db3b3e8f4 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -907,6 +907,11 @@ static void multifd_send_terminate_threads(Error *err) } } +/* in case multifd_send_state has been freed earlier */ +if (!multifd_send_state) { +return; +} + for (i = 0; i < migrate_multifd_channels(); i++) { MultiFDSendParams *p = &multifd_send_state->params[i]; @@ -922,7 +927,7 @@ int multifd_save_cleanup(Error **errp) int i; int ret = 0; -if (!migrate_use_multifd()) { +if (!migrate_use_multifd() || !multifd_send_state) { return 0; } multifd_send_terminate_threads(NULL); @@ -960,7 +965,7 @@ static void multifd_send_sync_main(void) { int i; -if (!migrate_use_multifd()) { +if (!migrate_use_multifd() || !multifd_send_state) { return; } if (multifd_send_state->pages->used) { @@ -1070,6 +1075,10 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque) QIOChannel *sioc = QIO_CHANNEL(qio_task_get_source(task)); Error *local_err = NULL; +if (!multifd_send_state) { +return; +} + if (qio_task_propagate_error(task, &local_err)) { if (multifd_save_cleanup(&local_err) != 0) { migrate_set_error(migrate_get_current(), local_err); @@ -1131,7 +1140,7 @@ struct { uint64_t packet_num; } *multifd_recv_state; -static void multifd_recv_terminate_threads(Error *err) +static void multifd_recv_terminate_threads(Error *err, bool channel) { int i; @@ -1145,6 +1154,11 @@ static void multifd_recv_terminate_threads(Error *err) } } +/* in case p->c is not initialized */ +if (!channel) { +return; +} + for (i = 0; i < migrate_multifd_channels(); i++) { MultiFDRecvParams *p = &multifd_recv_state->params[i]; @@ -1166,7 +1180,7 @@ int multifd_load_cleanup(Error **errp) if (!migrate_use_multifd()) { return 0; } -multifd_recv_terminate_threads(NULL); +multifd_recv_terminate_threads(NULL, true); for (i = 0; i < migrate_multifd_channels(); i++) { MultiFDRecvParams *p = &multifd_recv_state->params[i]; @@ -1269,7 +1283,7 @@ static void *multifd_recv_thread(void *opaque) } if (local_err) { -multifd_recv_terminate_threads(local_err); +multifd_recv_terminate_threads(local_err, true); } qemu_mutex_lock(&p->mutex); p->running = false; @@ -1331,7 +1345,7 @@ bool multifd_recv_new_channel(QIOChannel *ioc) id = multifd_recv_initial_packet(ioc, &local_err); if (id < 0) { -multifd_recv_terminate_threads(local_err); +multifd_recv_terminate_threads(local_err, false); return false; } @@ -1339,7 +1353,7 @@ bool multifd_recv_new_channel(QIOChannel *ioc) if (p->c != NULL) { error_setg(&local_err, "multifd: received id '%d' already setup'", id); -multifd_recv_terminate_threads(local_err); +multifd_recv_terminate_threads(local_err, true); return false; } p->c = ioc; -- 2.13.7
[Qemu-devel] [PATCH RFC v6 2/7] qemu_init_vcpu: add a new Error parameter to propagate
This patch is to pave the way for a later patch as it is too long: "qemu_thread_create: propagate the error to callers to handle." The callers of qemu_init_vcpu() already passed the **errp to handle errors. In view of this, add a new Error parameter to all the functions called by qemu_init_vcpu() to propagate the error and let the further callers check it. Besides, make qemu_init_vcpu() return a Boolean value to let its callers know whether it succeeds. Signed-off-by: Fei Li Reviewed-by: Fam Zheng --- accel/tcg/user-exec-stub.c | 3 ++- cpus.c | 34 +- include/qom/cpu.h | 2 +- target/alpha/cpu.c | 4 +++- target/arm/cpu.c| 4 +++- target/cris/cpu.c | 4 +++- target/hppa/cpu.c | 4 +++- target/i386/cpu.c | 4 +++- target/lm32/cpu.c | 4 +++- target/m68k/cpu.c | 4 +++- target/microblaze/cpu.c | 4 +++- target/mips/cpu.c | 4 +++- target/moxie/cpu.c | 4 +++- target/nios2/cpu.c | 4 +++- target/openrisc/cpu.c | 4 +++- target/ppc/translate_init.inc.c | 4 +++- target/riscv/cpu.c | 4 +++- target/s390x/cpu.c | 4 +++- target/sh4/cpu.c| 4 +++- target/sparc/cpu.c | 4 +++- target/tilegx/cpu.c | 4 +++- target/tricore/cpu.c| 4 +++- target/unicore32/cpu.c | 4 +++- target/xtensa/cpu.c | 4 +++- 24 files changed, 87 insertions(+), 36 deletions(-) diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c index a32b4496af..f8c38a375c 100644 --- a/accel/tcg/user-exec-stub.c +++ b/accel/tcg/user-exec-stub.c @@ -10,8 +10,9 @@ void cpu_resume(CPUState *cpu) { } -void qemu_init_vcpu(CPUState *cpu) +bool qemu_init_vcpu(CPUState *cpu, Error **errp) { +return true; } /* User mode emulation does not support record/replay yet. */ diff --git a/cpus.c b/cpus.c index 3978f63d8f..ed71618e1f 100644 --- a/cpus.c +++ b/cpus.c @@ -1919,7 +1919,7 @@ void cpu_remove_sync(CPUState *cpu) /* For temporary buffers for forming a name */ #define VCPU_THREAD_NAME_SIZE 16 -static void qemu_tcg_init_vcpu(CPUState *cpu) +static void qemu_tcg_init_vcpu(CPUState *cpu, Error **errp) { char thread_name[VCPU_THREAD_NAME_SIZE]; static QemuCond *single_tcg_halt_cond; @@ -1975,7 +1975,7 @@ static void qemu_tcg_init_vcpu(CPUState *cpu) } } -static void qemu_hax_start_vcpu(CPUState *cpu) +static void qemu_hax_start_vcpu(CPUState *cpu, Error **errp) { char thread_name[VCPU_THREAD_NAME_SIZE]; @@ -1992,7 +1992,7 @@ static void qemu_hax_start_vcpu(CPUState *cpu) #endif } -static void qemu_kvm_start_vcpu(CPUState *cpu) +static void qemu_kvm_start_vcpu(CPUState *cpu, Error **errp) { char thread_name[VCPU_THREAD_NAME_SIZE]; @@ -2005,7 +2005,7 @@ static void qemu_kvm_start_vcpu(CPUState *cpu) cpu, QEMU_THREAD_JOINABLE); } -static void qemu_hvf_start_vcpu(CPUState *cpu) +static void qemu_hvf_start_vcpu(CPUState *cpu, Error **errp) { char thread_name[VCPU_THREAD_NAME_SIZE]; @@ -2023,7 +2023,7 @@ static void qemu_hvf_start_vcpu(CPUState *cpu) cpu, QEMU_THREAD_JOINABLE); } -static void qemu_whpx_start_vcpu(CPUState *cpu) +static void qemu_whpx_start_vcpu(CPUState *cpu, Error **errp) { char thread_name[VCPU_THREAD_NAME_SIZE]; @@ -2039,7 +2039,7 @@ static void qemu_whpx_start_vcpu(CPUState *cpu) #endif } -static void qemu_dummy_start_vcpu(CPUState *cpu) +static void qemu_dummy_start_vcpu(CPUState *cpu, Error **errp) { char thread_name[VCPU_THREAD_NAME_SIZE]; @@ -2052,11 +2052,12 @@ static void qemu_dummy_start_vcpu(CPUState *cpu) QEMU_THREAD_JOINABLE); } -void qemu_init_vcpu(CPUState *cpu) +bool qemu_init_vcpu(CPUState *cpu, Error **errp) { cpu->nr_cores = smp_cores; cpu->nr_threads = smp_threads; cpu->stopped = true; +Error *local_err = NULL; if (!cpu->as) { /* If the target cpu hasn't set up any address spaces itself, @@ -2067,22 +2068,29 @@ void qemu_init_vcpu(CPUState *cpu) } if (kvm_enabled()) { -qemu_kvm_start_vcpu(cpu); +qemu_kvm_start_vcpu(cpu, &local_err); } else if (hax_enabled()) { -qemu_hax_start_vcpu(cpu); +qemu_hax_start_vcpu(cpu, &local_err); } else if (hvf_enabled()) { -qemu_hvf_start_vcpu(cpu); +qemu_hvf_start_vcpu(cpu, &local_err); } else if (tcg_enabled()) { -qemu_tcg_init_vcpu(cpu); +qemu_tcg_init_vcpu(cpu, &local_err); } else if (whpx_enabled()) { -qemu_whpx_start_vcpu(cpu); +qemu_whpx_start_vcpu(cpu, &local_err); } else { -qemu_dummy_start_vcpu(cpu); +qemu_dummy_start_vcpu(cpu, &local_err); +} + +if (local_err) { +error_propagate(err
[Qemu-devel] [PATCH RFC v6 6/7] migration: fix some error handling
Add error handling for qemu_ram_foreach_migratable_block() when it fails. Always call migrate_set_error() to set the error state without relying on whether multifd_save_cleanup() succeeds. As the passed &local_err is never used in multifd_save_cleanup(), remove it. Signed-off-by: Fei Li --- migration/migration.c| 5 + migration/postcopy-ram.c | 3 +++ migration/ram.c | 7 +++ migration/ram.h | 2 +- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 87dfc7374f..3b8b7ab4f9 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1377,7 +1377,6 @@ static void migrate_fd_cleanup(void *opaque) qemu_savevm_state_cleanup(); if (s->to_dst_file) { -Error *local_err = NULL; QEMUFile *tmp; trace_migrate_fd_cleanup(); @@ -1388,9 +1387,7 @@ static void migrate_fd_cleanup(void *opaque) } qemu_mutex_lock_iothread(); -if (multifd_save_cleanup(&local_err) != 0) { -error_report_err(local_err); -} +multifd_save_cleanup(); qemu_mutex_lock(&s->qemu_file_lock); tmp = s->to_dst_file; s->to_dst_file = NULL; diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index e5c02a32c5..4ca2bc02b3 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -1117,6 +1117,9 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis) /* Mark so that we get notified of accesses to unwritten areas */ if (qemu_ram_foreach_migratable_block(ram_block_enable_notify, mis)) { +error_report("ram_block_enable_notify failed"); +close(mis->userfault_event_fd); +close(mis->userfault_fd); return -1; } diff --git a/migration/ram.c b/migration/ram.c index 8f03afe228..57cb1bee3d 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -922,7 +922,7 @@ static void multifd_send_terminate_threads(Error *err) } } -int multifd_save_cleanup(Error **errp) +int multifd_save_cleanup(void) { int i; int ret = 0; @@ -1082,9 +1082,8 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque) if (qio_task_propagate_error(task, &local_err)) { migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED); -if (multifd_save_cleanup(&local_err) != 0) { -migrate_set_error(migrate_get_current(), local_err); -} +multifd_save_cleanup(); +migrate_set_error(migrate_get_current(), local_err); } else { p->c = QIO_CHANNEL(sioc); qio_channel_set_delay(p->c, false); diff --git a/migration/ram.h b/migration/ram.h index 046d3074be..0d1215209e 100644 --- a/migration/ram.h +++ b/migration/ram.h @@ -43,7 +43,7 @@ uint64_t ram_bytes_remaining(void); uint64_t ram_bytes_total(void); int multifd_save_setup(void); -int multifd_save_cleanup(Error **errp); +int multifd_save_cleanup(void); int multifd_load_setup(void); int multifd_load_cleanup(Error **errp); bool multifd_recv_all_channels_created(void); -- 2.13.7
[Qemu-devel] [PATCH RFC v6 3/7] qemu_thread_join: fix segmentation fault
To avoid the segmentation fault in qemu_thread_join(), just directly return when the QemuThread *thread failed to be created in either qemu-thread-posix.c or qemu-thread-win32.c. Signed-off-by: Fei Li Reviewed-by: Fam Zheng --- util/qemu-thread-posix.c | 3 +++ util/qemu-thread-win32.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index dfa66ff2fb..289af4fab5 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -562,6 +562,9 @@ void *qemu_thread_join(QemuThread *thread) int err; void *ret; +if (!thread->thread) { +return NULL; +} err = pthread_join(thread->thread, &ret); if (err) { error_exit(err, __func__); diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c index 4a363ca675..1a27e1cf6f 100644 --- a/util/qemu-thread-win32.c +++ b/util/qemu-thread-win32.c @@ -366,7 +366,7 @@ void *qemu_thread_join(QemuThread *thread) HANDLE handle; data = thread->data; -if (data->mode == QEMU_THREAD_DETACHED) { +if (data == NULL || data->mode == QEMU_THREAD_DETACHED) { return NULL; } -- 2.13.7
[Qemu-devel] [PATCH v2 4/6] monitor: check if chardev can switch gcontext for OOB
Not all backends are able to switch gcontext. Those backends cannot drive a OOB monitor (the monitor would then be blocking on main thread). For example, ringbuf, spice, or more esoteric input chardevs like braille or MUX. We currently forbid MUX because not all frontends are ready to run outside main loop. Extend to add a context-switching feature check. Note: this patch will conflict with Peter "[PATCH v9 3/6] monitor: remove "x-oob", turn oob on by default", but can be trivially updated. Signed-off-by: Marc-André Lureau --- monitor.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/monitor.c b/monitor.c index 511dd11d1c..fffeb27ef9 100644 --- a/monitor.c +++ b/monitor.c @@ -4560,9 +4560,11 @@ void monitor_init(Chardev *chr, int flags) bool use_oob = flags & MONITOR_USE_OOB; if (use_oob) { -if (CHARDEV_IS_MUX(chr)) { +if (CHARDEV_IS_MUX(chr) || +!qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT)) { error_report("Monitor out-of-band is not supported with " - "MUX typed chardev backend"); + "%s typed chardev backend", + object_get_typename(OBJECT(chr))); exit(1); } if (use_readline) { -- 2.19.0.271.gfe8321ec05
[Qemu-devel] [PATCH RFC v6 7/7] qemu_thread_create: propagate the error to callers to handle
Make qemu_thread_create() return a Boolean to indicate if it succeeds rather than failing with an error. And add an Error parameter to hold the error message and let the callers handle it. Signed-off-by: Fei Li --- cpus.c | 45 ++- dump.c | 6 -- hw/misc/edu.c | 6 -- hw/ppc/spapr_hcall.c| 10 +++-- hw/rdma/rdma_backend.c | 4 +++- hw/usb/ccid-card-emulated.c | 13 include/qemu/thread.h | 4 ++-- io/task.c | 3 ++- iothread.c | 16 +- migration/migration.c | 52 + migration/postcopy-ram.c| 14 ++-- migration/ram.c | 41 ++- migration/savevm.c | 11 +++--- tests/atomic_add-bench.c| 3 ++- tests/iothread.c| 2 +- tests/qht-bench.c | 3 ++- tests/rcutorture.c | 3 ++- tests/test-aio.c| 2 +- tests/test-rcu-list.c | 3 ++- ui/vnc-jobs.c | 17 ++- ui/vnc-jobs.h | 2 +- ui/vnc.c| 4 +++- util/compatfd.c | 12 +-- util/oslib-posix.c | 17 +++ util/qemu-thread-posix.c| 24 ++--- util/qemu-thread-win32.c| 16 ++ util/rcu.c | 3 ++- util/thread-pool.c | 4 +++- 28 files changed, 240 insertions(+), 100 deletions(-) diff --git a/cpus.c b/cpus.c index ed71618e1f..0510f90e06 100644 --- a/cpus.c +++ b/cpus.c @@ -1949,15 +1949,20 @@ static void qemu_tcg_init_vcpu(CPUState *cpu, Error **errp) snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG", cpu->cpu_index); -qemu_thread_create(cpu->thread, thread_name, qemu_tcg_cpu_thread_fn, - cpu, QEMU_THREAD_JOINABLE); +if (!qemu_thread_create(cpu->thread, thread_name, +qemu_tcg_cpu_thread_fn, cpu, +QEMU_THREAD_JOINABLE, errp)) { +return; +} } else { /* share a single thread for all cpus with TCG */ snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG"); -qemu_thread_create(cpu->thread, thread_name, - qemu_tcg_rr_cpu_thread_fn, - cpu, QEMU_THREAD_JOINABLE); +if (!qemu_thread_create(cpu->thread, thread_name, +qemu_tcg_rr_cpu_thread_fn, cpu, +QEMU_THREAD_JOINABLE, errp)) { +return; +} single_tcg_halt_cond = cpu->halt_cond; single_tcg_cpu_thread = cpu->thread; @@ -1985,8 +1990,10 @@ static void qemu_hax_start_vcpu(CPUState *cpu, Error **errp) snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HAX", cpu->cpu_index); -qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn, - cpu, QEMU_THREAD_JOINABLE); +if (!qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn, +cpu, QEMU_THREAD_JOINABLE, errp)) { +return; +} #ifdef _WIN32 cpu->hThread = qemu_thread_get_handle(cpu->thread); #endif @@ -2001,8 +2008,10 @@ static void qemu_kvm_start_vcpu(CPUState *cpu, Error **errp) qemu_cond_init(cpu->halt_cond); snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM", cpu->cpu_index); -qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn, - cpu, QEMU_THREAD_JOINABLE); +if (!qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn, +cpu, QEMU_THREAD_JOINABLE, errp)) { +/* keep 'if' here in case there is further error handling logic */ +} } static void qemu_hvf_start_vcpu(CPUState *cpu, Error **errp) @@ -2019,8 +2028,10 @@ static void qemu_hvf_start_vcpu(CPUState *cpu, Error **errp) snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HVF", cpu->cpu_index); -qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn, - cpu, QEMU_THREAD_JOINABLE); +if (!qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn, +cpu, QEMU_THREAD_JOINABLE, errp)) { +/* keep 'if' here in case there is further error handling logic */ +} } static void qemu_whpx_start_vcpu(CPUState *cpu, Error **errp) @@ -2032,8 +2043,10 @@ static void qemu_whpx_start_vcpu(CPUState *cpu, Error **errp) qemu_cond_init(cpu->halt_cond); snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/WHPX", cpu->cpu_index); -qemu_thread_create(cpu->thread, thread_name, qemu_whpx_cpu_thread_fn, -
Re: [Qemu-devel] [PATCH] hw/arm/virt: Set VIRT_COMPAT_3_0 compat
On 24 October 2018 at 12:25, Andrew Jones wrote: > On Wed, Oct 24, 2018 at 10:56:02AM +0200, Eric Auger wrote: >> We are missing the VIRT_COMPAT_3_0 definition and setting. >> Let's add them. >> >> Signed-off-by: Eric Auger >> --- >> hw/arm/virt.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 9f677825f9..a2b8d8f7c2 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -1871,6 +1871,9 @@ static void virt_machine_3_1_options(MachineClass *mc) >> } >> DEFINE_VIRT_MACHINE_AS_LATEST(3, 1) >> >> +#define VIRT_COMPAT_3_0 \ >> +HW_COMPAT_3_0 >> + >> static void virt_3_0_instance_init(Object *obj) >> { >> virt_3_1_instance_init(obj); >> @@ -1879,6 +1882,7 @@ static void virt_3_0_instance_init(Object *obj) >> static void virt_machine_3_0_options(MachineClass *mc) >> { >> virt_machine_3_1_options(mc); >> +SET_MACHINE_COMPAT(mc, VIRT_COMPAT_3_0); >> } >> DEFINE_VIRT_MACHINE(3, 0) >> >> -- >> 2.17.2 >> >> > > Oops, I should have done this with the 3.1 machine type. Thanks for the > catching and patching. > > Reviewed-by: Andrew Jones Applied to target-arm.next, thanks. -- PMM
Re: [Qemu-devel] [PATCH v2] strongarm: mask off high[32:28] bits from dir and state registers
On 26 October 2018 at 08:30, P J P wrote: > From: Prasad J Pandit > > The high[32:28] bits of 'direction' and 'state' registers of > SA-1100/SA-1110 device are reserved. Setting them may lead to > OOB 's->handler[]' array access issue. Mask off [32:28] bits to > avoid it. There is no bit 32 in a 32-bit value; you mean 31. > > Reported-by: Moguofang > Signed-off-by: Prasad J Pandit > --- > hw/arm/strongarm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > Update v2: mask off high[32:28] bits > -> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg05746.html > > diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c > index ec2627374d..dd8c4b1f2e 100644 > --- a/hw/arm/strongarm.c > +++ b/hw/arm/strongarm.c > @@ -587,12 +587,12 @@ static void strongarm_gpio_write(void *opaque, hwaddr > offset, > > switch (offset) { > case GPDR:/* GPIO Pin-Direction registers */ > -s->dir = value; > +s->dir = value & 0x3f; The commit message says it's masking [31:28], but the code is masking [31:22]. The SA1110 spec suggests the commit message is correct and the code is not. thanks -- PMM
Re: [Qemu-devel] ?==?utf-8?q? ?==?utf-8?q? [PATCH 5/6] Determine the desired FPU mode
exit() error codes are taken and left over from related kernel code. Will be set to 1 in next series version. Also, appropriate error messages printing will be added. Regards, Stefan Original Message Subject: Re: [Qemu-devel] [PATCH 5/6] Determine the desired FPU mode Date: Friday, October 26, 2018 20:12 CEST From: Peter Maydell To: Stefan Markovic CC: QEMU Developers , Petar Jovanovic , Riku Voipio , Aleksandar Markovic , Aurelien Jarno , Laurent Vivier References: <1540563667-23300-1-git-send-email-stefan.marko...@rt-rk.com> <1540563667-23300-6-git-send-email-stefan.marko...@rt-rk.com> On 26 October 2018 at 15:21, Stefan Markovic wrote: > From: Stefan Markovic > > Floating-point mode is calculated from MIPS.abiflags FP ABI value > (based on kernel implementation). Illegal combinations are rejected. > > Signed-off-by: Stefan Markovic > --- > linux-user/mips/cpu_loop.c | 75 ++ > 1 file changed, 75 insertions(+) > + if ((info->fp_abi > MAX_FP_ABI && info->fp_abi != MIPS_ABI_FP_UNKNOWN) > + || (info->interp_fp_abi > MAX_FP_ABI && > + info->interp_fp_abi != MIPS_ABI_FP_UNKNOWN)) { > + fprintf(stderr, "qemu: Program and interpreter have " > + "unexpected FPU modes\n"); > + exit(137); Why are we exit()ing with a funny exit status code here? If this is a "can't happen" case, then we should assert(). If it is a "can happen if fed an odd binary" case, then we should just exit(1) as we do already in this function for an unsupported NaN mode. > + } > + > + prog_req = (info->fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req > + : fpu_reqs[info->fp_abi]; > + interp_req = (info->interp_fp_abi == MIPS_ABI_FP_UNKNOWN) ? none_req > + : fpu_reqs[info->interp_fp_abi]; > + > + prog_req.single &= interp_req.single; > + prog_req.soft &= interp_req.soft; > + prog_req.fr1 &= interp_req.fr1; > + prog_req.frdefault &= interp_req.frdefault; > + prog_req.fre &= interp_req.fre; > + > + bool cpu_has_mips_r2_r6 = env->insn_flags & ISA_MIPS32R2 || > + env->insn_flags & ISA_MIPS64R2 || > + env->insn_flags & ISA_MIPS32R6 || > + env->insn_flags & ISA_MIPS64R6; > + > + if (prog_req.fre && !prog_req.frdefault && !prog_req.fr1) { > + env->CP0_Config5 |= (1 << CP0C5_FRE); > + if (env->active_fpu.fcr0 & (1 << FCR0_FREP)) { > + env->hflags |= MIPS_HFLAG_FRE; > + } > + } else if ((prog_req.fr1 && prog_req.frdefault) || > + (prog_req.single && !prog_req.frdefault)) { > + if ((env->active_fpu.fcr0 & (1 << FCR0_F64) > + && cpu_has_mips_r2_r6) || prog_req.fr1) { > + env->CP0_Status |= (1 << CP0St_FR); > + env->hflags |= MIPS_HFLAG_F64; > + } > + } else if (!prog_req.fre && !prog_req.frdefault && > + !prog_req.fr1 && !prog_req.single && !prog_req.soft) { > + exit(137); > + } Ditto here (and we haven't printed any error message here...) thanks -- PMM
Re: [Qemu-devel] [PATCH 5/9] qom/globals: generalize object_property_set_globals()
On Wed, 12 Sep 2018 16:55:27 +0400 Marc-André Lureau wrote: > Handle calls of object_property_set_globals() with any object type, > but only apply globals to TYPE_DEVICE & TYPE_USER_CREATABLE. > > Signed-off-by: Marc-André Lureau > --- > qom/globals.c | 22 ++ > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/qom/globals.c b/qom/globals.c > index 587f4a1b5c..8664baebe0 100644 > --- a/qom/globals.c > +++ b/qom/globals.c > @@ -15,22 +15,28 @@ void object_property_register_global(GlobalProperty *prop) > > void object_property_set_globals(Object *obj) > { > -DeviceState *dev = DEVICE(obj); > GList *l; > +DeviceState *dev = (DeviceState *)object_dynamic_cast(obj, TYPE_DEVICE); > + > +if (!dev && !IS_USER_CREATABLE(obj)) { > +/* only TYPE_DEVICE and TYPE_USER_CREATABLE support globals */ > +return; > +} more dynamic casts but now imposed on every create object :( Maybe we should add ObjectClass::check/set_globals hook? It would be cheap to check and only objects we intend to work with it would be affected. On top of that hooks could be different so that device/user_creatable specifics won't be in generic code (like it's implemented here). > > for (l = global_props; l; l = l->next) { > GlobalProperty *prop = l->data; > Error *err = NULL; > > -if (object_dynamic_cast(OBJECT(dev), prop->driver) == NULL) { > +if (object_dynamic_cast(obj, prop->driver) == NULL) { > continue; > } > prop->used = true; > -object_property_parse(OBJECT(dev), prop->value, prop->property, > &err); > +object_property_parse(obj, prop->value, prop->property, &err); > if (err != NULL) { > error_prepend(&err, "can't apply global %s.%s=%s: ", >prop->driver, prop->property, prop->value); > -if (!dev->hotplugged && prop->errp) { > + > +if (dev && !dev->hotplugged && prop->errp) { > error_propagate(prop->errp, err); > } else { > assert(prop->user_provided); > @@ -56,15 +62,15 @@ int object_property_check_globals(void) > continue; > } > oc = object_class_by_name(prop->driver); > -oc = object_class_dynamic_cast(oc, TYPE_DEVICE); > -if (!oc) { > +dc = (DeviceClass *)object_class_dynamic_cast(oc, TYPE_DEVICE); > +if (!IS_USER_CREATABLE_CLASS(oc) && !dc) { > warn_report("global %s.%s has invalid class name", > prop->driver, prop->property); > ret = 1; > continue; > } > -dc = DEVICE_CLASS(oc); > -if (!dc->hotpluggable && !prop->used) { > + > +if (dc && !dc->hotpluggable) { > warn_report("global %s.%s=%s not used", > prop->driver, prop->property, prop->value); > ret = 1;
Re: [Qemu-devel] [PATCH 0/2] Deprecate the "collie" machine and Strongarm devices
On 27 October 2018 at 12:04, Guenter Roeck wrote: > On 10/26/18 3:12 AM, Peter Maydell wrote: >> Hi Guenter; there's a proposal here to deprecate (and eventually >> remove) the 'collie' board (strongarm) from QEMU. Is that one of >> the ones you're currently using in your automated testing of Linux >> kernels on QEMU? >> > > Yes. I can run the test with older versions of qemu, so it is ok for me > if it is removed (as long as that removal is not backported). Mmm, but if we have an active user who's testing them then they probably shouldn't be in the frontline of boards to remove. Which other boards do you test with mainline QEMU? thanks -- PMM
Re: [Qemu-devel] vhost-user devices work with chardev from different threads
29.10.2018, 09:46, "Marc-André Lureau" : > Hi > > On Mon, Oct 22, 2018 at 5:24 PM Yury Kotov wrote: > >> Hi, >> >> I examined vhost-user devices and found some chardev using strangeness. >> >> Is it ok, that vhost-user's set_status do sync chardev io ops from KVM >> thread? >> It seems that chardev doesn't support working with different threads. >> >> For example, I think such race is possible (two simultaneous events): >> 1) Vhost-user server was killed (main thread will handle G_IO_HUP), >> 2) Virtio-pci bus handles guest driver's set_status command. >> Some io op from vhost-user backend will fail because of 1). >> >> So, it is possible to enter tcp_chr_disconnect twice from the main thread >> and >> from KVM thread. >> >> Backtrace examples: >> >> KVM thread (handle set_status of guest): >> tcp_chr_disconnect >> tcp_chr_write (cc->chr_write) >> qemu_chr_write_buffer >> qemu_chr_write >> qemu_chr_fe_write_all >> vhost_user_write >> vhost_user_set_mem_table (hdev->vhost_ops->vhost_set_mem_table) >> vhost_dev_start >> vhost_user_blk_start (or another vhost-user device) >> vhost_user_blk_set_status (vdc->set_status) >> >> Main thread (handle vhost server disconnect): >> tcp_chr_disconnect >> tcp_chr_hup >> g_main_context_dispatch >> glib_pollfds_poll >> os_host_main_loop_wait >> main_loop_wait >> main_loop >> main >> >> Is it really a problem or do I misunderstand? > > I think you are correct, it's a problem. Are you working on a > solution? (either using the chardev lock, or perhaps relying on main > thread HUP handler only, or a combination of both approaches) So, chardev is thread safe by design? I thought it is vhost's problem... Anyway I plan to fix that. > > thanks > > -- > Marc-André Lureau
[Qemu-devel] [PULL 4/4] audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string
From: Mao Zhongyi Cc: Jan Kiszka Cc: Philippe Mathieu-Daudé Cc: Peter Maydell Signed-off-by: Mao Zhongyi Reviewed-by: Philippe Mathieu-Daudé Message-id: 20181022074050.19638-4-maozhon...@cmss.chinamobile.com Signed-off-by: Gerd Hoffmann --- include/hw/audio/wm8750.h | 1 + hw/arm/musicpal.c | 2 +- hw/audio/marvell_88w8618.c | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/hw/audio/wm8750.h b/include/hw/audio/wm8750.h index 84e7a119bb..e12cb886d1 100644 --- a/include/hw/audio/wm8750.h +++ b/include/hw/audio/wm8750.h @@ -17,6 +17,7 @@ #include "hw/hw.h" #define TYPE_WM8750 "wm8750" +#define TYPE_MV88W8618_AUDIO "mv88w8618_audio" typedef void data_req_cb(void *opaque, int free_out, int free_in); diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index ac266f9253..9648b3af44 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -1693,7 +1693,7 @@ static void musicpal_init(MachineState *machine) } wm8750_dev = i2c_create_slave(i2c, TYPE_WM8750, MP_WM_ADDR); -dev = qdev_create(NULL, "mv88w8618_audio"); +dev = qdev_create(NULL, TYPE_MV88W8618_AUDIO); s = SYS_BUS_DEVICE(dev); object_property_set_link(OBJECT(dev), OBJECT(wm8750_dev), TYPE_WM8750, NULL); diff --git a/hw/audio/marvell_88w8618.c b/hw/audio/marvell_88w8618.c index 1c7080e844..6600ab4851 100644 --- a/hw/audio/marvell_88w8618.c +++ b/hw/audio/marvell_88w8618.c @@ -39,7 +39,6 @@ #define MP_AUDIO_CLOCK_24MHZ(1 << 9) #define MP_AUDIO_MONO (1 << 14) -#define TYPE_MV88W8618_AUDIO "mv88w8618_audio" #define MV88W8618_AUDIO(obj) \ OBJECT_CHECK(mv88w8618_audio_state, (obj), TYPE_MV88W8618_AUDIO) -- 2.9.3
[Qemu-devel] [PULL 0/4] Audio 20181029 patches
The following changes since commit 285278ca785f5fa9a570927e1c0958a2ca2b2150: Merge remote-tracking branch 'remotes/famz/tags/testing-pull-request' into staging (2018-10-27 19:55:08 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/audio-20181029-pull-request for you to fetch changes up to d436d4e7a50a7c4fddc0569c2107fe5eaf0e1fbc: audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string (2018-10-29 13:50:15 +0100) audio: qom cleanups. Li Qiang (1): hw: AC97: make it more QOMconventional Mao Zhongyi (3): audio: use TYPE_WM8750 instead of a hardcoded string audio: use object link instead of qdev property to pass wm8750 reference audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string include/hw/audio/wm8750.h | 1 + hw/arm/musicpal.c | 5 +++-- hw/audio/ac97.c| 12 hw/audio/marvell_88w8618.c | 14 ++ 4 files changed, 18 insertions(+), 14 deletions(-) -- 2.9.3
[Qemu-devel] [PULL 1/4] hw: AC97: make it more QOMconventional
From: Li Qiang Signed-off-by: Li Qiang Reviewed-by: Philippe Mathieu-Daudé Message-id: 20181013060809.52496-1-liq...@163.com Signed-off-by: Gerd Hoffmann --- hw/audio/ac97.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c index 337402e9c6..d799533aa9 100644 --- a/hw/audio/ac97.c +++ b/hw/audio/ac97.c @@ -123,6 +123,10 @@ enum { #define MUTE_SHIFT 15 +#define TYPE_AC97 "AC97" +#define AC97(obj) \ +OBJECT_CHECK(AC97LinkState, (obj), TYPE_AC97) + #define REC_MASK 7 enum { REC_MIC = 0, @@ -1340,7 +1344,7 @@ static void ac97_on_reset (DeviceState *dev) static void ac97_realize(PCIDevice *dev, Error **errp) { -AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev); +AC97LinkState *s = AC97(dev); uint8_t *c = s->dev.config; /* TODO: no need to override */ @@ -1389,7 +1393,7 @@ static void ac97_realize(PCIDevice *dev, Error **errp) static void ac97_exit(PCIDevice *dev) { -AC97LinkState *s = DO_UPCAST(AC97LinkState, dev, dev); +AC97LinkState *s = AC97(dev); AUD_close_in(&s->card, s->voice_pi); AUD_close_out(&s->card, s->voice_po); @@ -1399,7 +1403,7 @@ static void ac97_exit(PCIDevice *dev) static int ac97_init (PCIBus *bus) { -pci_create_simple (bus, -1, "AC97"); +pci_create_simple(bus, -1, TYPE_AC97); return 0; } @@ -1427,7 +1431,7 @@ static void ac97_class_init (ObjectClass *klass, void *data) } static const TypeInfo ac97_info = { -.name = "AC97", +.name = TYPE_AC97, .parent= TYPE_PCI_DEVICE, .instance_size = sizeof (AC97LinkState), .class_init= ac97_class_init, -- 2.9.3
[Qemu-devel] [PATCH v5 02/11] hw/m68k: implement ADB bus support for via
From: Laurent Vivier Co-developed-by: Mark Cave-Ayland Signed-off-by: Mark Cave-Ayland Signed-off-by: Laurent Vivier --- hw/misc/mac_via.c | 190 ++ include/hw/misc/mac_via.h | 7 ++ 2 files changed, 197 insertions(+) diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c index d6d6b86e1a..0fc8d0a038 100644 --- a/hw/misc/mac_via.c +++ b/hw/misc/mac_via.c @@ -237,10 +237,16 @@ * Table 19-10 ADB transaction states */ +#define ADB_STATE_NEW 0 +#define ADB_STATE_EVEN 1 +#define ADB_STATE_ODD 2 +#define ADB_STATE_IDLE 3 + #define VIA1B_vADB_StateMask(VIA1B_vADBS1 | VIA1B_vADBS2) #define VIA1B_vADB_StateShift 4 #define VIA_TIMER_FREQ (783360) +#define VIA_ADB_POLL_FREQ 50 /* XXX: not real */ /* VIA returns time offset from Jan 1, 1904, not 1970 */ #define RTC_OFFSET 2082844800 @@ -422,6 +428,181 @@ static void via1_rtc_update(MacVIAState *m) } } +static int adb_via_poll(MacVIAState *s, int state, uint8_t *data) +{ +if (state != ADB_STATE_IDLE) { +return 0; +} + +if (s->adb_data_in_size < s->adb_data_in_index) { +return 0; +} + +if (s->adb_data_out_index != 0) { +return 0; +} + +s->adb_data_in_index = 0; +s->adb_data_out_index = 0; +s->adb_data_in_size = adb_poll(&s->adb_bus, s->adb_data_in, 0x); + +if (s->adb_data_in_size) { +*data = s->adb_data_in[s->adb_data_in_index++]; +qemu_irq_raise(s->adb_data_ready); +} + +return s->adb_data_in_size; +} + +static int adb_via_send(MacVIAState *s, int state, uint8_t data) +{ +switch (state) { +case ADB_STATE_NEW: +s->adb_data_out_index = 0; +break; +case ADB_STATE_EVEN: +if ((s->adb_data_out_index & 1) == 0) { +return 0; +} +break; +case ADB_STATE_ODD: +if (s->adb_data_out_index & 1) { +return 0; +} +break; +case ADB_STATE_IDLE: +return 0; +} + +assert(s->adb_data_out_index < sizeof(s->adb_data_out) - 1); + +s->adb_data_out[s->adb_data_out_index++] = data; +qemu_irq_raise(s->adb_data_ready); +return 1; +} + +static int adb_via_receive(MacVIAState *s, int state, uint8_t *data) +{ +switch (state) { +case ADB_STATE_NEW: +return 0; + +case ADB_STATE_EVEN: +if (s->adb_data_in_size <= 0) { +qemu_irq_raise(s->adb_data_ready); +return 0; +} + +if (s->adb_data_in_index >= s->adb_data_in_size) { +*data = 0; +qemu_irq_raise(s->adb_data_ready); +return 1; +} + +if ((s->adb_data_in_index & 1) == 0) { +return 0; +} + +break; + +case ADB_STATE_ODD: +if (s->adb_data_in_size <= 0) { +qemu_irq_raise(s->adb_data_ready); +return 0; +} + +if (s->adb_data_in_index >= s->adb_data_in_size) { +*data = 0; +qemu_irq_raise(s->adb_data_ready); +return 1; +} + +if (s->adb_data_in_index & 1) { +return 0; +} + +break; + +case ADB_STATE_IDLE: +if (s->adb_data_out_index == 0) { +return 0; +} + +s->adb_data_in_size = adb_request(&s->adb_bus, s->adb_data_in, + s->adb_data_out, + s->adb_data_out_index); +s->adb_data_out_index = 0; +s->adb_data_in_index = 0; +if (s->adb_data_in_size < 0) { +*data = 0xff; +qemu_irq_raise(s->adb_data_ready); +return -1; +} + +if (s->adb_data_in_size == 0) { +return 0; +} + +break; +} + +assert(s->adb_data_in_index < sizeof(s->adb_data_in) - 1); + +*data = s->adb_data_in[s->adb_data_in_index++]; +qemu_irq_raise(s->adb_data_ready); +if (*data == 0xff || *data == 0) { +return 0; +} +return 1; +} + +static void via1_adb_update(MacVIAState *m) +{ +MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(&m->mos6522_via1); +MOS6522State *s = MOS6522(v1s); +int state; +int ret; + +state = (s->b & VIA1B_vADB_StateMask) >> VIA1B_vADB_StateShift; + +if (s->acr & VIA1ACR_vShiftOut) { +/* output mode */ +ret = adb_via_send(m, state, s->sr); +if (ret > 0) { +s->b &= ~VIA1B_vADBInt; +} else { +s->b |= VIA1B_vADBInt; +} +} else { +/* input mode */ +ret = adb_via_receive(m, state, &s->sr); +if (ret > 0 && s->sr != 0xff) { +s->b &= ~VIA1B_vADBInt; +} else { +s->b |= VIA1B_vADBInt; +} +} +} + +static void via_adb_poll(void *opaque) +{ +MacVIAState *m = opaque; +MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(&m->mos6522_via1); +MOS6522State *s = MOS6522(v1s); +int state; + +