Re: [PATCH v4 2/5] virtio-scsi: introduce a constant for fixed virtqueues

2020-05-30 Thread Raphael Norwitz
On Wed, May 27, 2020 at 6:32 AM Stefan Hajnoczi  wrote:
>
> The event and control virtqueues are always present, regardless of the
> multi-queue configuration.  Define a constant so that virtqueue number
> calculations are easier to read.
>
> Signed-off-by: Stefan Hajnoczi 
> Reviewed-by: Cornelia Huck 
> ---
>  include/hw/virtio/virtio-scsi.h | 3 +++
>  hw/scsi/vhost-user-scsi.c   | 2 +-
>  hw/scsi/virtio-scsi.c   | 7 ---
>  hw/virtio/vhost-scsi-pci.c  | 3 ++-
>  hw/virtio/vhost-user-scsi-pci.c | 3 ++-
>  hw/virtio/virtio-scsi-pci.c | 3 ++-
>  6 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index 24e768909d..9f293bcb80 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -36,6 +36,9 @@
>  #define VIRTIO_SCSI_MAX_TARGET  255
>  #define VIRTIO_SCSI_MAX_LUN 16383
>
> +/* Number of virtqueues that are always present */
> +#define VIRTIO_SCSI_VQ_NUM_FIXED2
> +
>  typedef struct virtio_scsi_cmd_req VirtIOSCSICmdReq;
>  typedef struct virtio_scsi_cmd_resp VirtIOSCSICmdResp;
>  typedef struct virtio_scsi_ctrl_tmf_req VirtIOSCSICtrlTMFReq;
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index cbb5d97599..f8bd158c31 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -115,7 +115,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, 
> Error **errp)
>  goto free_virtio;
>  }
>
> -vsc->dev.nvqs = 2 + vs->conf.num_queues;
> +vsc->dev.nvqs = VIRTIO_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
>  vsc->dev.vqs = g_new0(struct vhost_virtqueue, vsc->dev.nvqs);
>  vsc->dev.vq_index = 0;
>  vsc->dev.backend_features = 0;
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 9b72094a61..f3d60683bd 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -191,7 +191,7 @@ static void virtio_scsi_save_request(QEMUFile *f, 
> SCSIRequest *sreq)
>  VirtIOSCSIReq *req = sreq->hba_private;
>  VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(req->dev);
>  VirtIODevice *vdev = VIRTIO_DEVICE(req->dev);
> -uint32_t n = virtio_get_queue_index(req->vq) - 2;
> +uint32_t n = virtio_get_queue_index(req->vq) - VIRTIO_SCSI_VQ_NUM_FIXED;
>
>  assert(n < vs->conf.num_queues);
>  qemu_put_be32s(f, );
> @@ -892,10 +892,11 @@ void virtio_scsi_common_realize(DeviceState *dev,
>  sizeof(VirtIOSCSIConfig));
>
>  if (s->conf.num_queues == 0 ||
> -s->conf.num_queues > VIRTIO_QUEUE_MAX - 2) {
> +s->conf.num_queues > VIRTIO_QUEUE_MAX - 
> VIRTIO_SCSI_VQ_NUM_FIXED) {
>  error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
>   "must be a positive integer less than %d.",
> -   s->conf.num_queues, VIRTIO_QUEUE_MAX - 2);
> +   s->conf.num_queues,
> +   VIRTIO_QUEUE_MAX - VIRTIO_SCSI_VQ_NUM_FIXED);
>  virtio_cleanup(vdev);
>  return;
>  }
> diff --git a/hw/virtio/vhost-scsi-pci.c b/hw/virtio/vhost-scsi-pci.c
> index 5da6bb6449..2b25db9a3c 100644
> --- a/hw/virtio/vhost-scsi-pci.c
> +++ b/hw/virtio/vhost-scsi-pci.c
> @@ -50,7 +50,8 @@ static void vhost_scsi_pci_realize(VirtIOPCIProxy 
> *vpci_dev, Error **errp)
>  VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
>
>  if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> -vpci_dev->nvectors = vs->conf.num_queues + 3;
> +vpci_dev->nvectors = vs->conf.num_queues +
> + VIRTIO_SCSI_VQ_NUM_FIXED + 1;
>  }
>
>  qdev_set_parent_bus(vdev, BUS(_dev->bus));
> diff --git a/hw/virtio/vhost-user-scsi-pci.c b/hw/virtio/vhost-user-scsi-pci.c
> index 6f3375fe55..80710ab170 100644
> --- a/hw/virtio/vhost-user-scsi-pci.c
> +++ b/hw/virtio/vhost-user-scsi-pci.c
> @@ -56,7 +56,8 @@ static void vhost_user_scsi_pci_realize(VirtIOPCIProxy 
> *vpci_dev, Error **errp)
>  VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
>
>  if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> -vpci_dev->nvectors = vs->conf.num_queues + 3;
> +vpci_dev->nvectors = vs->conf.num_queues +
> + VIRTIO_SCSI_VQ_NUM_FIXED + 1;
>  }
>
>  qdev_set_parent_bus(vdev, BUS(_dev->bus));
> diff --git a/hw/virtio/virtio-scsi-pci.c b/hw/virtio/virtio-scsi-pci.c
> index e82e7e5680..c52d68053a 100644
> --- a/hw/virtio/virtio-scsi-pci.c
> +++ b/hw/virtio/virtio-scsi-pci.c
> @@ -51,7 +51,8 @@ static void virtio_scsi_pci_realize(VirtIOPCIProxy 
> *vpci_dev, Error **errp)
>  char *bus_name;
>
>  if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> -vpci_dev->nvectors = vs->conf.num_queues + 3;
> +vpci_dev->nvectors = vs->conf.num_queues +
> + VIRTIO_SCSI_VQ_NUM_FIXED + 1;
>  }
>
>  /*
> --
> 2.25.4
>

Reviewed-by: Raphael Norwitz 



Re: [PATCH v4 5/5] vhost-user-blk: default num_queues to -smp N

2020-05-30 Thread Raphael Norwitz
I'm happy with the code but as David pointed out with virtio-scsi, we
should probably add a comment about virtio_pci_optimal_num_queues()
capping the number of VQs here too.


On Wed, May 27, 2020 at 6:34 AM Stefan Hajnoczi  wrote:
>
> Automatically size the number of request virtqueues to match the number
> of vCPUs.  This ensures that completion interrupts are handled on the
> same vCPU that submitted the request.  No IPI is necessary to complete
> an I/O request and performance is improved.
>
> Signed-off-by: Stefan Hajnoczi 
> Reviewed-by: Cornelia Huck 
> ---
>  include/hw/virtio/vhost-user-blk.h | 2 ++
>  hw/block/vhost-user-blk.c  | 6 +-
>  hw/core/machine.c  | 1 +
>  hw/virtio/vhost-user-blk-pci.c | 4 
>  4 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/virtio/vhost-user-blk.h 
> b/include/hw/virtio/vhost-user-blk.h
> index 34ad6f0c0e..292d17147c 100644
> --- a/include/hw/virtio/vhost-user-blk.h
> +++ b/include/hw/virtio/vhost-user-blk.h
> @@ -25,6 +25,8 @@
>  #define VHOST_USER_BLK(obj) \
>  OBJECT_CHECK(VHostUserBlk, (obj), TYPE_VHOST_USER_BLK)
>
> +#define VHOST_USER_BLK_AUTO_NUM_QUEUES UINT16_MAX
> +
>  typedef struct VHostUserBlk {
>  VirtIODevice parent_obj;
>  CharBackend chardev;
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 9d8c0b3909..7a8639516f 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -385,6 +385,9 @@ static void vhost_user_blk_device_realize(DeviceState 
> *dev, Error **errp)
>  return;
>  }
>
> +if (s->num_queues == VHOST_USER_BLK_AUTO_NUM_QUEUES) {
> +s->num_queues = 1;
> +}
>  if (!s->num_queues || s->num_queues > VIRTIO_QUEUE_MAX) {
>  error_setg(errp, "vhost-user-blk: invalid number of IO queues");
>  return;
> @@ -496,7 +499,8 @@ static const VMStateDescription vmstate_vhost_user_blk = {
>
>  static Property vhost_user_blk_properties[] = {
>  DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> -DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues, 1),
> +DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
> +   VHOST_USER_BLK_AUTO_NUM_QUEUES),
>  DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
>  DEFINE_PROP_BIT("config-wce", VHostUserBlk, config_wce, 0, true),
>  DEFINE_PROP_END_OF_LIST(),
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 4aba3bdd3c..8cc4b54eec 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -32,6 +32,7 @@ GlobalProperty hw_compat_5_0[] = {
>  { "virtio-blk-device", "num-queues", "1"},
>  { "virtio-scsi-device", "num_queues", "1"},
>  { "vhost-scsi", "num_queues", "1"},
> +{ "vhost-user-blk", "num-queues", "1"},
>  { "vhost-user-scsi", "num_queues", "1"},
>  };
>  const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
> diff --git a/hw/virtio/vhost-user-blk-pci.c b/hw/virtio/vhost-user-blk-pci.c
> index 58d7c31735..1c8ab7f5e6 100644
> --- a/hw/virtio/vhost-user-blk-pci.c
> +++ b/hw/virtio/vhost-user-blk-pci.c
> @@ -54,6 +54,10 @@ static void vhost_user_blk_pci_realize(VirtIOPCIProxy 
> *vpci_dev, Error **errp)
>  VHostUserBlkPCI *dev = VHOST_USER_BLK_PCI(vpci_dev);
>  DeviceState *vdev = DEVICE(>vdev);
>
> +if (dev->vdev.num_queues == VHOST_USER_BLK_AUTO_NUM_QUEUES) {
> +dev->vdev.num_queues = virtio_pci_optimal_num_queues(0);
> +}
> +
>  if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
>  vpci_dev->nvectors = dev->vdev.num_queues + 1;
>  }
> --
> 2.25.4
>



[Bug 1881450] [NEW] Emulation of a math function fails for m68k Linux user mode

2020-05-30 Thread Ahmed Karaman
Public bug reported:

Please check the attached math-example.c file.
When running the m68k executable under QEMU, it results in an 
"Illegal instruction" error.
Other targets don't produce this error.

Steps to reproduce the bug:

1. Download the math-example.c attached file.
2. Compile it by running:
   m68k-linux-gnu-gcc -O2 -static math-example.c -o math-example-m68k -lm
3. Run the executable with QEMU:
   /build/qemu-5.0.0/build-gcc/m68k-linux-user/qemu-m68k math-example-m68k

The output of execution is:
   Profiling function expm1f():
   qemu: uncaught target signal 4 (Illegal instruction) - core dumped
   Illegal instruction (core dumped)

Output when running on other targets:
   Profiling function expm1f():
 Elapsed time: 47 ms
 Control result: 71804.953125

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: m68k

** Attachment added: "Emulating the expm1f() function."
   
https://bugs.launchpad.net/bugs/1881450/+attachment/5378913/+files/math-example.c

** Description changed:

  Please check the attached math-example.c file.
- When running the m68k executable under QEMU, it results in an "Illegal 
instruction" error.
+ When running the m68k executable under QEMU, it results in an 
+ "Illegal instruction" error.
  Other targets don't produce this error.
  
  Steps to reproduce the bug:
  
  1. Download the math-example.c attached file.
  2. Compile it by running:
- m68k-linux-gnu-gcc -O2 -static math-example.c -o math-example-m68k -lm
+    m68k-linux-gnu-gcc -O2 -static math-example.c -o math-example-m68k -lm
  3. Run the executable with QEMU:
- /build/qemu-5.0.0/build-gcc/m68k-linux-user/qemu-m68k 
math-example-m68k 
+    /build/qemu-5.0.0/build-gcc/m68k-linux-user/qemu-m68k math-example-m68k
  
  The output of execution is:
- Profiling function expm1f():
- qemu: uncaught target signal 4 (Illegal instruction) - core dumped
- Illegal instruction (core dumped)
+    Profiling function expm1f():
+    qemu: uncaught target signal 4 (Illegal instruction) - core dumped
+    Illegal instruction (core dumped)
  
- Expected output:
- Profiling function expm1f():
-   Elapsed time: 47 ms
-   Control result: 71804.953125
+ Output when running on other targets:
+    Profiling function expm1f():
+  Elapsed time: 47 ms
+  Control result: 71804.953125

** Attachment removed: "math-example.c"
   
https://bugs.launchpad.net/qemu/+bug/1881450/+attachment/5378913/+files/math-example.c

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1881450

Title:
  Emulation of a math function fails for m68k Linux user mode

Status in QEMU:
  New

Bug description:
  Please check the attached math-example.c file.
  When running the m68k executable under QEMU, it results in an 
  "Illegal instruction" error.
  Other targets don't produce this error.

  Steps to reproduce the bug:

  1. Download the math-example.c attached file.
  2. Compile it by running:
     m68k-linux-gnu-gcc -O2 -static math-example.c -o math-example-m68k -lm
  3. Run the executable with QEMU:
     /build/qemu-5.0.0/build-gcc/m68k-linux-user/qemu-m68k math-example-m68k

  The output of execution is:
     Profiling function expm1f():
     qemu: uncaught target signal 4 (Illegal instruction) - core dumped
     Illegal instruction (core dumped)

  Output when running on other targets:
     Profiling function expm1f():
   Elapsed time: 47 ms
   Control result: 71804.953125

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1881450/+subscriptions



[Bug 1881450] Re: Emulation of a math function fails for m68k Linux user mode

2020-05-30 Thread Ahmed Karaman
** Attachment added: "math-example.c"
   
https://bugs.launchpad.net/qemu/+bug/1881450/+attachment/5378914/+files/math-example.c

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1881450

Title:
  Emulation of a math function fails for m68k Linux user mode

Status in QEMU:
  New

Bug description:
  Please check the attached math-example.c file.
  When running the m68k executable under QEMU, it results in an 
  "Illegal instruction" error.
  Other targets don't produce this error.

  Steps to reproduce the bug:

  1. Download the math-example.c attached file.
  2. Compile it by running:
     m68k-linux-gnu-gcc -O2 -static math-example.c -o math-example-m68k -lm
  3. Run the executable with QEMU:
     /build/qemu-5.0.0/build-gcc/m68k-linux-user/qemu-m68k math-example-m68k

  The output of execution is:
     Profiling function expm1f():
     qemu: uncaught target signal 4 (Illegal instruction) - core dumped
     Illegal instruction (core dumped)

  Output when running on other targets:
     Profiling function expm1f():
   Elapsed time: 47 ms
   Control result: 71804.953125

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1881450/+subscriptions



Re: [PATCH v4 2/2] vhost-user-blk: delay vhost_user_blk_disconnect

2020-05-30 Thread Raphael Norwitz
On Thu, May 28, 2020 at 5:13 AM Dima Stepanov  wrote:
>
> A socket write during vhost-user communication may trigger a disconnect
> event, calling vhost_user_blk_disconnect() and clearing all the
> vhost_dev structures holding data that vhost-user functions expect to
> remain valid to roll back initialization correctly. Delay the cleanup to
> keep vhost_dev structure valid.
> There are two possible states to handle:
> 1. RUN_STATE_PRELAUNCH: skip bh oneshot call and perform disconnect in
> the caller routine.
> 2. RUN_STATE_RUNNING: delay by using bh
>
> BH changes are based on the similar changes for the vhost-user-net
> device:
>   commit e7c83a885f865128ae3cf1946f8cb538b63cbfba
>   "vhost-user: delay vhost_user_stop"
>
> Signed-off-by: Dima Stepanov 

Reviewed-by: Raphael Norwitz 

Li Feng - would you also like to sign off here?

> ---
>  hw/block/vhost-user-blk.c | 38 +-
>  1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 9d8c0b3..76838e7 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -349,6 +349,19 @@ static void vhost_user_blk_disconnect(DeviceState *dev)
>  vhost_dev_cleanup(>dev);
>  }
>
> +static void vhost_user_blk_event(void *opaque, QEMUChrEvent event);
> +
> +static void vhost_user_blk_chr_closed_bh(void *opaque)
> +{
> +DeviceState *dev = opaque;
> +VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +VHostUserBlk *s = VHOST_USER_BLK(vdev);
> +
> +vhost_user_blk_disconnect(dev);
> +qemu_chr_fe_set_handlers(>chardev, NULL, NULL, vhost_user_blk_event,
> +NULL, opaque, NULL, true);
> +}
> +
>  static void vhost_user_blk_event(void *opaque, QEMUChrEvent event)
>  {
>  DeviceState *dev = opaque;
> @@ -363,7 +376,30 @@ static void vhost_user_blk_event(void *opaque, 
> QEMUChrEvent event)
>  }
>  break;
>  case CHR_EVENT_CLOSED:
> -vhost_user_blk_disconnect(dev);
> +/*
> + * A close event may happen during a read/write, but vhost
> + * code assumes the vhost_dev remains setup, so delay the
> + * stop & clear. There are two possible paths to hit this
> + * disconnect event:
> + * 1. When VM is in the RUN_STATE_PRELAUNCH state. The
> + * vhost_user_blk_device_realize() is a caller.
> + * 2. In tha main loop phase after VM start.
> + *
> + * For p2 the disconnect event will be delayed. We can't
> + * do the same for p1, because we are not running the loop
> + * at this moment. So just skip this step and perform
> + * disconnect in the caller function.
> + *
> + * TODO: maybe it is a good idea to make the same fix
> + * for other vhost-user devices.
> + */
> +if (runstate_is_running()) {
> +AioContext *ctx = qemu_get_current_aio_context();
> +
> +qemu_chr_fe_set_handlers(>chardev, NULL, NULL, NULL, NULL,
> +NULL, NULL, false);
> +aio_bh_schedule_oneshot(ctx, vhost_user_blk_chr_closed_bh, 
> opaque);
> +}
>  break;
>  case CHR_EVENT_BREAK:
>  case CHR_EVENT_MUX_IN:
> --
> 2.7.4
>
>



