[PATCH 0/3] build: fix for SunOS systems.
David Carlier (3): configure: fix for SunOS based systems. exec: posix_madvise usage on SunOS. contrib: ivshmem client and server build fix for SunOS. configure | 2 +- contrib/ivshmem-client/ivshmem-client.c | 12 ++-- contrib/ivshmem-server/ivshmem-server.c | 12 ++-- exec.c | 8 4 files changed, 21 insertions(+), 13 deletions(-) -- 2.25.4
Re: Is traceroute supposed to work in user mode networking (slirp) ?
Hello, Ottavio Caruso, le mar. 14 juil. 2020 12:15:48 +0100, a ecrit: > I cannot get traceroute to work with standard udp from any of my > guests. > > $ traceroute 8.8.8.8 > traceroute to 8.8.8.8 (8.8.8.8), 64 hops max, 40 byte packets > 1 * * * That was because - libslirp was not forwarding the ttl value, thus always set to 64 hops - libslirp was not reporting icmp errors. I had a try at both, and that indeed seems to be fixing the issue: https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/48 https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/49 > Any clues? Is this intended behaviour? Any workarounds that don't > involve tap/tun/bridges? Not without updating libslirp with the abovementioned patches. Samuel
Re: [PATCH] linux-user: syscall: ioctls: support DRM_IOCTL_I915_GETPARAM
On 2020/7/14 上午2:46, Laurent Vivier wrote: >> +gparam->value = lock_user(VERIFY_WRITE, target_gparam->value, >> + sizeof(*gparam->value), 0); > > I don't think you should use directly the guest memory. > You should have something like that: > > int value; > > gparam->value = &value; > >> +if (!gparam->value) { >> +unlock_user_struct(target_gparam, arg, 0); >> +return -TARGET_EFAULT; >> +} >> + >> +ret = get_errno(safe_ioctl(fd, ie->host_cmd, gparam)); > > and then: > > put_user_s32(value, target_gparam->value); > OK, thanks. It will be better. >> + >> +unlock_user(gparam->value, target_gparam->value, >> sizeof(*gparam->value)); >> +unlock_user_struct(target_gparam, arg, 0); >> +return ret; >> +} >> + >> +static abi_long do_ioctl_drm_i915(const IOCTLEntry *ie, uint8_t *buf_temp, >> + int fd, int cmd, abi_long arg) >> +{ >> +switch (ie->host_cmd) { >> +case DRM_IOCTL_I915_GETPARAM: >> +return do_ioctl_drm_i915_getparam(ie, >> + (struct drm_i915_getparam >> *)buf_temp, >> + fd, arg); >> +default: >> +return -TARGET_ENOSYS; >> +} >> +} > > There is a better way to register a struct with convertion functions: > you might use STRUCT_SPECIAL() to declare the structure rather than > IOCTL_SPECIAL() to declare the ioctl command. > (I think STRUCT_SPECIAL() could also be used with DRM_IOCTL_VERSION) > For me, STRUCT_SPECIAL sounds good for the complex structures, but for drm_i915_getparam which is simple enough, it is not quite suitable. For me, STRUCT_SPECIAL is much suitable for DRM_IOCTL_VERSION. Welcome your additional ideas. >> >> static IOCTLEntry ioctl_entries[] = { >> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h >> index 3c261cff0e..9082f6c2bc 100644 >> --- a/linux-user/syscall_defs.h >> +++ b/linux-user/syscall_defs.h >> @@ -1170,6 +1170,9 @@ struct target_rtc_pll_info { >> /* drm ioctls */ >> #define TARGET_DRM_IOCTL_VERSION TARGET_IOWRU('d', 0x00) >> >> +/* drm i915 ioctls */ >> +#define TARGET_DRM_IOCTL_I915_GETPARAM TARGET_IOWRU('d', 0x46) > > why do you use the U variant? > > TARGET_IOWR('d', 0x46, struct target_drm_i915_getparam) > Because qemu will automatically set the size with the target structure size in syscall_init(). It'll be more easier. e.g. usb ioctls and device mapper ioctls also do like this.
Re: [PULL 0/1] bitmaps patches for 2020-07-17 [-rc1]
On Fri, 17 Jul 2020 at 16:17, Eric Blake wrote: > > The following changes since commit 151f76c689b1ff4c2c59e6d8469a0d4fe5346f55: > > Merge remote-tracking branch 'remotes/ehabkost/tags/x86-next-pull-request' > into staging (2020-07-16 21:46:18 +0100) > > are available in the Git repository at: > > https://repo.or.cz/qemu/ericb.git tags/pull-bitmaps-2020-07-17 > > for you to fetch changes up to 7cb015197b383a62f5729d2c92b1050db0185c1c: > > migration/block-dirty-bitmap: fix add_bitmaps_to_list (2020-07-17 08:18:51 > -0500) > > I had been waiting to see if I had more than one patch to bundle, but > given that we are now coming up on -rc1 and this is a bugfix, it's time > for the pull request of this in isolation. > > > bitmaps patches for 2020-07-17 > > - improve corner-case of bitmap migration > > > Vladimir Sementsov-Ogievskiy (1): > migration/block-dirty-bitmap: fix add_bitmaps_to_list Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1 for any user-visible changes. -- PMM
Re: [PATCH v2 06/11] cputlb: ensure we save the IOTLB data in case of reset
On Mon, Jul 13, 2020 at 21:04:10 +0100, Alex Bennée wrote: > Any write to a device might cause a re-arrangement of memory > triggering a TLB flush and potential re-size of the TLB invalidating > previous entries. This would cause users of qemu_plugin_get_hwaddr() > to see the warning: > > invalid use of qemu_plugin_get_hwaddr > > because of the failed tlb_lookup which should always succeed. To > prevent this we save the IOTLB data in case it is later needed by a > plugin doing a lookup. > > Signed-off-by: Alex Bennée (snip) > +/* > + * Save a potentially trashed IOTLB entry for later lookup by plugin. > + * > + * We also need to track the thread storage address because the RCU > + * cleanup that runs when we leave the critical region (the current > + * execution) is actually in a different thread. As I mentioned in the previous iteration of this series, this comment is outdated -- there is no thread storage nor RCU to worry about here. > + * This almost never fails as the memory access being instrumented > + * should have just filled the TLB. The one corner case is io_writex > + * which can cause TLB flushes and potential resizing of the TLBs > + * loosing the information we need. In those cases we need to recover > + * data from a copy of the io_tlb entry. > */ s/loosing/losing/ About the approach in this patch: it works as long as the caller is in the same vCPU thread, otherwise we'd need a seqlock to avoid races between readers and the writing vCPU. I see that qemu_plugin_get_hwaddr does not even take a vCPU index, so this should be OK -- as long as this is called only from a mem callback, it's in the same vCPU thread and it's therefore safe. With the above comments fixed, Reviewed-by: Emilio G. Cota Thanks, Emilio
Re: [PATCH v2] goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/TIME_HIGH
On 7/17/20 5:49 PM, Jessica Clarke wrote: > The specification says: > >0x00 TIME_LOW R: Get current time, then return low-order 32-bits. >0x04 TIME_HIGH R: Return high 32-bits from previous TIME_LOW read. > >... > >To read the value, the kernel must perform an IO_READ(TIME_LOW), >which returns an unsigned 32-bit value, before an IO_READ(TIME_HIGH), >which returns a signed 32-bit value, corresponding to the higher half >of the full value. > > However, we were just returning the current time for both. If the guest > is unlucky enough to read TIME_LOW and TIME_HIGH either side of an > overflow of the lower half, it will see time be in the future, before > jumping backwards on the next read, and Linux currently relies on the > atomicity guaranteed by the spec so is affected by this. Fix this > violation of the spec by caching the correct value for TIME_HIGH > whenever TIME_LOW is read, and returning that value for any TIME_HIGH > read. > > Signed-off-by: Jessica Clarke > --- > Changes since v1: > > * Add time_high to goldfish_rtc_vmstate and increment version. > > hw/rtc/goldfish_rtc.c | 17 ++--- > include/hw/rtc/goldfish_rtc.h | 1 + > 2 files changed, 15 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PULL 0/1] s390x: documentation fix
On Fri, 17 Jul 2020 at 14:56, Cornelia Huck wrote: > > The following changes since commit 151f76c689b1ff4c2c59e6d8469a0d4fe5346f55: > > Merge remote-tracking branch 'remotes/ehabkost/tags/x86-next-pull-request' > into staging (2020-07-16 21:46:18 +0100) > > are available in the Git repository at: > > https://github.com/cohuck/qemu tags/s390x-20200717 > > for you to fetch changes up to 9ece07d7a3941eeb845a6f000a191bba19557231: > > docs/s390x: fix vfio-ccw type (2020-07-17 12:56:22 +0200) > > > Fix typo in newly added documentation. > > > > Cornelia Huck (1): > docs/s390x: fix vfio-ccw type > > docs/system/s390x/vfio-ccw.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1 for any user-visible changes. -- PMM
Re: [PATCH 0/3] configure: fix forSunOS based systems
Hi David, Your git-sendemail seems broken... Usually 0/N is for the cover letter, patches are 1/N to N/N. Also patches are sent as replies to the cover (as a thread). In your series all patches are posted as new thread... On 7/18/20 3:19 PM, David CARLIER wrote: > From 63a3c9639d708a796abd958607aa6376fc9b99f2 Mon Sep 17 00:00:00 2001 > From: David Carlier > Date: Sat, 18 Jul 2020 13:27:52 +0100 > Subject: [PATCH 1/3] configure: fix for SunOS based systems. > > local directive make the configure fails on these systems. > > Signed-off-by: David Carlier > --- > configure | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/configure b/configure > index e93836aaae..5d100eba72 100755 > --- a/configure > +++ b/configure > @@ -59,7 +59,7 @@ error_exit() { > do_compiler() { > # Run the compiler, capturing its output to the log. First argument > # is compiler binary to execute. > -local compiler="$1" > +compiler="$1" > shift > if test -n "$BASH_VERSION"; then eval ' > echo >>config.log " >
Re: [PATCH v2] goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/TIME_HIGH
On Sat, 18 Jul 2020 at 15:45, Jessica Clarke wrote: > On 18 Jul 2020, at 08:42, Philippe Mathieu-Daudé wrote: > > Maybe easier to cache the whole u64, this matches RTC_ALARM_LOW / > > RTC_ALARM_HIGH pattern (goldfish_rtc_vmstate change not included): > > We could, but why waste space storing an extra 32 bits you never need? > I don't think saving all 64 bits makes it any easier to read, I'd > personally even argue it makes it slightly less obvious what's going on. You could go either way. (The original Google-written version of this device model went for store-the-whole-u64: https://android.googlesource.com/platform/external/qemu/+/refs/heads/emu-2.0-release/hw/android/goldfish/timer.c but we don't need to follow their implementation.) Since "save the high half" is the version you've written and tested, I vote we go with that :-) Reviewed-by: Peter Maydell NB: this is a migration compatibility break for the risc-v 'virt' board : up to Alistair whether that's OK or if the more awkward compat-maintaining vmstate subsection is worth the effort. thanks -- PMM
RE: [PATCH v2 4/4] net/colo: Match is-enabled probe to tracepoint
> -Original Message- > From: Roman Bolshakov > Sent: Friday, July 17, 2020 5:35 PM > To: qemu-devel@nongnu.org > Cc: Daniel P. Berrangé ; Stefan Hajnoczi > ; Cameron Esfahani ; Roman > Bolshakov ; Philippe Mathieu-Daudé > ; Zhang, Chen ; Li Zhijian > ; Jason Wang > Subject: [PATCH v2 4/4] net/colo: Match is-enabled probe to tracepoint > > Build of QEMU with dtrace fails on macOS: > > LINKx86_64-softmmu/qemu-system-x86_64 > error: probe colo_compare_miscompare doesn't exist > error: Could not register probes > ld: error creating dtrace DOF section for architecture x86_64 > > The reason of the error is explained by Adam Leventhal [1]: > > Note that is-enabled probes don't have the stability magic so I'm not > sure how things would work if only is-enabled probes were used. > > net/colo code uses is-enabled probes to determine if other probes should be > used but colo_compare_miscompare itself is not used explicitly. > Linker doesn't include the symbol and build fails. > > The issue can be resolved if is-enabled probe matches the actual trace point > that is used inside the test. Packet dump toggle is replaced with a compile- > time conditional definition. > > 1. http://markmail.org/message/6grq2ygr5nwdwsnb > > Fixes: f4b618360e ("colo-compare: add TCP, UDP, ICMP packet comparison") > Cc: Philippe Mathieu-Daudé > Cc: Cameron Esfahani > Signed-off-by: Roman Bolshakov > --- > net/colo-compare.c| 42 ++ > net/filter-rewriter.c | 10 -- > net/trace-events | 2 -- > 3 files changed, 30 insertions(+), 24 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c index > 398b7546ff..e0be98e494 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -54,6 +54,8 @@ static NotifierList colo_compare_notifiers = #define > REGULAR_PACKET_CHECK_MS 3000 #define DEFAULT_TIME_OUT_MS 3000 > > +/* #define DEBUG_COLO_PACKETS */ > + > static QemuMutex colo_compare_mutex; > static bool colo_compare_active; > static QemuMutex event_mtx; > @@ -327,7 +329,7 @@ static int colo_compare_packet_payload(Packet > *ppkt, > uint16_t len) > > { > -if > (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) > { > +if (trace_event_get_state_backends(TRACE_COLO_COMPARE_IP_INFO)) > { > char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20]; > > strcpy(pri_ip_src, inet_ntoa(ppkt->ip->ip_src)); @@ -492,12 +494,12 > @@ sec: > g_queue_push_head(&conn->primary_list, ppkt); > g_queue_push_head(&conn->secondary_list, spkt); > > -if > (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) > { > -qemu_hexdump((char *)ppkt->data, stderr, > -"colo-compare ppkt", ppkt->size); > -qemu_hexdump((char *)spkt->data, stderr, > -"colo-compare spkt", spkt->size); > -} > +#ifdef DEBUG_COLO_PACKETS > +qemu_hexdump((char *)ppkt->data, stderr, > + "colo-compare ppkt", ppkt->size); > +qemu_hexdump((char *)spkt->data, stderr, > + "colo-compare spkt", spkt->size); #endif > > colo_compare_inconsistency_notify(s); > } > @@ -533,12 +535,12 @@ static int colo_packet_compare_udp(Packet *spkt, > Packet *ppkt) > ppkt->size - offset)) { > trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size); > trace_colo_compare_udp_miscompare("Secondary pkt size", spkt- > >size); > -if > (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) > { > -qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt", > - ppkt->size); > -qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt", > - spkt->size); > -} > +#ifdef DEBUG_COLO_PACKETS > +qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt", > + ppkt->size); > +qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt", > + spkt->size); > +#endif Hi Roman, I think change the " trace_event_get_state_backends()" to "trace_colo_compare_main("Dump packet hex: ")" is a better choice here. It will keep the original code logic and avoid the problem here. Thanks Zhang Chen > return -1; > } else { > return 0; > @@ -576,12 +578,12 @@ static int colo_packet_compare_icmp(Packet *spkt, > Packet *ppkt) > ppkt->size); > trace_colo_compare_icmp_miscompare("Secondary pkt size", > spkt->size); > -if > (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) > { > -qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt", > - ppkt->size); > -
Re: [PULL 00/12] Block layer patches for 5.1.0-rc1
On Fri, 17 Jul 2020 at 13:55, Kevin Wolf wrote: > > The following changes since commit 151f76c689b1ff4c2c59e6d8469a0d4fe5346f55: > > Merge remote-tracking branch 'remotes/ehabkost/tags/x86-next-pull-request' > into staging (2020-07-16 21:46:18 +0100) > > are available in the Git repository at: > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to a8c5cf27c945d392edd85b0b0c64cd5c52cae658: > > file-posix: Fix leaked fd in raw_open_common() error path (2020-07-17 > 14:20:57 +0200) > > > Block layer patches: > > - file-posix: Fix read-only Linux block devices with auto-read-only > - Require aligned image size with O_DIRECT to avoid assertion failure > - Allow byte-aligned direct I/O on NFS instead of guessing 4k alignment > - Fix nbd_export_close_all() crash > - Fix race in iotests case 030 > - qemu-img resize: Require --shrink for shrinking all image formats > - crypto: use a stronger private key for tests > - Remove VXHS block device > - MAINTAINERS: vvfat: set status to odd fixes > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1 for any user-visible changes. -- PMM
Re: [PATCH 2/3] exec: posix_madvise usage on SunOS.
It probably does not indeed (maybe Solaris madvise does not provide but memcntl ?), I was just trying to fix the build only :-) On Sat, 18 Jul 2020 at 15:15, Peter Maydell wrote: > > On Sat, 18 Jul 2020 at 14:21, David CARLIER wrote: > > > > From a9e3cced279ae55a59847ba232f7828bc2479367 Mon Sep 17 00:00:00 2001 > > From: David Carlier > > Date: Sat, 18 Jul 2020 13:29:44 +0100 > > Subject: [PATCH 2/3] exec: posix_madvise usage on SunOS. > > > > with _XOPEN_SOURCE set, the older mman.h API based on caddr_t handling > > is not accessible thus using posix_madvise here. > > > > Signed-off-by: David Carlier > > --- > > exec.c | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/exec.c b/exec.c > > index 6f381f98e2..0466a75b89 100644 > > --- a/exec.c > > +++ b/exec.c > > @@ -3964,7 +3964,15 @@ int ram_block_discard_range(RAMBlock *rb, > > uint64_t start, size_t length) > > * fallocate'd away). > > */ > > #if defined(CONFIG_MADVISE) > > +#if !defined(CONFIG_SOLARIS) > > ret = madvise(host_startaddr, length, MADV_DONTNEED); > > +#else > > +/* > > + * mmap and its caddr_t based api is not accessible > > + * with _XOPEN_SOURCE set on illumos > > + */ > > +ret = posix_madvise(host_startaddr, length, > > POSIX_MADV_DONTNEED); > > +#endif > > Hi. I'm not sure this patch will do the right thing, because > I don't think that Solaris's POSIX_MADV_DONTNEED provides > the semantics that this QEMU function says it needs. The > comment at the top of the function says: > > * Unmap pages of memory from start to start+length such that > * they a) read as 0, b) Trigger whatever fault mechanism > * the OS provides for postcopy. > * The pages must be unmapped by the end of the function. > > (Aside: the use of 'unmap' in this comment is a bit confusing, > because it clearly doesn't mean 'unmap' if it wants read-as-0. > And the reference to faults on postcopy is incomprehensible > to me: if memory is read-as-0 it isn't going to fault.) > > Linux's madvise(MADV_DONTNEED) does guarantee us this > read-as-zero behaviour. (It's a silly API choice that Linux > put this behaviour behind madvise, which is supposed to be > merely advisory, but that's how it is.) The Solaris > posix_madvise() manpage says it is merely advisory and > doesn't affect the behaviour of accesses to the memory. > > If posix_madvise() behaviour was OK in this function, the > right way to fix this would be to use qemu_madvise() > instead, which already provides this "if host has > madvise(), use it, otherwise use posix_madvise()" logic. > But I suspect that the direct madvise() here is deliberate. > > Side note: not sure the current code is correct for the > BSDs either -- they have madvise() but don't provide > Linux's really-read-as-zero guarantee for MADV_DONTNEED. > So we should probably be doing something else there, and > whatever that something-else is is probably also what > Solaris wants. > > We use ram_block_discard_range() only in migration and > in virtio-balloon and virtio-mem; I've cc'd some people > who hopefully understand what the requirements on this > function are and might have a view on what the not-Linux > implementation should look like. (David Gilbert: git > blame says you wrote this code :-)) > > thanks > -- PMM
[Bug 1603970] Re: KVM freezes after live migration (AMD 4184 -> 4234)
Looking through old bug tickets... can you still reproduce this issue with the latest version of QEMU? Or could we close this ticket nowadays? ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1603970 Title: KVM freezes after live migration (AMD 4184 -> 4234) Status in QEMU: Incomplete Bug description: Hi, i have two host systems with different CPU types: Host A: AMD Opteron(tm) Processor 4234 Host B: AMD Opteron(tm) Processor 4184 Live migration from B -> A works as expected, migration from A -> B always ends in a freezed KVM. If the KVM is frozen, VNC output is still present, however, you can't type anything. CPU usage is always at 100% for one core (so if i set two cores, one is at 100% the other one at 0% usage). My command to launch the KVM is the following: /usr/bin/kvm -id 104 -chardev socket,id=qmp,path=/var/run/qemu- server/104.qmp,server,nowait -mon chardev=qmp,mode=control -pidfile /var/run/qemu-server/104.pid -daemonize -smbios type=1,uuid=26dd83a9-b9bd-4641-8016-c55f255f1bdf -name kilian-test -smp 1,sockets=1,cores=1,maxcpus=1 -nodefaults -boot menu=on,strict=on ,reboot-timeout=1000 -vga cirrus -vnc unix:/var/run/qemu- server/104.vnc,x509,password -cpu kvm64,+lahf_lm,+sep,+kvm_pv_unhalt,+kvm_pv_eoi,enforce -m 512 -object memory-backend-ram,id=ram-node0,size=512M -numa node,nodeid=0,cpus=0,memdev=ram-node0 -k de -device pci- bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f -device pci- bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e -device piix3-usb- uhci,id=uhci,bus=pci.0,addr=0x1.0x2 -device usb- tablet,id=tablet,bus=uhci.0,port=1 -device virtio-balloon- pci,id=balloon0,bus=pci.0,addr=0x3 -iscsi initiator- name=iqn.1993-08.org.debian:01:5ca1e9d334b2 -drive file=/mnt/pve/nfs_synology/images/104/vm-104-disk-2.qcow2,if=none,id =drive-virtio0,format=qcow2,cache=none,aio=native,detect-zeroes=on -device virtio-blk-pci,drive=drive- virtio0,id=virtio0,bus=pci.0,addr=0xa,bootindex=100 -netdev type=tap,id=net0,ifname=tap104i0,script=/var/lib/qemu-server/pve- bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on -device virtio-net- pci,mac=66:33:31:36:35:36,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300 KVM / QEMU version: QEMU emulator version 2.5.1.1 I have tried to set different CPU types, but no one works (qemu64, vm64, Opteron_G1, ...). I have found an email from 2014 where another user reports exactly the same problem: http://lists.gnu.org/archive/html/qemu-discuss/2014-02/msg2.html Greets Kilian To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1603970/+subscriptions
[Bug 1629483] Re: Build fails on optionrom
Looking through old bug tickets... can you still reproduce this issue with the latest version of QEMU? Or could we close this ticket nowadays? ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1629483 Title: Build fails on optionrom Status in QEMU: Incomplete Bug description: Git pseudo-bisected (focused on optionrom commits) it to this commit. commit cdbd727c20ad7aac7797dc8c95e485e1a4c6901b Author: Richard Henderson Date: Thu Jul 7 21:49:36 2016 -0700 build: Use $(AS) for optionrom explicitly Build output (non-verbose): ASoptionrom/linuxboot.o cpp: fatal error: '-c' is not a valid option to the preprocessor compilation terminated. cpp: fatal error: '-c' is not a valid option to the preprocessor compilation terminated. CCoptionrom/linuxboot_dma.o CC/home/bkamath/dev/workspace/block-2/mothra/output/sp0/targetqga/main.o ASoptionrom/kvmvapic.o cpp: fatal error: '-c' is not a valid option to the preprocessor compilation terminated. Steps to reproduce: Using buildroot and overriding qemu version to 2.7.0 Fedora 24, cpp (GCC) 6.2.1 20160916 (Red Hat 6.2.1-2) I tried first just building without the -c option but it hangs indefinitely. Reverting the above listed commit fixes the problem on my platform. I didn't dive much further into this, because this seems like a regression. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1629483/+subscriptions
[Bug 1213196] Re: -serial tcp should hang up when DTR goes low
** Changed in: qemu Importance: Undecided => Wishlist -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1213196 Title: -serial tcp should hang up when DTR goes low Status in QEMU: New Bug description: In keeping with the spirit of serial modem control signals, de- asserting DTR should cause the TCP connection to break; asserting DTR should cause QEMU to initiate a new connection or for it to accept another (in server mode; this may involve waiting for one to arrive, too). To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1213196/+subscriptions
Re: [Bug 1878057] Re: null-ptr dereference in megasas_command_complete
I ran this through my minimization script to remove the extraneous qtest commands: cat << EOF | ./i386-softmmu/qemu-system-i386 \ -M pc-q35-5.0 -no-shutdown -M q35 -device megasas \ -device scsi-cd,drive=null0 \ -blockdev driver=null-co,read-zeroes=on,node-name=null0 \ -nographic -qtest stdio -monitor none -serial none outl 0xcf8 0x80001814 outl 0xcfc 0xc021 outl 0xcf8 0x80001804 outw 0xcfc 0x7 outl 0xcf8 0x80001810 outl 0xcfc 0xe10c write 0x44b20 0x1 0x35 write 0x44b00 0x1 0x03 write 0xc021e10c0040 0x4 0x014b0400 write 0xc021e10c00c0 0x1 0x04 EOF On 200718 1024, Philippe Mathieu-Daudé wrote: > Might be relevant: > > commit 6df5718bd3ec56225c44cf96440c723c1b611b87 > Author: Hannes Reinecke > Date: Wed Oct 29 13:00:15 2014 +0100 > > megasas: Rework frame queueing algorithm > > Windows requires the frames to be unmapped, otherwise we run > into a race condition where the updated frame data is not > visible to the guest. > With that we can simplify the queue algorithm and use a bitmap > for tracking free frames. > > /* > * This absolutely needs to be locked if > * qemu ever goes multithreaded. > */ > static MegasasCmd *megasas_enqueue_frame(MegasasState *s, > hwaddr frame, uint64_t context, int count) > > Using -trace scsi\* -trace megasas\*: > > megasas_qf_enqueue frame 0x0 count 0 context 0x0 head 0x0 tail 0x0 busy 1 > megasas_handle_scsi LD SCSI dev 1/0/0 sdev 0x573f5560 xfer 0 > scsi_req_parsed target 0 lun 0 tag 0 command 53 dir 0 length 0 > scsi_req_parsed_lba target 0 lun 0 tag 0 command 53 lba 0 > scsi_req_alloc target 0 lun 0 tag 0 > scsi_disk_new_request Command: lun=0 tag=0x0 data= 0x35 0x00 0x00 0x00 0x00 > 0x00 0x00 0x00 0x00 0x00 > megasas_scsi_nodata scmd 0: no data to be transferred > megasas_mmio_invalid_writel addr 0x44: 0x3101 > megasas_mmio_invalid_writel addr 0x48: 0x44b0100 > megasas_mmio_invalid_writel addr 0x4c: 0x380100 > megasas_mmio_invalid_writel addr 0x50: 0x4b01 > megasas_mmio_invalid_writel addr 0x54: 0x3f010004 > megasas_mmio_invalid_writel addr 0x58: 0x100 > megasas_mmio_invalid_writel addr 0x5c: 0x100044b > megasas_mmio_invalid_writel addr 0x60: 0x46 > megasas_mmio_invalid_writel addr 0x64: 0x44b01 > megasas_mmio_invalid_writel addr 0x68: 0x4d01 > megasas_mmio_invalid_writel addr 0x6c: 0x44b0100 > megasas_mmio_invalid_writel addr 0x70: 0x540100 > megasas_mmio_invalid_writel addr 0x74: 0x4b01 > megasas_mmio_invalid_writel addr 0x78: 0x5b010004 > megasas_mmio_invalid_writel addr 0x7c: 0x100 > megasas_mmio_invalid_writel addr 0x80: 0x100044b > megasas_mmio_invalid_writel addr 0x84: 0x62 > megasas_mmio_invalid_writel addr 0x88: 0x44b01 > megasas_mmio_invalid_writel addr 0x8c: 0x6901 > megasas_mmio_invalid_writel addr 0x90: 0x44b0100 > megasas_mmio_invalid_writel addr 0x94: 0x700100 > megasas_mmio_invalid_writel addr 0x98: 0x4b01 > megasas_mmio_invalid_writel addr 0x9c: 0x77010004 > megasas_mmio_writel reg MFI_ODCR0: 0x100 > megasas_mmio_invalid_writel addr 0xa4: 0x100044b > megasas_mmio_invalid_writel addr 0xa8: 0x7e > megasas_mmio_invalid_writel addr 0xac: 0x44b01 > megasas_mmio_invalid_writel addr 0xb0: 0x8501 > megasas_mmio_invalid_writel addr 0xb4: 0x44b0100 > megasas_mmio_invalid_writel addr 0xb8: 0x8c0100 > megasas_mmio_invalid_writel addr 0xbc: 0x4b01 > megasas_mmio_writel reg MFI_IQPL: 0x4 > megasas_qf_new frame 0x1 addr 0x0 > megasas_enqueue_frame fr: 0x7fffa1e0 > megasas_qf_enqueue frame 0x1 count 2 context 0x0 head 0x0 tail 0x0 busy 2 > megasas_init_firmware pa 0x0 > megasas_init_queue queue at 0x0 len 0 head 0x0 tail 0x0 flags 0x0 > megasas_unmap_frame fr: 0x7fffa1e44b00 > megasas_unmap_frame fr: 0x7fffa1e0 > megasas_qf_complete_noirq context 0x0 > scsi_req_dequeue target 0 lun 0 tag 0 > scsi_aio_complete > megasas_command_complete scmd 0: status 0x0, residual 0 > megasas_scsi_complete scmd 0: status 0x0, len 0/0 > > The frame is unmapped when the complete callback occurs. > Then SIGSEGV in megasas_command_complete(): > > 1856 static void megasas_command_complete(SCSIRequest *req, uint32_t status, > 1857 size_t resid) > 1858 { > 1859 MegasasCmd *cmd = req->hba_private; > 1860 uint8_t cmd_status = MFI_STAT_OK; > 1861 > 1862 trace_megasas_command_complete(cmd->index, status, resid); > 1863 > 1864 if (req->io_canceled) { > 1865 return; > 1866 } > 1867 > 1868 if (cmd->dcmd_opcode != -1) { > 1869 /* > 1870 * Internal command complete > 1871 */ > 1872 cmd_status = megasas_finish_internal_dcmd(cmd, req, resid); > 1873 if (cmd_status == MFI_STAT_INVALID_STATUS) { > 1874 return; > 1875 } > 1876 } else { > 1877 req->status = status; > 1878 trace_megasas_scsi_complete(cmd->index, req->status, > 1879 cmd->iov_size, req->cmd.xfer); > 1880 if (req->status != GOOD) { > 1881 cm
[Bug 1852781] Re: qemu s390x on focal - applications breaking
Seems like you have to set all to "incomplete" to restart the expire counter again... ** Changed in: ubuntu-z-systems Status: Expired => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1852781 Title: qemu s390x on focal - applications breaking Status in QEMU: Incomplete Status in Ubuntu on IBM z Systems: Incomplete Bug description: Running qemu-system-s390x (1:4.0+dfsg-0ubuntu10) on an x86-64 Focal host with an upgrade of a Eoan s390x VM to a Focal s390x is triggering random breakage, for example: sudo apt-get update && sudo apt-get dist-upgrade ... ... Unpacking debianutils (4.9) over (4.8.6.3) ... Setting up debianutils (4.9) ... Use of uninitialized value $ARGV[0] in string ne at /usr/sbin/update-mime line 43. (Reading database ... 83640 files and directories currently installed.) Preparing to unpack .../bash_5.0-5ubuntu1_s390x.deb ... Unpacking bash (5.0-5ubuntu1) over (5.0-4ubuntu1) ... Setting up bash (5.0-5ubuntu1) ... [12124.788618] User process fault: interruption code 0007 ilc:3 in bash[2aa3d78+149000] dpkg: error processing package bash (--configure): installed bash package post-installation script subprocess was killed by signal (Floating point exception), core du mped Errors were encountered while processing: bash E: Sub-process /usr/bin/dpkg returned an error code (1) And now bash is completely broken: cking@eoan-s390x:~$ bash [12676.204389] User process fault: interruption code 0007 ilc:3 in bash[2aa1478+149000] Floating point exception (core dumped) The upgrade works OK on a s390x, so I'm assuming it's something to do with the qemu emulation. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1852781/+subscriptions
Re: [PATCH v2] goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/TIME_HIGH
On 18 Jul 2020, at 08:42, Philippe Mathieu-Daudé wrote: > On 7/18/20 2:49 AM, Jessica Clarke wrote: >> The specification says: >> >> 0x00 TIME_LOW R: Get current time, then return low-order 32-bits. >> 0x04 TIME_HIGH R: Return high 32-bits from previous TIME_LOW read. >> >> ... >> >> To read the value, the kernel must perform an IO_READ(TIME_LOW), >> which returns an unsigned 32-bit value, before an IO_READ(TIME_HIGH), >> which returns a signed 32-bit value, corresponding to the higher half >> of the full value. > > What a odd design choice... It actually makes a lot of sense. You know software always needs both halves, and needs them to be atomic, so this is an easy way to provide atomicity across two seemingly-independent reads. >> However, we were just returning the current time for both. If the guest >> is unlucky enough to read TIME_LOW and TIME_HIGH either side of an >> overflow of the lower half, it will see time be in the future, before >> jumping backwards on the next read, and Linux currently relies on the >> atomicity guaranteed by the spec so is affected by this. Fix this >> violation of the spec by caching the correct value for TIME_HIGH >> whenever TIME_LOW is read, and returning that value for any TIME_HIGH >> read. >> >> Signed-off-by: Jessica Clarke >> --- >> Changes since v1: >> >> * Add time_high to goldfish_rtc_vmstate and increment version. >> >> hw/rtc/goldfish_rtc.c | 17 ++--- >> include/hw/rtc/goldfish_rtc.h | 1 + >> 2 files changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c >> index 01e9d2b083..6ddd45cce0 100644 >> --- a/hw/rtc/goldfish_rtc.c >> +++ b/hw/rtc/goldfish_rtc.c >> @@ -94,12 +94,22 @@ static uint64_t goldfish_rtc_read(void *opaque, hwaddr >> offset, >> GoldfishRTCState *s = opaque; >> uint64_t r = 0; >> >> +/* >> + * From the documentation linked at the top of the file: >> + * >> + * To read the value, the kernel must perform an IO_READ(TIME_LOW), >> which >> + * returns an unsigned 32-bit value, before an IO_READ(TIME_HIGH), >> which >> + * returns a signed 32-bit value, corresponding to the higher half of >> the >> + * full value. >> + */ >> switch (offset) { >> case RTC_TIME_LOW: >> -r = goldfish_rtc_get_count(s) & 0x; >> +r = goldfish_rtc_get_count(s); >> +s->time_high = r >> 32; >> +r &= 0x; >> break; >> case RTC_TIME_HIGH: >> -r = goldfish_rtc_get_count(s) >> 32; >> +r = s->time_high; >> break; >> case RTC_ALARM_LOW: >> r = s->alarm_next & 0x; >> @@ -216,7 +226,7 @@ static const MemoryRegionOps goldfish_rtc_ops = { >> >> static const VMStateDescription goldfish_rtc_vmstate = { >> .name = TYPE_GOLDFISH_RTC, >> -.version_id = 1, >> +.version_id = 2, >> .pre_save = goldfish_rtc_pre_save, >> .post_load = goldfish_rtc_post_load, >> .fields = (VMStateField[]) { >> @@ -225,6 +235,7 @@ static const VMStateDescription goldfish_rtc_vmstate = { >> VMSTATE_UINT32(alarm_running, GoldfishRTCState), >> VMSTATE_UINT32(irq_pending, GoldfishRTCState), >> VMSTATE_UINT32(irq_enabled, GoldfishRTCState), >> +VMSTATE_UINT32(time_high, GoldfishRTCState), >> VMSTATE_END_OF_LIST() >> } >> }; >> diff --git a/include/hw/rtc/goldfish_rtc.h b/include/hw/rtc/goldfish_rtc.h >> index 16f9f9e29d..9bd8924f5f 100644 >> --- a/include/hw/rtc/goldfish_rtc.h >> +++ b/include/hw/rtc/goldfish_rtc.h >> @@ -41,6 +41,7 @@ typedef struct GoldfishRTCState { >> uint32_t alarm_running; >> uint32_t irq_pending; >> uint32_t irq_enabled; >> +uint32_t time_high; >> } GoldfishRTCState; > > Maybe easier to cache the whole u64, this matches RTC_ALARM_LOW / > RTC_ALARM_HIGH pattern (goldfish_rtc_vmstate change not included): We could, but why waste space storing an extra 32 bits you never need? I don't think saving all 64 bits makes it any easier to read, I'd personally even argue it makes it slightly less obvious what's going on. Jess
[Bug 1816805] Re: Cannot create cdrom device with open tray and cache option
** Changed in: qemu Assignee: John Snow (jnsnow) => (unassigned) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1816805 Title: Cannot create cdrom device with open tray and cache option Status in QEMU: Incomplete Bug description: When trying to create cdrom device with open tray and either of "cache" or "discard" options specified, I get the following error: qemu-system-x86_64: -drive if=none,id=drive- ide0-0-0,readonly=on,cache=writeback,discard=unmap,throttling.iops- total=900: Must specify either driver or file This bug essentially forbids live migration of VMs with open cdrom trays. I was able to find the same bug at RedHat: https://bugzilla.redhat.com/show_bug.cgi?id=1338638 The bug was encountered in versions 2.5 and 2.11. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1816805/+subscriptions
[Bug 1859656] Re: [2.6] Unable to reboot s390x KVM machine after initial deploy
** Project changed: qemu => qemu (Ubuntu) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1859656 Title: [2.6] Unable to reboot s390x KVM machine after initial deploy Status in MAAS: Triaged Status in Ubuntu on IBM z Systems: Triaged Status in qemu package in Ubuntu: Incomplete Bug description: MAAS version: 2.6.1 (7832-g17912cdc9-0ubuntu1~18.04.1) Arch: S390x Appears that MAAS can not find the s390x bootloader to boot from the disk, not sure how maas determines this. However this was working in the past. I had originally thought that if the maas machine was deployed then it defaulted to boot from disk. If I force the VM to book from disk, the VM starts up as expected. Reproduce: - Deploy Disco on S390x KVM instance - Reboot it on the KVM console... Connected to domain s2lp6g001 Escape character is ^] done Using IPv4 address: 10.246.75.160 Using TFTP server: 10.246.72.3 Bootfile name: 'boots390x.bin' Receiving data: 0 KBytes TFTP error: file not found: boots390x.bin Trying pxelinux.cfg files... Receiving data: 0 KBytes Receiving data: 0 KBytes Failed to load OS from network ==> /var/log/maas/rackd.log <== 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] boots390x.bin requested by 10.246.75.160 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] s390x/65a9ca43-9541-49be-b315-e2ca85936ea2 requested by 10.246.75.160 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] s390x/01-52-54-00-e5-d7-bb requested by 10.246.75.160 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] s390x/0AF64BA0 requested by 10.246.75.160 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] s390x/0AF64BA requested by 10.246.75.160 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] s390x/0AF64B requested by 10.246.75.160 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] s390x/0AF64 requested by 10.246.75.160 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] s390x/0AF6 requested by 10.246.75.160 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] s390x/0AF requested by 10.246.75.160 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] s390x/0A requested by 10.246.75.160 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] s390x/0 requested by 10.246.75.160 2020-01-14 18:21:24 provisioningserver.rackdservices.tftp: [info] s390x/default requested by 10.246.75.160 To manage notifications about this bug go to: https://bugs.launchpad.net/maas/+bug/1859656/+subscriptions
Re: instance_init() and the realize() functions
On 18/07/2020 09.09, Pratik Parvati wrote: > Hi team, > > Could someone please guild me to understand the difference > between *instance_init()* and the*realize()* functions? The > *class_init() *function is straight forward (it is similar to the > constructor in C++ OOP); But I am, finding hard to quote the difference > between *instance_init()* and *realize()*. > > What is the code flow when both functions are defined? Beside the other good answers that you've already got, maybe this helps a little bit, too: http://people.redhat.com/~thuth/blog/qemu/2018/09/10/instance-init-realize.html Cheers, Thomas
Re: [PATCH 3/3] contrib: ivshmem client and server build fix for SunOS.
On Sat, 18 Jul 2020 at 14:22, David CARLIER wrote: > > From 1c5225132a01ad67c87d603659ef4e4bcd46a16d Mon Sep 17 00:00:00 2001 > From: David Carlier > Date: Sat, 18 Jul 2020 13:32:47 +0100 > Subject: [PATCH 3/3] contrib: ivshmem client and server build fix for SunOS. > > sun is a macro on these systems, thus renaming the variables on the > client and server. > > Signed-off-by: David Carlier > --- > contrib/ivshmem-client/ivshmem-client.c | 12 ++-- > contrib/ivshmem-server/ivshmem-server.c | 12 ++-- > 2 files changed, 12 insertions(+), 12 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH 2/3] exec: posix_madvise usage on SunOS.
On Sat, 18 Jul 2020 at 14:21, David CARLIER wrote: > > From a9e3cced279ae55a59847ba232f7828bc2479367 Mon Sep 17 00:00:00 2001 > From: David Carlier > Date: Sat, 18 Jul 2020 13:29:44 +0100 > Subject: [PATCH 2/3] exec: posix_madvise usage on SunOS. > > with _XOPEN_SOURCE set, the older mman.h API based on caddr_t handling > is not accessible thus using posix_madvise here. > > Signed-off-by: David Carlier > --- > exec.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/exec.c b/exec.c > index 6f381f98e2..0466a75b89 100644 > --- a/exec.c > +++ b/exec.c > @@ -3964,7 +3964,15 @@ int ram_block_discard_range(RAMBlock *rb, > uint64_t start, size_t length) > * fallocate'd away). > */ > #if defined(CONFIG_MADVISE) > +#if !defined(CONFIG_SOLARIS) > ret = madvise(host_startaddr, length, MADV_DONTNEED); > +#else > +/* > + * mmap and its caddr_t based api is not accessible > + * with _XOPEN_SOURCE set on illumos > + */ > +ret = posix_madvise(host_startaddr, length, > POSIX_MADV_DONTNEED); > +#endif Hi. I'm not sure this patch will do the right thing, because I don't think that Solaris's POSIX_MADV_DONTNEED provides the semantics that this QEMU function says it needs. The comment at the top of the function says: * Unmap pages of memory from start to start+length such that * they a) read as 0, b) Trigger whatever fault mechanism * the OS provides for postcopy. * The pages must be unmapped by the end of the function. (Aside: the use of 'unmap' in this comment is a bit confusing, because it clearly doesn't mean 'unmap' if it wants read-as-0. And the reference to faults on postcopy is incomprehensible to me: if memory is read-as-0 it isn't going to fault.) Linux's madvise(MADV_DONTNEED) does guarantee us this read-as-zero behaviour. (It's a silly API choice that Linux put this behaviour behind madvise, which is supposed to be merely advisory, but that's how it is.) The Solaris posix_madvise() manpage says it is merely advisory and doesn't affect the behaviour of accesses to the memory. If posix_madvise() behaviour was OK in this function, the right way to fix this would be to use qemu_madvise() instead, which already provides this "if host has madvise(), use it, otherwise use posix_madvise()" logic. But I suspect that the direct madvise() here is deliberate. Side note: not sure the current code is correct for the BSDs either -- they have madvise() but don't provide Linux's really-read-as-zero guarantee for MADV_DONTNEED. So we should probably be doing something else there, and whatever that something-else is is probably also what Solaris wants. We use ram_block_discard_range() only in migration and in virtio-balloon and virtio-mem; I've cc'd some people who hopefully understand what the requirements on this function are and might have a view on what the not-Linux implementation should look like. (David Gilbert: git blame says you wrote this code :-)) thanks -- PMM
Re: tests/vm infrastructure fails to notice that QEMU dying is a failure
On Fri, 17 Jul 2020 at 18:24, John Snow wrote: > > - The real problem, though: Why is QEMU hanging? It might need a longer > timeout, or it might be having problems with the console socket again. > > (CC Robert Foley who has been working on the Console socket draining > problems. Maybe he has some insight here?) When we did see the console issues we would see a hung stack like this: #0 0xd43d141c in qemu_chr_write_buffer #1 0xd43d194c in qemu_chr_write #2 0xd43d3968 in qemu_chr_fe_write_all #3 0xd417cf80 in pl011_write #4 0xd3f3c7b0 in memory_region_write_accessor #5 0xd3f3a1fc in access_with_adjusted_size #6 0xd3f3e828 in memory_region_dispatch_write #7 0xd3f517b0 in io_writex #8 0x574a1d34 in code_gen_buffer () #9 0xd3f67228 in cpu_tb_exec #10 0xd3f67228 in cpu_loop_exec_tb #11 0xd3f67228 in cpu_exec #12 0xd3f2dbe4 in tcg_cpu_exec #13 0xd3f305e8 in qemu_tcg_cpu_thread_fn #14 0xd4441d88 in qemu_thread_start #15 0x85bec088 in start_thread #16 0x85b5c4ec in thread_start However, since we added console socket draining thread, it seems to have fixed this and presently basevm.py should be using this console draining for the vm-build-openbsd. When QEMU is hanging and exceeding our shutdown timeout, could we (optionally) send something like a SIGABRT to QEMU to force a core dump so we can get the stack and see where QEMU is hung? I suppose that presumes it is reproducible, but it might help to remove doubt in cases where QEMU hangs. -Rob > > --js >
[PATCH 3/3] contrib: ivshmem client and server build fix for SunOS.
>From 1c5225132a01ad67c87d603659ef4e4bcd46a16d Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 18 Jul 2020 13:32:47 +0100 Subject: [PATCH 3/3] contrib: ivshmem client and server build fix for SunOS. sun is a macro on these systems, thus renaming the variables on the client and server. Signed-off-by: David Carlier --- contrib/ivshmem-client/ivshmem-client.c | 12 ++-- contrib/ivshmem-server/ivshmem-server.c | 12 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/contrib/ivshmem-client/ivshmem-client.c b/contrib/ivshmem-client/ivshmem-client.c index b1274b236a..182c79d27c 100644 --- a/contrib/ivshmem-client/ivshmem-client.c +++ b/contrib/ivshmem-client/ivshmem-client.c @@ -178,7 +178,7 @@ ivshmem_client_init(IvshmemClient *client, const char *unix_sock_path, int ivshmem_client_connect(IvshmemClient *client) { -struct sockaddr_un sun; +struct sockaddr_un s_un; int fd, ret; int64_t tmp; @@ -192,16 +192,16 @@ ivshmem_client_connect(IvshmemClient *client) return -1; } -sun.sun_family = AF_UNIX; -ret = snprintf(sun.sun_path, sizeof(sun.sun_path), "%s", +s_un.sun_family = AF_UNIX; +ret = snprintf(s_un.sun_path, sizeof(s_un.sun_path), "%s", client->unix_sock_path); -if (ret < 0 || ret >= sizeof(sun.sun_path)) { +if (ret < 0 || ret >= sizeof(s_un.sun_path)) { IVSHMEM_CLIENT_DEBUG(client, "could not copy unix socket path\n"); goto err_close; } -if (connect(client->sock_fd, (struct sockaddr *)&sun, sizeof(sun)) < 0) { -IVSHMEM_CLIENT_DEBUG(client, "cannot connect to %s: %s\n", sun.sun_path, +if (connect(client->sock_fd, (struct sockaddr *)&s_un, sizeof(s_un)) < 0) { +IVSHMEM_CLIENT_DEBUG(client, "cannot connect to %s: %s\n", s_un.sun_path, strerror(errno)); goto err_close; } diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c index 88daee812d..39a6ffdb5d 100644 --- a/contrib/ivshmem-server/ivshmem-server.c +++ b/contrib/ivshmem-server/ivshmem-server.c @@ -288,7 +288,7 @@ ivshmem_server_init(IvshmemServer *server, const char *unix_sock_path, int ivshmem_server_start(IvshmemServer *server) { -struct sockaddr_un sun; +struct sockaddr_un s_un; int shm_fd, sock_fd, ret; /* open shm file */ @@ -327,15 +327,15 @@ ivshmem_server_start(IvshmemServer *server) goto err_close_shm; } -sun.sun_family = AF_UNIX; -ret = snprintf(sun.sun_path, sizeof(sun.sun_path), "%s", +s_un.sun_family = AF_UNIX; +ret = snprintf(s_un.sun_path, sizeof(s_un.sun_path), "%s", server->unix_sock_path); -if (ret < 0 || ret >= sizeof(sun.sun_path)) { +if (ret < 0 || ret >= sizeof(s_un.sun_path)) { IVSHMEM_SERVER_DEBUG(server, "could not copy unix socket path\n"); goto err_close_sock; } -if (bind(sock_fd, (struct sockaddr *)&sun, sizeof(sun)) < 0) { -IVSHMEM_SERVER_DEBUG(server, "cannot connect to %s: %s\n", sun.sun_path, +if (bind(sock_fd, (struct sockaddr *)&s_un, sizeof(s_un)) < 0) { +IVSHMEM_SERVER_DEBUG(server, "cannot connect to %s: %s\n", s_un.sun_path, strerror(errno)); goto err_close_sock; } -- 2.25.4
[PATCH 2/3] exec: posix_madvise usage on SunOS.
>From a9e3cced279ae55a59847ba232f7828bc2479367 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 18 Jul 2020 13:29:44 +0100 Subject: [PATCH 2/3] exec: posix_madvise usage on SunOS. with _XOPEN_SOURCE set, the older mman.h API based on caddr_t handling is not accessible thus using posix_madvise here. Signed-off-by: David Carlier --- exec.c | 8 1 file changed, 8 insertions(+) diff --git a/exec.c b/exec.c index 6f381f98e2..0466a75b89 100644 --- a/exec.c +++ b/exec.c @@ -3964,7 +3964,15 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) * fallocate'd away). */ #if defined(CONFIG_MADVISE) +#if !defined(CONFIG_SOLARIS) ret = madvise(host_startaddr, length, MADV_DONTNEED); +#else +/* + * mmap and its caddr_t based api is not accessible + * with _XOPEN_SOURCE set on illumos + */ +ret = posix_madvise(host_startaddr, length, POSIX_MADV_DONTNEED); +#endif if (ret) { ret = -errno; error_report("ram_block_discard_range: Failed to discard range " -- 2.25.4
[PATCH 0/3] configure: fix forSunOS based systems
>From 63a3c9639d708a796abd958607aa6376fc9b99f2 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 18 Jul 2020 13:27:52 +0100 Subject: [PATCH 1/3] configure: fix for SunOS based systems. local directive make the configure fails on these systems. Signed-off-by: David Carlier --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index e93836aaae..5d100eba72 100755 --- a/configure +++ b/configure @@ -59,7 +59,7 @@ error_exit() { do_compiler() { # Run the compiler, capturing its output to the log. First argument # is compiler binary to execute. -local compiler="$1" +compiler="$1" shift if test -n "$BASH_VERSION"; then eval ' echo >>config.log " -- 2.25.4
[PATCH] build: fix for SunOS based systems.
>From 1c5225132a01ad67c87d603659ef4e4bcd46a16d Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 18 Jul 2020 13:33:47 +0100 Subject: [PATCH 0/3] build: fix for SunOS systems. David Carlier (3): configure: fix for SunOS based systems. exec: posix_madvise usage on SunOS. contrib: ivshmem client and server build fix for SunOS. configure | 2 +- contrib/ivshmem-client/ivshmem-client.c | 12 ++-- contrib/ivshmem-server/ivshmem-server.c | 12 ++-- exec.c | 8 4 files changed, 21 insertions(+), 13 deletions(-) -- 2.25.4
[Bug 1880287] Re: gcc crashes in hppa emulation
** Tags added: linux-user -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1880287 Title: gcc crashes in hppa emulation Status in QEMU: New Bug description: There seems to be a translation bug in the qemu-hppa (qemu v5.0.0) emulation: A stripped down testcase (taken from Linux kernel build) is attached. In there is "a.sh", a shell script which calls gcc-9 (fails with both debian gcc-9.3.0-11 or gcc-9.3.0-12). and "a.iii", the preprocessed source. When starting a.sh, in the emulation gcc crashes with segfault. On real hardware gcc succeeds to compile the source. In a hppa-user chroot running "apt update && apt install gcc-9" should be sufficient to get the needed reproducer environment. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1880287/+subscriptions
Re: [Bug 1878057] Re: null-ptr dereference in megasas_command_complete
Cc'ing Hannes who doesn't have a Launchpad account. On 7/18/20 12:24 PM, Philippe Mathieu-Daudé wrote: > Might be relevant: > > commit 6df5718bd3ec56225c44cf96440c723c1b611b87 > Author: Hannes Reinecke > Date: Wed Oct 29 13:00:15 2014 +0100 > > megasas: Rework frame queueing algorithm > > Windows requires the frames to be unmapped, otherwise we run > into a race condition where the updated frame data is not > visible to the guest. > With that we can simplify the queue algorithm and use a bitmap > for tracking free frames. > > /* > * This absolutely needs to be locked if > * qemu ever goes multithreaded. > */ > static MegasasCmd *megasas_enqueue_frame(MegasasState *s, > hwaddr frame, uint64_t context, int count) > > Using -trace scsi\* -trace megasas\*: > > megasas_qf_enqueue frame 0x0 count 0 context 0x0 head 0x0 tail 0x0 busy 1 > megasas_handle_scsi LD SCSI dev 1/0/0 sdev 0x573f5560 xfer 0 > scsi_req_parsed target 0 lun 0 tag 0 command 53 dir 0 length 0 > scsi_req_parsed_lba target 0 lun 0 tag 0 command 53 lba 0 > scsi_req_alloc target 0 lun 0 tag 0 > scsi_disk_new_request Command: lun=0 tag=0x0 data= 0x35 0x00 0x00 0x00 0x00 > 0x00 0x00 0x00 0x00 0x00 > megasas_scsi_nodata scmd 0: no data to be transferred > megasas_mmio_invalid_writel addr 0x44: 0x3101 > megasas_mmio_invalid_writel addr 0x48: 0x44b0100 > megasas_mmio_invalid_writel addr 0x4c: 0x380100 > megasas_mmio_invalid_writel addr 0x50: 0x4b01 > megasas_mmio_invalid_writel addr 0x54: 0x3f010004 > megasas_mmio_invalid_writel addr 0x58: 0x100 > megasas_mmio_invalid_writel addr 0x5c: 0x100044b > megasas_mmio_invalid_writel addr 0x60: 0x46 > megasas_mmio_invalid_writel addr 0x64: 0x44b01 > megasas_mmio_invalid_writel addr 0x68: 0x4d01 > megasas_mmio_invalid_writel addr 0x6c: 0x44b0100 > megasas_mmio_invalid_writel addr 0x70: 0x540100 > megasas_mmio_invalid_writel addr 0x74: 0x4b01 > megasas_mmio_invalid_writel addr 0x78: 0x5b010004 > megasas_mmio_invalid_writel addr 0x7c: 0x100 > megasas_mmio_invalid_writel addr 0x80: 0x100044b > megasas_mmio_invalid_writel addr 0x84: 0x62 > megasas_mmio_invalid_writel addr 0x88: 0x44b01 > megasas_mmio_invalid_writel addr 0x8c: 0x6901 > megasas_mmio_invalid_writel addr 0x90: 0x44b0100 > megasas_mmio_invalid_writel addr 0x94: 0x700100 > megasas_mmio_invalid_writel addr 0x98: 0x4b01 > megasas_mmio_invalid_writel addr 0x9c: 0x77010004 > megasas_mmio_writel reg MFI_ODCR0: 0x100 > megasas_mmio_invalid_writel addr 0xa4: 0x100044b > megasas_mmio_invalid_writel addr 0xa8: 0x7e > megasas_mmio_invalid_writel addr 0xac: 0x44b01 > megasas_mmio_invalid_writel addr 0xb0: 0x8501 > megasas_mmio_invalid_writel addr 0xb4: 0x44b0100 > megasas_mmio_invalid_writel addr 0xb8: 0x8c0100 > megasas_mmio_invalid_writel addr 0xbc: 0x4b01 > megasas_mmio_writel reg MFI_IQPL: 0x4 > megasas_qf_new frame 0x1 addr 0x0 > megasas_enqueue_frame fr: 0x7fffa1e0 > megasas_qf_enqueue frame 0x1 count 2 context 0x0 head 0x0 tail 0x0 busy 2 > megasas_init_firmware pa 0x0 > megasas_init_queue queue at 0x0 len 0 head 0x0 tail 0x0 flags 0x0 > megasas_unmap_frame fr: 0x7fffa1e44b00 > megasas_unmap_frame fr: 0x7fffa1e0 > megasas_qf_complete_noirq context 0x0 > scsi_req_dequeue target 0 lun 0 tag 0 > scsi_aio_complete > megasas_command_complete scmd 0: status 0x0, residual 0 > megasas_scsi_complete scmd 0: status 0x0, len 0/0 > > The frame is unmapped when the complete callback occurs. > Then SIGSEGV in megasas_command_complete(): > > 1856 static void megasas_command_complete(SCSIRequest *req, uint32_t status, > 1857 size_t resid) > 1858 { > 1859 MegasasCmd *cmd = req->hba_private; > 1860 uint8_t cmd_status = MFI_STAT_OK; > 1861 > 1862 trace_megasas_command_complete(cmd->index, status, resid); > 1863 > 1864 if (req->io_canceled) { > 1865 return; > 1866 } > 1867 > 1868 if (cmd->dcmd_opcode != -1) { > 1869 /* > 1870 * Internal command complete > 1871 */ > 1872 cmd_status = megasas_finish_internal_dcmd(cmd, req, resid); > 1873 if (cmd_status == MFI_STAT_INVALID_STATUS) { > 1874 return; > 1875 } > 1876 } else { > 1877 req->status = status; > 1878 trace_megasas_scsi_complete(cmd->index, req->status, > 1879 cmd->iov_size, req->cmd.xfer); > 1880 if (req->status != GOOD) { > 1881 cmd_status = MFI_STAT_SCSI_DONE_WITH_ERROR; > 1882 } > 1883 if (req->status == CHECK_CONDITION) { > 1884 megasas_copy_sense(cmd); > 1885 } > 1886 > 1887 cmd->frame->header.scsi_status = req->status; > > ^^ This is NULL. > > 1888 } > 1889 cmd->frame->header.cmd_status = cmd_status; > 1890 megasas_complete_command(cmd); > 1891 } > > ** Changed in: qemu >Status: New => Confirmed >
[Bug 1878057] Re: null-ptr dereference in megasas_command_complete
Might be relevant: commit 6df5718bd3ec56225c44cf96440c723c1b611b87 Author: Hannes Reinecke Date: Wed Oct 29 13:00:15 2014 +0100 megasas: Rework frame queueing algorithm Windows requires the frames to be unmapped, otherwise we run into a race condition where the updated frame data is not visible to the guest. With that we can simplify the queue algorithm and use a bitmap for tracking free frames. /* * This absolutely needs to be locked if * qemu ever goes multithreaded. */ static MegasasCmd *megasas_enqueue_frame(MegasasState *s, hwaddr frame, uint64_t context, int count) Using -trace scsi\* -trace megasas\*: megasas_qf_enqueue frame 0x0 count 0 context 0x0 head 0x0 tail 0x0 busy 1 megasas_handle_scsi LD SCSI dev 1/0/0 sdev 0x573f5560 xfer 0 scsi_req_parsed target 0 lun 0 tag 0 command 53 dir 0 length 0 scsi_req_parsed_lba target 0 lun 0 tag 0 command 53 lba 0 scsi_req_alloc target 0 lun 0 tag 0 scsi_disk_new_request Command: lun=0 tag=0x0 data= 0x35 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 megasas_scsi_nodata scmd 0: no data to be transferred megasas_mmio_invalid_writel addr 0x44: 0x3101 megasas_mmio_invalid_writel addr 0x48: 0x44b0100 megasas_mmio_invalid_writel addr 0x4c: 0x380100 megasas_mmio_invalid_writel addr 0x50: 0x4b01 megasas_mmio_invalid_writel addr 0x54: 0x3f010004 megasas_mmio_invalid_writel addr 0x58: 0x100 megasas_mmio_invalid_writel addr 0x5c: 0x100044b megasas_mmio_invalid_writel addr 0x60: 0x46 megasas_mmio_invalid_writel addr 0x64: 0x44b01 megasas_mmio_invalid_writel addr 0x68: 0x4d01 megasas_mmio_invalid_writel addr 0x6c: 0x44b0100 megasas_mmio_invalid_writel addr 0x70: 0x540100 megasas_mmio_invalid_writel addr 0x74: 0x4b01 megasas_mmio_invalid_writel addr 0x78: 0x5b010004 megasas_mmio_invalid_writel addr 0x7c: 0x100 megasas_mmio_invalid_writel addr 0x80: 0x100044b megasas_mmio_invalid_writel addr 0x84: 0x62 megasas_mmio_invalid_writel addr 0x88: 0x44b01 megasas_mmio_invalid_writel addr 0x8c: 0x6901 megasas_mmio_invalid_writel addr 0x90: 0x44b0100 megasas_mmio_invalid_writel addr 0x94: 0x700100 megasas_mmio_invalid_writel addr 0x98: 0x4b01 megasas_mmio_invalid_writel addr 0x9c: 0x77010004 megasas_mmio_writel reg MFI_ODCR0: 0x100 megasas_mmio_invalid_writel addr 0xa4: 0x100044b megasas_mmio_invalid_writel addr 0xa8: 0x7e megasas_mmio_invalid_writel addr 0xac: 0x44b01 megasas_mmio_invalid_writel addr 0xb0: 0x8501 megasas_mmio_invalid_writel addr 0xb4: 0x44b0100 megasas_mmio_invalid_writel addr 0xb8: 0x8c0100 megasas_mmio_invalid_writel addr 0xbc: 0x4b01 megasas_mmio_writel reg MFI_IQPL: 0x4 megasas_qf_new frame 0x1 addr 0x0 megasas_enqueue_frame fr: 0x7fffa1e0 megasas_qf_enqueue frame 0x1 count 2 context 0x0 head 0x0 tail 0x0 busy 2 megasas_init_firmware pa 0x0 megasas_init_queue queue at 0x0 len 0 head 0x0 tail 0x0 flags 0x0 megasas_unmap_frame fr: 0x7fffa1e44b00 megasas_unmap_frame fr: 0x7fffa1e0 megasas_qf_complete_noirq context 0x0 scsi_req_dequeue target 0 lun 0 tag 0 scsi_aio_complete megasas_command_complete scmd 0: status 0x0, residual 0 megasas_scsi_complete scmd 0: status 0x0, len 0/0 The frame is unmapped when the complete callback occurs. Then SIGSEGV in megasas_command_complete(): 1856 static void megasas_command_complete(SCSIRequest *req, uint32_t status, 1857 size_t resid) 1858 { 1859 MegasasCmd *cmd = req->hba_private; 1860 uint8_t cmd_status = MFI_STAT_OK; 1861 1862 trace_megasas_command_complete(cmd->index, status, resid); 1863 1864 if (req->io_canceled) { 1865 return; 1866 } 1867 1868 if (cmd->dcmd_opcode != -1) { 1869 /* 1870 * Internal command complete 1871 */ 1872 cmd_status = megasas_finish_internal_dcmd(cmd, req, resid); 1873 if (cmd_status == MFI_STAT_INVALID_STATUS) { 1874 return; 1875 } 1876 } else { 1877 req->status = status; 1878 trace_megasas_scsi_complete(cmd->index, req->status, 1879 cmd->iov_size, req->cmd.xfer); 1880 if (req->status != GOOD) { 1881 cmd_status = MFI_STAT_SCSI_DONE_WITH_ERROR; 1882 } 1883 if (req->status == CHECK_CONDITION) { 1884 megasas_copy_sense(cmd); 1885 } 1886 1887 cmd->frame->header.scsi_status = req->status; ^^ This is NULL. 1888 } 1889 cmd->frame->header.cmd_status = cmd_status; 1890 megasas_complete_command(cmd); 1891 } ** Changed in: qemu Status: New => Confirmed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1878057 Title: null-ptr dereference in megasas_command_complete Status in QEMU: Confirmed Bug description: Hello, While fuzzing, I found an input that triggers a null-pointer dereference in megasas_command_complete: ==1495
Re: instance_init() and the realize() functions
> On 18 Jul 2020, at 12:43, Peter Maydell wrote: > > Note that for a lot > of device state struct members (where they correspond to guest > registers state), you want to set their values in the > device's reset method, not in instance_init or realize. > That's because the guest-visible register state needs to > be set back to its initial value on a system reset anyway, > and QEMU guarantees that we will reset a device before > first use. Yes, that's a good point, but must be used with caution; since .reset is called **after** .realize, if the dynamically allocated structures are not set to zero, leaving state members uninitialised up to .reset is risky, it may lead to unexpected behaviour during .realize or other parts of the code. That's why I mentioned that .instance_init should ensure that all members have default values and properties associated with them, such that later code, like .realize, has a deterministic behaviour. Of course, for consistent results, both .instance_init and .reset can call a common internal function. Regards, Liviu
[Bug 1878259] Re: Null-pointer dereference in megasas_handle_frame
Fixed in commit 77f55eac6c433e23e82a1b88b2d74f385c4c7d82. ** Changed in: qemu Status: New => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1878259 Title: Null-pointer dereference in megasas_handle_frame Status in QEMU: Fix Committed Bug description: Hello, While fuzzing, I found an input that triggers a null-pointer dereference in megasas_handle_frame: ==1595==ERROR: AddressSanitizer: SEGV on unknown address 0x (pc 0x55e3e83e6e08 bp 0x7ffdb04c63b0 sp 0x7ffd ==1595==The signal is caused by a READ memory access. ==1595==Hint: address points to the zero page. #0 0x55e3e83e6e08 in megasas_handle_frame /home/alxndr/Development/qemu/hw/scsi/megasas.c:1952:36 #1 0x55e3e83e6e08 in megasas_mmio_write /home/alxndr/Development/qemu/hw/scsi/megasas.c:2122:9 #2 0x55e3e7d088d6 in memory_region_write_accessor /home/alxndr/Development/qemu/memory.c:483:5 #3 0x55e3e7d0827f in access_with_adjusted_size /home/alxndr/Development/qemu/memory.c:544:18 #4 0x55e3e7d0827f in memory_region_dispatch_write /home/alxndr/Development/qemu/memory.c:1476:16 #5 0x55e3e7c1d1d3 in flatview_write_continue /home/alxndr/Development/qemu/exec.c:3137:23 #6 0x55e3e7c15b97 in flatview_write /home/alxndr/Development/qemu/exec.c:3177:14 #7 0x55e3e7c15b97 in address_space_write /home/alxndr/Development/qemu/exec.c:3268:18 #8 0x55e3e7d03bc4 in qtest_process_command /home/alxndr/Development/qemu/qtest.c:567:9 #9 0x55e3e7cfe74d in qtest_process_inbuf /home/alxndr/Development/qemu/qtest.c:710:9 #10 0x55e3e8804cad in fd_chr_read /home/alxndr/Development/qemu/chardev/char-fd.c:68:9 #11 0x7f602ef2a897 in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4e897) #12 0x55e3e8948384 in glib_pollfds_poll /home/alxndr/Development/qemu/util/main-loop.c:219:9 #13 0x55e3e8948384 in os_host_main_loop_wait /home/alxndr/Development/qemu/util/main-loop.c:242:5 #14 0x55e3e8948384 in main_loop_wait /home/alxndr/Development/qemu/util/main-loop.c:518:11 #15 0x55e3e7f27676 in qemu_main_loop /home/alxndr/Development/qemu/softmmu/vl.c:1664:9 #16 0x55e3e8851c6a in main /home/alxndr/Development/qemu/softmmu/main.c:49:5 #17 0x7f602dadae0a in __libc_start_main /build/glibc-GwnBeO/glibc-2.30/csu/../csu/libc-start.c:308:16 #18 0x55e3e7b5c7b9 in _start (/home/alxndr/Development/qemu/build/i386-softmmu/qemu-system-i386+0x9027b9) I can reproduce it in qemu 5.0 using: cat << EOF | ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -qtest stdio -nographic -monitor none -serial none -M q35 -device megasas outl 0xcf8 0x80001814 outl 0xcfc 0xc021 outl 0xcf8 0x80001818 outl 0xcf8 0x80001804 outw 0xcfc 0x7 outl 0xcf8 0x80001810 outl 0xcfc 0xe10c outl 0xcf8 0x8000f810 outl 0xcf8 0x8000fa24 outl 0xcfc 0xe10c4000 outl 0xcf8 0x8000fa04 outw 0xcfc 0x7 outl 0xcf8 0x8000fb20 write 0xe10c4384 0x15 0x838383838383838383838383838383838383838383 write 0xc021e10c00c0 0x4 0x082c04dd EOF I also attached the commands to this launchpad report, in case the formatting is broken: qemu-system-i386 -qtest stdio -nographic -monitor none -serial none -M q35 -device megasas < attachment Please let me know if I can provide any further info. -Alex To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1878259/+subscriptions
Re: instance_init() and the realize() functions
On Sat, 18 Jul 2020 at 09:27, Liviu Ionescu wrote: > For me it was difficult to draw a line of what initialisations should be done > in .instance_init and what in .realize, but given that .realise is called > when the whole hierarchy is ready, it might do links between objects, which > are not available when .instance_init is called. Yeah, we have not been able as a project to come up with a nice rule for how to do this split. There are a few definite rules: * anything that can fail and return an error must go in realize (because instance_init has no failure-return mechanism) * anything you need to do to set up a QOM property on the object must go in instance_init (so that the property can be set by the user of the device object before realizing it) * anything that changes the state of the simulation must go in realize (some QMP monitor commands to introspect objects will do instance_init/look at object/delete) but a lot of things could validly go in either method, and we haven't set a convention for those cases. (There's a bunch of inconclusive discussions in the mailing list archives.) > A simple rule would be for .instance_init to ensure that all members have > default values and properties associated with them, and defer all functional > initialisations to .realize. Yeah, this is a reasonable default rule. Note that for a lot of device state struct members (where they correspond to guest registers state), you want to set their values in the device's reset method, not in instance_init or realize. That's because the guest-visible register state needs to be set back to its initial value on a system reset anyway, and QEMU guarantees that we will reset a device before first use. thanks -- PMM
[PATCH-for-5.1] hw/misc/milkymist-pfpu: Fix pFPU region size
The last microcode word (address 0x6000.6ffc) is not reachable. Correct the programmable FPU I/O size (which is 4 KiB) to be able to use all the microcode area. Signed-off-by: Philippe Mathieu-Daudé --- hw/misc/milkymist-pfpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/misc/milkymist-pfpu.c b/hw/misc/milkymist-pfpu.c index 516825e83d..4fbe3e8971 100644 --- a/hw/misc/milkymist-pfpu.c +++ b/hw/misc/milkymist-pfpu.c @@ -507,7 +507,7 @@ static void milkymist_pfpu_realize(DeviceState *dev, Error **errp) sysbus_init_irq(sbd, &s->irq); memory_region_init_io(&s->regs_region, OBJECT(dev), &pfpu_mmio_ops, s, -"milkymist-pfpu", MICROCODE_END * 4); + "milkymist-pfpu", 0x1000); sysbus_init_mmio(sbd, &s->regs_region); } -- 2.21.3
Re: tests/vm infrastructure fails to notice that QEMU dying is a failure
On Fri, 17 Jul 2020 at 23:24, John Snow wrote: > - The real problem, though: Why is QEMU hanging? It might need a longer > timeout, or it might be having problems with the console socket again. The host machine seemed to be under really heavy I/O load at the time. thanks -- PMM
Re: instance_init() and the realize() functions
> On 18 Jul 2020, at 10:09, Pratik Parvati wrote: > > The class_init() function is straight forward (it is similar to the > constructor in C++ OOP The C++ constructor initialises class **instances**, i.e. C++ objects, not C++ classes. In QEMU, the OOP functionality is implemented with nested structures used to store class and instance definitions, and callbacks as virtual methods. The .class_init callbacks are called early, by a mechanism similar to C++ static constructors, and they initialise the structures used to store the class definitions. They are recursively chained, i.e. first the parent callback is called to initialise the parent structure members, then the current callback is called, to fill in its own members. The .instance_init callback are automatically called when new **instances** of a class are created. Similarly, they are also recursively chained. .instance_init may also create children objects, recursively. .realize is a bit trickier. If .instance_init is the very first thing automatically called when creating an object, .realize is the very last thing, it is called manually when the whole hierarchy of objects is created and it signals that everything is ready, a kind of 'rien ne va plus'. .realize usually has to manually call the parent .realize. --- For me it was difficult to draw a line of what initialisations should be done in .instance_init and what in .realize, but given that .realise is called when the whole hierarchy is ready, it might do links between objects, which are not available when .instance_init is called. A simple rule would be for .instance_init to ensure that all members have default values and properties associated with them, and defer all functional initialisations to .realize. Hope this helps, Liviu
[Bug 1884693] Re: Assertion failure in address_space_unmap through ahci_map_clb_address
Proposed fix: https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg05637.html -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1884693 Title: Assertion failure in address_space_unmap through ahci_map_clb_address Status in QEMU: Confirmed Bug description: Hello, Reproducer: cat << EOF | ./i386-softmmu/qemu-system-i386 -qtest stdio -monitor none -serial none -M pc-q35-5.0 -nographic outl 0xcf8 0x8000fa24 outl 0xcfc 0xe1068000 outl 0xcf8 0x8000fa04 outw 0xcfc 0x7 outl 0xcf8 0x8000fb20 write 0xe1068304 0x1 0x21 write 0xe1068318 0x1 0x21 write 0xe1068384 0x1 0x21 write 0xe1068398 0x2 0x21 EOF Stack trace: #0 0x55bfabfe9ea0 in __libc_start_main /build/glibc-GwnBeO/glibc-2.30/csu/../csu/libc-start.c:308:16 #1 0x55bfabfc8ef9 in __sanitizer_print_stack_trace (build/i386-softmmu/qemu-fuzz-i386+0x7b7ef9) #2 0x55bfabfaf933 in fuzzer::PrintStackTrace() FuzzerUtil.cpp:210:5 #3 0x7f88df76110f (/lib/x86_64-linux-gnu/libpthread.so.0+0x1410f) #4 0x7f88df5a4760 in __libc_signal_restore_set /build/glibc-GwnBeO/glibc-2.30/signal/../sysdeps/unix/sysv/linux/internal-signals.h:84:10 #5 0x7f88df5a4760 in raise /build/glibc-GwnBeO/glibc-2.30/signal/../sysdeps/unix/sysv/linux/raise.c:48:3 #6 0x7f88df58e55a in abort /build/glibc-GwnBeO/glibc-2.30/stdlib/abort.c:79:7 #7 0x7f88df58e42e in __assert_fail_base /build/glibc-GwnBeO/glibc-2.30/assert/assert.c:92:3 #8 0x7f88df59d091 in __assert_fail /build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3 #9 0x55bfabff7182 in address_space_unmap exec.c:3602:9 #10 0x55bfac4a452f in dma_memory_unmap include/sysemu/dma.h:148:5 #11 0x55bfac4a452f in map_page hw/ide/ahci.c:254:9 #12 0x55bfac4a1f98 in ahci_map_clb_address hw/ide/ahci.c:748:5 #13 0x55bfac4a1f98 in ahci_cond_start_engines hw/ide/ahci.c:276:14 #14 0x55bfac4a074e in ahci_port_write hw/ide/ahci.c:339:9 #15 0x55bfac4a074e in ahci_mem_write hw/ide/ahci.c:513:9 #16 0x55bfac0e0dc2 in memory_region_write_accessor memory.c:483:5 #17 0x55bfac0e0bde in access_with_adjusted_size memory.c:544:18 #18 0x55bfac0e0917 in memory_region_dispatch_write memory.c #19 0x55bfabffa4fd in flatview_write_continue exec.c:3146:23 #20 0x55bfabff569b in flatview_write exec.c:3186:14 #21 0x55bfabff569b in address_space_write exec.c:3280:18 #22 0x55bfac8982a9 in op_write_pattern tests/qtest/fuzz/general_fuzz.c:407:5 #23 0x55bfac897749 in general_fuzz tests/qtest/fuzz/general_fuzz.c:481:17 #24 0x55bfac8930a2 in LLVMFuzzerTestOneInput tests/qtest/fuzz/fuzz.c:136:5 #25 0x55bfabfb0e68 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) FuzzerLoop.cpp:558:15 #26 0x55bfabfb0485 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) FuzzerLoop.cpp:470:3 #27 0x55bfabfb18a1 in fuzzer::Fuzzer::MutateAndTestOne() FuzzerLoop.cpp:701:19 #28 0x55bfabfb2305 in fuzzer::Fuzzer::Loop(std::vector >&) FuzzerLoop.cpp:837:5 #29 0x55bfabfa2018 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) FuzzerDriver.cpp:846:6 #30 0x55bfabfb8722 in main FuzzerMain.cpp:19:10 #31 0x7f88df58fe0a in __libc_start_main /build/glibc-GwnBeO/glibc-2.30/csu/../csu/libc-start.c:308:16 #32 0x55bfabf97869 in _start (build/i386-softmmu/qemu-fuzz-i386+0x786869) The same error can be triggered through ahci_map_fis_address @ hw/ide/ahci.c:721:5 Found with generic device fuzzer: https://patchew.org/QEMU/20200611055651.13784-1-alx...@bu.edu/ Please let me know if I can provide any further info. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1884693/+subscriptions
Re: [PATCH v2] goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/TIME_HIGH
On 7/18/20 2:49 AM, Jessica Clarke wrote: > The specification says: > >0x00 TIME_LOW R: Get current time, then return low-order 32-bits. >0x04 TIME_HIGH R: Return high 32-bits from previous TIME_LOW read. > >... > >To read the value, the kernel must perform an IO_READ(TIME_LOW), >which returns an unsigned 32-bit value, before an IO_READ(TIME_HIGH), >which returns a signed 32-bit value, corresponding to the higher half >of the full value. What a odd design choice... > > However, we were just returning the current time for both. If the guest > is unlucky enough to read TIME_LOW and TIME_HIGH either side of an > overflow of the lower half, it will see time be in the future, before > jumping backwards on the next read, and Linux currently relies on the > atomicity guaranteed by the spec so is affected by this. Fix this > violation of the spec by caching the correct value for TIME_HIGH > whenever TIME_LOW is read, and returning that value for any TIME_HIGH > read. > > Signed-off-by: Jessica Clarke > --- > Changes since v1: > > * Add time_high to goldfish_rtc_vmstate and increment version. > > hw/rtc/goldfish_rtc.c | 17 ++--- > include/hw/rtc/goldfish_rtc.h | 1 + > 2 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c > index 01e9d2b083..6ddd45cce0 100644 > --- a/hw/rtc/goldfish_rtc.c > +++ b/hw/rtc/goldfish_rtc.c > @@ -94,12 +94,22 @@ static uint64_t goldfish_rtc_read(void *opaque, hwaddr > offset, > GoldfishRTCState *s = opaque; > uint64_t r = 0; > > +/* > + * From the documentation linked at the top of the file: > + * > + * To read the value, the kernel must perform an IO_READ(TIME_LOW), > which > + * returns an unsigned 32-bit value, before an IO_READ(TIME_HIGH), > which > + * returns a signed 32-bit value, corresponding to the higher half of > the > + * full value. > + */ > switch (offset) { > case RTC_TIME_LOW: > -r = goldfish_rtc_get_count(s) & 0x; > +r = goldfish_rtc_get_count(s); > +s->time_high = r >> 32; > +r &= 0x; > break; > case RTC_TIME_HIGH: > -r = goldfish_rtc_get_count(s) >> 32; > +r = s->time_high; > break; > case RTC_ALARM_LOW: > r = s->alarm_next & 0x; > @@ -216,7 +226,7 @@ static const MemoryRegionOps goldfish_rtc_ops = { > > static const VMStateDescription goldfish_rtc_vmstate = { > .name = TYPE_GOLDFISH_RTC, > -.version_id = 1, > +.version_id = 2, > .pre_save = goldfish_rtc_pre_save, > .post_load = goldfish_rtc_post_load, > .fields = (VMStateField[]) { > @@ -225,6 +235,7 @@ static const VMStateDescription goldfish_rtc_vmstate = { > VMSTATE_UINT32(alarm_running, GoldfishRTCState), > VMSTATE_UINT32(irq_pending, GoldfishRTCState), > VMSTATE_UINT32(irq_enabled, GoldfishRTCState), > +VMSTATE_UINT32(time_high, GoldfishRTCState), > VMSTATE_END_OF_LIST() > } > }; > diff --git a/include/hw/rtc/goldfish_rtc.h b/include/hw/rtc/goldfish_rtc.h > index 16f9f9e29d..9bd8924f5f 100644 > --- a/include/hw/rtc/goldfish_rtc.h > +++ b/include/hw/rtc/goldfish_rtc.h > @@ -41,6 +41,7 @@ typedef struct GoldfishRTCState { > uint32_t alarm_running; > uint32_t irq_pending; > uint32_t irq_enabled; > +uint32_t time_high; > } GoldfishRTCState; Maybe easier to cache the whole u64, this matches RTC_ALARM_LOW / RTC_ALARM_HIGH pattern (goldfish_rtc_vmstate change not included): -- >8 -- --- a/include/hw/rtc/goldfish_rtc.h +++ b/include/hw/rtc/goldfish_rtc.h @@ -37,6 +37,7 @@ typedef struct GoldfishRTCState { uint64_t tick_offset; uint64_t tick_offset_vmstate; +uint64_t rtc_time; /* Updated when RTC_TIME_LOW is read */ uint64_t alarm_next; uint32_t alarm_running; uint32_t irq_pending; --- a/hw/rtc/goldfish_rtc.c +++ b/hw/rtc/goldfish_rtc.c @@ -96,10 +96,11 @@ static uint64_t goldfish_rtc_read(void *opaque, hwaddr offset, switch (offset) { case RTC_TIME_LOW: -r = goldfish_rtc_get_count(s) & 0x; +s->rtc_time = goldfish_rtc_get_count(s); +r = s->rtc_time & 0x; break; case RTC_TIME_HIGH: -r = goldfish_rtc_get_count(s) >> 32; +r = s->rtc_time >> 32; break; case RTC_ALARM_LOW: r = s->alarm_next & 0x; ---
[PATCH-for-5.1] hw/ide/ahci: Do not dma_memory_unmap(NULL)
libFuzzer triggered the following assertion: cat << EOF | qemu-system-i386 -M pc-q35-5.0 \ -nographic -monitor none -serial none -qtest stdio outl 0xcf8 0x8000fa24 outl 0xcfc 0xe1068000 outl 0xcf8 0x8000fa04 outw 0xcfc 0x7 outl 0xcf8 0x8000fb20 write 0xe1068304 0x1 0x21 write 0xe1068318 0x1 0x21 write 0xe1068384 0x1 0x21 write 0xe1068398 0x2 0x21 EOF qemu-system-i386: exec.c:3621: address_space_unmap: Assertion `mr != NULL' failed. Aborted (core dumped) This is because we don't check the return value from dma_memory_map() which can return NULL, then we call dma_memory_unmap(NULL) which is illegal. Fix by only unmap if the value is not NULL (and the size is not the expected one). Cc: qemu-sta...@nongnu.org Reported-by: Alexander Bulekov Fixes: f6ad2e32f8 ("ahci: add ahci emulation") BugLink: https://bugs.launchpad.net/qemu/+bug/1884693 Signed-off-by: Philippe Mathieu-Daudé --- hw/ide/ahci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 009120f88b..4f596cb9ce 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -250,7 +250,7 @@ static void map_page(AddressSpace *as, uint8_t **ptr, uint64_t addr, } *ptr = dma_memory_map(as, addr, &len, DMA_DIRECTION_FROM_DEVICE); -if (len < wanted) { +if (len < wanted && *ptr) { dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len); *ptr = NULL; } -- 2.21.3
instance_init() and the realize() functions
Hi team, Could someone please guild me to understand the difference between *instance_init()* and the* realize()* functions? The *class_init() *function is straight forward (it is similar to the constructor in C++ OOP); But I am, finding hard to quote the difference between *instance_init()* and *realize()*. What is the code flow when both functions are defined? Thanks & Regards, Pratik