[PATCH 0/3] build: fix for SunOS systems.

2020-07-18 Thread David Carlier
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) ?

2020-07-18 Thread Samuel Thibault
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

2020-07-18 Thread Chen Gang
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]

2020-07-18 Thread Peter Maydell
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

2020-07-18 Thread Emilio G. Cota
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

2020-07-18 Thread Richard Henderson
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

2020-07-18 Thread Peter Maydell
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

2020-07-18 Thread Philippe Mathieu-Daudé
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

2020-07-18 Thread Peter Maydell
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

2020-07-18 Thread Zhang, Chen


> -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

2020-07-18 Thread Peter Maydell
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.

2020-07-18 Thread David CARLIER
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)

2020-07-18 Thread Thomas Huth
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

2020-07-18 Thread Thomas Huth
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

2020-07-18 Thread Thomas Huth
** 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

2020-07-18 Thread Alexander Bulekov
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

2020-07-18 Thread Thomas Huth
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

2020-07-18 Thread Jessica Clarke
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

2020-07-18 Thread Thomas Huth
** 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

2020-07-18 Thread Thomas Huth
** 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

2020-07-18 Thread Thomas Huth
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.

2020-07-18 Thread Peter Maydell
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.

2020-07-18 Thread Peter Maydell
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

2020-07-18 Thread Robert Foley
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.

2020-07-18 Thread David CARLIER
>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.

2020-07-18 Thread David CARLIER
>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

2020-07-18 Thread David CARLIER
>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.

2020-07-18 Thread David CARLIER
>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

2020-07-18 Thread Philippe Mathieu-Daudé
** 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

2020-07-18 Thread Philippe Mathieu-Daudé
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

2020-07-18 Thread Philippe Mathieu-Daudé
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

2020-07-18 Thread Liviu Ionescu



> 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

2020-07-18 Thread Philippe Mathieu-Daudé
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

2020-07-18 Thread Peter Maydell
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

2020-07-18 Thread Philippe Mathieu-Daudé
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

2020-07-18 Thread Peter Maydell
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

2020-07-18 Thread Liviu Ionescu



> 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

2020-07-18 Thread Philippe Mathieu-Daudé
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

2020-07-18 Thread Philippe Mathieu-Daudé
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)

2020-07-18 Thread Philippe Mathieu-Daudé
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

2020-07-18 Thread Pratik Parvati
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