Re: [PATCH v3] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION

2020-05-30 Thread Chen Gang
On 2020/5/28 下午6:25, Laurent Vivier wrote:
>> diff --git a/configure b/configure
>> index e225a1e3ff..2c2c489d1e 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3912,6 +3912,24 @@ EOF
>>  fi
>>  fi
>>  
>> +#
>> +# libdrm check
>> +
>> +cat > $TMPC << EOF
>> +#include 
>> +#include 
>> +int main(void)
>> +{
>> +struct drm_version version;
>> +ioctl(-1, DRM_IOCTL_VERSION, );
>> +return 0;
>> +}
>> +EOF
>> +if ! compile_prog "" ; then
>> +error_exit "libdrm check failed" \
>> +"Make sure to have the libdrm/drm.h installed."
>> +fi
> 
> You break the build of qemu if libdrm is not available, not a good idea.
> 
> In fact, you should only check for the include with something like
> "check_include libdrm/drm.h" and then define a HAVE_DRM_H to use it
> around the new code:
> 
> #ifdef HAVE_DRM_H
> #include 
> #endif
> 
> ...
> #ifdef HAVE_DRM_H
> static inline abi_long target_to_host_drmversion(...
> ...
> #endif
> ...
> 
> #ifdef HAVE_DRM_H
> IOCTL_SPECIAL(DRM_IOCTL_VERSION,...
> ...
> #endif
> 

OK, thanks. I'll send patch v4

Chen Gang





Re: [PATCH] tests/acceptance: Add boot tests for sh4 QEMU advent calendar image

2020-05-30 Thread Aleksandar Markovic
суб, 30. мај 2020. у 19:25 Thomas Huth  је написао/ла:
>
> On 30/05/2020 10.54, Aleksandar Markovic wrote:
> > 18:44 Pet, 15.05.2020. Thomas Huth  > > је написао/ла:
> >>
> >> Now that we can select the second serial console in the acceptance tests
> >> (see commit 746f244d9720 "Allow to use other serial consoles than
> > default"),
> >> we can also test the sh4 image from the QEMU advent calendar 2018.
> >>
> >> Signed-off-by: Thomas Huth mailto:th...@redhat.com>>
> >> ---
> >>  I've split my original patch that also added yet another mips64 test...
> >>  I hope it's easier to review/ack/merge this way this only addresses
> > sh4 here.
> >>
> >
> > It makes sense to me, Thomas. If I were you, I would do the same.
> >
> > But, sorry, do you intend to send mips64 counterpart in future, or it is
> > already sent, but I somehow missed it?
>
>  Hi Aleksandar,
>
> I was planning to send a separate patch, but then I noticed that there
> are already quite a lot of tests for the various flavors of the
> mips-malta machine in tests/acceptance/boot_linux_console.py ... so I
> currently doubt that the malta advent calendar image will add much value
> here?
>

I'm fine with your judgement.

Glad to see sh4 advent calendar.

Best wishes,
Aleksandar


>  Thomas
>



Re: [PATCH 0/4] python: pylint and flake8 support

2020-05-30 Thread Philippe Mathieu-Daudé
On 5/29/20 12:21 AM, John Snow wrote:
> This is a quick series to delint the files under python/qemu, with one
> extra fix outside of that domain.
> 
> This was split out from my longer series attempting to package
> python/qemu. This part is a nice standalone chunk.
> 
> John Snow (4):
>   scripts/qmp: Fix shebang and imports
>   python/machine.py: remove bare except
>   python/qemu: delint and add pylintrc
>   python/qemu: delint; add flake8 config
> 
>  python/qemu/.flake8|  2 ++
>  python/qemu/accel.py   |  9 ---
>  python/qemu/machine.py | 52 +++--
>  python/qemu/pylintrc   | 58 ++
>  python/qemu/qmp.py |  4 +--
>  python/qemu/qtest.py   | 42 +++---
>  scripts/qmp/qmp|  4 ++-
>  scripts/qmp/qom-fuse   |  4 ++-
>  scripts/qmp/qom-get|  6 +++--
>  scripts/qmp/qom-list   |  6 +++--
>  scripts/qmp/qom-set|  6 +++--
>  scripts/qmp/qom-tree   |  6 +++--
>  12 files changed, 150 insertions(+), 49 deletions(-)
>  create mode 100644 python/qemu/.flake8
>  create mode 100644 python/qemu/pylintrc
> 

Thanks, patches 1/3/4 applied to my python-next tree:
https://gitlab.com/philmd/qemu/commits/python-next

I skipped patch #2 which doesn't apply on top on Vladimir's "hard-kill"
patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg680200.html




Re: [PATCH v6 0/4] vhost-user block device backend implementation

2020-05-30 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/20200530171441.660814-1-coiby...@gmail.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==6732==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-coroutine" 
==6780==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==6780==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 
0x7ffebc79d000; bottom 0x7fa7567c8000; size: 0x005765fd5000 (375373254656)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
PASS 11 fdc-test /x86_64/fdc/read_no_dma_18
==6795==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 27 test-aio /aio-gsource/event/wait/no-flush-cb
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-aio-multithread -m=quick -k --tap < /dev/null | 
./scripts/tap-driver.pl --test-name="test-aio-multithread" 
==6800==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=./qemu-img 
QTEST_QEMU_STORAGE_DAEMON_BINARY=./qemu-storage-daemon tests/qtest/ide-test 
-m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ide-test" 
PASS 2 test-aio-multithread /aio/multi/schedule
==6822==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==6828==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
==6839==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==6845==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
==6851==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
---
PASS 6 test-throttle /throttle/detach_attach
PASS 7 test-throttle /throttle/config_functions
PASS 8 test-throttle /throttle/accounting
==6868==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 9 test-throttle /throttle/groups
PASS 10 test-throttle /throttle/config/enabled
PASS 11 test-throttle /throttle/config/conflicting
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-thread-pool" 
==6872==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
---
PASS 12 test-hbitmap /hbitmap/set/two-elem
PASS 13 test-hbitmap /hbitmap/set/general
PASS 14 test-hbitmap /hbitmap/set/twice
==6943==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false 

Re: [PATCH] hw/display/cirrus_vga: Fix code mis-indentation

2020-05-30 Thread Thomas Huth
On 29/05/2020 18.54, Philippe Mathieu-Daudé wrote:
> While replacing fprintf() by qemu_log_mask() in commit
> 2b55f4d3504, we incorrectly used a 'tab = 4 spaces'
> alignment, leading to misindented new code. Fix now.
> 
> Reported-by: Peter Maydell 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/display/cirrus_vga.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index 92c197cdde..212d6f5e61 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -1032,9 +1032,9 @@ static void cirrus_bitblt_start(CirrusVGAState * s)
>  } else {
>   if (s->cirrus_blt_mode & CIRRUS_BLTMODE_TRANSPARENTCOMP) {
>   if (s->cirrus_blt_pixelwidth > 2) {
> -qemu_log_mask(LOG_GUEST_ERROR,
> -  "cirrus: src transparent without colorexpand "
> -  "must be 8bpp or 16bpp\n");
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "cirrus: src transparent without 
> colorexpand "
> +  "must be 8bpp or 16bpp\n");
>   goto bitblt_ignore;
>   }
>   if (s->cirrus_blt_mode & CIRRUS_BLTMODE_BACKWARDS) {
> 

I think it would be better to fix the TABs in the whole surounding area,
too. Or maybe even in the whole file. Otherwise this problem will happen
soon again...

 Thomas




[Bug 1878255] Re: Assertion failure in bdrv_aio_cancel, through ide

2020-05-30 Thread Alexander Bulekov
> Not all of those register writes are actually important for the bug,
so I simplified them to the fewest writes and fewest bits.

Thanks for bringing this up. I tried to trim long write commands from
both "sides", but there can still be useless data in the middle. I'll
work on something that can split them up so only the relevant data
remains in the future.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1878255

Title:
  Assertion failure in bdrv_aio_cancel, through ide

Status in QEMU:
  New

Bug description:
  Hello,
  While fuzzing, I found an input that triggers an assertion failure in 
bdrv_aio_cancel, through ide:

  #1  0x7685755b in __GI_abort () at abort.c:79
  #2  0x56a8d396 in bdrv_aio_cancel (acb=0x60761290) at 
/home/alxndr/Development/qemu/block/io.c:2746
  #3  0x56a58525 in blk_aio_cancel (acb=0x2) at 
/home/alxndr/Development/qemu/block/block-backend.c:1540
  #4  0x56552f5b in ide_reset (s=) at 
/home/alxndr/Development/qemu/hw/ide/core.c:1318
  #5  0x56552aeb in ide_bus_reset (bus=0x62d17398) at 
/home/alxndr/Development/qemu/hw/ide/core.c:2422
  #6  0x56579ba5 in ahci_reset_port (s=, port=) at /home/alxndr/Development/qemu/hw/ide/ahci.c:650
  #7  0x5657bd8d in ahci_port_write (s=0x61e14d70, port=0x2, 
offset=, val=0x10) at 
/home/alxndr/Development/qemu/hw/ide/ahci.c:360
  #8  0x5657bd8d in ahci_mem_write (opaque=, 
addr=, val=, size=) at 
/home/alxndr/Development/qemu/hw/ide/ahci.c:513
  #9  0x560028d7 in memory_region_write_accessor (mr=, 
addr=, value=, size=, 
shift=, mask=, attrs=...) at 
/home/alxndr/Development/qemu/memory.c:483
  #10 0x56002280 in access_with_adjusted_size (addr=, 
value=, size=, access_size_min=, 
access_size_max=, access_fn=, mr=0x61e14da0, 
attrs=...) at /home/alxndr/Development/qemu/memory.c:544
  #11 0x56002280 in memory_region_dispatch_write (mr=, 
addr=, data=0x10, op=, attrs=...) at 
/home/alxndr/Development/qemu/memory.c:1476
  #12 0x55f171d4 in flatview_write_continue (fv=, 
addr=0xe106c22c, attrs=..., ptr=, len=0x1, addr1=0x7fffb8d0, 
l=, mr=0x61e14da0) at 
/home/alxndr/Development/qemu/exec.c:3137
  #13 0x55f0fb98 in flatview_write (fv=0x6063b180, addr=, attrs=..., buf=, len=) at 
/home/alxndr/Development/qemu/exec.c:3177

  I can reproduce it in qemu 5.0 using:

  cat << EOF | ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -qtest 
stdio -monitor none -serial none -M pc-q35-5.0  -nographic
  outl 0xcf8 0x8000fa24
  outl 0xcfc 0xe106c000
  outl 0xcf8 0x8000fa04
  outw 0xcfc 0x7
  outl 0xcf8 0x8000fb20
  write 0x0 0x3 0x2780e7
  write 0xe106c22c 0xd 0x1130c218021130c218021130c2
  write 0xe106c218 0x15 0x110010110010110010110010110010110010110010
  EOF

  I also attached the commands to this launchpad report, in case the
  formatting is broken:

  qemu-system-i386 -qtest stdio -monitor none -serial none -M pc-q35-5.0
  -nographic < 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/1878255/+subscriptions



Re: [PATCH] tests/acceptance: Add boot tests for sh4 QEMU advent calendar image

2020-05-30 Thread Thomas Huth
On 30/05/2020 10.54, Aleksandar Markovic wrote:
> 18:44 Pet, 15.05.2020. Thomas Huth  > је написао/ла:
>>
>> Now that we can select the second serial console in the acceptance tests
>> (see commit 746f244d9720 "Allow to use other serial consoles than
> default"),
>> we can also test the sh4 image from the QEMU advent calendar 2018.
>>
>> Signed-off-by: Thomas Huth mailto:th...@redhat.com>>
>> ---
>>  I've split my original patch that also added yet another mips64 test...
>>  I hope it's easier to review/ack/merge this way this only addresses
> sh4 here.
>>
> 
> It makes sense to me, Thomas. If I were you, I would do the same.
> 
> But, sorry, do you intend to send mips64 counterpart in future, or it is
> already sent, but I somehow missed it?

 Hi Aleksandar,

I was planning to send a separate patch, but then I noticed that there
are already quite a lot of tests for the various flavors of the
mips-malta machine in tests/acceptance/boot_linux_console.py ... so I
currently doubt that the malta advent calendar image will add much value
here?

 Thomas




[PATCH v6 3/4] vhost-user block device backend server

2020-05-30 Thread Coiby Xu
By making use of libvhost-user, block device drive can be shared to
the connected vhost-user client. Only one client can connect to the
server one time.

Since vhost-user-server needs a block drive to be created first, delay
the creation of this object.

Signed-off-by: Coiby Xu 
---
 block/Makefile.objs  |   1 +
 block/export/vhost-user-blk-server.c | 715 +++
 block/export/vhost-user-blk-server.h |  34 ++
 softmmu/vl.c |   4 +
 4 files changed, 754 insertions(+)
 create mode 100644 block/export/vhost-user-blk-server.c
 create mode 100644 block/export/vhost-user-blk-server.h

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 3635b6b4c1..0eb7eff470 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -24,6 +24,7 @@ block-obj-y += throttle-groups.o
 block-obj-$(CONFIG_LINUX) += nvme.o
 
 block-obj-y += nbd.o
+block-obj-$(CONFIG_LINUX) += export/vhost-user-blk-server.o 
../contrib/libvhost-user/libvhost-user.o
 block-obj-$(CONFIG_SHEEPDOG) += sheepdog.o
 block-obj-$(CONFIG_LIBISCSI) += iscsi.o
 block-obj-$(if $(CONFIG_LIBISCSI),y,n) += iscsi-opts.o
diff --git a/block/export/vhost-user-blk-server.c 
b/block/export/vhost-user-blk-server.c
new file mode 100644
index 00..630aeb4ac4
--- /dev/null
+++ b/block/export/vhost-user-blk-server.c
@@ -0,0 +1,715 @@
+/*
+ * Sharing QEMU block devices via vhost-user protocal
+ *
+ * Author: Coiby Xu 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "block/block.h"
+#include "vhost-user-blk-server.h"
+#include "qapi/error.h"
+#include "qom/object_interfaces.h"
+#include "sysemu/block-backend.h"
+
+enum {
+VHOST_USER_BLK_MAX_QUEUES = 1,
+};
+struct virtio_blk_inhdr {
+unsigned char status;
+};
+
+static QTAILQ_HEAD(, VuBlockDev) vu_block_devs =
+ QTAILQ_HEAD_INITIALIZER(vu_block_devs);
+
+
+typedef struct VuBlockReq {
+VuVirtqElement *elem;
+int64_t sector_num;
+size_t size;
+struct virtio_blk_inhdr *in;
+struct virtio_blk_outhdr out;
+VuServer *server;
+struct VuVirtq *vq;
+} VuBlockReq;
+
+
+static void vu_block_req_complete(VuBlockReq *req)
+{
+VuDev *vu_dev = >server->vu_dev;
+
+/* IO size with 1 extra status byte */
+vu_queue_push(vu_dev, req->vq, req->elem, req->size + 1);
+vu_queue_notify(vu_dev, req->vq);
+
+if (req->elem) {
+free(req->elem);
+}
+
+g_free(req);
+}
+
+static VuBlockDev *get_vu_block_device_by_server(VuServer *server)
+{
+return container_of(server, VuBlockDev, vu_server);
+}
+
+static int coroutine_fn
+vu_block_discard_write_zeroes(VuBlockReq *req, struct iovec *iov,
+  uint32_t iovcnt, uint32_t type)
+{
+struct virtio_blk_discard_write_zeroes desc;
+ssize_t size = iov_to_buf(iov, iovcnt, 0, , sizeof(desc));
+if (unlikely(size != sizeof(desc))) {
+error_report("Invalid size %ld, expect %ld", size, sizeof(desc));
+return -EINVAL;
+}
+
+VuBlockDev *vdev_blk = get_vu_block_device_by_server(req->server);
+uint64_t range[2] = { le64toh(desc.sector) << 9,
+  le32toh(desc.num_sectors) << 9 };
+if (type == VIRTIO_BLK_T_DISCARD) {
+if (blk_co_pdiscard(vdev_blk->backend, range[0], range[1]) == 0) {
+return 0;
+}
+} else if (type == VIRTIO_BLK_T_WRITE_ZEROES) {
+if (blk_co_pwrite_zeroes(vdev_blk->backend,
+ range[0], range[1], 0) == 0) {
+return 0;
+}
+}
+
+return -EINVAL;
+}
+
+
+static void coroutine_fn vu_block_flush(VuBlockReq *req)
+{
+VuBlockDev *vdev_blk = get_vu_block_device_by_server(req->server);
+BlockBackend *backend = vdev_blk->backend;
+blk_co_flush(backend);
+}
+
+
+struct req_data {
+VuServer *server;
+VuVirtq *vq;
+VuVirtqElement *elem;
+};
+
+static void coroutine_fn vu_block_virtio_process_req(void *opaque)
+{
+struct req_data *data = opaque;
+VuServer *server = data->server;
+VuVirtq *vq = data->vq;
+VuVirtqElement *elem = data->elem;
+uint32_t type;
+VuBlockReq *req;
+
+VuBlockDev *vdev_blk = get_vu_block_device_by_server(server);
+BlockBackend *backend = vdev_blk->backend;
+
+struct iovec *in_iov = elem->in_sg;
+struct iovec *out_iov = elem->out_sg;
+unsigned in_num = elem->in_num;
+unsigned out_num = elem->out_num;
+/* refer to hw/block/virtio_blk.c */
+if (elem->out_num < 1 || elem->in_num < 1) {
+error_report("virtio-blk request missing headers");
+free(elem);
+return;
+}
+
+req = g_new0(VuBlockReq, 1);
+req->server = server;
+req->vq = vq;
+req->elem = elem;
+
+if (unlikely(iov_to_buf(out_iov, out_num, 0, >out,
+sizeof(req->out)) != sizeof(req->out))) {
+

[PATCH v6 2/4] generic vhost user server

2020-05-30 Thread Coiby Xu
Sharing QEMU devices via vhost-user protocol.

Only one vhost-user client can connect to the server one time.

Signed-off-by: Coiby Xu 
---
 util/Makefile.objs   |   1 +
 util/vhost-user-server.c | 404 +++
 util/vhost-user-server.h |  59 ++
 3 files changed, 464 insertions(+)
 create mode 100644 util/vhost-user-server.c
 create mode 100644 util/vhost-user-server.h

diff --git a/util/Makefile.objs b/util/Makefile.objs
index fe339c2636..f54a6c80ec 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -40,6 +40,7 @@ util-obj-y += readline.o
 util-obj-y += rcu.o
 util-obj-$(CONFIG_MEMBARRIER) += sys_membarrier.o
 util-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
+util-obj-$(CONFIG_LINUX) += vhost-user-server.o
 util-obj-y += qemu-coroutine-sleep.o
 util-obj-y += qemu-co-shared-resource.o
 util-obj-y += coroutine-$(CONFIG_COROUTINE_BACKEND).o
diff --git a/util/vhost-user-server.c b/util/vhost-user-server.c
new file mode 100644
index 00..414d0d4e47
--- /dev/null
+++ b/util/vhost-user-server.c
@@ -0,0 +1,404 @@
+/*
+ * Sharing QEMU devices via vhost-user protocol
+ *
+ * Author: Coiby Xu 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include 
+#include "qemu/main-loop.h"
+#include "vhost-user-server.h"
+
+static void vmsg_close_fds(VhostUserMsg *vmsg)
+{
+int i;
+for (i = 0; i < vmsg->fd_num; i++) {
+close(vmsg->fds[i]);
+}
+}
+
+static void vmsg_unblock_fds(VhostUserMsg *vmsg)
+{
+int i;
+for (i = 0; i < vmsg->fd_num; i++) {
+qemu_set_nonblock(vmsg->fds[i]);
+}
+}
+
+static void vu_accept(QIONetListener *listener, QIOChannelSocket *sioc,
+  gpointer opaque);
+
+static void close_client(VuServer *server)
+{
+vu_deinit(>vu_dev);
+server->sioc = NULL;
+object_unref(OBJECT(server->ioc));
+
+server->sioc_slave = NULL;
+object_unref(OBJECT(server->ioc_slave));
+/*
+ * Set the callback function for network listener so another
+ * vhost-user client can connect to this server
+ */
+qio_net_listener_set_client_func(server->listener,
+ vu_accept,
+ server,
+ NULL);
+}
+
+static void panic_cb(VuDev *vu_dev, const char *buf)
+{
+VuServer *server = container_of(vu_dev, VuServer, vu_dev);
+
+if (buf) {
+error_report("vu_panic: %s", buf);
+}
+
+if (server->sioc) {
+close_client(server);
+}
+
+if (server->device_panic_notifier) {
+server->device_panic_notifier(server);
+}
+}
+
+static QIOChannel *slave_io_channel(VuServer *server, int fd,
+Error **local_err)
+{
+if (server->sioc_slave) {
+if (fd == server->sioc_slave->fd) {
+return server->ioc_slave;
+}
+} else {
+server->sioc_slave = qio_channel_socket_new_fd(fd, local_err);
+if (!*local_err) {
+server->ioc_slave = QIO_CHANNEL(server->sioc_slave);
+return server->ioc_slave;
+}
+}
+
+return NULL;
+}
+
+static bool coroutine_fn
+vu_message_read(VuDev *vu_dev, int conn_fd, VhostUserMsg *vmsg)
+{
+struct iovec iov = {
+.iov_base = (char *)vmsg,
+.iov_len = VHOST_USER_HDR_SIZE,
+};
+int rc, read_bytes = 0;
+Error *local_err = NULL;
+/*
+ * Store fds/nfds returned from qio_channel_readv_full into
+ * temporary variables.
+ *
+ * VhostUserMsg is a packed structure, gcc will complain about passing
+ * pointer to a packed structure member if we pass _num
+ * and  directly when calling qio_channel_readv_full,
+ * thus two temporary variables nfds and fds are used here.
+ */
+size_t nfds = 0, nfds_t = 0;
+int *fds = NULL, *fds_t = NULL;
+VuServer *server = container_of(vu_dev, VuServer, vu_dev);
+QIOChannel *ioc = NULL;
+
+if (conn_fd == server->sioc->fd) {
+ioc = server->ioc;
+} else {
+/* Slave communication will also use this function to read msg */
+ioc = slave_io_channel(server, conn_fd, _err);
+}
+
+if (!ioc) {
+error_report_err(local_err);
+goto fail;
+}
+
+assert(qemu_in_coroutine());
+do {
+/*
+ * qio_channel_readv_full may have short reads, keeping calling it
+ * until getting VHOST_USER_HDR_SIZE or 0 bytes in total
+ */
+rc = qio_channel_readv_full(ioc, , 1, _t, _t, _err);
+if (rc < 0) {
+if (rc == QIO_CHANNEL_ERR_BLOCK) {
+qio_channel_yield(ioc, G_IO_IN);
+continue;
+} else {
+error_report_err(local_err);
+return false;
+}
+}
+read_bytes += rc;
+if (nfds_t > 0) {
+

[PATCH v6 1/4] Allow vu_message_read to be replaced

2020-05-30 Thread Coiby Xu
Allow vu_message_read to be replaced by one which will make use of the
QIOChannel functions. Thus reading vhost-user message won't stall the
guest.

Signed-off-by: Coiby Xu 
---
 contrib/libvhost-user/libvhost-user-glib.c |  2 +-
 contrib/libvhost-user/libvhost-user.c  | 11 ++-
 contrib/libvhost-user/libvhost-user.h  | 21 +
 tests/vhost-user-bridge.c  |  2 ++
 tools/virtiofsd/fuse_virtio.c  |  4 ++--
 5 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/contrib/libvhost-user/libvhost-user-glib.c 
b/contrib/libvhost-user/libvhost-user-glib.c
index 53f1ca4cdd..0df2ec9271 100644
--- a/contrib/libvhost-user/libvhost-user-glib.c
+++ b/contrib/libvhost-user/libvhost-user-glib.c
@@ -147,7 +147,7 @@ vug_init(VugDev *dev, uint16_t max_queues, int socket,
 g_assert(dev);
 g_assert(iface);
 
-if (!vu_init(>parent, max_queues, socket, panic, set_watch,
+if (!vu_init(>parent, max_queues, socket, panic, NULL, set_watch,
  remove_watch, iface)) {
 return false;
 }
diff --git a/contrib/libvhost-user/libvhost-user.c 
b/contrib/libvhost-user/libvhost-user.c
index 3bca996c62..0c7368baa2 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -67,8 +67,6 @@
 /* The version of inflight buffer */
 #define INFLIGHT_VERSION 1
 
-#define VHOST_USER_HDR_SIZE offsetof(VhostUserMsg, payload.u64)
-
 /* The version of the protocol we support */
 #define VHOST_USER_VERSION 1
 #define LIBVHOST_USER_DEBUG 0
@@ -412,7 +410,7 @@ vu_process_message_reply(VuDev *dev, const VhostUserMsg 
*vmsg)
 goto out;
 }
 
-if (!vu_message_read(dev, dev->slave_fd, _reply)) {
+if (!dev->read_msg(dev, dev->slave_fd, _reply)) {
 goto out;
 }
 
@@ -647,7 +645,7 @@ vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg 
*vmsg)
 /* Wait for QEMU to confirm that it's registered the handler for the
  * faults.
  */
-if (!vu_message_read(dev, dev->sock, vmsg) ||
+if (!dev->read_msg(dev, dev->sock, vmsg) ||
 vmsg->size != sizeof(vmsg->payload.u64) ||
 vmsg->payload.u64 != 0) {
 vu_panic(dev, "failed to receive valid ack for postcopy 
set-mem-table");
@@ -1653,7 +1651,7 @@ vu_dispatch(VuDev *dev)
 int reply_requested;
 bool need_reply, success = false;
 
-if (!vu_message_read(dev, dev->sock, )) {
+if (!dev->read_msg(dev, dev->sock, )) {
 goto end;
 }
 
@@ -1704,6 +1702,7 @@ vu_deinit(VuDev *dev)
 }
 
 if (vq->kick_fd != -1) {
+dev->remove_watch(dev, vq->kick_fd);
 close(vq->kick_fd);
 vq->kick_fd = -1;
 }
@@ -1751,6 +1750,7 @@ vu_init(VuDev *dev,
 uint16_t max_queues,
 int socket,
 vu_panic_cb panic,
+vu_read_msg_cb read_msg,
 vu_set_watch_cb set_watch,
 vu_remove_watch_cb remove_watch,
 const VuDevIface *iface)
@@ -1768,6 +1768,7 @@ vu_init(VuDev *dev,
 
 dev->sock = socket;
 dev->panic = panic;
+dev->read_msg = read_msg ? read_msg : vu_message_read;
 dev->set_watch = set_watch;
 dev->remove_watch = remove_watch;
 dev->iface = iface;
diff --git a/contrib/libvhost-user/libvhost-user.h 
b/contrib/libvhost-user/libvhost-user.h
index f30394fab6..d756da8548 100644
--- a/contrib/libvhost-user/libvhost-user.h
+++ b/contrib/libvhost-user/libvhost-user.h
@@ -30,6 +30,8 @@
 
 #define VHOST_MEMORY_MAX_NREGIONS 8
 
+#define VHOST_USER_HDR_SIZE offsetof(VhostUserMsg, payload.u64)
+
 typedef enum VhostSetConfigType {
 VHOST_SET_CONFIG_TYPE_MASTER = 0,
 VHOST_SET_CONFIG_TYPE_MIGRATION = 1,
@@ -205,6 +207,7 @@ typedef uint64_t (*vu_get_features_cb) (VuDev *dev);
 typedef void (*vu_set_features_cb) (VuDev *dev, uint64_t features);
 typedef int (*vu_process_msg_cb) (VuDev *dev, VhostUserMsg *vmsg,
   int *do_reply);
+typedef bool (*vu_read_msg_cb) (VuDev *dev, int sock, VhostUserMsg *vmsg);
 typedef void (*vu_queue_set_started_cb) (VuDev *dev, int qidx, bool started);
 typedef bool (*vu_queue_is_processed_in_order_cb) (VuDev *dev, int qidx);
 typedef int (*vu_get_config_cb) (VuDev *dev, uint8_t *config, uint32_t len);
@@ -373,6 +376,23 @@ struct VuDev {
 bool broken;
 uint16_t max_queues;
 
+/* @read_msg: custom method to read vhost-user message
+ *
+ * Read data from vhost_user socket fd and fill up
+ * the passed VhostUserMsg *vmsg struct.
+ *
+ * If reading fails, it should close the received set of file
+ * descriptors as socket message's auxiliary data.
+ *
+ * For the details, please refer to vu_message_read in libvhost-user.c
+ * which will be used by default if not custom method is provided when
+ * calling vu_init
+ *
+ * Returns: true if vhost-user message successfully received,
+ *  otherwise return false.
+ *
+ */
+vu_read_msg_cb read_msg;
 /* 

[PATCH v6 4/4] new qTest case to test the vhost-user-blk-server

2020-05-30 Thread Coiby Xu
This test case has the same tests as tests/virtio-blk-test.c except for
tests have block_resize. Since vhost-user server can only server one
client one time, two instances of qemu-storage-daemon are launched
for the hotplug test.

In order to not block scripts/tap-driver.pl, vhost-user-blk-server will
send "quit" command to qemu-storage-daemon's QMP monitor. So a function
is added to libqtest.c to establish socket connection with socket
server.

Signed-off-by: Coiby Xu 
---
 tests/Makefile.include  |   3 +-
 tests/qtest/Makefile.include|   2 +
 tests/qtest/libqos/vhost-user-blk.c | 126 +
 tests/qtest/libqos/vhost-user-blk.h |  44 ++
 tests/qtest/libqtest.c  |  44 +-
 tests/qtest/libqtest.h  |  38 ++
 tests/qtest/vhost-user-blk-test.c   | 741 
 7 files changed, 966 insertions(+), 32 deletions(-)
 create mode 100644 tests/qtest/libqos/vhost-user-blk.c
 create mode 100644 tests/qtest/libqos/vhost-user-blk.h
 create mode 100644 tests/qtest/vhost-user-blk-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 03a74b60f6..f7136b2fc6 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -636,7 +636,8 @@ endef
 $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: %-softmmu/all 
$(check-qtest-y)
$(call do_test_human,$(check-qtest-$*-y:%=tests/qtest/%$(EXESUF)) 
$(check-qtest-generic-y:%=tests/qtest/%$(EXESUF)), \
  QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
- QTEST_QEMU_IMG=qemu-img$(EXESUF))
+ QTEST_QEMU_IMG=./qemu-img$(EXESUF) \
+ QTEST_QEMU_STORAGE_DAEMON_BINARY=./qemu-storage-daemon$(EXESUF))
 
 check-unit: $(check-unit-y)
$(call do_test_human, $^)
diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
index 9e5a51d033..b6f081cb26 100644
--- a/tests/qtest/Makefile.include
+++ b/tests/qtest/Makefile.include
@@ -186,6 +186,7 @@ libqos-obj-y += tests/qtest/libqos/virtio.o
 libqos-obj-$(CONFIG_VIRTFS) += tests/qtest/libqos/virtio-9p.o
 libqos-obj-y += tests/qtest/libqos/virtio-balloon.o
 libqos-obj-y += tests/qtest/libqos/virtio-blk.o
+libqos-obj-$(CONFIG_LINUX) += tests/qtest/libqos/vhost-user-blk.o
 libqos-obj-y += tests/qtest/libqos/virtio-mmio.o
 libqos-obj-y += tests/qtest/libqos/virtio-net.o
 libqos-obj-y += tests/qtest/libqos/virtio-pci.o
@@ -230,6 +231,7 @@ qos-test-obj-$(CONFIG_VHOST_NET_USER) += 
tests/qtest/vhost-user-test.o $(chardev
 qos-test-obj-y += tests/qtest/virtio-test.o
 qos-test-obj-$(CONFIG_VIRTFS) += tests/qtest/virtio-9p-test.o
 qos-test-obj-y += tests/qtest/virtio-blk-test.o
+qos-test-obj-$(CONFIG_LINUX) += tests/qtest/vhost-user-blk-test.o
 qos-test-obj-y += tests/qtest/virtio-net-test.o
 qos-test-obj-y += tests/qtest/virtio-rng-test.o
 qos-test-obj-y += tests/qtest/virtio-scsi-test.o
diff --git a/tests/qtest/libqos/vhost-user-blk.c 
b/tests/qtest/libqos/vhost-user-blk.c
new file mode 100644
index 00..ec46b7ddb4
--- /dev/null
+++ b/tests/qtest/libqos/vhost-user-blk.c
@@ -0,0 +1,126 @@
+/*
+ * libqos driver framework
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito 
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see 
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/module.h"
+#include "standard-headers/linux/virtio_blk.h"
+#include "libqos/qgraph.h"
+#include "libqos/vhost-user-blk.h"
+
+#define PCI_SLOT0x04
+#define PCI_FN  0x00
+
+/* virtio-blk-device */
+static void *qvhost_user_blk_get_driver(QVhostUserBlk *v_blk,
+const char *interface)
+{
+if (!g_strcmp0(interface, "vhost-user-blk")) {
+return v_blk;
+}
+if (!g_strcmp0(interface, "virtio")) {
+return v_blk->vdev;
+}
+
+fprintf(stderr, "%s not present in vhost-user-blk-device\n", interface);
+g_assert_not_reached();
+}
+
+static void *qvhost_user_blk_device_get_driver(void *object,
+   const char *interface)
+{
+QVhostUserBlkDevice *v_blk = object;
+return qvhost_user_blk_get_driver(_blk->blk, interface);
+}
+
+static void *vhost_user_blk_device_create(void *virtio_dev,
+  QGuestAllocator *t_alloc,
+  void *addr)
+{
+QVhostUserBlkDevice *vhost_user_blk = g_new0(QVhostUserBlkDevice, 1);
+QVhostUserBlk *interface = _user_blk->blk;
+

[PATCH v6 0/4] vhost-user block device backend implementation

2020-05-30 Thread Coiby Xu
v6
 - add missing license header and include guard
 - vhost-user server only serve one client one time
 - a bug fix in custom vu_message_read
 - using qemu-storage-daemon to start vhost-user-blk-server
 - a bug fix to pass docker-test-clang@ubuntu

Coiby Xu (4):
  Allow vu_message_read to be replaced
  generic vhost user server
  vhost-user block device backend server
  new qTest case to test the vhost-user-blk-server

 block/Makefile.objs|   1 +
 block/export/vhost-user-blk-server.c   | 715 
 block/export/vhost-user-blk-server.h   |  34 +
 contrib/libvhost-user/libvhost-user-glib.c |   2 +-
 contrib/libvhost-user/libvhost-user.c  |  11 +-
 contrib/libvhost-user/libvhost-user.h  |  21 +
 softmmu/vl.c   |   4 +
 tests/Makefile.include |   3 +-
 tests/qtest/Makefile.include   |   2 +
 tests/qtest/libqos/vhost-user-blk.c| 126 
 tests/qtest/libqos/vhost-user-blk.h|  44 ++
 tests/qtest/libqtest.c |  44 +-
 tests/qtest/libqtest.h |  38 ++
 tests/qtest/vhost-user-blk-test.c  | 741 +
 tests/vhost-user-bridge.c  |   2 +
 tools/virtiofsd/fuse_virtio.c  |   4 +-
 util/Makefile.objs |   1 +
 util/vhost-user-server.c   | 404 +++
 util/vhost-user-server.h   |  59 ++
 19 files changed, 2216 insertions(+), 40 deletions(-)
 create mode 100644 block/export/vhost-user-blk-server.c
 create mode 100644 block/export/vhost-user-blk-server.h
 create mode 100644 tests/qtest/libqos/vhost-user-blk.c
 create mode 100644 tests/qtest/libqos/vhost-user-blk.h
 create mode 100644 tests/qtest/vhost-user-blk-test.c
 create mode 100644 util/vhost-user-server.c
 create mode 100644 util/vhost-user-server.h

--
2.26.2




[Bug 1878255] Re: Assertion failure in bdrv_aio_cancel, through ide

2020-05-30 Thread John Snow
Forgot to mention:

4. the last write to PxSCTL is what actually causes the reset by
clearing the DET bit that was armed.

In response to Philippe: Yes, if you had a malicious kernel or root
access to the guest, you could emit a sequence of PIO and memory write
operations to trip this. Even the reproducer CLI omits -accel qtest, so
at a minimum a malicious firmware image that's guaranteed not to be
interrupted could trigger the race condition.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1878255

Title:
  Assertion failure in bdrv_aio_cancel, through ide

Status in QEMU:
  New

Bug description:
  Hello,
  While fuzzing, I found an input that triggers an assertion failure in 
bdrv_aio_cancel, through ide:

  #1  0x7685755b in __GI_abort () at abort.c:79
  #2  0x56a8d396 in bdrv_aio_cancel (acb=0x60761290) at 
/home/alxndr/Development/qemu/block/io.c:2746
  #3  0x56a58525 in blk_aio_cancel (acb=0x2) at 
/home/alxndr/Development/qemu/block/block-backend.c:1540
  #4  0x56552f5b in ide_reset (s=) at 
/home/alxndr/Development/qemu/hw/ide/core.c:1318
  #5  0x56552aeb in ide_bus_reset (bus=0x62d17398) at 
/home/alxndr/Development/qemu/hw/ide/core.c:2422
  #6  0x56579ba5 in ahci_reset_port (s=, port=) at /home/alxndr/Development/qemu/hw/ide/ahci.c:650
  #7  0x5657bd8d in ahci_port_write (s=0x61e14d70, port=0x2, 
offset=, val=0x10) at 
/home/alxndr/Development/qemu/hw/ide/ahci.c:360
  #8  0x5657bd8d in ahci_mem_write (opaque=, 
addr=, val=, size=) at 
/home/alxndr/Development/qemu/hw/ide/ahci.c:513
  #9  0x560028d7 in memory_region_write_accessor (mr=, 
addr=, value=, size=, 
shift=, mask=, attrs=...) at 
/home/alxndr/Development/qemu/memory.c:483
  #10 0x56002280 in access_with_adjusted_size (addr=, 
value=, size=, access_size_min=, 
access_size_max=, access_fn=, mr=0x61e14da0, 
attrs=...) at /home/alxndr/Development/qemu/memory.c:544
  #11 0x56002280 in memory_region_dispatch_write (mr=, 
addr=, data=0x10, op=, attrs=...) at 
/home/alxndr/Development/qemu/memory.c:1476
  #12 0x55f171d4 in flatview_write_continue (fv=, 
addr=0xe106c22c, attrs=..., ptr=, len=0x1, addr1=0x7fffb8d0, 
l=, mr=0x61e14da0) at 
/home/alxndr/Development/qemu/exec.c:3137
  #13 0x55f0fb98 in flatview_write (fv=0x6063b180, addr=, attrs=..., buf=, len=) at 
/home/alxndr/Development/qemu/exec.c:3177

  I can reproduce it in qemu 5.0 using:

  cat << EOF | ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -qtest 
stdio -monitor none -serial none -M pc-q35-5.0  -nographic
  outl 0xcf8 0x8000fa24
  outl 0xcfc 0xe106c000
  outl 0xcf8 0x8000fa04
  outw 0xcfc 0x7
  outl 0xcf8 0x8000fb20
  write 0x0 0x3 0x2780e7
  write 0xe106c22c 0xd 0x1130c218021130c218021130c2
  write 0xe106c218 0x15 0x110010110010110010110010110010110010110010
  EOF

  I also attached the commands to this launchpad report, in case the
  formatting is broken:

  qemu-system-i386 -qtest stdio -monitor none -serial none -M pc-q35-5.0
  -nographic < 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/1878255/+subscriptions



[PATCH] migration/vmstate: Remove unnecessary MemoryRegion forward declaration

2020-05-30 Thread Philippe Mathieu-Daudé
"migration/vmstate.h" only uses pointer to MemoryRegion, which
is already forward declared in "qemu/typedefs.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
CI: https://travis-ci.org/github/philmd/qemu/builds/692879495
---
 include/migration/vmstate.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 30667631bc..eafa39f560 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1199,7 +1199,6 @@ static inline int vmstate_register(VMStateIf *obj, int 
instance_id,
 void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
 void *opaque);
 
-struct MemoryRegion;
 void vmstate_register_ram(struct MemoryRegion *memory, DeviceState *dev);
 void vmstate_unregister_ram(struct MemoryRegion *memory, DeviceState *dev);
 void vmstate_register_ram_global(struct MemoryRegion *memory);
-- 
2.21.3




[PATCH v4] hw/net/imx_fec: Convert debug fprintf() to trace events

2020-05-30 Thread Philippe Mathieu-Daudé
From: Jean-Christophe Dubois 

Signed-off-by: Jean-Christophe Dubois 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Message-Id: <20200530102707.195131-1-...@tribudubois.net>
[PMD: Fixed 32-bit format string using PRIx32/PRIx64]
Signed-off-by: Philippe Mathieu-Daudé 
---
Based-on: <20200530102707.195131-1-...@tribudubois.net>
---
 hw/net/imx_fec.c| 106 +++-
 hw/net/trace-events |  18 
 2 files changed, 63 insertions(+), 61 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 7adcc9df65..eefedc252d 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -31,34 +31,11 @@
 #include "qemu/module.h"
 #include "net/checksum.h"
 #include "net/eth.h"
+#include "trace.h"
 
 /* For crc32 */
 #include 
 
-#ifndef DEBUG_IMX_FEC
-#define DEBUG_IMX_FEC 0
-#endif
-
-#define FEC_PRINTF(fmt, args...) \
-do { \
-if (DEBUG_IMX_FEC) { \
-fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_FEC, \
- __func__, ##args); \
-} \
-} while (0)
-
-#ifndef DEBUG_IMX_PHY
-#define DEBUG_IMX_PHY 0
-#endif
-
-#define PHY_PRINTF(fmt, args...) \
-do { \
-if (DEBUG_IMX_PHY) { \
-fprintf(stderr, "[%s.phy]%s: " fmt , TYPE_IMX_FEC, \
- __func__, ##args); \
-} \
-} while (0)
-
 #define IMX_MAX_DESC1024
 
 static const char *imx_default_reg_name(IMXFECState *s, uint32_t index)
@@ -262,43 +239,45 @@ static void imx_eth_update(IMXFECState *s);
  * For now we don't handle any GPIO/interrupt line, so the OS will
  * have to poll for the PHY status.
  */
-static void phy_update_irq(IMXFECState *s)
+static void imx_phy_update_irq(IMXFECState *s)
 {
 imx_eth_update(s);
 }
 
-static void phy_update_link(IMXFECState *s)
+static void imx_phy_update_link(IMXFECState *s)
 {
 /* Autonegotiation status mirrors link status.  */
 if (qemu_get_queue(s->nic)->link_down) {
-PHY_PRINTF("link is down\n");
+trace_imx_phy_update_link("down");
 s->phy_status &= ~0x0024;
 s->phy_int |= PHY_INT_DOWN;
 } else {
-PHY_PRINTF("link is up\n");
+trace_imx_phy_update_link("up");
 s->phy_status |= 0x0024;
 s->phy_int |= PHY_INT_ENERGYON;
 s->phy_int |= PHY_INT_AUTONEG_COMPLETE;
 }
-phy_update_irq(s);
+imx_phy_update_irq(s);
 }
 
 static void imx_eth_set_link(NetClientState *nc)
 {
-phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
+imx_phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
 }
 
-static void phy_reset(IMXFECState *s)
+static void imx_phy_reset(IMXFECState *s)
 {
+trace_imx_phy_reset();
+
 s->phy_status = 0x7809;
 s->phy_control = 0x3000;
 s->phy_advertise = 0x01e1;
 s->phy_int_mask = 0;
 s->phy_int = 0;
-phy_update_link(s);
+imx_phy_update_link(s);
 }
 
-static uint32_t do_phy_read(IMXFECState *s, int reg)
+static uint32_t imx_phy_read(IMXFECState *s, int reg)
 {
 uint32_t val;
 
@@ -332,7 +311,7 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)
 case 29:/* Interrupt source.  */
 val = s->phy_int;
 s->phy_int = 0;
-phy_update_irq(s);
+imx_phy_update_irq(s);
 break;
 case 30:/* Interrupt mask */
 val = s->phy_int_mask;
@@ -352,14 +331,14 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)
 break;
 }
 
-PHY_PRINTF("read 0x%04x @ %d\n", val, reg);
+trace_imx_phy_read(val, reg);
 
 return val;
 }
 
-static void do_phy_write(IMXFECState *s, int reg, uint32_t val)
+static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
 {
-PHY_PRINTF("write 0x%04x @ %d\n", val, reg);
+trace_imx_phy_write(val, reg);
 
 if (reg > 31) {
 /* we only advertise one phy */
@@ -369,7 +348,7 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
 switch (reg) {
 case 0: /* Basic Control */
 if (val & 0x8000) {
-phy_reset(s);
+imx_phy_reset(s);
 } else {
 s->phy_control = val & 0x7980;
 /* Complete autonegotiation immediately.  */
@@ -383,7 +362,7 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
 break;
 case 30:/* Interrupt mask */
 s->phy_int_mask = val & 0xff;
-phy_update_irq(s);
+imx_phy_update_irq(s);
 break;
 case 17:
 case 18:
@@ -402,6 +381,8 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
 static void imx_fec_read_bd(IMXFECBufDesc *bd, dma_addr_t addr)
 {
 dma_memory_read(_space_memory, addr, bd, sizeof(*bd));
+
+trace_imx_fec_read_bd(addr, bd->flags, bd->length, bd->data);
 }
 
 static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t addr)
@@ -412,6 +393,9 @@ static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t 
addr)
 static void imx_enet_read_bd(IMXENETBufDesc *bd, dma_addr_t 

Re: [PATCH v3] hw/net/imx_fec.c: Convert debug fprintf() to trace event

2020-05-30 Thread Philippe Mathieu-Daudé
On 5/30/20 12:27 PM, Jean-Christophe Dubois wrote:
> Signed-off-by: Jean-Christophe Dubois 
> ---
> 
>  v2: fix coding style issues.
>  v3: improve tracing code based on feedback
>  * change some tracing function names
>  * remove unnecessary cast
>  * add register index in addition to name

Thanks for the changes!

> 
>  hw/net/imx_fec.c| 106 +++-
>  hw/net/trace-events |  18 
>  2 files changed, 63 insertions(+), 61 deletions(-)
> 
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index 7adcc9df654..eefedc252de 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -31,34 +31,11 @@
>  #include "qemu/module.h"
>  #include "net/checksum.h"
>  #include "net/eth.h"
> +#include "trace.h"
>  
>  /* For crc32 */
>  #include 
>  
> -#ifndef DEBUG_IMX_FEC
> -#define DEBUG_IMX_FEC 0
> -#endif
> -
> -#define FEC_PRINTF(fmt, args...) \
> -do { \
> -if (DEBUG_IMX_FEC) { \
> -fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_FEC, \
> - __func__, ##args); \
> -} \
> -} while (0)
> -
> -#ifndef DEBUG_IMX_PHY
> -#define DEBUG_IMX_PHY 0
> -#endif
> -
> -#define PHY_PRINTF(fmt, args...) \
> -do { \
> -if (DEBUG_IMX_PHY) { \
> -fprintf(stderr, "[%s.phy]%s: " fmt , TYPE_IMX_FEC, \
> - __func__, ##args); \
> -} \
> -} while (0)
> -
>  #define IMX_MAX_DESC1024
>  
>  static const char *imx_default_reg_name(IMXFECState *s, uint32_t index)
> @@ -262,43 +239,45 @@ static void imx_eth_update(IMXFECState *s);
>   * For now we don't handle any GPIO/interrupt line, so the OS will
>   * have to poll for the PHY status.
>   */
> -static void phy_update_irq(IMXFECState *s)
> +static void imx_phy_update_irq(IMXFECState *s)
>  {
>  imx_eth_update(s);
>  }
>  
> -static void phy_update_link(IMXFECState *s)
> +static void imx_phy_update_link(IMXFECState *s)
>  {
>  /* Autonegotiation status mirrors link status.  */
>  if (qemu_get_queue(s->nic)->link_down) {
> -PHY_PRINTF("link is down\n");
> +trace_imx_phy_update_link("down");
>  s->phy_status &= ~0x0024;
>  s->phy_int |= PHY_INT_DOWN;
>  } else {
> -PHY_PRINTF("link is up\n");
> +trace_imx_phy_update_link("up");
>  s->phy_status |= 0x0024;
>  s->phy_int |= PHY_INT_ENERGYON;
>  s->phy_int |= PHY_INT_AUTONEG_COMPLETE;
>  }
> -phy_update_irq(s);
> +imx_phy_update_irq(s);
>  }
>  
>  static void imx_eth_set_link(NetClientState *nc)
>  {
> -phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
> +imx_phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
>  }
>  
> -static void phy_reset(IMXFECState *s)
> +static void imx_phy_reset(IMXFECState *s)
>  {
> +trace_imx_phy_reset();
> +
>  s->phy_status = 0x7809;
>  s->phy_control = 0x3000;
>  s->phy_advertise = 0x01e1;
>  s->phy_int_mask = 0;
>  s->phy_int = 0;
> -phy_update_link(s);
> +imx_phy_update_link(s);
>  }
>  
> -static uint32_t do_phy_read(IMXFECState *s, int reg)
> +static uint32_t imx_phy_read(IMXFECState *s, int reg)
>  {
>  uint32_t val;
>  
> @@ -332,7 +311,7 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)
>  case 29:/* Interrupt source.  */
>  val = s->phy_int;
>  s->phy_int = 0;
> -phy_update_irq(s);
> +imx_phy_update_irq(s);
>  break;
>  case 30:/* Interrupt mask */
>  val = s->phy_int_mask;
> @@ -352,14 +331,14 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)
>  break;
>  }
>  
> -PHY_PRINTF("read 0x%04x @ %d\n", val, reg);
> +trace_imx_phy_read(val, reg);
>  
>  return val;
>  }
>  
> -static void do_phy_write(IMXFECState *s, int reg, uint32_t val)
> +static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
>  {
> -PHY_PRINTF("write 0x%04x @ %d\n", val, reg);
> +trace_imx_phy_write(val, reg);
>  
>  if (reg > 31) {
>  /* we only advertise one phy */
> @@ -369,7 +348,7 @@ static void do_phy_write(IMXFECState *s, int reg, 
> uint32_t val)
>  switch (reg) {
>  case 0: /* Basic Control */
>  if (val & 0x8000) {
> -phy_reset(s);
> +imx_phy_reset(s);
>  } else {
>  s->phy_control = val & 0x7980;
>  /* Complete autonegotiation immediately.  */
> @@ -383,7 +362,7 @@ static void do_phy_write(IMXFECState *s, int reg, 
> uint32_t val)
>  break;
>  case 30:/* Interrupt mask */
>  s->phy_int_mask = val & 0xff;
> -phy_update_irq(s);
> +imx_phy_update_irq(s);
>  break;
>  case 17:
>  case 18:
> @@ -402,6 +381,8 @@ static void do_phy_write(IMXFECState *s, int reg, 
> uint32_t val)
>  static void imx_fec_read_bd(IMXFECBufDesc *bd, dma_addr_t addr)
>  {
>  dma_memory_read(_space_memory, addr, bd, sizeof(*bd));
> +
> +  

Re: 5.1 proposed schedule

2020-05-30 Thread Peter Maydell
On Sat, 30 May 2020 at 09:03, Aleksandar Markovic
 wrote:
> I really like "Tuesdays" concept. It worked very well for me as a 
> submaintainer. I don't
> know its origin, but it works, bringing some degree of order and 
> predictability, and at the
> same seemingly not imposing larger than necessary burden, and the end-of-week 
> rush.

Yeah, it's just a pattern I've adopted because it seems to work -- the
first and last
days in the week didn't seem like a good idea because Monday wouldn't
give us any
time to try to investigate or fix something that came up over the weekend, and I
definitely didn't want to be trying to get tags or releases out last
thing on Friday
(it doesn't give us useful room to slip a tag by a day or so if
necessary). And I
don't work Wednesdays, so Tuesday is a natural choice.

thanks
-- PMM



[PATCH v3] hw/net/imx_fec.c: Convert debug fprintf() to trace event

2020-05-30 Thread Jean-Christophe Dubois
Signed-off-by: Jean-Christophe Dubois 
---

 v2: fix coding style issues.
 v3: improve tracing code based on feedback
 * change some tracing function names
 * remove unnecessary cast
 * add register index in addition to name

 hw/net/imx_fec.c| 106 +++-
 hw/net/trace-events |  18 
 2 files changed, 63 insertions(+), 61 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 7adcc9df654..eefedc252de 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -31,34 +31,11 @@
 #include "qemu/module.h"
 #include "net/checksum.h"
 #include "net/eth.h"
+#include "trace.h"
 
 /* For crc32 */
 #include 
 
-#ifndef DEBUG_IMX_FEC
-#define DEBUG_IMX_FEC 0
-#endif
-
-#define FEC_PRINTF(fmt, args...) \
-do { \
-if (DEBUG_IMX_FEC) { \
-fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_FEC, \
- __func__, ##args); \
-} \
-} while (0)
-
-#ifndef DEBUG_IMX_PHY
-#define DEBUG_IMX_PHY 0
-#endif
-
-#define PHY_PRINTF(fmt, args...) \
-do { \
-if (DEBUG_IMX_PHY) { \
-fprintf(stderr, "[%s.phy]%s: " fmt , TYPE_IMX_FEC, \
- __func__, ##args); \
-} \
-} while (0)
-
 #define IMX_MAX_DESC1024
 
 static const char *imx_default_reg_name(IMXFECState *s, uint32_t index)
@@ -262,43 +239,45 @@ static void imx_eth_update(IMXFECState *s);
  * For now we don't handle any GPIO/interrupt line, so the OS will
  * have to poll for the PHY status.
  */
-static void phy_update_irq(IMXFECState *s)
+static void imx_phy_update_irq(IMXFECState *s)
 {
 imx_eth_update(s);
 }
 
-static void phy_update_link(IMXFECState *s)
+static void imx_phy_update_link(IMXFECState *s)
 {
 /* Autonegotiation status mirrors link status.  */
 if (qemu_get_queue(s->nic)->link_down) {
-PHY_PRINTF("link is down\n");
+trace_imx_phy_update_link("down");
 s->phy_status &= ~0x0024;
 s->phy_int |= PHY_INT_DOWN;
 } else {
-PHY_PRINTF("link is up\n");
+trace_imx_phy_update_link("up");
 s->phy_status |= 0x0024;
 s->phy_int |= PHY_INT_ENERGYON;
 s->phy_int |= PHY_INT_AUTONEG_COMPLETE;
 }
-phy_update_irq(s);
+imx_phy_update_irq(s);
 }
 
 static void imx_eth_set_link(NetClientState *nc)
 {
-phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
+imx_phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
 }
 
-static void phy_reset(IMXFECState *s)
+static void imx_phy_reset(IMXFECState *s)
 {
+trace_imx_phy_reset();
+
 s->phy_status = 0x7809;
 s->phy_control = 0x3000;
 s->phy_advertise = 0x01e1;
 s->phy_int_mask = 0;
 s->phy_int = 0;
-phy_update_link(s);
+imx_phy_update_link(s);
 }
 
-static uint32_t do_phy_read(IMXFECState *s, int reg)
+static uint32_t imx_phy_read(IMXFECState *s, int reg)
 {
 uint32_t val;
 
@@ -332,7 +311,7 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)
 case 29:/* Interrupt source.  */
 val = s->phy_int;
 s->phy_int = 0;
-phy_update_irq(s);
+imx_phy_update_irq(s);
 break;
 case 30:/* Interrupt mask */
 val = s->phy_int_mask;
@@ -352,14 +331,14 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)
 break;
 }
 
-PHY_PRINTF("read 0x%04x @ %d\n", val, reg);
+trace_imx_phy_read(val, reg);
 
 return val;
 }
 
-static void do_phy_write(IMXFECState *s, int reg, uint32_t val)
+static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
 {
-PHY_PRINTF("write 0x%04x @ %d\n", val, reg);
+trace_imx_phy_write(val, reg);
 
 if (reg > 31) {
 /* we only advertise one phy */
@@ -369,7 +348,7 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
 switch (reg) {
 case 0: /* Basic Control */
 if (val & 0x8000) {
-phy_reset(s);
+imx_phy_reset(s);
 } else {
 s->phy_control = val & 0x7980;
 /* Complete autonegotiation immediately.  */
@@ -383,7 +362,7 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
 break;
 case 30:/* Interrupt mask */
 s->phy_int_mask = val & 0xff;
-phy_update_irq(s);
+imx_phy_update_irq(s);
 break;
 case 17:
 case 18:
@@ -402,6 +381,8 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
 static void imx_fec_read_bd(IMXFECBufDesc *bd, dma_addr_t addr)
 {
 dma_memory_read(_space_memory, addr, bd, sizeof(*bd));
+
+trace_imx_fec_read_bd(addr, bd->flags, bd->length, bd->data);
 }
 
 static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t addr)
@@ -412,6 +393,9 @@ static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t 
addr)
 static void imx_enet_read_bd(IMXENETBufDesc *bd, dma_addr_t addr)
 {
 dma_memory_read(_space_memory, addr, bd, sizeof(*bd));
+
+trace_imx_enet_read_bd(addr, bd->flags, 

Re: [PATCH] hw/net/imx_fec.c: Convert debug fprintf() to trace event

2020-05-30 Thread Jean-Christophe DUBOIS

Le 30/05/2020 à 09:49, Philippe Mathieu-Daudé a écrit :

Hi Jean-Christophe,

On 5/29/20 8:00 PM, Jean-Christophe Dubois wrote:

Signed-off-by: Jean-Christophe Dubois 
---
  hw/net/imx_fec.c| 101 ++--
  hw/net/trace-events |  18 
  2 files changed, 58 insertions(+), 61 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 7adcc9df654..823dac0603b 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -31,34 +31,11 @@
  #include "qemu/module.h"
  #include "net/checksum.h"
  #include "net/eth.h"
+#include "trace.h"
  
  /* For crc32 */

  #include 
  
-#ifndef DEBUG_IMX_FEC

-#define DEBUG_IMX_FEC 0
-#endif
-
-#define FEC_PRINTF(fmt, args...) \
-do { \
-if (DEBUG_IMX_FEC) { \
-fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_FEC, \
- __func__, ##args); \
-} \
-} while (0)
-
-#ifndef DEBUG_IMX_PHY
-#define DEBUG_IMX_PHY 0
-#endif
-
-#define PHY_PRINTF(fmt, args...) \
-do { \
-if (DEBUG_IMX_PHY) { \
-fprintf(stderr, "[%s.phy]%s: " fmt , TYPE_IMX_FEC, \
- __func__, ##args); \
-} \
-} while (0)
-
  #define IMX_MAX_DESC1024
  
  static const char *imx_default_reg_name(IMXFECState *s, uint32_t index)

@@ -262,43 +239,45 @@ static void imx_eth_update(IMXFECState *s);
   * For now we don't handle any GPIO/interrupt line, so the OS will
   * have to poll for the PHY status.
   */
-static void phy_update_irq(IMXFECState *s)
+static void imx_phy_update_irq(IMXFECState *s)
  {
  imx_eth_update(s);
  }
  
-static void phy_update_link(IMXFECState *s)

+static void imx_phy_update_link(IMXFECState *s)
  {
  /* Autonegotiation status mirrors link status.  */
  if (qemu_get_queue(s->nic)->link_down) {
-PHY_PRINTF("link is down\n");
+trace_imx_phy_update_link("down");
  s->phy_status &= ~0x0024;
  s->phy_int |= PHY_INT_DOWN;
  } else {
-PHY_PRINTF("link is up\n");
+trace_imx_phy_update_link("up");
  s->phy_status |= 0x0024;
  s->phy_int |= PHY_INT_ENERGYON;
  s->phy_int |= PHY_INT_AUTONEG_COMPLETE;
  }
-phy_update_irq(s);
+imx_phy_update_irq(s);
  }
  
  static void imx_eth_set_link(NetClientState *nc)

  {
-phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
+imx_phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
  }
  
-static void phy_reset(IMXFECState *s)

+static void imx_phy_reset(IMXFECState *s)
  {
+trace_imx_phy_reset();
+
  s->phy_status = 0x7809;
  s->phy_control = 0x3000;
  s->phy_advertise = 0x01e1;
  s->phy_int_mask = 0;
  s->phy_int = 0;
-phy_update_link(s);
+imx_phy_update_link(s);
  }
  
-static uint32_t do_phy_read(IMXFECState *s, int reg)

+static uint32_t imx_phy_read(IMXFECState *s, int reg)
  {
  uint32_t val;
  
@@ -332,7 +311,7 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)

  case 29:/* Interrupt source.  */
  val = s->phy_int;
  s->phy_int = 0;
-phy_update_irq(s);
+imx_phy_update_irq(s);
  break;
  case 30:/* Interrupt mask */
  val = s->phy_int_mask;
@@ -352,14 +331,14 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)
  break;
  }
  
-PHY_PRINTF("read 0x%04x @ %d\n", val, reg);

+trace_imx_phy_read(val, reg);
  
  return val;

  }
  
-static void do_phy_write(IMXFECState *s, int reg, uint32_t val)

+static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
  {
-PHY_PRINTF("write 0x%04x @ %d\n", val, reg);
+trace_imx_phy_write(val, reg);
  
  if (reg > 31) {

  /* we only advertise one phy */
@@ -369,7 +348,7 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
  switch (reg) {
  case 0: /* Basic Control */
  if (val & 0x8000) {
-phy_reset(s);
+imx_phy_reset(s);
  } else {
  s->phy_control = val & 0x7980;
  /* Complete autonegotiation immediately.  */
@@ -383,7 +362,7 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
  break;
  case 30:/* Interrupt mask */
  s->phy_int_mask = val & 0xff;
-phy_update_irq(s);
+imx_phy_update_irq(s);
  break;
  case 17:
  case 18:
@@ -402,6 +381,8 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
  static void imx_fec_read_bd(IMXFECBufDesc *bd, dma_addr_t addr)
  {
  dma_memory_read(_space_memory, addr, bd, sizeof(*bd));
+
+trace_imx_fec_read_bd(addr, bd->flags, bd->length, bd->data);
  }
  
  static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t addr)

@@ -412,6 +393,9 @@ static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t 
addr)
  static void imx_enet_read_bd(IMXENETBufDesc *bd, dma_addr_t addr)
  {
  dma_memory_read(_space_memory, addr, bd, sizeof(*bd));
+
+

Re: [Libguestfs] Provide NBD via Browser over Websockets

2020-05-30 Thread Richard W.M. Jones
On Fri, May 29, 2020 at 09:08:29PM +, Eric Wheeler wrote:
> On Fri, 29 May 2020, Richard W.M. Jones wrote:
> > On Fri, May 29, 2020 at 08:58:06AM -0500, Eric Blake wrote:
> > > On 5/29/20 8:50 AM, Daniel P. Berrangé wrote:
> > > 
> > > >>>(2) You need to persuade qemu's NBD client to read from a WebSocket.
> > > >>>I didn't really know anything about WebSockets until today but it
> > > >>>seems as if they are a full-duplex protocol layered on top of HTTP [a].
> > > >>>Is there a WebSocket proxy that turns WS into plain TCP (a bit like
> > > >>>stunnel)?  Google suggests [b].
> > > >>>
> > > >>>[a] https://en.wikipedia.org/wiki/WebSocket#Protocol_handshake
> > > >>>[b] https://github.com/novnc/websockify
> > > >>
> > > >>qemu already knows how to connect as a client to websockets; Dan 
> > > >>Berrange
> > > >>knows more about that setup.  I suspect it would not be too difficult to
> > > >>teach the qemu NBD client code to use a WebSocket instead of a Unix or 
> > > >>TCP
> > > >>socket as its data source.
> > > >
> > > >Actually the inverse. The QIOChannelWebsocket impl is only the server
> > > >side of the problem, as used by QEMU's VNC server. We've never 
> > > >implemented
> > > >the client side. There is nothing especially stopping us doing that - 
> > > >just
> > > >needs someone motivated with time to work on it.
> > > 
> > > In the meantime, you may still be able to set up something like:
> > > 
> > > local machine:
> > > iso -> NBD server -> Unix socket -> websockify -> WebSocket
> > 
> > I guess the idea is to have a zero-install solution for the browser.
> > As I said in the email earlier this is very common for IPMI-type
> > remote access to blade servers and in my experience is implemented
> > using a Java applet and a proprietary protocol terminated at the BMC
> > (which then emulates a virtual CDROM to the server).  There are some
> > HP blade servers on Red Hat's internal Beaker instance where you can
> > play with this.  For qemu we wouldn't need to invent a new protocol
> > when NBD is available and already implemented (albeit not yet on top
> > of WebSockets).
> > 
> > The NBD server must run inside the browser and therefore be either
> > written from scratch in Javascript, or an existing server
> > cross-compiled to WASM (if that is possible - I don't really know).
> 
> Interesting idea about WASM.  I'll see if I can build one of the simple 
> nbd servers that are around.  Not sure how to link it to the JS file IO, 
> however.

After reading a bit about compiling to WebSockets it sounds like you
can cross-compile a C program, but there's no library support at all.
IOW to port an existing server you'd have to implement enough of POSIX
to make it work.  nbdkit has a liberal license deliberately to make it
possible to chop it up and incorporate it into completely forked
codebases (nbdkit is a plot to make NBD more popular).

But since NBD is pretty simple, a fresh Javascript server might be
easier, especially if you stick to only implementing reads.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top




[PATCH 2/5] hw/sh4: Extract timer definitions to 'hw/timer/tmu012.h'

2020-05-30 Thread Philippe Mathieu-Daudé
Extract timer definitions to 'hw/timer/tmu012.h'.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Message-Id: <20200504081653.14841-3-f4...@amsat.org>
---
 include/hw/sh4/sh.h   |  9 -
 include/hw/timer/tmu012.h | 23 +++
 hw/sh4/sh7750.c   |  1 +
 hw/timer/sh_timer.c   |  2 ++
 4 files changed, 26 insertions(+), 9 deletions(-)
 create mode 100644 include/hw/timer/tmu012.h

diff --git a/include/hw/sh4/sh.h b/include/hw/sh4/sh.h
index fe773cb01d..93f464bf4c 100644
--- a/include/hw/sh4/sh.h
+++ b/include/hw/sh4/sh.h
@@ -27,15 +27,6 @@ typedef struct {
 
 int sh7750_register_io_device(struct SH7750State *s,
  sh7750_io_device * device);
-/* sh_timer.c */
-#define TMU012_FEAT_TOCR   (1 << 0)
-#define TMU012_FEAT_3CHAN  (1 << 1)
-#define TMU012_FEAT_EXTCLK (1 << 2)
-void tmu012_init(MemoryRegion *sysmem, hwaddr base,
- int feat, uint32_t freq,
-qemu_irq ch0_irq, qemu_irq ch1_irq,
-qemu_irq ch2_irq0, qemu_irq ch2_irq1);
-
 
 /* sh_serial.c */
 #define SH_SERIAL_FEAT_SCIF (1 << 0)
diff --git a/include/hw/timer/tmu012.h b/include/hw/timer/tmu012.h
new file mode 100644
index 00..808ed8de1d
--- /dev/null
+++ b/include/hw/timer/tmu012.h
@@ -0,0 +1,23 @@
+/*
+ * SuperH Timer
+ *
+ * Copyright (c) 2007 Magnus Damm
+ *
+ * This code is licensed under the GPL.
+ */
+
+#ifndef HW_TIMER_TMU012_H
+#define HW_TIMER_TMU012_H
+
+#include "exec/hwaddr.h"
+
+#define TMU012_FEAT_TOCR   (1 << 0)
+#define TMU012_FEAT_3CHAN  (1 << 1)
+#define TMU012_FEAT_EXTCLK (1 << 2)
+
+void tmu012_init(MemoryRegion *sysmem, hwaddr base,
+ int feat, uint32_t freq,
+ qemu_irq ch0_irq, qemu_irq ch1_irq,
+ qemu_irq ch2_irq0, qemu_irq ch2_irq1);
+
+#endif
diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
index d660714443..f8ac3ec6e3 100644
--- a/hw/sh4/sh7750.c
+++ b/hw/sh4/sh7750.c
@@ -30,6 +30,7 @@
 #include "sh7750_regs.h"
 #include "sh7750_regnames.h"
 #include "hw/sh4/sh_intc.h"
+#include "hw/timer/tmu012.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
 
diff --git a/hw/timer/sh_timer.c b/hw/timer/sh_timer.c
index 13c4051808..b9cbacf5d0 100644
--- a/hw/timer/sh_timer.c
+++ b/hw/timer/sh_timer.c
@@ -9,10 +9,12 @@
  */
 
 #include "qemu/osdep.h"
+#include "exec/memory.h"
 #include "hw/hw.h"
 #include "hw/irq.h"
 #include "hw/sh4/sh.h"
 #include "qemu/timer.h"
+#include "hw/timer/tmu012.h"
 #include "hw/ptimer.h"
 
 //#define DEBUG_TIMER
-- 
2.21.3




[PATCH 4/5] tests/acceptance: Add boot tests for sh4 QEMU advent calendar image

2020-05-30 Thread Philippe Mathieu-Daudé
From: Thomas Huth 

Now that we can select the second serial console in the acceptance tests
(see commit 746f244d9720 "Allow to use other serial consoles than default"),
we can also test the sh4 image from the QEMU advent calendar 2018.

Signed-off-by: Thomas Huth 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Message-Id: <20200515164337.4899-1-th...@redhat.com>
[PMD: Split .travis.yml change in separate patch]
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/acceptance/boot_linux_console.py | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index c6b06a1a13..0653c8c1bf 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -826,12 +826,12 @@ def test_m68k_q800(self):
 console_pattern = 'No filesystem could mount root'
 self.wait_for_console_pattern(console_pattern)
 
-def do_test_advcal_2018(self, day, tar_hash, kernel_name):
+def do_test_advcal_2018(self, day, tar_hash, kernel_name, console=0):
 tar_url = ('https://www.qemu-advent-calendar.org'
'/2018/download/day' + day + '.tar.xz')
 file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
 archive.extract(file_path, self.workdir)
-self.vm.set_console()
+self.vm.set_console(console_index=console)
 self.vm.add_args('-kernel',
  self.workdir + '/day' + day + '/' + kernel_name)
 self.vm.launch()
@@ -905,6 +905,15 @@ def test_ppc_mac99(self):
 self.vm.add_args('-M', 'graphics=off')
 self.do_test_advcal_2018('15', tar_hash, 'invaders.elf')
 
+def test_sh4_r2d(self):
+"""
+:avocado: tags=arch:sh4
+:avocado: tags=machine:r2d
+"""
+tar_hash = 'fe06a4fd8ccbf2e27928d64472939d47829d4c7e'
+self.vm.add_args('-append', 'console=ttySC1')
+self.do_test_advcal_2018('09', tar_hash, 'zImage', console=1)
+
 def test_sparc_ss20(self):
 """
 :avocado: tags=arch:sparc
-- 
2.21.3




[PATCH 5/5] .travis.yml: Test SH4 QEMU advent calendar image

2020-05-30 Thread Philippe Mathieu-Daudé
From: Thomas Huth 

Now that we can select the second serial console in the acceptance tests
(see commit 746f244d9720 "Allow to use other serial consoles than default"),
we can also test the sh4 image from the QEMU advent calendar 2018.

Signed-off-by: Thomas Huth 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Message-Id: <20200515164337.4899-1-th...@redhat.com>
[PMD: Split tests/acceptance/boot_linux_console.py in previous commit]
Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 564be50a3c..e2003565d8 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -293,7 +293,7 @@ jobs:
 - name: "GCC check-acceptance"
   dist: bionic
   env:
-- CONFIG="--enable-tools 
--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
+- CONFIG="--enable-tools 
--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sh4-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
 - TEST_CMD="make check-acceptance"
 - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-acceptance"
   after_script:
-- 
2.21.3




[PATCH 3/5] hw/timer/sh_timer: Remove unused 'qemu/timer.h' include

2020-05-30 Thread Philippe Mathieu-Daudé
Remove unused "qemu/timer.h" include.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Message-Id: <20200504081653.14841-4-f4...@amsat.org>
---
 hw/timer/sh_timer.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/timer/sh_timer.c b/hw/timer/sh_timer.c
index b9cbacf5d0..bb0e1c8ee5 100644
--- a/hw/timer/sh_timer.c
+++ b/hw/timer/sh_timer.c
@@ -13,7 +13,6 @@
 #include "hw/hw.h"
 #include "hw/irq.h"
 #include "hw/sh4/sh.h"
-#include "qemu/timer.h"
 #include "hw/timer/tmu012.h"
 #include "hw/ptimer.h"
 
-- 
2.21.3




[PATCH 0/5] hw/sh4: current patch queue

2020-05-30 Thread Philippe Mathieu-Daudé
Hi,

As there is no SH4 active maintainer, I gathered various
patches in a single series, in case someone is willing to
apply them.

CI report:
https://travis-ci.org/github/philmd/qemu/builds/692828388

Regards,

Phil.

Philippe Mathieu-Daudé (3):
  hw/sh4: Use MemoryRegion typedef
  hw/sh4: Extract timer definitions to 'hw/timer/tmu012.h'
  hw/timer/sh_timer: Remove unused 'qemu/timer.h' include

Thomas Huth (2):
  tests/acceptance: Add boot tests for sh4 QEMU advent calendar image
  .travis.yml: Test SH4 QEMU advent calendar image

 include/hw/sh4/sh.h| 12 +---
 include/hw/timer/tmu012.h  | 23 +++
 hw/sh4/sh7750.c|  1 +
 hw/timer/sh_timer.c|  3 ++-
 .travis.yml|  2 +-
 tests/acceptance/boot_linux_console.py | 13 +++--
 6 files changed, 39 insertions(+), 15 deletions(-)
 create mode 100644 include/hw/timer/tmu012.h

-- 
2.21.3




[PATCH 1/5] hw/sh4: Use MemoryRegion typedef

2020-05-30 Thread Philippe Mathieu-Daudé
Use the MemoryRegion type defined in "qemu/typedefs.h",
to keep the repository style consistent.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Message-Id: <20200504081653.14841-2-f4...@amsat.org>
---
 include/hw/sh4/sh.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/hw/sh4/sh.h b/include/hw/sh4/sh.h
index 767a2df7e2..fe773cb01d 100644
--- a/include/hw/sh4/sh.h
+++ b/include/hw/sh4/sh.h
@@ -10,9 +10,8 @@
 
 /* sh7750.c */
 struct SH7750State;
-struct MemoryRegion;
 
-struct SH7750State *sh7750_init(SuperHCPU *cpu, struct MemoryRegion *sysmem);
+struct SH7750State *sh7750_init(SuperHCPU *cpu, MemoryRegion *sysmem);
 
 typedef struct {
 /* The callback will be triggered if any of the designated lines change */
@@ -32,7 +31,7 @@ int sh7750_register_io_device(struct SH7750State *s,
 #define TMU012_FEAT_TOCR   (1 << 0)
 #define TMU012_FEAT_3CHAN  (1 << 1)
 #define TMU012_FEAT_EXTCLK (1 << 2)
-void tmu012_init(struct MemoryRegion *sysmem, hwaddr base,
+void tmu012_init(MemoryRegion *sysmem, hwaddr base,
  int feat, uint32_t freq,
 qemu_irq ch0_irq, qemu_irq ch1_irq,
 qemu_irq ch2_irq0, qemu_irq ch2_irq1);
-- 
2.21.3




[PATCH] target/arm/cpu: adjust virtual time for cortex series cpu

2020-05-30 Thread Ying Fang
Virtual time adjustment was implemented for virt-5.0 machine type,
but the cpu property was enabled only for host-passthrough and
max cpu model. Let's add it for arm cortex series cpu which has
the gernic timer feature enabled.

Signed-off-by: Ying Fang 
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 32bec156f2..a564141b22 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1973,6 +1973,9 @@ static void cortex_a7_initfn(Object *obj)
 cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
 cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
 define_arm_cp_regs(cpu, cortexa15_cp_reginfo); /* Same as A15 */
+if (kvm_enabled()) {
+kvm_arm_add_vcpu_properties(obj);
+}
 }
 
 static void cortex_a15_initfn(Object *obj)
@@ -2015,6 +2018,9 @@ static void cortex_a15_initfn(Object *obj)
 cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
 cpu->ccsidr[2] = 0x711fe07a; /* 4096K L2 unified cache */
 define_arm_cp_regs(cpu, cortexa15_cp_reginfo);
+if (kvm_enabled()) {
+kvm_arm_add_vcpu_properties(obj);
+}
 }
 
 #ifndef TARGET_AARCH64
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index cbc5c3868f..3922347b83 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -137,6 +137,9 @@ static void aarch64_a57_initfn(Object *obj)
 cpu->gic_vpribits = 5;
 cpu->gic_vprebits = 5;
 define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
+if (kvm_enabled()) {
+kvm_arm_add_vcpu_properties(obj);
+}
 }
 
 static void aarch64_a53_initfn(Object *obj)
@@ -190,6 +193,9 @@ static void aarch64_a53_initfn(Object *obj)
 cpu->gic_vpribits = 5;
 cpu->gic_vprebits = 5;
 define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
+if (kvm_enabled()) {
+kvm_arm_add_vcpu_properties(obj);
+}
 }
 
 static void aarch64_a72_initfn(Object *obj)
@@ -241,6 +247,9 @@ static void aarch64_a72_initfn(Object *obj)
 cpu->gic_vpribits = 5;
 cpu->gic_vprebits = 5;
 define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
+if (kvm_enabled()) {
+kvm_arm_add_vcpu_properties(obj);
+}
 }
 
 void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
-- 
2.23.0





About the kvm-no-adjvtime CPU property

2020-05-30 Thread Ying Fang

About the kvm-no-adjvtime CPU property

Hi Andrew,
To adjust virutal time, a new kvm cpu property kvm-no-adjvtime
was introduced to 5.0 virt machine types. However the cpu
property was enabled only for host-passthrough and max cpu model.
As for other cpu model like cortex-a57, cortex-a53, cortex-a72,
this kvm-adjvtime is not enabled by default, which means the
virutal time can not be adjust for them.

Here, for example, if VM is configured with kvm enabled:

  
cortex-a72


  
  

  

We cannot adjust virtual time even if 5.0 virt machine is used.
So i'd like to add it to other cpu model, do you have any
suggestions here ?




Re: [PATCH] tests/acceptance: Add boot tests for sh4 QEMU advent calendar image

2020-05-30 Thread Aleksandar Markovic
18:44 Pet, 15.05.2020. Thomas Huth  је написао/ла:
>
> Now that we can select the second serial console in the acceptance tests
> (see commit 746f244d9720 "Allow to use other serial consoles than
default"),
> we can also test the sh4 image from the QEMU advent calendar 2018.
>
> Signed-off-by: Thomas Huth 
> ---
>  I've split my original patch that also added yet another mips64 test...
>  I hope it's easier to review/ack/merge this way this only addresses sh4
here.
>

It makes sense to me, Thomas. If I were you, I would do the same.

But, sorry, do you intend to send mips64 counterpart in future, or it is
already sent, but I somehow missed it?

Thanks in advance!
Aleksandar

>  .travis.yml|  2 +-
>  tests/acceptance/boot_linux_console.py | 13 +++--
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index fe708792ca..84b7f83ac4 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -291,7 +291,7 @@ jobs:
>  - name: "GCC check-acceptance"
>dist: bionic
>env:
> -- CONFIG="--enable-tools
--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
> +- CONFIG="--enable-tools
--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sh4-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
>  - TEST_CMD="make check-acceptance"
>  - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-acceptance"
>after_script:
> diff --git a/tests/acceptance/boot_linux_console.py
b/tests/acceptance/boot_linux_console.py
> index c6b06a1a13..0653c8c1bf 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -826,12 +826,12 @@ class BootLinuxConsole(Test):
>  console_pattern = 'No filesystem could mount root'
>  self.wait_for_console_pattern(console_pattern)
>
> -def do_test_advcal_2018(self, day, tar_hash, kernel_name):
> +def do_test_advcal_2018(self, day, tar_hash, kernel_name, console=0):
>  tar_url = ('https://www.qemu-advent-calendar.org'
> '/2018/download/day' + day + '.tar.xz')
>  file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
>  archive.extract(file_path, self.workdir)
> -self.vm.set_console()
> +self.vm.set_console(console_index=console)
>  self.vm.add_args('-kernel',
>   self.workdir + '/day' + day + '/' + kernel_name)
>  self.vm.launch()
> @@ -905,6 +905,15 @@ class BootLinuxConsole(Test):
>  self.vm.add_args('-M', 'graphics=off')
>  self.do_test_advcal_2018('15', tar_hash, 'invaders.elf')
>
> +def test_sh4_r2d(self):
> +"""
> +:avocado: tags=arch:sh4
> +:avocado: tags=machine:r2d
> +"""
> +tar_hash = 'fe06a4fd8ccbf2e27928d64472939d47829d4c7e'
> +self.vm.add_args('-append', 'console=ttySC1')
> +self.do_test_advcal_2018('09', tar_hash, 'zImage', console=1)
> +
>  def test_sparc_ss20(self):
>  """
>  :avocado: tags=arch:sparc
> --
> 2.18.1
>
>


[PATCH v2] hw/net/imx_fec.c: Convert debug fprintf() to trace event

2020-05-30 Thread Jean-Christophe Dubois
Signed-off-by: Jean-Christophe Dubois 
---

 v2: fix coding style issues.

 hw/net/imx_fec.c| 101 ++--
 hw/net/trace-events |  18 
 2 files changed, 58 insertions(+), 61 deletions(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 7adcc9df654..853d47deeb6 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -31,34 +31,11 @@
 #include "qemu/module.h"
 #include "net/checksum.h"
 #include "net/eth.h"
+#include "trace.h"
 
 /* For crc32 */
 #include 
 
-#ifndef DEBUG_IMX_FEC
-#define DEBUG_IMX_FEC 0
-#endif
-
-#define FEC_PRINTF(fmt, args...) \
-do { \
-if (DEBUG_IMX_FEC) { \
-fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_FEC, \
- __func__, ##args); \
-} \
-} while (0)
-
-#ifndef DEBUG_IMX_PHY
-#define DEBUG_IMX_PHY 0
-#endif
-
-#define PHY_PRINTF(fmt, args...) \
-do { \
-if (DEBUG_IMX_PHY) { \
-fprintf(stderr, "[%s.phy]%s: " fmt , TYPE_IMX_FEC, \
- __func__, ##args); \
-} \
-} while (0)
-
 #define IMX_MAX_DESC1024
 
 static const char *imx_default_reg_name(IMXFECState *s, uint32_t index)
@@ -262,43 +239,45 @@ static void imx_eth_update(IMXFECState *s);
  * For now we don't handle any GPIO/interrupt line, so the OS will
  * have to poll for the PHY status.
  */
-static void phy_update_irq(IMXFECState *s)
+static void imx_phy_update_irq(IMXFECState *s)
 {
 imx_eth_update(s);
 }
 
-static void phy_update_link(IMXFECState *s)
+static void imx_phy_update_link(IMXFECState *s)
 {
 /* Autonegotiation status mirrors link status.  */
 if (qemu_get_queue(s->nic)->link_down) {
-PHY_PRINTF("link is down\n");
+trace_imx_phy_update_link("down");
 s->phy_status &= ~0x0024;
 s->phy_int |= PHY_INT_DOWN;
 } else {
-PHY_PRINTF("link is up\n");
+trace_imx_phy_update_link("up");
 s->phy_status |= 0x0024;
 s->phy_int |= PHY_INT_ENERGYON;
 s->phy_int |= PHY_INT_AUTONEG_COMPLETE;
 }
-phy_update_irq(s);
+imx_phy_update_irq(s);
 }
 
 static void imx_eth_set_link(NetClientState *nc)
 {
-phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
+imx_phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
 }
 
-static void phy_reset(IMXFECState *s)
+static void imx_phy_reset(IMXFECState *s)
 {
+trace_imx_phy_reset();
+
 s->phy_status = 0x7809;
 s->phy_control = 0x3000;
 s->phy_advertise = 0x01e1;
 s->phy_int_mask = 0;
 s->phy_int = 0;
-phy_update_link(s);
+imx_phy_update_link(s);
 }
 
-static uint32_t do_phy_read(IMXFECState *s, int reg)
+static uint32_t imx_phy_read(IMXFECState *s, int reg)
 {
 uint32_t val;
 
@@ -332,7 +311,7 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)
 case 29:/* Interrupt source.  */
 val = s->phy_int;
 s->phy_int = 0;
-phy_update_irq(s);
+imx_phy_update_irq(s);
 break;
 case 30:/* Interrupt mask */
 val = s->phy_int_mask;
@@ -352,14 +331,14 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)
 break;
 }
 
-PHY_PRINTF("read 0x%04x @ %d\n", val, reg);
+trace_imx_phy_read(val, reg);
 
 return val;
 }
 
-static void do_phy_write(IMXFECState *s, int reg, uint32_t val)
+static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
 {
-PHY_PRINTF("write 0x%04x @ %d\n", val, reg);
+trace_imx_phy_write(val, reg);
 
 if (reg > 31) {
 /* we only advertise one phy */
@@ -369,7 +348,7 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
 switch (reg) {
 case 0: /* Basic Control */
 if (val & 0x8000) {
-phy_reset(s);
+imx_phy_reset(s);
 } else {
 s->phy_control = val & 0x7980;
 /* Complete autonegotiation immediately.  */
@@ -383,7 +362,7 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
 break;
 case 30:/* Interrupt mask */
 s->phy_int_mask = val & 0xff;
-phy_update_irq(s);
+imx_phy_update_irq(s);
 break;
 case 17:
 case 18:
@@ -402,6 +381,8 @@ static void do_phy_write(IMXFECState *s, int reg, uint32_t 
val)
 static void imx_fec_read_bd(IMXFECBufDesc *bd, dma_addr_t addr)
 {
 dma_memory_read(_space_memory, addr, bd, sizeof(*bd));
+
+trace_imx_fec_read_bd(addr, bd->flags, bd->length, bd->data);
 }
 
 static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t addr)
@@ -412,6 +393,9 @@ static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t 
addr)
 static void imx_enet_read_bd(IMXENETBufDesc *bd, dma_addr_t addr)
 {
 dma_memory_read(_space_memory, addr, bd, sizeof(*bd));
+
+trace_imx_enet_read_bd(addr, bd->flags, bd->length, bd->data,
+   bd->option, bd->status);
 }
 
 static void imx_enet_write_bd(IMXENETBufDesc *bd, dma_addr_t addr)
@@ -471,11 +455,9 @@ 

Re: [PATCH] qemu-img: Fix doc typo for 'bitmap' subcommand

2020-05-30 Thread Vladimir Sementsov-Ogievskiy

29.05.2020 17:45, Eric Blake wrote:

Prefer a consistent naming for the --merge argument.

Fixes: 3b51ab4bf
Signed-off-by: Eric Blake 


Reviewed-by: Vladimir Sementsov-Ogievskiy 


--
Best regards,
Vladimir



Re: 5.1 proposed schedule

2020-05-30 Thread Aleksandar Markovic
16:36 Pet, 29.05.2020. Peter Maydell  је
написао/ла:
>
> On Tue, 26 May 2020 at 11:07, Peter Maydell 
wrote:
> >
> > Here's a draft schedule for 5.1:
> >
> > 2019-07-06: softfreeze
>
> this should have read 2020-07-07 (Tuesday)...
>

I really like "Tuesdays" concept. It worked very well for me as a
submaintainer. I don't know its origin, but it works, bringing some degree
of order and predictability, and at the same seemingly not imposing larger
than necessary burden, and the end-of-week rush.

Just from my gut feeling, "Mondays" or "Fridays" wouldn't work that well.

Aleksandar

> > 2019-07-14: hardfreeze, rc0
> > 2019-07-21: rc1
> > 2019-07-28: rc2
> > 2019-08-04: rc3
> > 2019-08-11: release, or rc4 if we need it
> > 2019-08-18: release if we needed an rc4
>
> ...and these are all obviously supposed to be 2020,
> though the month/day numbers are otherwise correct.
>
> thanks
> -- PMM
>


Re: [PATCH] tests/acceptance: Add boot tests for sh4 QEMU advent calendar image

2020-05-30 Thread Philippe Mathieu-Daudé
On 5/15/20 6:43 PM, Thomas Huth wrote:
> Now that we can select the second serial console in the acceptance tests
> (see commit 746f244d9720 "Allow to use other serial consoles than default"),
> we can also test the sh4 image from the QEMU advent calendar 2018.
> 
> Signed-off-by: Thomas Huth 

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

> ---
>  I've split my original patch that also added yet another mips64 test...
>  I hope it's easier to review/ack/merge this way this only addresses sh4 here.
> 
>  .travis.yml|  2 +-
>  tests/acceptance/boot_linux_console.py | 13 +++--
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index fe708792ca..84b7f83ac4 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -291,7 +291,7 @@ jobs:
>  - name: "GCC check-acceptance"
>dist: bionic
>env:
> -- CONFIG="--enable-tools 
> --target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
> +- CONFIG="--enable-tools 
> --target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sh4-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
>  - TEST_CMD="make check-acceptance"
>  - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-acceptance"
>after_script:
> diff --git a/tests/acceptance/boot_linux_console.py 
> b/tests/acceptance/boot_linux_console.py
> index c6b06a1a13..0653c8c1bf 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -826,12 +826,12 @@ class BootLinuxConsole(Test):
>  console_pattern = 'No filesystem could mount root'
>  self.wait_for_console_pattern(console_pattern)
>  
> -def do_test_advcal_2018(self, day, tar_hash, kernel_name):
> +def do_test_advcal_2018(self, day, tar_hash, kernel_name, console=0):
>  tar_url = ('https://www.qemu-advent-calendar.org'
> '/2018/download/day' + day + '.tar.xz')
>  file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
>  archive.extract(file_path, self.workdir)
> -self.vm.set_console()
> +self.vm.set_console(console_index=console)
>  self.vm.add_args('-kernel',
>   self.workdir + '/day' + day + '/' + kernel_name)
>  self.vm.launch()
> @@ -905,6 +905,15 @@ class BootLinuxConsole(Test):
>  self.vm.add_args('-M', 'graphics=off')
>  self.do_test_advcal_2018('15', tar_hash, 'invaders.elf')
>  
> +def test_sh4_r2d(self):
> +"""
> +:avocado: tags=arch:sh4
> +:avocado: tags=machine:r2d
> +"""
> +tar_hash = 'fe06a4fd8ccbf2e27928d64472939d47829d4c7e'
> +self.vm.add_args('-append', 'console=ttySC1')
> +self.do_test_advcal_2018('09', tar_hash, 'zImage', console=1)
> +
>  def test_sparc_ss20(self):
>  """
>  :avocado: tags=arch:sparc
> 




Re: [PATCH v2 54/58] qdev: Make qdev_realize() support bus-less devices

2020-05-30 Thread Philippe Mathieu-Daudé
On 5/29/20 3:45 PM, Markus Armbruster wrote:
> So far, qdev_realize() supports only devices that plug into a bus:
> argument @bus cannot be null.  Extend it to support bus-less devices,
> too.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  hw/core/qdev.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 78a06db76e..50336168f2 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -408,7 +408,7 @@ void qdev_init_nofail(DeviceState *dev)
>  /*
>   * Realize @dev.
>   * @dev must not be plugged into a bus.
> - * Plug @dev into @bus.  This takes a reference to @dev.
> + * If @bus, plug @dev into @bus.  This takes a reference to @dev.
>   * If @dev has no QOM parent, make one up, taking another reference.
>   * On success, return true.
>   * On failure, store an error through @errp and return false.
> @@ -418,9 +418,12 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error 
> **errp)
>  Error *err = NULL;
>  
>  assert(!dev->realized && !dev->parent_bus);
> -assert(bus);

This simpler form is easier to review IMHO:

   if (!bus) {
assert(!DEVICE_GET_CLASS(dev)->bus_type);
   }

Whichever you prefer (the simpler the better):
Reviewed-by: Philippe Mathieu-Daudé 

>  
> -qdev_set_parent_bus(dev, bus);
> +if (bus) {
> +qdev_set_parent_bus(dev, bus);
> +} else {
> +assert(!DEVICE_GET_CLASS(dev)->bus_type);
> +}
>  
>  object_property_set_bool(OBJECT(dev), true, "realized", );
>  if (err) {
> 




Re: [PATCH v2 58/58] MAINTAINERS: Make section QOM cover hw/core/*bus.c as well

2020-05-30 Thread Philippe Mathieu-Daudé
On 5/29/20 3:45 PM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster 
> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bb9861f33b..e6957dac1a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2279,6 +2279,8 @@ R: Eduardo Habkost 
>  S: Supported
>  F: docs/qdev-device-use.txt
>  F: hw/core/qdev*
> +F: hw/core/bus.c
> +F: hw/core/sysbus.c
>  F: include/hw/qdev*
>  F: include/monitor/qdev.h
>  F: include/qom/
> 

Reviewed-by: Philippe Mathieu-Daudé 




[PULL 1/2] hw/m68k/mcf5206: Reduce m5206_mbar_read/write() offset arg to 16-bit

2020-05-30 Thread Thomas Huth
From: Philippe Mathieu-Daudé 

All calls to m5206_mbar_read/m5206_mbar_write are used with
'offset = hwaddr & 0x3ff', so we are sure the offset fits
in 16-bit.

Suggested-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20200526094052.1723-2-f4...@amsat.org>
Signed-off-by: Thomas Huth 
---
 hw/m68k/mcf5206.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c
index b155dd8170..187291e1f6 100644
--- a/hw/m68k/mcf5206.c
+++ b/hw/m68k/mcf5206.c
@@ -273,7 +273,7 @@ static void m5206_mbar_reset(m5206_mbar_state *s)
 }
 
 static uint64_t m5206_mbar_read(m5206_mbar_state *s,
-uint64_t offset, unsigned size)
+uint16_t offset, unsigned size)
 {
 if (offset >= 0x100 && offset < 0x120) {
 return m5206_timer_read(s->timer[0], offset - 0x100);
@@ -306,11 +306,11 @@ static uint64_t m5206_mbar_read(m5206_mbar_state *s,
 case 0x170: return s->uivr[0];
 case 0x1b0: return s->uivr[1];
 }
-hw_error("Bad MBAR read offset 0x%x", (int)offset);
+hw_error("Bad MBAR read offset 0x%"PRIx16, offset);
 return 0;
 }
 
-static void m5206_mbar_write(m5206_mbar_state *s, uint32_t offset,
+static void m5206_mbar_write(m5206_mbar_state *s, uint16_t offset,
  uint64_t value, unsigned size)
 {
 if (offset >= 0x100 && offset < 0x120) {
@@ -360,7 +360,7 @@ static void m5206_mbar_write(m5206_mbar_state *s, uint32_t 
offset,
 s->uivr[1] = value;
 break;
 default:
-hw_error("Bad MBAR write offset 0x%x", (int)offset);
+hw_error("Bad MBAR write offset 0x%"PRIx16, offset);
 break;
 }
 }
-- 
2.26.2




[PULL 0/2] m68k coldfire machine cleanup patches

2020-05-30 Thread Thomas Huth
 Hi Peter,

the following changes since commit c86274bc2e34295764fb44c2aef3cf29623f9b4b:

  Merge remote-tracking branch 
'remotes/stsquad/tags/pull-testing-tcg-plugins-270520-1' into staging 
(2020-05-29 17:41:45 +0100)

are available in the Git repository at:

  https://gitlab.com/huth/qemu.git tags/pull-request-2020-05-30

for you to fetch changes up to b809667808b1f742a85d6cce0d77800be20bcaa0:

  hw/m68k/mcf52xx: Replace hw_error() by qemu_log_mask() (2020-05-30 09:17:46 
+0200)


* Replace hw_error() with qemu_log_mask() in the m68k coldfire machine code


Philippe Mathieu-Daudé (2):
  hw/m68k/mcf5206: Reduce m5206_mbar_read/write() offset arg to 16-bit
  hw/m68k/mcf52xx: Replace hw_error() by qemu_log_mask()

 hw/m68k/mcf5206.c  | 14 +-
 hw/m68k/mcf5208.c  | 16 ++--
 hw/m68k/mcf_intc.c | 10 +++---
 hw/net/mcf_fec.c   |  9 ++---
 4 files changed, 32 insertions(+), 17 deletions(-)



[PULL 2/2] hw/m68k/mcf52xx: Replace hw_error() by qemu_log_mask()

2020-05-30 Thread Thomas Huth
From: Philippe Mathieu-Daudé 

hw_error() calls exit(). This a bit overkill when we can log
the accesses as unimplemented or guest error.

When fuzzing the devices, we don't want the whole process to
exit. Replace some hw_error() calls by qemu_log_mask().

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20200526094052.1723-3-f4...@amsat.org>
Reviewed-by: Thomas Huth 
Signed-off-by: Thomas Huth 
---
 hw/m68k/mcf5206.c  | 10 +++---
 hw/m68k/mcf5208.c  | 16 ++--
 hw/m68k/mcf_intc.c | 10 +++---
 hw/net/mcf_fec.c   |  9 ++---
 4 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c
index 187291e1f6..a2fef04f8e 100644
--- a/hw/m68k/mcf5206.c
+++ b/hw/m68k/mcf5206.c
@@ -8,6 +8,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
+#include "qemu/log.h"
 #include "cpu.h"
 #include "hw/hw.h"
 #include "hw/irq.h"
@@ -225,7 +226,8 @@ static void m5206_mbar_update(m5206_mbar_state *s)
 break;
 default:
 /* Unknown vector.  */
-error_report("Unhandled vector for IRQ %d", irq);
+qemu_log_mask(LOG_UNIMP, "%s: Unhandled vector for IRQ %d\n",
+  __func__, irq);
 vector = 0xf;
 break;
 }
@@ -306,7 +308,8 @@ static uint64_t m5206_mbar_read(m5206_mbar_state *s,
 case 0x170: return s->uivr[0];
 case 0x1b0: return s->uivr[1];
 }
-hw_error("Bad MBAR read offset 0x%"PRIx16, offset);
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad MBAR offset 0x%"PRIx16"\n",
+  __func__, offset);
 return 0;
 }
 
@@ -360,7 +363,8 @@ static void m5206_mbar_write(m5206_mbar_state *s, uint16_t 
offset,
 s->uivr[1] = value;
 break;
 default:
-hw_error("Bad MBAR write offset 0x%"PRIx16, offset);
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad MBAR offset 0x%"PRIx16"\n",
+  __func__, offset);
 break;
 }
 }
diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index b84c152ce3..2ab9701ad6 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -9,10 +9,10 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 #include "qemu/error-report.h"
+#include "qemu/log.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "cpu.h"
-#include "hw/hw.h"
 #include "hw/irq.h"
 #include "hw/m68k/mcf.h"
 #include "hw/m68k/mcf_fec.h"
@@ -111,8 +111,9 @@ static void m5208_timer_write(void *opaque, hwaddr offset,
 case 4:
 break;
 default:
-hw_error("m5208_timer_write: Bad offset 0x%x\n", (int)offset);
-break;
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
+  __func__, offset);
+return;
 }
 m5208_timer_update(s);
 }
@@ -136,7 +137,8 @@ static uint64_t m5208_timer_read(void *opaque, hwaddr addr,
 case 4:
 return ptimer_get_count(s->timer);
 default:
-hw_error("m5208_timer_read: Bad offset 0x%x\n", (int)addr);
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
+  __func__, addr);
 return 0;
 }
 }
@@ -164,7 +166,8 @@ static uint64_t m5208_sys_read(void *opaque, hwaddr addr,
 return 0;
 
 default:
-hw_error("m5208_sys_read: Bad offset 0x%x\n", (int)addr);
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
+  __func__, addr);
 return 0;
 }
 }
@@ -172,7 +175,8 @@ static uint64_t m5208_sys_read(void *opaque, hwaddr addr,
 static void m5208_sys_write(void *opaque, hwaddr addr,
 uint64_t value, unsigned size)
 {
-hw_error("m5208_sys_write: Bad offset 0x%x\n", (int)addr);
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
+  __func__, addr);
 }
 
 static const MemoryRegionOps m5208_sys_ops = {
diff --git a/hw/m68k/mcf_intc.c b/hw/m68k/mcf_intc.c
index d9e03a06ab..bc20742d9a 100644
--- a/hw/m68k/mcf_intc.c
+++ b/hw/m68k/mcf_intc.c
@@ -8,6 +8,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/module.h"
+#include "qemu/log.h"
 #include "cpu.h"
 #include "hw/hw.h"
 #include "hw/irq.h"
@@ -80,7 +81,9 @@ static uint64_t mcf_intc_read(void *opaque, hwaddr addr,
 case 0xe1: case 0xe2: case 0xe3: case 0xe4:
 case 0xe5: case 0xe6: case 0xe7:
 /* LnIACK */
-hw_error("mcf_intc_read: LnIACK not implemented\n");
+qemu_log_mask(LOG_UNIMP, "%s: LnIACK not implemented (offset 
0x%02x)\n",
+  __func__, offset);
+/* fallthru */
 default:
 return 0;
 }
@@ -127,8 +130,9 @@ static void mcf_intc_write(void *opaque, hwaddr addr,
 }
 break;
 default:
-hw_error("mcf_intc_write: Bad write offset %d\n", offset);
-break;
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%02x\n",
+  __func__, offset);
+return;

Re: [PATCH v2 01/58] qdev: Rename qbus_realize() to qbus_init()

2020-05-30 Thread Philippe Mathieu-Daudé
On 5/29/20 3:44 PM, Markus Armbruster wrote:
> qbus_realize() does not actually realize.  Rename it to qbus_init().
> 
> Signed-off-by: Markus Armbruster 
> ---
>  hw/core/bus.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH] hw/net/imx_fec.c: Convert debug fprintf() to trace event

2020-05-30 Thread Philippe Mathieu-Daudé
Hi Jean-Christophe,

On 5/29/20 8:00 PM, Jean-Christophe Dubois wrote:
> Signed-off-by: Jean-Christophe Dubois 
> ---
>  hw/net/imx_fec.c| 101 ++--
>  hw/net/trace-events |  18 
>  2 files changed, 58 insertions(+), 61 deletions(-)
> 
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index 7adcc9df654..823dac0603b 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -31,34 +31,11 @@
>  #include "qemu/module.h"
>  #include "net/checksum.h"
>  #include "net/eth.h"
> +#include "trace.h"
>  
>  /* For crc32 */
>  #include 
>  
> -#ifndef DEBUG_IMX_FEC
> -#define DEBUG_IMX_FEC 0
> -#endif
> -
> -#define FEC_PRINTF(fmt, args...) \
> -do { \
> -if (DEBUG_IMX_FEC) { \
> -fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_FEC, \
> - __func__, ##args); \
> -} \
> -} while (0)
> -
> -#ifndef DEBUG_IMX_PHY
> -#define DEBUG_IMX_PHY 0
> -#endif
> -
> -#define PHY_PRINTF(fmt, args...) \
> -do { \
> -if (DEBUG_IMX_PHY) { \
> -fprintf(stderr, "[%s.phy]%s: " fmt , TYPE_IMX_FEC, \
> - __func__, ##args); \
> -} \
> -} while (0)
> -
>  #define IMX_MAX_DESC1024
>  
>  static const char *imx_default_reg_name(IMXFECState *s, uint32_t index)
> @@ -262,43 +239,45 @@ static void imx_eth_update(IMXFECState *s);
>   * For now we don't handle any GPIO/interrupt line, so the OS will
>   * have to poll for the PHY status.
>   */
> -static void phy_update_irq(IMXFECState *s)
> +static void imx_phy_update_irq(IMXFECState *s)
>  {
>  imx_eth_update(s);
>  }
>  
> -static void phy_update_link(IMXFECState *s)
> +static void imx_phy_update_link(IMXFECState *s)
>  {
>  /* Autonegotiation status mirrors link status.  */
>  if (qemu_get_queue(s->nic)->link_down) {
> -PHY_PRINTF("link is down\n");
> +trace_imx_phy_update_link("down");
>  s->phy_status &= ~0x0024;
>  s->phy_int |= PHY_INT_DOWN;
>  } else {
> -PHY_PRINTF("link is up\n");
> +trace_imx_phy_update_link("up");
>  s->phy_status |= 0x0024;
>  s->phy_int |= PHY_INT_ENERGYON;
>  s->phy_int |= PHY_INT_AUTONEG_COMPLETE;
>  }
> -phy_update_irq(s);
> +imx_phy_update_irq(s);
>  }
>  
>  static void imx_eth_set_link(NetClientState *nc)
>  {
> -phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
> +imx_phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
>  }
>  
> -static void phy_reset(IMXFECState *s)
> +static void imx_phy_reset(IMXFECState *s)
>  {
> +trace_imx_phy_reset();
> +
>  s->phy_status = 0x7809;
>  s->phy_control = 0x3000;
>  s->phy_advertise = 0x01e1;
>  s->phy_int_mask = 0;
>  s->phy_int = 0;
> -phy_update_link(s);
> +imx_phy_update_link(s);
>  }
>  
> -static uint32_t do_phy_read(IMXFECState *s, int reg)
> +static uint32_t imx_phy_read(IMXFECState *s, int reg)
>  {
>  uint32_t val;
>  
> @@ -332,7 +311,7 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)
>  case 29:/* Interrupt source.  */
>  val = s->phy_int;
>  s->phy_int = 0;
> -phy_update_irq(s);
> +imx_phy_update_irq(s);
>  break;
>  case 30:/* Interrupt mask */
>  val = s->phy_int_mask;
> @@ -352,14 +331,14 @@ static uint32_t do_phy_read(IMXFECState *s, int reg)
>  break;
>  }
>  
> -PHY_PRINTF("read 0x%04x @ %d\n", val, reg);
> +trace_imx_phy_read(val, reg);
>  
>  return val;
>  }
>  
> -static void do_phy_write(IMXFECState *s, int reg, uint32_t val)
> +static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
>  {
> -PHY_PRINTF("write 0x%04x @ %d\n", val, reg);
> +trace_imx_phy_write(val, reg);
>  
>  if (reg > 31) {
>  /* we only advertise one phy */
> @@ -369,7 +348,7 @@ static void do_phy_write(IMXFECState *s, int reg, 
> uint32_t val)
>  switch (reg) {
>  case 0: /* Basic Control */
>  if (val & 0x8000) {
> -phy_reset(s);
> +imx_phy_reset(s);
>  } else {
>  s->phy_control = val & 0x7980;
>  /* Complete autonegotiation immediately.  */
> @@ -383,7 +362,7 @@ static void do_phy_write(IMXFECState *s, int reg, 
> uint32_t val)
>  break;
>  case 30:/* Interrupt mask */
>  s->phy_int_mask = val & 0xff;
> -phy_update_irq(s);
> +imx_phy_update_irq(s);
>  break;
>  case 17:
>  case 18:
> @@ -402,6 +381,8 @@ static void do_phy_write(IMXFECState *s, int reg, 
> uint32_t val)
>  static void imx_fec_read_bd(IMXFECBufDesc *bd, dma_addr_t addr)
>  {
>  dma_memory_read(_space_memory, addr, bd, sizeof(*bd));
> +
> +trace_imx_fec_read_bd(addr, bd->flags, bd->length, bd->data);
>  }
>  
>  static void imx_fec_write_bd(IMXFECBufDesc *bd, dma_addr_t addr)
> @@ -412,6 +393,9 @@ static void imx_fec_write_bd(IMXFECBufDesc *bd, 
> 

Re: [Bug 1878255] Re: Assertion failure in bdrv_aio_cancel, through ide

2020-05-30 Thread Philippe Mathieu-Daudé
On 5/30/20 12:59 AM, John Snow wrote:
> outl 0xcf8 0x8000fa24
> outl 0xcfc 0xe106c000 (Writes e106c00 to BAR5 for 0:31:2)

We might eventually display this in the reproducer output.

> 
> outl 0xcf8 0x8000fa04
> outw 0xcfc 0x7 (Enables BM, Memory IO and PIO for 0:31:2)
> 
> outl 0xcf8 0x8000fb20 (Enables 0:31:3, I guess? My PCI knowledge is
> iffy. We set the enable bit and select BAR4, but then we don't actually
> write to 0xcfc again to set anything.)
> 
> 
> write 0x0 0x3 0x2780e7
> - write these three bytes to addr 0 in memory.
> 
> write 0xe106c22c 0xd 0x1130c218021130c218021130c2
> - ahci_port_write ahci(0x555c950f71a0)[2]: port write [reg:PxSCTL] @ 0x2c: 
> 0x18c23011
> - ahci_port_write ahci(0x555c950f71a0)[2]: port write [reg:PxSERR] @ 0x30: 
> 0xc2301102
> - ahci_port_write ahci(0x555c950f71a0)[2]: port write [reg:PxSACT] @ 0x34: 
> 0x30110218
> - ahci_port_write ahci(0x555c950f71a0)[2]: port write [reg:PxCI] @ 0x38: 
> 0x00c2
> 
> write 0xe106c218 0x15 0x110010110010110010110010110010110010110010
> 
> - ahci_port_write ahci(0x555c950f71a0)[2]: port write [reg:PxCMD] @ 0x18: 
> 0x11100011
> - ahci_port_write ahci(0x555c950f71a0)[2]: port write [reg:Reserved] @ 0x1c: 
> 0x00111000
> - ahci_port_write ahci(0x555c950f71a0)[2]: port write [reg:PxTFD] @ 0x20: 
> 0x10001110
> - ahci_port_write ahci(0x555c950f71a0)[2]: port write [reg:PxSIG] @ 0x24: 
> 0x11100011
> - ahci_port_write ahci(0x555c950f71a0)[2]: port write [reg:PxSSTS] @ 0x28: 
> 0x00111000
> - ahci_port_write ahci(0x555c950f71a0)[2]: port write [reg:PxSCTL] @ 0x2c: 
> 0x0010
> 
> Not all of those register writes are actually important for the bug, so
> I simplified them to the fewest writes and fewest bits.
> 
> outl 0xcf8 0x8000fa24
> outl 0xcfc 0xe106c000
> outl 0xcf8 0x8000fa04
> outw 0xcfc 0x7
> outl 0xcf8 0x8000fb20
> write 0x0 0x3 0x2780e7
> write 0xe106c22c 0x4 0x0100
> write 0xe106c238 0x2 0x02
> write 0xe106c218 0x4 0x1100
> write 0xe106c22c 0x1 0x00
> 
> 
> 1. PxSCTL write arms the DET bit. It isn't intended to be left on when 
> PxCMD.ST (Start) is issued. It's not clear what should happen if this DOES 
> occur. (Undefined behavior, at the very least.)
> See AHCI 1.3 section 3.3.1.1 "Offset 2Ch: PxSCTL – Port x Serial ATA Control 
> (SCR2: SControl)"
> 
> This bit is intended to send a reset signal to attached SATA drives.
> QEMU just synchronously resets the drive because we can.
> 
> 
> 2. PxCI is not intended to be written to when PxCMD.ST is unset. The spec 
> suggests that when ST transitions from '1' to '0' that this field is cleared, 
> but it does not suggest what happens when it transitions from '0' to '1'. 
> QEMU will happily set the register.
> 
> 
> 3. PxCMD write: This sets PxCMD.ST and PxCMD.FRE, which engages the AHCI 
> device in earnest.
> 
> At this point, AHCI sees outstanding commands and tries to process them.
> The FIS receive address is never programmed, so it's at zero. We start
> reading a FIS there:
> 
> 15712@1590789960.784835:handle_cmd_fis_dump ahci(0x55b4c56621a0)[2]: FIS:
> 0x00: 27 80 e7 00 00 00 00 00 00 00 00 00 00 00 00 00 
> 0x10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> 0x20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> 0x30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> 0x40: 34 40 70 01 01 14 eb 20 00 00 00 00 01 00 00 00 
> 0x50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> 0x60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> 0x70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> 
> This is translated as:
> 0x27 SATA_FIS_TYPE_REGISTER_H2D
> 0x80 SATA_FIS_REG_H2D_UPDATE_COMMAND_REGISTER
> 0xe7 command = FLUSH CACHE
> 
> This will engage ide_flush_cache() (core.c)
> 
> 
> At this point I get a little confused. I wouldn't think we'd have a 
> BlockBackend here for ide_flush to work on, but it seems to think we do and 
> allows the flush command to proceed. We then immediately try to cancel this 
> flush, but bdrv_aio_cancel can't tolerate an aiocb with a null BDS and panics.
> 
> ...Hm, it should be the case that blk_do_flush detects this as
> ENOMEDIUM, but are we maybe just canceling this request too fast? I
> actually can't trigger this through the console, but I can trigger it by
> redirecting input from a .txt file.
> 
> Yup, OK: if you look in blk_aio_prwv, we schedule a oneshot to invoke
> the callback on a synchronous failure, but we are managing to inject the
> reset command before the oneshot gets a chance to run.

What is not clear to me is, can this be reproduced by a real guest, or
only replaying the fuzzer payload (via the qtest chardev)?

Very nicely detailed analysis btw!

Various parts are worth being copied in the fix commit description.

> 
> I think either blk_aio_cancel or bdrv_aio_cancel needs to check that
> there isn't a dangling BH callback -- it seems wrong to make it as far
> as bdrv_aio_cancel when there's no BDS, but the IDE device no longer has
> any idea why its callback hasn't returned yet, and blk_aio_cancel is 

Re: [PATCH] acpi: tpm: Do not build TCPA table for TPM 2

2020-05-30 Thread Auger Eric
Hi Stefan

On 5/30/20 12:23 AM, Stefan Berger wrote:
> On 5/29/20 3:28 PM, Stefan Berger wrote:
>> From: Stefan Berger 
>>
>> Do not build a TCPA table for TPM 2 anymore but create the log area when
>> building the TPM2 table. The TCPA table is only needed for TPM 1.2.

Now I understand the purpose of the tcpalog arg in build_tpm2() ;-)

Reviewed-by: Eric Auger 

I will send shortly 2 series, the one related to ARM/ACPI support and a
new one introducing ACPI table tests based on this new patch as both
conflict with this patch.

Thanks

Eric

> 
> Specs are here:
> https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf
> 
> 
> TCPA is a TPM 1.2 table and TPM2 tables is sufficient for TPM 2.0.
> 
> 
> 
> 




Re: [PATCH v2 00/58] qdev: Rework how we plug into the parent bus

2020-05-30 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200529134523.8477-1-arm...@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200529134523.8477-1-arm...@redhat.com
Subject: [PATCH v2 00/58] qdev: Rework how we plug into the parent bus
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20200529134523.8477-1-arm...@redhat.com -> 
patchew/20200529134523.8477-1-arm...@redhat.com
Switched to a new branch 'test'
d7ba4cb MAINTAINERS: Make section QOM cover hw/core/*bus.c as well
e12f9a9 qdev: qdev_init_nofail() is now unused, drop
c587de1 qdev: Convert bus-less devices to qdev_realize() with Coccinelle
e3ec37d qdev: Use qdev_realize() in qdev_device_add()
508be87 qdev: Make qdev_realize() support bus-less devices
2b3a896 s390x/event-facility: Simplify creation of SCLP event devices
35263dc microbit: Eliminate two local variables in microbit_init()
bf24af3 sysbus: sysbus_init_child_obj() is now unused, drop
0f6c400 sysbus: Convert qdev_set_parent_bus() use with Coccinelle, part 4
eaeff13 sysbus: Convert qdev_set_parent_bus() use with Coccinelle, part 3
17b4e10 sysbus: Convert qdev_set_parent_bus() use with Coccinelle, part 2
e2a4e40 sysbus: Convert qdev_set_parent_bus() use with Coccinelle, part 1
452882c qdev: Drop qdev_realize() support for null bus
99093fa sysbus: Convert to sysbus_realize() etc. with Coccinelle
0a36513 sysbus: New sysbus_realize(), sysbus_realize_and_unref()
8004fce sysbus: Tidy up sysbus_init_child_obj()'s @childsize arg, part 2
e347c20 hw/arm/armsse: Pass correct child size to sysbus_init_child_obj()
982e979 sysbus: Tidy up sysbus_init_child_obj()'s @childsize arg, part 1
50a9e3b microbit: Tidy up sysbus_init_child_obj() @child argument
23fad3c sysbus: Drop useless OBJECT() in sysbus_init_child_obj() calls
d944eab macio: Eliminate macio_init_child_obj()
8bb71b7 macio: Convert use of qdev_set_parent_bus()
94f09a8 qom: Less verbose object_initialize_child()
248945e qom: Tidy up a few object_initialize_child() calls
71344e4 auxbus: Eliminate aux_create_slave()
cfce609 auxbus: Convert a use of qdev_set_parent_bus()
ffced2a auxbus: New aux_bus_realize(), pairing with aux_bus_init()
ad1d6ea auxbus: Rename aux_init_bus() to aux_bus_init()
4302b25 qdev: qdev_create(), qdev_try_create() are now unused, drop
b29fdb6 usb: Eliminate usb_try_create_simple()
044a4d6 usb: usb_create() is now unused, drop
2119f64 usb: Convert uses of usb_create()
0869d6b usb: New usb_new(), usb_realize_and_unref()
7a38216 ssi: ssi_create_slave_no_init() is now unused, drop
1de789b ssi: Convert last use of ssi_create_slave_no_init() manually
f0966db ssi: Convert uses of ssi_create_slave_no_init() with Coccinelle
a44e783 ssi: ssi_auto_connect_slaves() never does anything, drop
dd97b52 isa: isa_create(), isa_try_create() are now unused, drop
facab3d isa: Convert uses of isa_create(), isa_try_create() manually
21c651e isa: Convert uses of isa_create() with Coccinelle
828f669 isa: New isa_new(), isa_realize_and_unref() etc.
7422b38 pci: pci_create(), pci_create_multifunction() are now unused, drop
d66dd66 pci: Convert uses of pci_create() etc. manually
47e9e7f pci: Convert uses of pci_create() etc. with Coccinelle
c2a7c32 hw/ppc: Eliminate two superfluous QOM casts
05dae68 pci: New pci_new(), pci_realize_and_unref() etc.
73a088d qdev: Convert uses of qdev_set_parent_bus() manually
a6a5708 qdev: Convert uses of qdev_set_parent_bus() with Coccinelle
04167e9 qdev: Convert uses of qdev_create() manually
3ddff84 qdev: Convert uses of qdev_create() with Coccinelle
3a0df8e qdev: Convert to qdev_unrealize() manually
b89c5d7 qdev: Convert to qdev_unrealize() with Coccinelle
5f39fa6 qdev: Convert to qbus_realize(), qbus_unrealize()
c056c1e qdev: Put qdev_new() to use with Coccinelle
8d85516 qdev: New qdev_new(), qdev_realize(), etc.
77a952f Revert "hw/versatile: realize the PCI root bus as part of the versatile 
init"
3e66cdd Revert "hw/prep: realize the PCI root bus as part of the prep init"
232f042 qdev: Rename qbus_realize() to qbus_init()

=== OUTPUT BEGIN ===
1/58 Checking commit 232f042d8722 (qdev: Rename qbus_realize() to qbus_init())
2/58 Checking commit 3e66cdd1d73c (Revert "hw/prep: realize the PCI root bus as 
part of the prep init")
3/58 Checking commit 77a952ffe9ae (Revert "hw/versatile: realize the PCI root 
bus as part of the versatile init")
4/58 Checking commit 8d855167bb6e (qdev: New qdev_new(), qdev_realize(), etc.)
5/58 Checking commit c056c1e1f85e (qdev: Put qdev_new() to use with Coccinelle)
6/58 Checking commit 5f39fa696d96 (qdev: Convert to qbus_realize(), 
qbus_unrealize())
7/58 Checking commit