[PATCH] target/xtensa: drop gen_io_end call

[Max Filippov]

Since commit
ba3e7926691e ("icount: clean up cpu_can_io at the entry to the block")
it has been unnecessary for target code to call gen_io_end() after an IO
instruction in icount mode; it is sufficient to call gen_io_start()
before it and to force the end of the TB.
Remaining call in xtensa target translator is for the opcodes that may
change IRQ state. All of them end current TB, so gen_io_end is not
needed. Drop gen_io_end call from the xtensa target translator.

Signed-off-by: Max Filippov 
---
 target/xtensa/translate.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 4bc15252c8a5..6346b2eef014 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -595,9 +595,6 @@ static int gen_postprocess(DisasContext *dc, int slot)
 gen_io_start();
 }
 gen_helper_check_interrupts(cpu_env);
-if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) {
-gen_io_end();
-}
 }
 #endif
 if (op_flags & XTENSA_OP_SYNC_REGISTER_WINDOW) {
-- 
2.20.1

Re: [PATCH v3 4/6] iotests: move bitmap helpers into their own file

[Vladimir Sementsov-Ogievskiy]

19.06.2020 22:56, Eric Blake wrote:

From: John Snow 

Signed-off-by: John Snow 
Message-Id: <20200514034922.24834-5-js...@redhat.com>
Signed-off-by: Eric Blake 
---
  tests/qemu-iotests/257| 110 +---
  tests/qemu-iotests/bitmaps.py | 131 ++
  2 files changed, 132 insertions(+), 109 deletions(-)
  create mode 100644 tests/qemu-iotests/bitmaps.py

diff --git a/tests/qemu-iotests/257 b/tests/qemu-iotests/257
index 004a433b8be2..2a81f9e30c56 100755
--- a/tests/qemu-iotests/257
+++ b/tests/qemu-iotests/257
@@ -24,120 +24,12 @@ import os

  import iotests
  from iotests import log, qemu_img
+from bitmaps import EmulatedBitmap, GROUPS


Exporting global variable of some unknown to this module structure doesn't seem
to be good module design. One day we may want to refactor this.. For now:

Reviewed-by: Vladimir Sementsov-Ogievskiy 


--
Best regards,
Vladimir

Re: what are the requirements on target/ code for -icount to work correctly?

[Max Filippov]

On Fri, Jun 19, 2020 at 10:05 AM Peter Maydell  wrote:
> I've just sent a patch that removes the target/arm gen_io_end() calls.
> I had a quick look at sparc, xtensa and ppc, but they were too complicated
> for a quick look to be sufficient :-)

I've checked the xtensa translator. The only gen_io_end() is for
opcodes that end TB when a full instruction is translated, because
they can change active interrupt requests. So it can be removed.
I'll send a patch.

One instance of gen_io_start is for the rsr.ccount opcode that reads
current cycle counter that translates to reading
QEMU_CLOCK_VIRTUAL clock.
Looks like this adds one more case to the
following list:

> As I understand it, the definition of "I/O insn" is anything that can
> either:
>
> - affect the icount deadline (e.g. by setting or removing a
> QEMU_CLOCK_VIRTUAL timer)
>
> - interrupt the current translation block with cpu_loop_exit,
> cpu_restore_state or similar.

-- 
Thanks.
-- Max

Re: [PATCH v3 2/6] blockdev: combine DriveBackupState and BlockdevBackupState

[Vladimir Sementsov-Ogievskiy]

19.06.2020 22:56, Eric Blake wrote:

From: John Snow 

They have the same fields -- rename it BlockJobActionState.


commit/abort/clean functions are identical after it. I think better to combine 
them as well here



Signed-off-by: John Snow 
Signed-off-by: Eric Blake 
---
  blockdev.c | 30 --
  1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 72df193ca73b..6d80af903c55 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1655,11 +1655,11 @@ static void external_snapshot_clean(BlkActionState 
*common)
  aio_context_release(aio_context);
  }

-typedef struct DriveBackupState {
+typedef struct BlockJobActionState {
  BlkActionState common;
  BlockDriverState *bs;
  BlockJob *job;
-} DriveBackupState;
+} BlockJobActionState;

  static BlockJob *do_backup_common(BackupCommon *backup,
BlockDriverState *bs,
@@ -1669,7 +1669,7 @@ static BlockJob *do_backup_common(BackupCommon *backup,

  static void drive_backup_prepare(BlkActionState *common, Error **errp)
  {
-DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
+BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, 
common);
  DriveBackup *backup;
  BlockDriverState *bs;
  BlockDriverState *target_bs;
@@ -1806,7 +1806,7 @@ out:

  static void drive_backup_commit(BlkActionState *common)
  {
-DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
+BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, 
common);
  AioContext *aio_context;

  aio_context = bdrv_get_aio_context(state->bs);
@@ -1820,7 +1820,7 @@ static void drive_backup_commit(BlkActionState *common)

  static void drive_backup_abort(BlkActionState *common)
  {
-DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
+BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, 
common);

  if (state->job) {
  AioContext *aio_context;
@@ -1836,7 +1836,7 @@ static void drive_backup_abort(BlkActionState *common)

  static void drive_backup_clean(BlkActionState *common)
  {
-DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
+BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, 
common);
  AioContext *aio_context;

  if (!state->bs) {
@@ -1851,15 +1851,9 @@ static void drive_backup_clean(BlkActionState *common)
  aio_context_release(aio_context);
  }

-typedef struct BlockdevBackupState {
-BlkActionState common;
-BlockDriverState *bs;
-BlockJob *job;
-} BlockdevBackupState;
-
  static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
  {
-BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
common);
+BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, 
common);
  BlockdevBackup *backup;
  BlockDriverState *bs;
  BlockDriverState *target_bs;
@@ -1907,7 +1901,7 @@ static void blockdev_backup_prepare(BlkActionState 
*common, Error **errp)

  static void blockdev_backup_commit(BlkActionState *common)
  {
-BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
common);
+BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, 
common);
  AioContext *aio_context;

  aio_context = bdrv_get_aio_context(state->bs);
@@ -1921,7 +1915,7 @@ static void blockdev_backup_commit(BlkActionState *common)

  static void blockdev_backup_abort(BlkActionState *common)
  {
-BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
common);
+BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, 
common);

  if (state->job) {
  AioContext *aio_context;
@@ -1937,7 +1931,7 @@ static void blockdev_backup_abort(BlkActionState *common)

  static void blockdev_backup_clean(BlkActionState *common)
  {
-BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
common);
+BlockJobActionState *state = DO_UPCAST(BlockJobActionState, common, 
common);
  AioContext *aio_context;

  if (!state->bs) {
@@ -2209,14 +2203,14 @@ static const BlkActionOps actions[] = {
  .clean = external_snapshot_clean,
  },
  [TRANSACTION_ACTION_KIND_DRIVE_BACKUP] = {
-.instance_size = sizeof(DriveBackupState),
+.instance_size = sizeof(BlockJobActionState),
  .prepare = drive_backup_prepare,
  .commit = drive_backup_commit,
  .abort = drive_backup_abort,
  .clean = drive_backup_clean,
  },
  [TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = {
-.instance_size = sizeof(BlockdevBackupState),
+.instance_size = sizeof(BlockJobActionState),
  .prepare = blockdev_backup_prepare,
  .commit = blockdev_backup_commit,
  .abort = blockdev_backup_abort,




--
Best regards,
Vladimir

Re: [PATCH 5/6] virtio-serial-bus: add terminal resize messages

[Michael S. Tsirkin]

On Sun, Jun 21, 2020 at 06:21:31PM +0200, Szymon Lukasz wrote:
> Implement the part of the virtio spec that allows to notify the virtio
> driver about terminal resizes. The virtio spec contains two methods to
> achieve that:
> 
> For legacy drivers, we have only one port and we put the terminal size
> in the config space and inject the config changed interrupt.
> 
> For multiport devices, we use the control virtqueue to send a packet
> containing the terminal size. Note that the Linux kernel expects
> the fields indicating the number of rows and columns in a packet to be
> in a different order than the one specified in the current version of
> the virtio spec. We follow the Linux implementation, so hopefully there
> is no implementation of this functionality conforming to the spec.
> 
> Signed-off-by: Szymon Lukasz 
> ---
>  hw/char/trace-events  |  1 +
>  hw/char/virtio-serial-bus.c   | 41 +--
>  include/hw/virtio/virtio-serial.h |  5 
>  3 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/char/trace-events b/hw/char/trace-events
> index d20eafd56f..be40df47ea 100644
> --- a/hw/char/trace-events
> +++ b/hw/char/trace-events
> @@ -10,6 +10,7 @@ serial_ioport_write(uint16_t addr, uint8_t value) "write 
> addr 0x%02x val 0x%02x"
>  
>  # virtio-serial-bus.c
>  virtio_serial_send_control_event(unsigned int port, uint16_t event, uint16_t 
> value) "port %u, event %u, value %u"
> +virtio_serial_send_console_resize(unsigned int port, uint16_t cols, uint16_t 
> rows) "port %u, cols %u, rows %u"
>  virtio_serial_throttle_port(unsigned int port, bool throttle) "port %u, 
> throttle %d"
>  virtio_serial_handle_control_message(uint16_t event, uint16_t value) "event 
> %u, value %u"
>  virtio_serial_handle_control_message_port(unsigned int port) "port %u"
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 262089c0c9..9d99161e13 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -261,6 +261,42 @@ static size_t send_control_event(VirtIOSerial *vser, 
> uint32_t port_id,
>  return send_control_msg(vser, , sizeof(cpkt));
>  }
>  
> +/*
> + * This struct should be added to the Linux kernel uapi headers
> + * and later imported to standard-headers/linux/virtio_console.h
> + */
> +struct virtio_console_resize {
> +__virtio16 rows;
> +__virtio16 cols;
> +};
> +
> +void virtio_serial_send_console_resize(VirtIOSerialPort *port,
> +   uint16_t cols, uint16_t rows)
> +{
> +VirtIOSerial *vser = port->vser;
> +VirtIODevice *vdev = VIRTIO_DEVICE(vser);
> +
> +if (virtio_vdev_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) {
> +struct {
> +struct virtio_console_control control;
> +struct virtio_console_resize resize;
> +} buffer;
> +
> +virtio_stl_p(vdev, , port->id);
> +virtio_stw_p(vdev, , VIRTIO_CONSOLE_RESIZE);
> +virtio_stw_p(vdev, , cols);
> +virtio_stw_p(vdev, , rows);
> +
> +trace_virtio_serial_send_console_resize(port->id, cols, rows);
> +send_control_msg(vser, , sizeof(buffer));
> +
> +} else if (virtio_vdev_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE)) {
> +vser->port0_cols = cols;
> +vser->port0_rows = rows;
> +virtio_notify_config(vdev);
> +}
> +}
> +
>  /* Functions for use inside qemu to open and read from/write to ports */
>  int virtio_serial_open(VirtIOSerialPort *port)
>  {
> @@ -559,6 +595,7 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t 
> features,
>  vser = VIRTIO_SERIAL(vdev);
>  
>  features |= vser->host_features;
> +virtio_add_feature(, VIRTIO_CONSOLE_F_SIZE);
>  if (vser->bus.max_nr_ports > 1) {
>  virtio_add_feature(, VIRTIO_CONSOLE_F_MULTIPORT);
>  }

Unconditonally adding a feature bit like that will break cross-version
migration. You need to add it through a bit property and
the disable for existing machine types.


> @@ -572,8 +609,8 @@ static void get_config(VirtIODevice *vdev, uint8_t 
> *config_data)
>  struct virtio_console_config *config =
>  (struct virtio_console_config *)config_data;
>  
> -config->cols = 0;
> -config->rows = 0;
> +config->cols = virtio_tswap16(vdev, vser->port0_cols);
> +config->rows = virtio_tswap16(vdev, vser->port0_rows);
>  config->max_nr_ports = virtio_tswap32(vdev,
>vser->serial.max_virtserial_ports);
>  }
> diff --git a/include/hw/virtio/virtio-serial.h 
> b/include/hw/virtio/virtio-serial.h
> index ed3e916b68..1d6436c0b1 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -188,6 +188,8 @@ struct VirtIOSerial {
>  virtio_serial_conf serial;
>  
>  uint64_t host_features;
> +
> +uint16_t port0_cols, port0_rows;
>  };
>  
>  /* Interface to the virtio-serial bus */
> @@ -222,6 +224,9 @@ size_t 

Re: Error compiling Qemu-4.1 on Linux

[Bin Meng]

Hi,

On Sun, Dec 15, 2019 at 2:40 PM  wrote:
>
> Hello
>
> Anything suspicious that anyone can spot in here?? Still cannot get qemu to 
> compile
>
> Keen to hear
>
> -Original Message-
> From: aijaz.b...@protonmail.com 
> Sent: Saturday, December 7, 2019 11:25 AM
> To: 'Daniel P. Berrangé' 
> Cc: qemu-devel@nongnu.org
> Subject: RE: Error compiling Qemu-4.1 on Linux
>
>
> That file IS present and its contents are:
>
> prefix=/usr
> exec_prefix=${prefix}
> libdir=${prefix}/lib/x86_64-linux-gnu
> includedir=${prefix}/include
>
> Name: GThread
> Description: Thread support for GLib
> Requires: glib-2.0
> Version: 2.50.3
> Libs: -L${libdir} -lgthread-2.0 -pthread
> Cflags: -pthread
>
> Let me know what is falling short here
>
> Regards
>
> -Original Message-
> From: Daniel P. Berrangé 
> Sent: Friday, December 6, 2019 10:38 PM
> To: aijaz.b...@protonmail.com
> Cc: qemu-devel@nongnu.org
> Subject: Re: Error compiling Qemu-4.1 on Linux
>
>
> On Fri, Dec 06, 2019 at 04:55:37PM +, aijaz.b...@protonmail.com wrote:
> > Here is the content of config.log: https://pastebin.com/6zrSXWAG
> >
> > I am configuring it for 'arm-softmmu'  as can be seen from the above
> > paste
>
> Looks like it is failing on
>
>   $ pkg-config  --atleast-version=2.40 gthread-2.0
>
> returning non-zero exit status.
>
>
> This suggests the file:
>
>   /usr/lib/x86_64-linux-gnu/pkgconfig/gthread-2.0.pc
>
> is missing on your install.
>
> Or do you have some PKG_CONFIG_LIBDIR env variable set that is mistakenly 
> pointing to a different directory.
>

This is failing for me today after I cleaned up my build directory and
re-configured everything from scratch. Something is wrong in the
configure scripts?

Regards,
Bin

[PATCH v3] migration: Count new_dirty instead of real_dirty

[Keqian Zhu]

real_dirty_pages becomes equal to total ram size after dirty log sync
in ram_init_bitmaps, the reason is that the bitmap of ramblock is
initialized to be all set, so old path counts them as "real dirty" at
beginning.

This causes wrong dirty rate and false positive throttling.

Signed-off-by: Keqian Zhu 
---
Changelog:

v3:
 - Address Dave's comments.

v2:
 - Use new_dirty_pages instead of accu_dirty_pages.
 - Adjust commit messages.
---
 include/exec/ram_addr.h | 5 +
 migration/ram.c | 8 +---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 7b5c24e928..3ef729a23c 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -442,8 +442,7 @@ static inline void 
cpu_physical_memory_clear_dirty_range(ram_addr_t start,
 static inline
 uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb,
ram_addr_t start,
-   ram_addr_t length,
-   uint64_t *real_dirty_pages)
+   ram_addr_t length)
 {
 ram_addr_t addr;
 unsigned long word = BIT_WORD((start + rb->offset) >> TARGET_PAGE_BITS);
@@ -469,7 +468,6 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb,
 if (src[idx][offset]) {
 unsigned long bits = atomic_xchg([idx][offset], 0);
 unsigned long new_dirty;
-*real_dirty_pages += ctpopl(bits);
 new_dirty = ~dest[k];
 dest[k] |= bits;
 new_dirty &= bits;
@@ -502,7 +500,6 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb,
 start + addr + offset,
 TARGET_PAGE_SIZE,
 DIRTY_MEMORY_MIGRATION)) {
-*real_dirty_pages += 1;
 long k = (start + addr) >> TARGET_PAGE_BITS;
 if (!test_and_set_bit(k, dest)) {
 num_dirty++;
diff --git a/migration/ram.c b/migration/ram.c
index 069b6e30bc..5554a7d2d8 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -859,9 +859,11 @@ static inline bool migration_bitmap_clear_dirty(RAMState 
*rs,
 /* Called with RCU critical section */
 static void ramblock_sync_dirty_bitmap(RAMState *rs, RAMBlock *rb)
 {
-rs->migration_dirty_pages +=
-cpu_physical_memory_sync_dirty_bitmap(rb, 0, rb->used_length,
-  >num_dirty_pages_period);
+uint64_t new_dirty_pages =
+cpu_physical_memory_sync_dirty_bitmap(rb, 0, rb->used_length);
+
+rs->migration_dirty_pages += new_dirty_pages;
+rs->num_dirty_pages_period += new_dirty_pages;
 }
 
 /**
-- 
2.19.1

Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration

[Yan Zhao]

On Fri, Jun 19, 2020 at 04:40:46PM -0600, Alex Williamson wrote:
> On Tue, 9 Jun 2020 20:37:31 -0400
> Yan Zhao  wrote:
> 
> > On Fri, Jun 05, 2020 at 03:39:50PM +0100, Dr. David Alan Gilbert wrote:
> > > > > > I tried to simplify the problem a bit, but we keep going backwards. 
> > > > > >  If
> > > > > > the requirement is that potentially any source device can migrate 
> > > > > > to any
> > > > > > target device and we cannot provide any means other than writing an
> > > > > > opaque source string into a version attribute on the target and
> > > > > > evaluating the result to determine compatibility, then we're 
> > > > > > requiring
> > > > > > userspace to do an exhaustive search to find a potential match.  
> > > > > > That
> > > > > > sucks. 
> > > > >  
> > hi Alex and Dave,
> > do you think it's good for us to put aside physical devices and mdev 
> > aggregation
> > for the moment, and use Alex's original idea that
> > 
> > +  Userspace should regard two mdev devices compatible when ALL of below
> > +  conditions are met:
> > +  (0) The mdev devices are of the same type
> > +  (1) success when reading from migration_version attribute of one mdev 
> > device.
> > +  (2) success when writing migration_version string of one mdev device to
> > +  migration_version attribute of the other mdev device.
> 
> I think Pandora's box is already opened, if we can't articulate how
> this solution would evolve to support features that we know are coming,
> why should we proceed with this approach?  We've already seen interest
> in breaking rule (0) in this thread, so we can't focus the solution on
> mdev devices.
> 
> Maybe the best we can do is to compare one instance of a device to
> another instance of a device, without any capability to predict
> compatibility prior to creating devices, in the case on mdev.  The
> string would need to include not only the device and vendor driver
> compatibility, but also anything that has modified the state of the
> device, such as creation time or post-creation time configuration.  The
> user is left on their own for creating a compatible device, or
> filtering devices to determine which might be, or which might generate,
> compatible devices.  It's not much of a solution, I wonder if anyone
> would even use it.
> 
> > and what about adding another sysfs attribute for vendors to put
> > recommended migration compatible device type. e.g.
> > #cat 
> > /sys/bus/pci/devices/:00:02.0/mdev_supported_types/i915-GVTg_V5_8/migration_compatible_devices
> > parent id: 8086 591d
> > mdev_type: i915-GVTg_V5_8
> > 
> > vendors are free to define the format and conent of this 
> > migration_compatible_devices
> > and it's even not to be a full list.
> > 
> > before libvirt or user to do live migration, they have to read and test
> > migration_version attributes of src/target devices to check migration 
> > compatibility.
> 
> AFAICT, free-form, vendor defined attributes are useless to libvirt.
> Vendors could already put this information in the description attribute
> and have it ignored by userspace tools due to the lack of defined
> format.  It's also not clear what value this provides when it's
> necessarily incomplete, a driver written today cannot know what future
> drivers might be compatible with its migration data.  Thanks,
>
hi Alex
maybe the problem can be divided into two pieces:
(1) how to create/locate two migration compatible devices. For normal
users, the most common and safest way to do it is to find a exact duplication
of the source device. so for mdev, it's probably to create a target mdev
of the same parent pci id, mdev type and creation parameters as the
source mdev; and for physical devices, it's to locate a target device of the
same pci id as the source device, plus some extra constraints (e.g. the
target NVMe device is configured to the same remote device as the source
NVMe device; or the target QAT device is supporting equal encryption
algorithm set as the source QAT device...).
I think a possible solution for this piece is to let vendor drivers provide a
creating/locating script to find such exact duplication of source device.
Then before libvirt is about to do live migration, it can use this script to
create a target vm of exactly duplicated configuration of the source vm.

(2) how to identify two devices are migration compatible after they are
created and even they are not exactly identical (e.g. their parent
devices are of minor difference in hardware SKUs). This identification is
necessary even after in step (1) when libvirt has created/located two
identical devices and are about to start live migration.
Also, users are free to create/configure target devices and use the
read-and-test interfaces defined in this series to check if they are
live migration compatible.
The read and test behavior in this patch set can grant vendor drivers the
freedom to decide whether to support migration between only exact identical
devices or able to support migration 

Re: [PATCH 7/7] hw/watchdog/wdt_aspeed: Reduce timer precision to micro-second

[Andrew Jeffery]

On Wed, 17 Jun 2020, at 13:11, Philippe Mathieu-Daudé wrote:
> Hi Andrew,
> 
> On 6/17/20 3:18 AM, Andrew Jeffery wrote:
> > On Tue, 16 Jun 2020, at 17:21, Philippe Mathieu-Daudé wrote:
> >> The current implementation uses nano-second precision, while
> >> the watchdog can not be more precise than a micro-second.
> > 
> > What's the basis for this assertion? It's true for the AST2500 and AST2600, 
> > but 
> > the AST2400 can run the watchdog from either a 1MHz clock source or the APB 
> > clock (which must be at least 16.5MHz on palmetto). The reset state on the
> > AST2400 configures the watchdog for the APB clock rate.
> > 
> > The Linux driver will eventually configure the watchdog for 1MHz mode
> > regardless so perhaps the AST2400 reset state is a bit of a corner case, but
> > I feel the assertion should be watered down a bit?
> 
> What about this description?
> 
> "The current implementation uses nano-second precision, but
>  is not more precise than micro-second precision.
>  Simplify by using a micro-second based timer.
>  Rename the timer 'timer_us' to have the unit explicit."

So is this a limitation of QEMUTimer? I was establishing that the hardware can 
operate at greater than 1 micro-second precision.

Andrew

Re: [PATCH 0/2] qcow2: Force preallocation with data-file-raw

[Nir Soffer]

On Fri, Jun 19, 2020 at 1:40 PM Max Reitz  wrote:
>
> Hi,
>
> As discussed here:
>
> https://lists.nongnu.org/archive/html/qemu-block/2020-02/msg00644.html
> https://lists.nongnu.org/archive/html/qemu-block/2020-04/msg00329.html
> https://lists.nongnu.org/archive/html/qemu-block/2020-06/msg00240.html
>
> I think that qcow2 images with data-file-raw should always have
> preallocated 1:1 L1/L2 tables, so that the image always looks the same
> whether you respect or ignore the qcow2 metadata.

I don't know the internals of qcow2 data_file, but are we really using
qcow2 metadata
when accessing the data file? This may have unwanted performance consequences.

If I understand correctly, qcow2 metadata is needed only for keeping
bitmaps (or maybe
future extensions) for raw data file, and reading from the qcow2 image
should be read
directly from the raw file without any extra work.

Writing to the data file should also bypass the qcow2 metadata, since the bitmap
is updated in memory.

>  The easiest way to
> achieve that is to enforce at least metadata preallocation whenever
> data-file-raw is given.

But preallocation is not free, even on file systems, it can be even
slow (NFS < 4.2).
With block storage this means you need to allocate the entire image size on
storage for writing the metadata.

While oVirt does not use qcow2 with data_file, having preallocated qcow2
will make this very hard to use, for example for 500 GiB disk we will have to
allocate 500 GiB disk for the raw data file and 500 GiB disk for the qcow2
metadata disk which will be 99% unused.

I don't think that kubevirt is planning to use this either, but if
they decide to use
this it may be a problem for them as well when using block storage.

It looks like we abuse preallocation for getting the side effect that
the backing file
will be rejected, instead of adding the validation rejecting backing
file in this case.

Nir


Nir

> Max Reitz (2):
>   qcow2: Force preallocation with data-file-raw
>   iotests/244: Test preallocation for data-file-raw
>
>  block/qcow2.c  | 22 +
>  tests/qemu-iotests/244 | 65 ++
>  tests/qemu-iotests/244.out | 32 ---
>  3 files changed, 114 insertions(+), 5 deletions(-)
>
> --
> 2.26.2
>
>

Re: [PATCH v3 3/7] hw/misc/led: Emit a trace event when LED intensity has changed

[Philippe Mathieu-Daudé]

On 6/21/20 9:23 PM, Richard Henderson wrote:
> On 6/20/20 4:07 PM, Philippe Mathieu-Daudé wrote:
>> Track the LED intensity, and emit a trace event when it changes.
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  include/hw/misc/led.h | 1 +
>>  hw/misc/led.c | 5 +
>>  hw/misc/trace-events  | 1 +
>>  3 files changed, 7 insertions(+)
>>
>> diff --git a/include/hw/misc/led.h b/include/hw/misc/led.h
>> index 883006bb8f..df5b32a2db 100644
>> --- a/include/hw/misc/led.h
>> +++ b/include/hw/misc/led.h
>> @@ -35,6 +35,7 @@ typedef struct LEDState {
>>  DeviceState parent_obj;
>>  /* Public */
>>  
>> +uint16_t current_intensity;
>>  qemu_irq irq;
> 
> Why not sort this new field next to the other uint16_t and (partially) fill 
> the
> hole?

>From a reviewer point of view, I prefer to keep the state fields
separated from the properties fields, wasting few bits of RAM.

Anyway I switched to a percent value. What is better to hold
it, an 'unsigned' or 'uint8_t' type?

> 
> Otherwise,
> Reviewed-by: Richard Henderson 

Thanks!

> 
> 
> r~
> 

Re: [PATCH] tests/qht-bench: Adjust rate computation and comparisons

[Emilio G. Cota]

On Sat, Jun 20, 2020 at 14:45:51 -0700, Richard Henderson wrote:
> Use <= comparisons vs the threshold, so that threshold UINT64_MAX
> is always true, corresponding to rate 1.0 being unity.  Simplify
> do_threshold scaling to 2**64, with a special case for 1.0.
> 
> Cc: Emilio G. Cota 
> Signed-off-by: Richard Henderson 
> ---
>  tests/qht-bench.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qht-bench.c b/tests/qht-bench.c
> index eb88a90137..21b1b7de82 100644
> --- a/tests/qht-bench.c
> +++ b/tests/qht-bench.c
> @@ -132,7 +132,7 @@ static void do_rz(struct thread_info *info)
>  {
>  struct thread_stats *stats = >stats;
>  
> -if (info->r < resize_threshold) {
> +if (info->r <= resize_threshold) {
>  size_t size = info->resize_down ? resize_min : resize_max;
>  bool resized;

This works, but only because info->r cannot be 0 since xorshift never
returns it. (xorshift returns a random number in the range [1, u64max],
a fact that I missed when I wrote this code.)
If r were 0, then we would resize even if resize_threshold == 0.0.

I think it will be easier to reason about this if we rename info->r
to info->seed, and then have a local r = info->seed - 1. Then we can keep
the "if random < threshold" form (and its negated "if random >= threshold"
as below), which (at least to me) is intuitive provided that random's range
is [0, threshold), e.g. [0.0, 1.0) with drand48(3).

> @@ -154,7 +154,7 @@ static void do_rw(struct thread_info *info)
>  uint32_t hash;
>  long *p;
>  
> -if (info->r >= update_threshold) {
> +if (info->r > update_threshold) {
>  bool read;
>  
>  p = [info->r & (lookup_range - 1)];
> @@ -281,11 +281,18 @@ static void pr_params(void)
>  
>  static void do_threshold(double rate, uint64_t *threshold)
>  {
> +/*
> + * For 0 <= rate <= 1, scale to fit in a uint64_t.
> + *
> + * For rate == 1, returning UINT64_MAX means 100% certainty: all
> + * uint64_t will match using <=.  The largest representable value
> + * for rate less than 1 is 0.999889; scaling that
> + * by 2**64 results in 0xf800.
> + */
>  if (rate == 1.0) {
>  *threshold = UINT64_MAX;
>  } else {
> -*threshold = (rate * 0xull)
> -   + (rate * 0xull);
> +*threshold = rate * 0x1p64;

I'm sorry this caused a breakage for some integration tests; I thought
this was fixed in May with:
  https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg01477.html

Just for my own education, why isn't nextafter needed here?

Thanks,
Emilio

Re: [PATCH v3 5/7] hw/misc/mps2-fpgaio: Use the LED device

[Philippe Mathieu-Daudé]

On 6/21/20 11:00 PM, Richard Henderson wrote:
> On 6/20/20 4:07 PM, Philippe Mathieu-Daudé wrote:
>> +DeviceState *led[2];
> 
> Perhaps better as LEDState?  And perhaps return that from create_led.

I guess I first thought about using an opaque structure
with forward typedef declaration, but in this case I also
prefer your suggestion.

Thanks :)

Re: [PATCH v3 6/7] hw/misc/mps2-scc: Use the LED device

[Richard Henderson]

On 6/20/20 4:07 PM, Philippe Mathieu-Daudé wrote:
> @@ -25,6 +25,7 @@ typedef struct {
>  
>  /*< public >*/
>  MemoryRegion iomem;
> +DeviceState *led[8];

LEDState?

> +for (size_t i = 0; i < ARRAY_SIZE(s->led); i++) {
> +led_set_state(LED(s->led[i]), !!extract32(value, i, 1));
> +}

No need for !!.


r~

Re: [PATCH v3 5/7] hw/misc/mps2-fpgaio: Use the LED device

[Richard Henderson]

On 6/20/20 4:07 PM, Philippe Mathieu-Daudé wrote:
> +DeviceState *led[2];

Perhaps better as LEDState?  And perhaps return that from create_led.


r~

Re: [PATCH v3 4/7] hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 #1

[Richard Henderson]

On 6/20/20 4:07 PM, Philippe Mathieu-Daudé wrote:
> The Witherspoon has 3 LEDs connected to a PCA9552. Add them.
> The names and reset values are taken from:
> https://github.com/open-power/witherspoon-xml/blob/master/witherspoon.xml
> 
> Example booting obmc-phosphor-image:
> 
>   $ qemu-system-arm -M witherspoon-bmc -trace led_change_intensity
>   1592693373.997015:led_change_intensity LED desc:'front-fault-4' color:green 
> intensity 0x -> 0x
>   1592693373.997632:led_change_intensity LED desc:'front-power-3' color:green 
> intensity 0x -> 0x
>   1592693373.998239:led_change_intensity LED desc:'front-id-5' color:green 
> intensity 0x -> 0x
>   1592693500.291805:led_change_intensity LED desc:'front-power-3' color:green 
> intensity 0x -> 0x
>   1592693500.312041:led_change_intensity LED desc:'front-power-3' color:green 
> intensity 0x -> 0x
>   1592693500.821254:led_change_intensity LED desc:'front-power-3' color:green 
> intensity 0x -> 0x
>   1592693501.331517:led_change_intensity LED desc:'front-power-3' color:green 
> intensity 0x -> 0x
>   1592693501.841367:led_change_intensity LED desc:'front-power-3' color:green 
> intensity 0x -> 0x
>   1592693502.350839:led_change_intensity LED desc:'front-power-3' color:green 
> intensity 0x -> 0x
>   1592693502.861134:led_change_intensity LED desc:'front-power-3' color:green 
> intensity 0x -> 0x
>   1592693503.371090:led_change_intensity LED desc:'front-power-3' color:green 
> intensity 0x -> 0x
> 
> We notice the front-power LED starts to blink.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/arm/aspeed.c | 17 +
>  hw/arm/Kconfig  |  1 +
>  2 files changed, 18 insertions(+)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v3 1/7] hw/misc: Add a LED device

[Richard Henderson]

On 6/21/20 1:35 PM, Philippe Mathieu-Daudé wrote:
>> Is color especially interesting, given that we only actually "display" the
>> color via tracing?
> 
> The idea of this device is to easily visualize events. Currently
> via tracing, but eventually an external UI could introspect the
> board for devices able to represent visual changes such LEDs, and
> automatically display them.
> To limit the list of representable object the visualizer has to
> support, I prefer to restrict this device to the existing LED
> physical colors.

Ok.  This does suggest that we do use then enum in the structure.

>> Indeed, why not insist that description is set?  If a board is forced to say
>> that the led is red, should it not also be forced to label it?
> 
> Because when we don't have access to the hardware schematics,
> we can not specify the label. I'll add a comment about this.

Well, if we don't have a hardware schematic label, then we must have the i/o
pin; we could insist that the board label the led in some way that makes some
sense.


r~

Re: [PATCH v3 1/7] hw/misc: Add a LED device

[Philippe Mathieu-Daudé]

Hi Richard,

On 6/21/20 4:00 AM, Richard Henderson wrote:
> On 6/20/20 4:07 PM, Philippe Mathieu-Daudé wrote:
>> Add a LED device which can be connected to a GPIO output.
>> LEDs are limited to a set of colors.
>> They can also be dimmed with PWM devices. For now we do
>> not implement the dimmed mode, but in preparation of a
>> future implementation, we start using the LED intensity.
>> When used with GPIOs, the intensity can only be either
>> minium or maximum. This depends of the polarity of the
>> GPIO.
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  include/hw/misc/led.h |  79 +++
>>  hw/misc/led.c | 121 ++
>>  MAINTAINERS   |   6 +++
>>  hw/misc/Kconfig   |   3 ++
>>  hw/misc/Makefile.objs |   1 +
>>  hw/misc/trace-events  |   3 ++
>>  6 files changed, 213 insertions(+)
>>  create mode 100644 include/hw/misc/led.h
>>  create mode 100644 hw/misc/led.c
>>
>> diff --git a/include/hw/misc/led.h b/include/hw/misc/led.h
>> new file mode 100644
>> index 00..821ee1247d
>> --- /dev/null
>> +++ b/include/hw/misc/led.h
>> @@ -0,0 +1,79 @@
>> +/*
>> + * QEMU single LED device
>> + *
>> + * Copyright (C) 2020 Philippe Mathieu-Daudé 
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +#ifndef HW_MISC_LED_H
>> +#define HW_MISC_LED_H
>> +
>> +#include "hw/qdev-core.h"
>> +
>> +#define TYPE_LED "led"
>> +#define LED(obj) OBJECT_CHECK(LEDState, (obj), TYPE_LED)
>> +
>> +typedef enum {
>> +LED_COLOR_UNKNOWN,
>> +LED_COLOR_RED,
>> +LED_COLOR_ORANGE,
>> +LED_COLOR_AMBER,
>> +LED_COLOR_YELLOW,
>> +LED_COLOR_GREEN,
>> +LED_COLOR_BLUE,
>> +LED_COLOR_VIOLET, /* PURPLE */
>> +LED_COLOR_WHITE,
>> +LED_COLOR_COUNT
>> +} LEDColor;
> 
> Is color especially interesting, given that we only actually "display" the
> color via tracing?

The idea of this device is to easily visualize events. Currently
via tracing, but eventually an external UI could introspect the
board for devices able to represent visual changes such LEDs, and
automatically display them.
To limit the list of representable object the visualizer has to
support, I prefer to restrict this device to the existing LED
physical colors.

> 
>> +/* Definitions useful when a LED is connected to a GPIO */
>> +#define LED_RESET_INTENSITY_ACTIVE_LOW  UINT16_MAX
>> +#define LED_RESET_INTENSITY_ACTIVE_HIGH 0U
>> +
>> +typedef struct LEDState {
>> +/* Private */
>> +DeviceState parent_obj;
>> +/* Public */
>> +
>> +/* Properties */
>> +char *description;
>> +char *color;
> 
> The enumeration is unused by the actual device, it would seem?

I want to, but it seems having a enum qdev property involves
a lot of code.

> 
>> +/**
>> + * led_set_intensity: set the state of a LED device
>> + * @s: the LED object
>> + * @is_on: boolean indicating whether the LED is emitting
>> + *
>> + * This utility is meant for LED connected to GPIO.
>> + */
>> +void led_set_state(LEDState *s, bool is_on);
> 
> Comment mismatch.
> 
> 
>> +void led_set_intensity(LEDState *s, uint16_t new_intensity)
>> +{
>> +trace_led_set_intensity(s->description ? s->description : "n/a",
>> +s->color, new_intensity);
> 
> Why not default description upon reset/realize?

Yes.

> 
>> +static void led_realize(DeviceState *dev, Error **errp)
>> +{
>> +LEDState *s = LED(dev);
>> +
>> +if (s->color == NULL) {
>> +error_setg(errp, "property 'color' not specified");
>> +return;
>> +}
>> +}
> 
> Indeed, why not insist that description is set?  If a board is forced to say
> that the led is red, should it not also be forced to label it?

Because when we don't have access to the hardware schematics,
we can not specify the label. I'll add a comment about this.

> 
>> +static Property led_properties[] = {
>> +DEFINE_PROP_STRING("color", LEDState, color),
> 
> It would appear that one can set any color via properties, including "plaid".
> So if you do want the char *color field, what's the point in the enum?

Good catch... I will either use an enum propery, or check the
property is in the allowed color names.

> 
>> +# led.c
>> +led_set_intensity(const char *color, const char *desc, uint16_t intensity) 
>> "LED desc:'%s' color:%s intensity: 0x%04"PRIx16
> 
> Is 0...65535 the best set of intensities?

You are right. I was thinking of PWM resolution (limiting to
16-bits). This is a different API to model, I mixed.

> Is that more valuable than e.g. a percentage?

Yes, the [0-100] range of integer is sufficient to represent
light intensity =)

Thanks for your review!

Phil.

Re: [PATCH v2 3/6] cputlb: ensure we save the IOTLB data in case of reset

[Emilio G. Cota]

On Wed, Jun 10, 2020 at 16:55:06 +0100, Alex Bennée wrote:
> Any write to a device might cause a re-arrangement of memory
> triggering a TLB flush and potential re-size of the TLB invalidating
> previous entries. This would cause users of qemu_plugin_get_hwaddr()
> to see the warning:
> 
>   invalid use of qemu_plugin_get_hwaddr
> 
> because of the failed tlb_lookup which should always succeed. To
> prevent this we save the IOTLB data in case it is later needed by a
> plugin doing a lookup.
> 
> Signed-off-by: Alex Bennée 
> 
> ---
> v2
>   - save the entry instead of re-running the tlb_fill.
> 
> squash! cputlb: ensure we save the IOTLB entry in case of reset
> ---
>  accel/tcg/cputlb.c | 63 --
>  1 file changed, 61 insertions(+), 2 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index eb2cf9de5e6..9bf9e479c7c 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1058,6 +1058,47 @@ static uint64_t io_readx(CPUArchState *env, 
> CPUIOTLBEntry *iotlbentry,
>  return val;
>  }
>  
> +#ifdef CONFIG_PLUGIN
> +
> +typedef struct SavedIOTLB {
> +struct rcu_head rcu;
> +struct SavedIOTLB **save_loc;
> +MemoryRegionSection *section;
> +hwaddr mr_offset;
> +} SavedIOTLB;
> +
> +static void clean_saved_entry(SavedIOTLB *s)
> +{
> +atomic_rcu_set(s->save_loc, NULL);

This will race with the CPU thread that sets saved_for_plugin in
save_iotlb_data().

> +g_free(s);
> +}
> +
> +static __thread SavedIOTLB *saved_for_plugin;

Apologies if this has been discussed, but why is this using TLS
variables and not state embedded in CPUState?
I see that qemu_plugin_get_hwaddr does not take a cpu_index, but
maybe it should? We could then just embed the RCU pointer in CPUState.

> +
> +/*
> + * Save a potentially trashed IOTLB entry for later lookup by plugin.
> + *
> + * We also need to track the thread storage address because the RCU
> + * cleanup that runs when we leave the critical region (the current
> + * execution) is actually in a different thread.
> + */
> +static void save_iotlb_data(MemoryRegionSection *section, hwaddr mr_offset)
> +{
> +SavedIOTLB *s = g_new(SavedIOTLB, 1);
> +s->save_loc = _for_plugin;
> +s->section = section;
> +s->mr_offset = mr_offset;
> +atomic_rcu_set(_for_plugin, s);
> +call_rcu(s, clean_saved_entry, rcu);

Here we could just publish the new pointer and g_free_rcu the old
one, if any.

> +}
> +
> +#else
> +static void save_iotlb_data(MemoryRegionSection *section, hwaddr mr_offset)
> +{
> +/* do nothing */
> +}
> +#endif
> +
>  static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
>int mmu_idx, uint64_t val, target_ulong addr,
>uintptr_t retaddr, MemOp op)
> @@ -1077,6 +1118,12 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
> *iotlbentry,
>  }
>  cpu->mem_io_pc = retaddr;
>  
> +/*
> + * The memory_region_dispatch may trigger a flush/resize
> + * so for plugins we save the iotlb_data just in case.
> + */
> +save_iotlb_data(section, mr_offset);
> +
>  if (mr->global_locking && !qemu_mutex_iothread_locked()) {
>  qemu_mutex_lock_iothread();
>  locked = true;
> @@ -1091,6 +1138,7 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
> *iotlbentry,
> MMU_DATA_STORE, mmu_idx, iotlbentry->attrs, r,
> retaddr);
>  }
> +

Stray whitespace change.

>  if (locked) {
>  qemu_mutex_unlock_iothread();
>  }
> @@ -1366,8 +1414,11 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr 
> addr,
>   * in the softmmu lookup code (or helper). We don't handle re-fills or
>   * checking the victim table. This is purely informational.
>   *
> - * This should never fail as the memory access being instrumented
> - * should have just filled the TLB.
> + * This almost never fails as the memory access being instrumented
> + * should have just filled the TLB. The one corner case is io_writex
> + * which can cause TLB flushes and potential resizing of the TLBs
> + * loosing the information we need. In those cases we need to recover
> + * data from a thread local copy of the io_tlb entry.
>   */
>  
>  bool tlb_plugin_lookup(CPUState *cpu, target_ulong addr, int mmu_idx,
> @@ -1391,6 +1442,14 @@ bool tlb_plugin_lookup(CPUState *cpu, target_ulong 
> addr, int mmu_idx,
>  data->v.ram.hostaddr = addr + tlbe->addend;
>  }
>  return true;
> +} else {
> +SavedIOTLB *saved = atomic_rcu_read(_for_plugin);
> +if (saved) {
> +data->is_io = true;
> +data->v.io.section = saved->section;
> +data->v.io.offset = saved->mr_offset;
> +return true;
> +}

Shouldn't we check that the contents of the saved IOTLB match the
parameters of the lookup? Otherwise passing a random address is likely
to land 

Re: [PATCH v3 3/7] hw/misc/led: Emit a trace event when LED intensity has changed

[Richard Henderson]

On 6/20/20 4:07 PM, Philippe Mathieu-Daudé wrote:
> Track the LED intensity, and emit a trace event when it changes.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/hw/misc/led.h | 1 +
>  hw/misc/led.c | 5 +
>  hw/misc/trace-events  | 1 +
>  3 files changed, 7 insertions(+)
> 
> diff --git a/include/hw/misc/led.h b/include/hw/misc/led.h
> index 883006bb8f..df5b32a2db 100644
> --- a/include/hw/misc/led.h
> +++ b/include/hw/misc/led.h
> @@ -35,6 +35,7 @@ typedef struct LEDState {
>  DeviceState parent_obj;
>  /* Public */
>  
> +uint16_t current_intensity;
>  qemu_irq irq;

Why not sort this new field next to the other uint16_t and (partially) fill the
hole?

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [PATCH v3 2/7] hw/misc/led: Add helper to connect LED to GPIO output

[Richard Henderson]

On 6/20/20 4:07 PM, Philippe Mathieu-Daudé wrote:
> Some devices expose GPIO lines. Add the create_led_by_gpio_id()
> helper to connect a LED to such GPIO.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Adding support for named GPIO is trivial. We don't need it yet.
> ---
>  include/hw/misc/led.h | 20 
>  hw/misc/led.c | 25 +
>  2 files changed, 45 insertions(+)

Modulo my question re colors, looks good.

Reviewed-by: Richard Henderson 

r~

Re: [PATCH] .travis.yml: Build acceptance tests with -O2 compiler optimization

[Philippe Mathieu-Daudé]

On 6/21/20 2:47 PM, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé  writes:
> 
>> On 6/21/20 1:29 AM, Philippe Mathieu-Daudé wrote:
>>> As we just want the tests to succeed, build them with compiler
>>> optimizations enabled to run the tests faster.
>>
>> Maybe it is a good opportunity to test -O3 instead...
>> Since this configuration is not covered.
> 
> Don't know if -O3 is worth it - even Gentoo developers warn against
> cranking it up too much.
> 
> In fact I'm surprised we don't build -O2 by default.

Do you mean in Travis or directly in ./configure?

> 
>>
>>>
>>> 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 74158f741b..61b247db9f 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,sh4-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
>>> +- CONFIG="--extra-cflags=-O2 --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:
>>>
> 
> 

Re: [PATCH v3 6/7] hw/misc/mps2-scc: Use the LED device

[Philippe Mathieu-Daudé]

On 6/21/20 1:07 AM, Philippe Mathieu-Daudé wrote:
> Per the 'ARM MPS2 and MPS2+ FPGA Prototyping Boards Technical
> Reference Manual' (100112_0200_07_en):
> 
>   2.1  Overview of the MPS2 and MPS2+ hardware
> 
>The MPS2 and MPS2+ FPGA Prototyping Boards contain the
>following components and interfaces:
> 
>* User switches and user LEDs:
> 
>  - Two green LEDs and two push buttons that connect to
>the FPGA.
>  - Eight green LEDs and one 8-way dip switch that connect
>to the MCC.
> 
> Add the 8 LEDs connected to the MCC.
> 
> This remplaces the 'mps2_scc_leds' trace events by the generic
> 'led_set_intensity' event.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> https://youtu.be/l9kD70uPchk?t=288
> ---
>  include/hw/misc/mps2-scc.h |  1 +
>  hw/misc/mps2-scc.c | 23 ---
>  hw/misc/Kconfig|  1 +
>  hw/misc/trace-events   |  1 -
>  4 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/include/hw/misc/mps2-scc.h b/include/hw/misc/mps2-scc.h
> index 7045473788..29e12b832b 100644
> --- a/include/hw/misc/mps2-scc.h
> +++ b/include/hw/misc/mps2-scc.h
> @@ -25,6 +25,7 @@ typedef struct {
>  
>  /*< public >*/
>  MemoryRegion iomem;
> +DeviceState *led[8];
>  
>  uint32_t cfg0;
>  uint32_t cfg1;
> diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c
> index 9d0909e7b3..2e59a382ec 100644
> --- a/hw/misc/mps2-scc.c
> +++ b/hw/misc/mps2-scc.c
> @@ -20,11 +20,13 @@
>  #include "qemu/osdep.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> +#include "qemu/bitops.h"
>  #include "trace.h"
>  #include "hw/sysbus.h"
>  #include "migration/vmstate.h"
>  #include "hw/registerfields.h"
>  #include "hw/misc/mps2-scc.h"
> +#include "hw/misc/led.h"
>  #include "hw/qdev-properties.h"
>  
>  REG32(CFG0, 0)
> @@ -152,18 +154,10 @@ static void mps2_scc_write(void *opaque, hwaddr offset, 
> uint64_t value,
>  s->cfg0 = value;
>  break;
>  case A_CFG1:
> -/* CFG1 bits [7:0] control the board LEDs. We don't currently have
> - * a mechanism for displaying this graphically, so use a trace event.
> - */
> -trace_mps2_scc_leds(value & 0x80 ? '*' : '.',
> -value & 0x40 ? '*' : '.',
> -value & 0x20 ? '*' : '.',
> -value & 0x10 ? '*' : '.',
> -value & 0x08 ? '*' : '.',
> -value & 0x04 ? '*' : '.',
> -value & 0x02 ? '*' : '.',
> -value & 0x01 ? '*' : '.');
>  s->cfg1 = value;
> +for (size_t i = 0; i < ARRAY_SIZE(s->led); i++) {
> +led_set_state(LED(s->led[i]), !!extract32(value, i, 1));
> +}
>  break;
>  case A_CFGDATA_OUT:
>  s->cfgdata_out = value;
> @@ -245,6 +239,13 @@ static void mps2_scc_init(Object *obj)
>  
>  memory_region_init_io(>iomem, obj, _scc_ops, s, "mps2-scc", 
> 0x1000);
>  sysbus_init_mmio(sbd, >iomem);
> +
> +for (size_t i = 0; i < ARRAY_SIZE(s->led); i++) {
> +char *name = g_strdup_printf("SCC LED%zu", i);
> +s->led[i] = create_led(obj, LED_COLOR_GREEN,
> +   name, LED_RESET_INTENSITY_ACTIVE_HIGH);
> +g_free(name);
> +}

This has to go ...

>  }
>  
>  static void mps2_scc_realize(DeviceState *dev, Error **errp)

... here to avoid:

**
ERROR:/tmp/qemu-test/src/hw/core/qdev.c:1074:device_finalize: assertion
failed: (dev->canonical_path)
Broken pipe
/tmp/qemu-test/src/tests/qtest/libqtest.c:175: kill_qemu() detected QEMU
death from signal 6 (Aborted) (core dumped)
ERROR - too few tests run (expected 6, got 5)

> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index 889757731b..f278f7f354 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -97,6 +97,7 @@ config MPS2_FPGAIO
>  
>  config MPS2_SCC
>  bool
> +select LED
>  
>  config TZ_MPC
>  bool
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 8bc7a675e8..490a0f341a 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -81,7 +81,6 @@ aspeed_scu_write(uint64_t offset, unsigned size, uint32_t 
> data) "To 0x%" PRIx64
>  mps2_scc_read(uint64_t offset, uint64_t data, unsigned size) "MPS2 SCC read: 
> offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>  mps2_scc_write(uint64_t offset, uint64_t data, unsigned size) "MPS2 SCC 
> write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>  mps2_scc_reset(void) "MPS2 SCC: reset"
> -mps2_scc_leds(char led7, char led6, char led5, char led4, char led3, char 
> led2, char led1, char led0) "MPS2 SCC LEDs: %c%c%c%c%c%c%c%c"
>  mps2_scc_cfg_write(unsigned function, unsigned device, uint32_t value) "MPS2 
> SCC config write: function %d device %d data 0x%" PRIx32
>  mps2_scc_cfg_read(unsigned function, unsigned device, uint32_t value) "MPS2 
> SCC config read: function %d 

Re: [PATCH v3 7/7] hw/arm/tosa: Replace fprintf() calls by LED devices

[Philippe Mathieu-Daudé]

On 6/21/20 1:07 AM, Philippe Mathieu-Daudé wrote:
> The recently added LED device reports LED status changes with
> the 'led_set_intensity' trace event. It is less invasive than
> the fprintf() calls. We need however to have a binary built
> with tracing support.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/arm/tosa.c  | 40 +---
>  hw/arm/Kconfig |  1 +
>  2 files changed, 14 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
> index 5dee2d76c6..74b5fd1049 100644
> --- a/hw/arm/tosa.c
> +++ b/hw/arm/tosa.c
> @@ -24,6 +24,7 @@
>  #include "hw/irq.h"
>  #include "hw/ssi/ssi.h"
>  #include "hw/sysbus.h"
> +#include "hw/misc/led.h"
>  #include "exec/address-spaces.h"
>  
>  #define TOSA_RAM0x0400
> @@ -65,27 +66,6 @@ static void tosa_microdrive_attach(PXA2xxState *cpu)
>  pxa2xx_pcmcia_attach(cpu->pcmcia[0], md);
>  }
>  
> -static void tosa_out_switch(void *opaque, int line, int level)
> -{
> -switch (line) {
> -case 0:
> -fprintf(stderr, "blue LED %s.\n", level ? "on" : "off");
> -break;
> -case 1:
> -fprintf(stderr, "green LED %s.\n", level ? "on" : "off");
> -break;
> -case 2:
> -fprintf(stderr, "amber LED %s.\n", level ? "on" : "off");
> -break;
> -case 3:
> -fprintf(stderr, "wlan LED %s.\n", level ? "on" : "off");
> -break;
> -default:
> -fprintf(stderr, "Uhandled out event: %d = %d\n", line, level);
> -break;
> -}
> -}
> -
>  static void tosa_reset(void *opaque, int line, int level)
>  {
>  if (level) {
> @@ -98,7 +78,6 @@ static void tosa_gpio_setup(PXA2xxState *cpu,
>  DeviceState *scp1,
>  TC6393xbState *tmio)
>  {
> -qemu_irq *outsignals = qemu_allocate_irqs(tosa_out_switch, cpu, 4);
>  qemu_irq reset;
>  
>  /* MMC/SD host */
> @@ -119,11 +98,6 @@ static void tosa_gpio_setup(PXA2xxState *cpu,
>  qdev_get_gpio_in(cpu->gpio, TOSA_GPIO_JC_CF_IRQ),
>  NULL);
>  
> -qdev_connect_gpio_out(scp1, TOSA_GPIO_BT_LED, outsignals[0]);
> -qdev_connect_gpio_out(scp1, TOSA_GPIO_NOTE_LED, outsignals[1]);
> -qdev_connect_gpio_out(scp1, TOSA_GPIO_CHRG_ERR_LED, outsignals[2]);
> -qdev_connect_gpio_out(scp1, TOSA_GPIO_WLAN_LED, outsignals[3]);
> -
>  qdev_connect_gpio_out(scp1, TOSA_GPIO_TC6393XB_L3V_ON, 
> tc6393xb_l3v_get(tmio));
>  
>  /* UDC Vbus */
> @@ -234,6 +208,18 @@ static void tosa_init(MachineState *machine)
>  
>  scp0 = sysbus_create_simple("scoop", 0x0880, NULL);
>  scp1 = sysbus_create_simple("scoop", 0x14800040, NULL);
> +create_led_by_gpio_id(OBJECT(machine), DEVICE(scp1),
> +  TOSA_GPIO_BT_LED,
> +  LED_COLOR_BLUE, "bluetooth", 0);
> +create_led_by_gpio_id(OBJECT(machine), DEVICE(scp1),
> +  TOSA_GPIO_NOTE_LED,
> +  LED_COLOR_GREEN, "note", 1);
> +create_led_by_gpio_id(OBJECT(machine), DEVICE(scp1),
> +  TOSA_GPIO_CHRG_ERR_LED,
> +  LED_COLOR_AMBER, "charger-error", 2);
> +create_led_by_gpio_id(OBJECT(machine), DEVICE(scp1),
> +  TOSA_GPIO_WLAN_LED,
> +  LED_COLOR_GREEN, "wlan", 3);

Oops, wrong last argument...

>  
>  tosa_gpio_setup(mpu, scp0, scp1, tmio);
>  
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 1a57a861ac..057c869d65 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -150,6 +150,7 @@ config TOSA
>  select ZAURUS  # scoop
>  select MICRODRIVE
>  select PXA2XX
> +select LED
>  
>  config SPITZ
>  bool
> 

[PATCH 6/6] virtio-console: notify the guest about terminal resizes

[Szymon Lukasz]

If a virtio serial port is a console port forward terminal resize
messages from the chardev backend to the guest.

Signed-off-by: Szymon Lukasz 
---
 hw/char/virtio-console.c | 64 +---
 1 file changed, 60 insertions(+), 4 deletions(-)

diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 97b9240ef5..1ea06aad08 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -29,6 +29,7 @@ typedef struct VirtConsole {
 
 CharBackend chr;
 guint watch;
+uint16_t cols, rows;
 } VirtConsole;
 
 /*
@@ -104,6 +105,36 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
 return ret;
 }
 
+static void virtconsole_send_resize(VirtIOSerialPort *port)
+{
+uint16_t cols, rows;
+VirtConsole *vcon = VIRTIO_CONSOLE(port);
+
+/*
+ * We probably shouldn't send these messages before
+ * we told the guest it is a console port (which we do
+ * by sending VIRTIO_CONSOLE_CONSOLE_PORT message).
+ * Instead of adding a new field to the device state
+ * lets just use the guest_connected field for that purpose
+ * since the guest should not care about the terminal size
+ * before opening the port.
+ */
+if (!port->guest_connected) {
+return;
+}
+
+if (qemu_chr_fe_get_winsize(>chr, , ) < 0) {
+cols = 0;
+rows = 0;
+}
+
+if (cols != vcon->cols || rows != vcon->rows) {
+vcon->cols = cols;
+vcon->rows = rows;
+virtio_serial_send_console_resize(port, cols, rows);
+}
+}
+
 /* Callback function that's called when the guest opens/closes the port */
 static void set_guest_connected(VirtIOSerialPort *port, int guest_connected)
 {
@@ -111,7 +142,9 @@ static void set_guest_connected(VirtIOSerialPort *port, int 
guest_connected)
 DeviceState *dev = DEVICE(port);
 VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
 
-if (!k->is_console) {
+if (k->is_console) {
+virtconsole_send_resize(port);
+} else {
 qemu_chr_fe_set_open(>chr, guest_connected);
 }
 
@@ -171,6 +204,22 @@ static void chr_event(void *opaque, QEMUChrEvent event)
 }
 }
 
+static void chr_event_console(void *opaque, QEMUChrEvent event)
+{
+VirtConsole *vcon = opaque;
+VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(vcon);
+
+switch (event) {
+case CHR_EVENT_OPENED:
+case CHR_EVENT_RESIZE:
+trace_virtio_console_chr_event(port->id, event);
+virtconsole_send_resize(port);
+break;
+default:
+break;
+}
+}
+
 static int chr_be_change(void *opaque)
 {
 VirtConsole *vcon = opaque;
@@ -179,7 +228,9 @@ static int chr_be_change(void *opaque)
 
 if (k->is_console) {
 qemu_chr_fe_set_handlers(>chr, chr_can_read, chr_read,
- NULL, chr_be_change, vcon, NULL, true);
+ chr_event_console, chr_be_change,
+ vcon, NULL, true);
+virtconsole_send_resize(port);
 } else {
 qemu_chr_fe_set_handlers(>chr, chr_can_read, chr_read,
  chr_event, chr_be_change, vcon, NULL, false);
@@ -207,7 +258,7 @@ static void virtconsole_enable_backend(VirtIOSerialPort 
*port, bool enable)
 VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
 
 qemu_chr_fe_set_handlers(>chr, chr_can_read, chr_read,
- k->is_console ? NULL : chr_event,
+ k->is_console ? chr_event_console : chr_event,
  chr_be_change, vcon, NULL, false);
 } else {
 qemu_chr_fe_set_handlers(>chr, NULL, NULL, NULL,
@@ -227,6 +278,11 @@ static void virtconsole_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+if (k->is_console) {
+vcon->cols = (uint16_t) -1;
+vcon->rows = (uint16_t) -1;
+}
+
 if (qemu_chr_fe_backend_connected(>chr)) {
 /*
  * For consoles we don't block guest data transfer just
@@ -239,7 +295,7 @@ static void virtconsole_realize(DeviceState *dev, Error 
**errp)
  */
 if (k->is_console) {
 qemu_chr_fe_set_handlers(>chr, chr_can_read, chr_read,
- NULL, chr_be_change,
+ chr_event_console, chr_be_change,
  vcon, NULL, true);
 virtio_serial_open(port);
 } else {
-- 
2.27.0

[PATCH 2/6] chardev: add support for retrieving the terminal size

[Szymon Lukasz]

Extend the class of chardevs with a new function - chr_get_winsize.
A chardev backend should implement if it is able to get the size of
the connected terminal and can detect changes in the terminal size,
i.e. if the backend cannot detect resizes it must not implement this
(e.g. if we have a tty backend connected to some (pseudo)terminal
there is no clean way to detect resizes since SIGWINCH is sent only
for the controlling terminal).

Signed-off-by: Szymon Lukasz 
---
 chardev/char-fe.c | 11 +++
 chardev/char-mux.c|  7 +++
 include/chardev/char-fe.h | 11 +++
 include/chardev/char.h|  1 +
 4 files changed, 30 insertions(+)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index f3530a90e6..802d3096cd 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -336,6 +336,17 @@ void qemu_chr_fe_set_echo(CharBackend *be, bool echo)
 }
 }
 
+int qemu_chr_fe_get_winsize(CharBackend *be, uint16_t *cols, uint16_t *rows)
+{
+Chardev *chr = be->chr;
+
+if (chr && CHARDEV_GET_CLASS(chr)->chr_get_winsize) {
+return CHARDEV_GET_CLASS(chr)->chr_get_winsize(chr, cols, rows);
+}
+
+return -1;
+}
+
 void qemu_chr_fe_set_open(CharBackend *be, int fe_open)
 {
 Chardev *chr = be->chr;
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 46c44af67c..368ce2334e 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -293,6 +293,12 @@ static void mux_chr_update_read_handlers(Chardev *chr)
   chr->gcontext, true, false);
 }
 
+static int mux_chr_get_winsize(Chardev *chr, uint16_t *cols, uint16_t *rows)
+{
+MuxChardev *d = MUX_CHARDEV(chr);
+return qemu_chr_fe_get_winsize(>chr, cols, rows);
+}
+
 void mux_set_focus(Chardev *chr, int focus)
 {
 MuxChardev *d = MUX_CHARDEV(chr);
@@ -385,6 +391,7 @@ static void char_mux_class_init(ObjectClass *oc, void *data)
 cc->chr_be_event = mux_chr_be_event;
 cc->chr_machine_done = open_muxes;
 cc->chr_update_read_handler = mux_chr_update_read_handlers;
+cc->chr_get_winsize = mux_chr_get_winsize;
 }
 
 static const TypeInfo char_mux_type_info = {
diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index a553843364..b7943df93a 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -154,6 +154,17 @@ int qemu_chr_fe_wait_connected(CharBackend *be, Error 
**errp);
  */
 void qemu_chr_fe_set_echo(CharBackend *be, bool echo);
 
+/**
+ * qemu_chr_fe_get_winsize:
+ * @cols: the address for storing columns
+ * @rows: the address for storing rows
+ *
+ * Get the terminal size of the backend.
+ *
+ * Returns: 0 on success and < 0 on error
+ */
+int qemu_chr_fe_get_winsize(CharBackend *be, uint16_t *cols, uint16_t *rows);
+
 /**
  * qemu_chr_fe_set_open:
  *
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 00589a6025..fb20707917 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -276,6 +276,7 @@ typedef struct ChardevClass {
 void (*chr_be_event)(Chardev *s, QEMUChrEvent event);
 /* Return 0 if succeeded, 1 if failed */
 int (*chr_machine_done)(Chardev *chr);
+int (*chr_get_winsize)(Chardev *chr, uint16_t *cols, uint16_t *rows);
 } ChardevClass;
 
 Chardev *qemu_chardev_new(const char *id, const char *typename,
-- 
2.27.0

[PATCH 4/6] char-stdio: add support for the terminal size

[Szymon Lukasz]

Implement chr_get_winsize for the stdio backend
and trigger CHR_EVENT_RESIZE upon SIGWINCH delivery.

Signed-off-by: Szymon Lukasz 
---
 chardev/char-stdio.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/chardev/char-stdio.c b/chardev/char-stdio.c
index 82eaebc1db..ab14edffc1 100644
--- a/chardev/char-stdio.c
+++ b/chardev/char-stdio.c
@@ -34,7 +34,9 @@
 #include "chardev/char-win-stdio.h"
 #else
 #include 
+#include 
 #include "chardev/char-fd.h"
+#include "qemu/main-loop.h"
 #endif
 
 #ifndef _WIN32
@@ -45,6 +47,13 @@ static bool stdio_in_use;
 static bool stdio_allow_signal;
 static bool stdio_echo_state;
 
+typedef struct {
+FDChardev parent;
+Notifier resize_notifier;
+} StdioChardev;
+
+#define STDIO_CHARDEV(obj) OBJECT_CHECK(StdioChardev, (obj), 
TYPE_CHARDEV_STDIO)
+
 static void term_exit(void)
 {
 if (stdio_in_use) {
@@ -82,11 +91,31 @@ static void term_stdio_handler(int sig)
 qemu_chr_set_echo_stdio(NULL, stdio_echo_state);
 }
 
+static int qemu_chr_get_winsize_stdio(Chardev *chr, uint16_t *cols, uint16_t 
*rows)
+{
+struct winsize ws;
+
+if (ioctl(1, TIOCGWINSZ, ) < 0) {
+return -1;
+}
+
+*cols = ws.ws_col;
+*rows = ws.ws_row;
+return 0;
+}
+
+static void term_resize_notify(Notifier *n, void *data)
+{
+StdioChardev *s = container_of(n, StdioChardev, resize_notifier);
+qemu_chr_be_event(CHARDEV(s), CHR_EVENT_RESIZE);
+}
+
 static void qemu_chr_open_stdio(Chardev *chr,
 ChardevBackend *backend,
 bool *be_opened,
 Error **errp)
 {
+StdioChardev *s = STDIO_CHARDEV(chr);
 ChardevStdio *opts = backend->u.stdio.data;
 struct sigaction act;
 
@@ -116,6 +145,9 @@ static void qemu_chr_open_stdio(Chardev *chr,
 stdio_allow_signal = opts->signal;
 }
 qemu_chr_set_echo_stdio(chr, false);
+
+s->resize_notifier.notify = term_resize_notify;
+sigwinch_add_notifier(>resize_notifier);
 }
 #endif
 
@@ -139,6 +171,7 @@ static void char_stdio_class_init(ObjectClass *oc, void 
*data)
 #ifndef _WIN32
 cc->open = qemu_chr_open_stdio;
 cc->chr_set_echo = qemu_chr_set_echo_stdio;
+cc->chr_get_winsize = qemu_chr_get_winsize_stdio;
 #endif
 }
 
@@ -155,6 +188,7 @@ static const TypeInfo char_stdio_type_info = {
 .parent = TYPE_CHARDEV_WIN_STDIO,
 #else
 .parent = TYPE_CHARDEV_FD,
+.instance_size = sizeof(StdioChardev),
 #endif
 .instance_finalize = char_stdio_finalize,
 .class_init = char_stdio_class_init,
-- 
2.27.0

[PATCH 3/6] chardev: add support for notifying about terminal resizes

[Szymon Lukasz]

Add a new chardev event, CHR_EVENT_RESIZE, which a backend should
trigger if detects the size of the connected terminal changed.

Signed-off-by: Szymon Lukasz 
---
 backends/cryptodev-vhost-user.c | 1 +
 chardev/char.c  | 1 +
 hw/block/vhost-user-blk.c   | 1 +
 hw/char/terminal3270.c  | 1 +
 hw/char/virtio-console.c| 1 +
 hw/ipmi/ipmi_bmc_extern.c   | 1 +
 hw/usb/ccid-card-passthru.c | 1 +
 hw/usb/dev-serial.c | 1 +
 hw/usb/redirect.c   | 1 +
 include/chardev/char.h  | 1 +
 monitor/hmp.c   | 1 +
 monitor/qmp.c   | 1 +
 net/vhost-user.c| 1 +
 13 files changed, 13 insertions(+)

diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
index 8b8cbc4223..bbf8ad426a 100644
--- a/backends/cryptodev-vhost-user.c
+++ b/backends/cryptodev-vhost-user.c
@@ -174,6 +174,7 @@ static void cryptodev_vhost_user_event(void *opaque, 
QEMUChrEvent event)
 case CHR_EVENT_BREAK:
 case CHR_EVENT_MUX_IN:
 case CHR_EVENT_MUX_OUT:
+case CHR_EVENT_RESIZE:
 /* Ignore */
 break;
 }
diff --git a/chardev/char.c b/chardev/char.c
index e3051295ac..904f8bf6e3 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -74,6 +74,7 @@ void qemu_chr_be_event(Chardev *s, QEMUChrEvent event)
 case CHR_EVENT_BREAK:
 case CHR_EVENT_MUX_IN:
 case CHR_EVENT_MUX_OUT:
+case CHR_EVENT_RESIZE:
 /* Ignore */
 break;
 }
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index a00b854736..1a656a27c3 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -403,6 +403,7 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent 
event)
 case CHR_EVENT_BREAK:
 case CHR_EVENT_MUX_IN:
 case CHR_EVENT_MUX_OUT:
+case CHR_EVENT_RESIZE:
 /* Ignore */
 break;
 }
diff --git a/hw/char/terminal3270.c b/hw/char/terminal3270.c
index 2c47ebf007..eadccbb617 100644
--- a/hw/char/terminal3270.c
+++ b/hw/char/terminal3270.c
@@ -169,6 +169,7 @@ static void chr_event(void *opaque, QEMUChrEvent event)
 case CHR_EVENT_BREAK:
 case CHR_EVENT_MUX_IN:
 case CHR_EVENT_MUX_OUT:
+case CHR_EVENT_RESIZE:
 /* Ignore */
 break;
 }
diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 4f46753ea3..97b9240ef5 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -165,6 +165,7 @@ static void chr_event(void *opaque, QEMUChrEvent event)
 case CHR_EVENT_BREAK:
 case CHR_EVENT_MUX_IN:
 case CHR_EVENT_MUX_OUT:
+case CHR_EVENT_RESIZE:
 /* Ignore */
 break;
 }
diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
index f9a13e0a44..9562584309 100644
--- a/hw/ipmi/ipmi_bmc_extern.c
+++ b/hw/ipmi/ipmi_bmc_extern.c
@@ -439,6 +439,7 @@ static void chr_event(void *opaque, QEMUChrEvent event)
 case CHR_EVENT_BREAK:
 case CHR_EVENT_MUX_IN:
 case CHR_EVENT_MUX_OUT:
+case CHR_EVENT_RESIZE:
 /* Ignore */
 break;
 }
diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
index bb325dbc4a..3c26b16ed0 100644
--- a/hw/usb/ccid-card-passthru.c
+++ b/hw/usb/ccid-card-passthru.c
@@ -321,6 +321,7 @@ static void ccid_card_vscard_event(void *opaque, 
QEMUChrEvent event)
 case CHR_EVENT_MUX_IN:
 case CHR_EVENT_MUX_OUT:
 case CHR_EVENT_CLOSED:
+case CHR_EVENT_RESIZE:
 /* Ignore */
 break;
 }
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 7e50e3ba47..e8e960d0e6 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -507,6 +507,7 @@ static void usb_serial_event(void *opaque, QEMUChrEvent 
event)
 break;
 case CHR_EVENT_MUX_IN:
 case CHR_EVENT_MUX_OUT:
+case CHR_EVENT_RESIZE:
 /* Ignore */
 break;
 }
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 417a60a2e6..b716c4fdd7 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1383,6 +1383,7 @@ static void usbredir_chardev_event(void *opaque, 
QEMUChrEvent event)
 case CHR_EVENT_BREAK:
 case CHR_EVENT_MUX_IN:
 case CHR_EVENT_MUX_OUT:
+case CHR_EVENT_RESIZE:
 /* Ignore */
 break;
 }
diff --git a/include/chardev/char.h b/include/chardev/char.h
index fb20707917..c3d108ce82 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -22,6 +22,7 @@ typedef enum {
 CHR_EVENT_OPENED, /* new connection established */
 CHR_EVENT_MUX_IN, /* mux-focus was set to this terminal */
 CHR_EVENT_MUX_OUT, /* mux-focus will move on */
+CHR_EVENT_RESIZE, /* the terminal size of the chardev changed */
 CHR_EVENT_CLOSED /* connection closed.  NOTE: currently this event
   * is only bound to the read port of the chardev.
   * Normally the read port and write port of a
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 

[PATCH 0/6] virtio-console: notify about the terminal size

[Szymon Lukasz]

The goal of this series is to have a nice terminal into a Linux guest
without having to set up networking and using, e.g. ssh.

The virtio spec allows a virtio-console device to notify the guest about
terminal resizes in the host. Linux Kernel implements the driver part of
the spec. This series implement the device part in QEMU.

In this series resize notifications are only supported for the stdio
backend but I think it should be easy to add support for the vc backend.
Support for tty/serial backends is complicated by the fact that there is
no clean way to detect resizes of the underlying terminal.

Also there is a problem with the virtio spec and Linux Kernel
implementation, the order of fields in virtio_console_resize struct
differs between the kernel and the spec. I do not know if there is any
implementation of the virtio-console driver that handles resize messages
and uses a different order than Linux.



Szymon Lukasz (6):
  main-loop: change the handling of SIGWINCH
  chardev: add support for retrieving the terminal size
  chardev: add support for notifying about terminal resizes
  char-stdio: add support for the terminal size
  virtio-serial-bus: add terminal resize messages
  virtio-console: notify the guest about terminal resizes

 backends/cryptodev-vhost-user.c   |  1 +
 chardev/char-fe.c | 11 ++
 chardev/char-mux.c|  7 
 chardev/char-stdio.c  | 34 
 chardev/char.c|  1 +
 hw/block/vhost-user-blk.c |  1 +
 hw/char/terminal3270.c|  1 +
 hw/char/trace-events  |  1 +
 hw/char/virtio-console.c  | 65 +--
 hw/char/virtio-serial-bus.c   | 41 ++-
 hw/ipmi/ipmi_bmc_extern.c |  1 +
 hw/usb/ccid-card-passthru.c   |  1 +
 hw/usb/dev-serial.c   |  1 +
 hw/usb/redirect.c |  1 +
 include/chardev/char-fe.h | 11 ++
 include/chardev/char.h|  2 +
 include/hw/virtio/virtio-serial.h |  5 +++
 include/qemu/main-loop.h  |  4 ++
 monitor/hmp.c |  1 +
 monitor/qmp.c |  1 +
 net/vhost-user.c  |  1 +
 ui/curses.c   | 11 +++---
 util/main-loop.c  | 21 ++
 23 files changed, 213 insertions(+), 11 deletions(-)

-- 
2.27.0

[PATCH 1/6] main-loop: change the handling of SIGWINCH

[Szymon Lukasz]

Block SIGWINCH, so it is delivered only via signalfd.
Install a handler that uses NotifierList to tell
interested parties about SIGWINCH delivery.

Signed-off-by: Szymon Lukasz 
---
 include/qemu/main-loop.h |  4 
 ui/curses.c  | 11 ++-
 util/main-loop.c | 21 +
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index a6d20b0719..f27dba1fd8 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -325,4 +325,8 @@ typedef struct MainLoopPoll {
 void main_loop_poll_add_notifier(Notifier *notify);
 void main_loop_poll_remove_notifier(Notifier *notify);
 
+#ifndef _WIN32
+void sigwinch_add_notifier(Notifier *n);
+#endif
+
 #endif
diff --git a/ui/curses.c b/ui/curses.c
index a59b23a9cf..e5895d506f 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -34,6 +34,7 @@
 #include 
 
 #include "qapi/error.h"
+#include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "ui/console.h"
 #include "ui/input.h"
@@ -146,7 +147,7 @@ static void curses_resize(DisplayChangeListener *dcl,
 }
 
 #if !defined(_WIN32) && defined(SIGWINCH) && defined(KEY_RESIZE)
-static volatile sig_atomic_t got_sigwinch;
+static bool got_sigwinch;
 static void curses_winch_check(void)
 {
 struct winsize {
@@ -169,17 +170,17 @@ static void curses_winch_check(void)
 invalidate = 1;
 }
 
-static void curses_winch_handler(int signum)
+static void curses_winch_handler(Notifier *n, void *data)
 {
 got_sigwinch = true;
 }
 
 static void curses_winch_init(void)
 {
-struct sigaction old, winch = {
-.sa_handler  = curses_winch_handler,
+static Notifier n = {
+.notify = curses_winch_handler
 };
-sigaction(SIGWINCH, , );
+sigwinch_add_notifier();
 }
 #else
 static void curses_winch_check(void) {}
diff --git a/util/main-loop.c b/util/main-loop.c
index eda63fe4e0..0f5c8f3af1 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -90,6 +90,7 @@ static int qemu_signal_init(Error **errp)
 sigaddset(, SIGIO);
 sigaddset(, SIGALRM);
 sigaddset(, SIGBUS);
+sigaddset(, SIGWINCH);
 /* SIGINT cannot be handled via signalfd, so that ^C can be used
  * to interrupt QEMU when it is being run under gdb.  SIGHUP and
  * SIGTERM are also handled asynchronously, even though it is not
@@ -111,6 +112,26 @@ static int qemu_signal_init(Error **errp)
 return 0;
 }
 
+static NotifierList sigwinch_notifiers =
+NOTIFIER_LIST_INITIALIZER(sigwinch_notifiers);
+
+static void sigwinch_handler(int signum)
+{
+notifier_list_notify(_notifiers, NULL);
+}
+
+void sigwinch_add_notifier(Notifier *n)
+{
+if (notifier_list_empty(_notifiers)) {
+struct sigaction action = {
+.sa_handler = sigwinch_handler,
+};
+sigaction(SIGWINCH, , NULL);
+}
+
+notifier_list_add(_notifiers, n);
+}
+
 #else /* _WIN32 */
 
 static int qemu_signal_init(Error **errp)
-- 
2.27.0

[PATCH 5/6] virtio-serial-bus: add terminal resize messages

[Szymon Lukasz]

Implement the part of the virtio spec that allows to notify the virtio
driver about terminal resizes. The virtio spec contains two methods to
achieve that:

For legacy drivers, we have only one port and we put the terminal size
in the config space and inject the config changed interrupt.

For multiport devices, we use the control virtqueue to send a packet
containing the terminal size. Note that the Linux kernel expects
the fields indicating the number of rows and columns in a packet to be
in a different order than the one specified in the current version of
the virtio spec. We follow the Linux implementation, so hopefully there
is no implementation of this functionality conforming to the spec.

Signed-off-by: Szymon Lukasz 
---
 hw/char/trace-events  |  1 +
 hw/char/virtio-serial-bus.c   | 41 +--
 include/hw/virtio/virtio-serial.h |  5 
 3 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/hw/char/trace-events b/hw/char/trace-events
index d20eafd56f..be40df47ea 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -10,6 +10,7 @@ serial_ioport_write(uint16_t addr, uint8_t value) "write addr 
0x%02x val 0x%02x"
 
 # virtio-serial-bus.c
 virtio_serial_send_control_event(unsigned int port, uint16_t event, uint16_t 
value) "port %u, event %u, value %u"
+virtio_serial_send_console_resize(unsigned int port, uint16_t cols, uint16_t 
rows) "port %u, cols %u, rows %u"
 virtio_serial_throttle_port(unsigned int port, bool throttle) "port %u, 
throttle %d"
 virtio_serial_handle_control_message(uint16_t event, uint16_t value) "event 
%u, value %u"
 virtio_serial_handle_control_message_port(unsigned int port) "port %u"
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 262089c0c9..9d99161e13 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -261,6 +261,42 @@ static size_t send_control_event(VirtIOSerial *vser, 
uint32_t port_id,
 return send_control_msg(vser, , sizeof(cpkt));
 }
 
+/*
+ * This struct should be added to the Linux kernel uapi headers
+ * and later imported to standard-headers/linux/virtio_console.h
+ */
+struct virtio_console_resize {
+__virtio16 rows;
+__virtio16 cols;
+};
+
+void virtio_serial_send_console_resize(VirtIOSerialPort *port,
+   uint16_t cols, uint16_t rows)
+{
+VirtIOSerial *vser = port->vser;
+VirtIODevice *vdev = VIRTIO_DEVICE(vser);
+
+if (virtio_vdev_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) {
+struct {
+struct virtio_console_control control;
+struct virtio_console_resize resize;
+} buffer;
+
+virtio_stl_p(vdev, , port->id);
+virtio_stw_p(vdev, , VIRTIO_CONSOLE_RESIZE);
+virtio_stw_p(vdev, , cols);
+virtio_stw_p(vdev, , rows);
+
+trace_virtio_serial_send_console_resize(port->id, cols, rows);
+send_control_msg(vser, , sizeof(buffer));
+
+} else if (virtio_vdev_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE)) {
+vser->port0_cols = cols;
+vser->port0_rows = rows;
+virtio_notify_config(vdev);
+}
+}
+
 /* Functions for use inside qemu to open and read from/write to ports */
 int virtio_serial_open(VirtIOSerialPort *port)
 {
@@ -559,6 +595,7 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t 
features,
 vser = VIRTIO_SERIAL(vdev);
 
 features |= vser->host_features;
+virtio_add_feature(, VIRTIO_CONSOLE_F_SIZE);
 if (vser->bus.max_nr_ports > 1) {
 virtio_add_feature(, VIRTIO_CONSOLE_F_MULTIPORT);
 }
@@ -572,8 +609,8 @@ static void get_config(VirtIODevice *vdev, uint8_t 
*config_data)
 struct virtio_console_config *config =
 (struct virtio_console_config *)config_data;
 
-config->cols = 0;
-config->rows = 0;
+config->cols = virtio_tswap16(vdev, vser->port0_cols);
+config->rows = virtio_tswap16(vdev, vser->port0_rows);
 config->max_nr_ports = virtio_tswap32(vdev,
   vser->serial.max_virtserial_ports);
 }
diff --git a/include/hw/virtio/virtio-serial.h 
b/include/hw/virtio/virtio-serial.h
index ed3e916b68..1d6436c0b1 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -188,6 +188,8 @@ struct VirtIOSerial {
 virtio_serial_conf serial;
 
 uint64_t host_features;
+
+uint16_t port0_cols, port0_rows;
 };
 
 /* Interface to the virtio-serial bus */
@@ -222,6 +224,9 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port);
  */
 void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
 
+void virtio_serial_send_console_resize(VirtIOSerialPort *port,
+   uint16_t cols, uint16_t rows);
+
 #define TYPE_VIRTIO_SERIAL "virtio-serial-device"
 #define VIRTIO_SERIAL(obj) \
 OBJECT_CHECK(VirtIOSerial, (obj), TYPE_VIRTIO_SERIAL)
-- 
2.27.0

[PATCH] i2c: Match parameters of i2c_start_transfer and i2c_send_recv

[BALATON Zoltan]

These functions have a parameter that decides the direction of
transfer but totally confusingly they don't match but inverted sense.
To avoid frequent mistakes when using these functions change
i2c_send_recv to match i2c_start_transfer. Also use bool in
i2c_start_transfer instead of int to match i2c_send_recv.

Signed-off-by: BALATON Zoltan 
---
Looks like hw/misc/auxbus.c already got this wrong and calls both
i2c_start_transfer and i2c_send_recv with same is_write parameter.
Although the name of the is_write variable suggest this may need to be
inverted I'm not sure what that value actially means and which usage
was correct so I did not touch it. Someone knowing this device might
want to review and fix it.

 hw/display/sm501.c   |  2 +-
 hw/i2c/core.c| 34 +-
 hw/i2c/ppc4xx_i2c.c  |  2 +-
 include/hw/i2c/i2c.h |  4 ++--
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 2db347dcbc..ccd0a6e376 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1034,7 +1034,7 @@ static void sm501_i2c_write(void *opaque, hwaddr addr, 
uint64_t value,
 int i;
 for (i = 0; i <= s->i2c_byte_count; i++) {
 res = i2c_send_recv(s->i2c_bus, >i2c_data[i],
-!(s->i2c_addr & 1));
+s->i2c_addr & 1);
 if (res) {
 s->i2c_status |= SM501_I2C_STATUS_ERROR;
 return;
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 1aac457a2a..c9d01df427 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -91,7 +91,7 @@ int i2c_bus_busy(I2CBus *bus)
  * without releasing the bus.  If that fails, the bus is still
  * in a transaction.
  */
-int i2c_start_transfer(I2CBus *bus, uint8_t address, int recv)
+int i2c_start_transfer(I2CBus *bus, uint8_t address, bool recv)
 {
 BusChild *kid;
 I2CSlaveClass *sc;
@@ -175,26 +175,14 @@ void i2c_end_transfer(I2CBus *bus)
 bus->broadcast = false;
 }
 
-int i2c_send_recv(I2CBus *bus, uint8_t *data, bool send)
+int i2c_send_recv(I2CBus *bus, uint8_t *data, bool recv)
 {
 I2CSlaveClass *sc;
 I2CSlave *s;
 I2CNode *node;
 int ret = 0;
 
-if (send) {
-QLIST_FOREACH(node, >current_devs, next) {
-s = node->elt;
-sc = I2C_SLAVE_GET_CLASS(s);
-if (sc->send) {
-trace_i2c_send(s->address, *data);
-ret = ret || sc->send(s, *data);
-} else {
-ret = -1;
-}
-}
-return ret ? -1 : 0;
-} else {
+if (recv) {
 ret = 0xff;
 if (!QLIST_EMPTY(>current_devs) && !bus->broadcast) {
 sc = I2C_SLAVE_GET_CLASS(QLIST_FIRST(>current_devs)->elt);
@@ -206,19 +194,31 @@ int i2c_send_recv(I2CBus *bus, uint8_t *data, bool send)
 }
 *data = ret;
 return 0;
+} else {
+QLIST_FOREACH(node, >current_devs, next) {
+s = node->elt;
+sc = I2C_SLAVE_GET_CLASS(s);
+if (sc->send) {
+trace_i2c_send(s->address, *data);
+ret = ret || sc->send(s, *data);
+} else {
+ret = -1;
+}
+}
+return ret ? -1 : 0;
 }
 }
 
 int i2c_send(I2CBus *bus, uint8_t data)
 {
-return i2c_send_recv(bus, , true);
+return i2c_send_recv(bus, , false);
 }
 
 uint8_t i2c_recv(I2CBus *bus)
 {
 uint8_t data = 0xff;
 
-i2c_send_recv(bus, , false);
+i2c_send_recv(bus, , true);
 return data;
 }
 
diff --git a/hw/i2c/ppc4xx_i2c.c b/hw/i2c/ppc4xx_i2c.c
index c0a8e04567..d3899203a4 100644
--- a/hw/i2c/ppc4xx_i2c.c
+++ b/hw/i2c/ppc4xx_i2c.c
@@ -239,7 +239,7 @@ static void ppc4xx_i2c_writeb(void *opaque, hwaddr addr, 
uint64_t value,
 }
 }
 if (!(i2c->sts & IIC_STS_ERR) &&
-i2c_send_recv(i2c->bus, >mdata[i], !recv)) {
+i2c_send_recv(i2c->bus, >mdata[i], recv)) {
 i2c->sts |= IIC_STS_ERR;
 i2c->extsts |= IIC_EXTSTS_XFRA;
 break;
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 4117211565..a09ab9230b 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -72,10 +72,10 @@ struct I2CBus {
 I2CBus *i2c_init_bus(DeviceState *parent, const char *name);
 void i2c_set_slave_address(I2CSlave *dev, uint8_t address);
 int i2c_bus_busy(I2CBus *bus);
-int i2c_start_transfer(I2CBus *bus, uint8_t address, int recv);
+int i2c_start_transfer(I2CBus *bus, uint8_t address, bool recv);
 void i2c_end_transfer(I2CBus *bus);
 void i2c_nack(I2CBus *bus);
-int i2c_send_recv(I2CBus *bus, uint8_t *data, bool send);
+int i2c_send_recv(I2CBus *bus, uint8_t *data, bool recv);
 int i2c_send(I2CBus *bus, uint8_t data);
 uint8_t i2c_recv(I2CBus *bus);
 
-- 

Re: Re: [PATCH] chardev/tcp: fix error message double free error

[lichun]

>Hi
>
>On Sun, Jun 21, 2020 at 10:54 AM lichun  wrote:
>>
>> Signed-off-by: lichun 
>> ---
>>  chardev/char-socket.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>> index afebeec5c3..3b6c1c5848 100644
>> --- a/chardev/char-socket.c
>> +++ b/chardev/char-socket.c
>> @@ -1086,7 +1086,10 @@ static void qemu_chr_socket_connected(QIOTask *task, 
>> void *opaque)
>>  if (qio_task_propagate_error(task, )) {
>>  tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
>>  check_report_connect_error(chr, err);
>> -    error_free(err);
>> +    /* If connect_err_reported is true, it means err is already freed */
>> +    if (!s->connect_err_reported) {
>> +    error_free(err);
>> +    }
>
>Good catch (did you find it with a static analysis tool?). 
Yeah, I find it with gdb.

>
>Instead of checking connect_err_reported here, I would rather let
>check_report_connect_error() handle error_free(). Can you update the
>patch? 
Of course, I will.
>
>thanks
>
>>  goto cleanup;
>>  }
>>
>> --
>> 2.18.4
>>
>

[PATCH v2] chardev/tcp: fix error message double free error

[lichun]

Signed-off-by: lichun 
---
 chardev/char-socket.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index afebeec5c3..569d54c144 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -142,6 +142,8 @@ static void check_report_connect_error(Chardev *chr,
   "Unable to connect character device %s: ",
   chr->label);
 s->connect_err_reported = true;
+} else {
+error_free(err);
 }
 qemu_chr_socket_restart_timer(chr);
 }
@@ -1086,7 +1088,6 @@ static void qemu_chr_socket_connected(QIOTask *task, void 
*opaque)
 if (qio_task_propagate_error(task, )) {
 tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
 check_report_connect_error(chr, err);
-error_free(err);
 goto cleanup;
 }
 
-- 
2.18.4

[no subject]

[Antonio Raffaele]

Hi I'm trying to create a qemu virtual machine that runs windows 10. I
would like to try to make it almost indistinguishable from a real computer
(I know it's impossible, but at least I get close). I have already changed
any suspicious identifiers (smbios, hard disk, card network and so on,
host-passthroug cpu etc.) But now checking the various components that the
guest computer recognizes, I realized (through the hwinfo64 program) that
in the bus section, then pcibus there are devices called "Red Hat , Device
ID "and with the same devicename, as device class have:" PCI-to-PCI Bridge
". Is there a way to change the devicename of these virtual compontents
(maybe even changing the qemu source)?

[PULL 11/15] hw/rx: Honor -accel qtest

[Philippe Mathieu-Daudé]

From: Richard Henderson 

Issue an error if no kernel, no bios, and not qtest'ing.
Fixes make check-qtest-rx: test/qom-test.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Yoshinori Sato 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20190531134315.4109-16-richard.hender...@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/rx/rx62n.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/rx/rx62n.c b/hw/rx/rx62n.c
index 85b7770023..d8f0fa4625 100644
--- a/hw/rx/rx62n.c
+++ b/hw/rx/rx62n.c
@@ -21,12 +21,14 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qemu/error-report.h"
 #include "hw/hw.h"
 #include "hw/rx/rx62n.h"
 #include "hw/loader.h"
 #include "hw/sysbus.h"
 #include "hw/qdev-properties.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
 #include "cpu.h"
 
 /*
@@ -208,7 +210,12 @@ static void rx62n_realize(DeviceState *dev, Error **errp)
 memory_region_add_subregion(s->sysmem, RX62N_CFLASH_BASE, >c_flash);
 
 if (!s->kernel) {
-rom_add_file_fixed(bios_name, RX62N_CFLASH_BASE, 0);
+if (bios_name) {
+rom_add_file_fixed(bios_name, RX62N_CFLASH_BASE, 0);
+}  else if (!qtest_enabled()) {
+error_report("No bios or kernel specified");
+exit(1);
+}
 }
 
 /* Initialize CPU */
-- 
2.21.3

[PULL 14/15] BootLinuxConsoleTest: Test the RX GDB simulator

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé 

Add two tests for the rx-gdbsim machine, based on the recommended
test setup from Yoshinori Sato:
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03586.html

- U-Boot prompt
- Linux kernel with Sash shell

These are very quick tests:

  $ avocado run -t arch:rx tests/acceptance/machine_rx_gdbsim.py
  JOB ID : 84a6ef01c0b87975ecbfcb31a920afd735753ace
  JOB LOG: 
/home/phil/avocado/job-results/job-2019-05-24T05.02-84a6ef0/job.log
   (1/2) tests/acceptance/machine_rx_gdbsim.py:RxGdbSimMachine.test_uboot: PASS 
(0.11 s)
   (2/2) tests/acceptance/machine_rx_gdbsim.py:RxGdbSimMachine.test_linux_sash: 
PASS (0.45 s)
  RESULTS: PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
CANCEL 0

Tests can also be run with:

  $ avocado --show=console run -t arch:rx tests/acceptance/machine_rx_gdbsim.py
  console: U-Boot 2016.05-rc3-23705-ga1ef3c71cb-dirty (Feb 05 2019 - 21:56:06 
+0900)
  console: Linux version 4.19.0+ (yo-satoh@yo-satoh-debian) (gcc version 9.0.0 
20181105 (experimental) (GCC)) #137 Wed Feb 20 23:20:02 JST 2019
  console: Built 1 zonelists, mobility grouping on.  Total pages: 8128
  ...
  console: SuperH (H)SCI(F) driver initialized
  console: 88240.serial: ttySC0 at MMIO 0x88240 (irq = 215, base_baud = 0) is a 
sci
  console: console [ttySC0] enabled
  console: 88248.serial: ttySC1 at MMIO 0x88248 (irq = 219, base_baud = 0) is a 
sci

Signed-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Yoshinori Sato 
Message-Id: <20200224141923.82118-22-ys...@users.sourceforge.jp>
[PMD: Replace obsolete set_machine() by machine tag, and rename as gdbsim]
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
 MAINTAINERS   |  1 +
 tests/acceptance/machine_rx_gdbsim.py | 68 +++
 2 files changed, 69 insertions(+)
 create mode 100644 tests/acceptance/machine_rx_gdbsim.py

diff --git a/MAINTAINERS b/MAINTAINERS
index a16e167721..1c9b4bc8e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1258,6 +1258,7 @@ rx-gdbsim
 M: Yoshinori Sato 
 S: Maintained
 F: hw/rx/rx-gdbsim.c
+F: tests/acceptance/machine_rx_gdbsim.py
 
 SH4 Machines
 
diff --git a/tests/acceptance/machine_rx_gdbsim.py 
b/tests/acceptance/machine_rx_gdbsim.py
new file mode 100644
index 00..a44f2c87da
--- /dev/null
+++ b/tests/acceptance/machine_rx_gdbsim.py
@@ -0,0 +1,68 @@
+# Functional test that boots a Linux kernel and checks the console
+#
+# Copyright (c) 2018 Red Hat, Inc.
+#
+# Author:
+#  Cleber Rosa 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from avocado_qemu import Test
+from avocado_qemu import exec_command_and_wait_for_pattern
+from avocado_qemu import wait_for_console_pattern
+from avocado.utils import archive
+
+
+class RxGdbSimMachine(Test):
+
+timeout = 30
+KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
+
+def test_uboot(self):
+"""
+U-Boot and checks that the console is operational.
+
+:avocado: tags=arch:rx
+:avocado: tags=machine:gdbsim-r5f562n8
+:avocado: tags=endian:little
+"""
+uboot_url = ('https://acc.dl.osdn.jp/users/23/23888/u-boot.bin.gz')
+uboot_hash = '9b78dbd43b40b2526848c0b1ce9de02c24f4dcdb'
+uboot_path = self.fetch_asset(uboot_url, asset_hash=uboot_hash)
+uboot_path = archive.uncompress(uboot_path, self.workdir)
+
+self.vm.set_console()
+self.vm.add_args('-bios', uboot_path,
+ '-no-reboot')
+self.vm.launch()
+uboot_version = 'U-Boot 2016.05-rc3-23705-ga1ef3c71cb-dirty'
+wait_for_console_pattern(self, uboot_version)
+gcc_version = 'rx-unknown-linux-gcc (GCC) 9.0.0 20181105 
(experimental)'
+# FIXME limit baudrate on chardev, else we type too fast
+#exec_command_and_wait_for_pattern(self, 'version', gcc_version)
+
+def test_linux_sash(self):
+"""
+Boots a Linux kernel and checks that the console is operational.
+
+:avocado: tags=arch:rx
+:avocado: tags=machine:gdbsim-r5f562n7
+:avocado: tags=endian:little
+"""
+dtb_url = ('https://acc.dl.osdn.jp/users/23/23887/rx-qemu.dtb')
+dtb_hash = '7b4e4e2c71905da44e86ce47adee2210b026ac18'
+dtb_path = self.fetch_asset(dtb_url, asset_hash=dtb_hash)
+kernel_url = ('http://acc.dl.osdn.jp/users/23/23845/zImage')
+kernel_hash = '39a81067f8d72faad90866ddfefa19165d68fc99'
+kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
+
+self.vm.set_console()
+kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'earlycon'
+self.vm.add_args('-kernel', kernel_path,
+ '-dtb', dtb_path,
+ '-no-reboot')
+self.vm.launch()
+wait_for_console_pattern(self, 'Sash command shell (version 1.1.1)',
+

[PULL 15/15] docs: Document the RX target

[Philippe Mathieu-Daudé]

From: Yoshinori Sato 

Add rx-virt target specification document.

Signed-off-by: Yoshinori Sato 
Message-Id: <20200308130637.37651-1-ys...@users.sourceforge.jp>
[PMD: Cover in MAINTAINERS, rename as gdbsim, use machine argument]
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
 docs/system/target-rx.rst | 36 
 docs/system/targets.rst   |  1 +
 MAINTAINERS   |  1 +
 3 files changed, 38 insertions(+)
 create mode 100644 docs/system/target-rx.rst

diff --git a/docs/system/target-rx.rst b/docs/system/target-rx.rst
new file mode 100644
index 00..4a20a89a06
--- /dev/null
+++ b/docs/system/target-rx.rst
@@ -0,0 +1,36 @@
+.. _RX-System-emulator:
+
+RX System emulator
+
+
+Use the executable ``qemu-system-rx`` to simulate RX target (GDB simulator).
+This target emulated following devices.
+
+-  R5F562N8 MCU
+
+   -  On-chip memory (ROM 512KB, RAM 96KB)
+   -  Interrupt Control Unit (ICUa)
+   -  8Bit Timer x 1CH (TMR0,1)
+   -  Compare Match Timer x 2CH (CMT0,1)
+   -  Serial Communication Interface x 1CH (SCI0)
+
+-  External memory 16MByte
+
+Example of ``qemu-system-rx`` usage for RX is shown below:
+
+Download  from
+https://osdn.net/users/ysato/pf/qemu/dl/u-boot.bin.gz
+
+Start emulation of rx-virt::
+  qemu-system-rx -M gdbsim-r5f562n8 -bios 
+
+Download ``kernel_image_file`` from
+https://osdn.net/users/ysato/pf/qemu/dl/zImage
+
+Download ``device_tree_blob`` from
+https://osdn.net/users/ysato/pf/qemu/dl/rx-virt.dtb
+
+Start emulation of rx-virt::
+  qemu-system-rx -M gdbsim-r5f562n8 \
+  -kernel  -dtb  \
+  -append "earlycon"
diff --git a/docs/system/targets.rst b/docs/system/targets.rst
index 0d8f91580a..99435a3eba 100644
--- a/docs/system/targets.rst
+++ b/docs/system/targets.rst
@@ -18,3 +18,4 @@ Contents:
target-m68k
target-xtensa
target-s390x
+   target-rx
diff --git a/MAINTAINERS b/MAINTAINERS
index 1c9b4bc8e7..5a46536d86 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1257,6 +1257,7 @@ RX Machines
 rx-gdbsim
 M: Yoshinori Sato 
 S: Maintained
+F: docs/system/target-rx.rst
 F: hw/rx/rx-gdbsim.c
 F: tests/acceptance/machine_rx_gdbsim.py
 
-- 
2.21.3

[PULL 08/15] hw/timer: RX62N compare match timer (CMT)

[Philippe Mathieu-Daudé]

From: Yoshinori Sato 

renesas_cmt: 16bit compare match timer modules.
This part use many renesas's CPU.
Hardware manual.
https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf

Signed-off-by: Yoshinori Sato 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20200224141923.82118-16-ys...@users.sourceforge.jp>
[PMD: Split from TMR, filled VMStateField for migration]
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/timer/renesas_cmt.h |  40 +
 hw/timer/renesas_cmt.c | 283 +
 MAINTAINERS|   4 +-
 hw/timer/Kconfig   |   3 +
 hw/timer/Makefile.objs |   1 +
 5 files changed, 329 insertions(+), 2 deletions(-)
 create mode 100644 include/hw/timer/renesas_cmt.h
 create mode 100644 hw/timer/renesas_cmt.c

diff --git a/include/hw/timer/renesas_cmt.h b/include/hw/timer/renesas_cmt.h
new file mode 100644
index 00..e28a15cb38
--- /dev/null
+++ b/include/hw/timer/renesas_cmt.h
@@ -0,0 +1,40 @@
+/*
+ * Renesas Compare-match timer Object
+ *
+ * Copyright (c) 2019 Yoshinori Sato
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_TIMER_RENESAS_CMT_H
+#define HW_TIMER_RENESAS_CMT_H
+
+#include "qemu/timer.h"
+#include "hw/sysbus.h"
+
+#define TYPE_RENESAS_CMT "renesas-cmt"
+#define RCMT(obj) OBJECT_CHECK(RCMTState, (obj), TYPE_RENESAS_CMT)
+
+enum {
+CMT_CH = 2,
+CMT_NR_IRQ = 1 * CMT_CH
+};
+
+typedef struct RCMTState {
+/*< private >*/
+SysBusDevice parent_obj;
+/*< public >*/
+
+uint64_t input_freq;
+MemoryRegion memory;
+
+uint16_t cmstr;
+uint16_t cmcr[CMT_CH];
+uint16_t cmcnt[CMT_CH];
+uint16_t cmcor[CMT_CH];
+int64_t tick[CMT_CH];
+qemu_irq cmi[CMT_CH];
+QEMUTimer timer[CMT_CH];
+} RCMTState;
+
+#endif
diff --git a/hw/timer/renesas_cmt.c b/hw/timer/renesas_cmt.c
new file mode 100644
index 00..2e0fd21a36
--- /dev/null
+++ b/hw/timer/renesas_cmt.c
@@ -0,0 +1,283 @@
+/*
+ * Renesas 16bit Compare-match timer
+ *
+ * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
+ *(Rev.1.40 R01UH0033EJ0140)
+ *
+ * Copyright (c) 2019 Yoshinori Sato
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/irq.h"
+#include "hw/registerfields.h"
+#include "hw/qdev-properties.h"
+#include "hw/timer/renesas_cmt.h"
+#include "migration/vmstate.h"
+
+/*
+ *  +0 CMSTR - common control
+ *  +2 CMCR  - ch0
+ *  +4 CMCNT - ch0
+ *  +6 CMCOR - ch0
+ *  +8 CMCR  - ch1
+ * +10 CMCNT - ch1
+ * +12 CMCOR - ch1
+ * If we think that the address of CH 0 has an offset of +2,
+ * we can treat it with the same address as CH 1, so define it like that.
+ */
+REG16(CMSTR, 0)
+  FIELD(CMSTR, STR0, 0, 1)
+  FIELD(CMSTR, STR1, 1, 1)
+  FIELD(CMSTR, STR,  0, 2)
+/* This addeess is channel offset */
+REG16(CMCR, 0)
+  FIELD(CMCR, CKS,  0, 2)
+  FIELD(CMCR, CMIE, 6, 1)
+REG16(CMCNT, 2)
+REG16(CMCOR, 4)
+
+static void update_events(RCMTState *cmt, int ch)
+{
+int64_t next_time;
+
+if ((cmt->cmstr & (1 << ch)) == 0) {
+/* count disable, so not happened next event. */
+return ;
+}
+next_time = cmt->cmcor[ch] - cmt->cmcnt[ch];
+next_time *= NANOSECONDS_PER_SECOND;
+next_time /= cmt->input_freq;
+/*
+ * CKS -> div rate
+ *  0 -> 8 (1 << 3)
+ *  1 -> 32 (1 << 5)
+ *  2 -> 128 (1 << 7)
+ *  3 -> 512 (1 << 9)
+ */
+next_time *= 1 << (3 + FIELD_EX16(cmt->cmcr[ch], CMCR, CKS) * 2);
+next_time += qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+timer_mod(>timer[ch], next_time);
+}
+
+static int64_t read_cmcnt(RCMTState *cmt, int ch)
+{
+int64_t delta, now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+
+if (cmt->cmstr & (1 << ch)) {
+delta = (now - cmt->tick[ch]);
+delta /= NANOSECONDS_PER_SECOND;
+delta /= cmt->input_freq;
+delta /= 1 << (3 + FIELD_EX16(cmt->cmcr[ch], CMCR, CKS) * 2);
+cmt->tick[ch] = now;
+return cmt->cmcnt[ch] + delta;
+} else {
+return cmt->cmcnt[ch];
+}
+}
+
+static uint64_t cmt_read(void *opaque, hwaddr offset, unsigned size)
+{
+RCMTState *cmt = opaque;
+int ch = offset / 0x08;
+uint64_t ret;
+
+if (offset == 

[PULL 12/15] hw/rx: Register R5F562N7 and R5F562N8 MCUs

[Philippe Mathieu-Daudé]

Make the current TYPE_RX62N_MCU an abstract class, and
generate TYPE_R5F562N7_MCU and TYPE_R5F562N8_MCU models.

Reviewed-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/rx/rx62n.h | 19 -
 hw/rx/rx62n.c | 92 ---
 2 files changed, 85 insertions(+), 26 deletions(-)

diff --git a/include/hw/rx/rx62n.h b/include/hw/rx/rx62n.h
index 7c6023bcd6..1d3e6a5cad 100644
--- a/include/hw/rx/rx62n.h
+++ b/include/hw/rx/rx62n.h
@@ -34,6 +34,9 @@
 #define TYPE_RX62N_MCU "rx62n-mcu"
 #define RX62N_MCU(obj) OBJECT_CHECK(RX62NState, (obj), TYPE_RX62N_MCU)
 
+#define TYPE_R5F562N7_MCU "r5f562n7-mcu"
+#define TYPE_R5F562N8_MCU "r5f562n8-mcu"
+
 #define RX62N_NR_TMR2
 #define RX62N_NR_CMT2
 #define RX62N_NR_SCI6
@@ -59,17 +62,11 @@ typedef struct RX62NState {
 MemoryRegion iomem3;
 MemoryRegion c_flash;
 qemu_irq irq[NR_IRQS];
+
+/* Input Clock (XTAL) frequency */
+uint32_t xtal_freq_hz;
+/* Peripheral Module Clock frequency */
+uint32_t pclk_freq_hz;
 } RX62NState;
 
-/*
- * RX62N Internal Memory
- * It is the value of R5F562N8.
- * Please change the size for R5F562N7.
- */
-#define RX62N_IRAM_SIZE (96 * KiB)
-#define RX62N_DFLASH_SIZE (32 * KiB)
-#define RX62N_CFLASH_SIZE (512 * KiB)
-
-#define RX62N_PCLK (48 * 1000 * 1000)
-
 #endif
diff --git a/hw/rx/rx62n.c b/hw/rx/rx62n.c
index d8f0fa4625..b9c217ebfa 100644
--- a/hw/rx/rx62n.c
+++ b/hw/rx/rx62n.c
@@ -5,6 +5,7 @@
  * (Rev.1.40 R01UH0033EJ0140)
  *
  * Copyright (c) 2019 Yoshinori Sato
+ * Copyright (c) 2020 Philippe Mathieu-Daudé
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -55,6 +56,25 @@
 #define RX62N_CMT_IRQ   28
 #define RX62N_SCI_IRQ   214
 
+#define RX62N_XTAL_MIN_HZ  (8 * 1000 * 1000)
+#define RX62N_XTAL_MAX_HZ (14 * 1000 * 1000)
+#define RX62N_PCLK_MAX_HZ (50 * 1000 * 1000)
+
+typedef struct RX62NClass {
+/*< private >*/
+DeviceClass parent_class;
+/*< public >*/
+const char *name;
+uint64_t ram_size;
+uint64_t rom_flash_size;
+uint64_t data_flash_size;
+} RX62NClass;
+
+#define RX62N_MCU_CLASS(klass) \
+OBJECT_CLASS_CHECK(RX62NClass, (klass), TYPE_RX62N_MCU)
+#define RX62N_MCU_GET_CLASS(obj) \
+OBJECT_GET_CLASS(RX62NClass, (obj), TYPE_RX62N_MCU)
+
 /*
  * IRQ -> IPR mapping table
  * 0x00 - 0x91: IPR no (IPR00 to IPR91)
@@ -148,7 +168,7 @@ static void register_tmr(RX62NState *s, int unit)
 object_initialize_child(OBJECT(s), "tmr[*]",
 >tmr[unit], TYPE_RENESAS_TMR);
 tmr = SYS_BUS_DEVICE(>tmr[unit]);
-qdev_prop_set_uint64(DEVICE(tmr), "input-freq", RX62N_PCLK);
+qdev_prop_set_uint64(DEVICE(tmr), "input-freq", s->pclk_freq_hz);
 sysbus_realize(tmr, _abort);
 
 irqbase = RX62N_TMR_IRQ + TMR_NR_IRQ * unit;
@@ -166,7 +186,7 @@ static void register_cmt(RX62NState *s, int unit)
 object_initialize_child(OBJECT(s), "cmt[*]",
 >cmt[unit], TYPE_RENESAS_CMT);
 cmt = SYS_BUS_DEVICE(>cmt[unit]);
-qdev_prop_set_uint64(DEVICE(cmt), "input-freq", RX62N_PCLK);
+qdev_prop_set_uint64(DEVICE(cmt), "input-freq", s->pclk_freq_hz);
 sysbus_realize(cmt, _abort);
 
 irqbase = RX62N_CMT_IRQ + CMT_NR_IRQ * unit;
@@ -185,7 +205,7 @@ static void register_sci(RX62NState *s, int unit)
 >sci[unit], TYPE_RENESAS_SCI);
 sci = SYS_BUS_DEVICE(>sci[unit]);
 qdev_prop_set_chr(DEVICE(sci), "chardev", serial_hd(unit));
-qdev_prop_set_uint64(DEVICE(sci), "input-freq", RX62N_PCLK);
+qdev_prop_set_uint64(DEVICE(sci), "input-freq", s->pclk_freq_hz);
 sysbus_realize(sci, _abort);
 
 irqbase = RX62N_SCI_IRQ + SCI_NR_IRQ * unit;
@@ -198,15 +218,31 @@ static void register_sci(RX62NState *s, int unit)
 static void rx62n_realize(DeviceState *dev, Error **errp)
 {
 RX62NState *s = RX62N_MCU(dev);
+RX62NClass *rxc = RX62N_MCU_GET_CLASS(dev);
+
+if (s->xtal_freq_hz == 0) {
+error_setg(errp, "\"xtal-frequency-hz\" property must be provided.");
+return;
+}
+/* XTAL range: 8-14 MHz */
+if (s->xtal_freq_hz < RX62N_XTAL_MIN_HZ
+|| s->xtal_freq_hz > RX62N_XTAL_MAX_HZ) {
+error_setg(errp, "\"xtal-frequency-hz\" property in incorrect range.");
+return;
+}
+/* Use a 4x fixed multiplier */
+s->pclk_freq_hz = 4 * s->xtal_freq_hz;
+/* PCLK range: 8-50 MHz */
+assert(s->pclk_freq_hz <= RX62N_PCLK_MAX_HZ);
 
 memory_region_init_ram(>iram, OBJECT(dev), "iram",
-   RX62N_IRAM_SIZE, _abort);
+   rxc->ram_size, _abort);
 memory_region_add_subregion(s->sysmem, RX62N_IRAM_BASE, >iram);
 memory_region_init_rom(>d_flash, OBJECT(dev), "flash-data",
-   RX62N_DFLASH_SIZE, _abort);
+   rxc->data_flash_size, _abort);
 

[PULL 13/15] hw/rx: Add RX GDB simulator

[Philippe Mathieu-Daudé]

From: Yoshinori Sato 

Add the RX machine internally simulated in GDB.

Signed-off-by: Yoshinori Sato 
Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
[PMD: Use TYPE_RX62N_CPU, use #define for RX62N_NR_TMR/CMT/SCI,
 renamed CPU -> MCU, device -> microcontroller]
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20200224141923.82118-18-ys...@users.sourceforge.jp>
[PMD: Split of MCU, rename gdbsim, Add gdbsim-r5f562n7/r5f562n8]
Signed-off-by: Philippe Mathieu-Daudé 
---
 default-configs/rx-softmmu.mak |   1 +
 include/hw/rx/rx62n.h  |   4 +
 hw/rx/rx-gdbsim.c  | 196 +
 MAINTAINERS|   7 ++
 hw/rx/Kconfig  |   4 +
 hw/rx/Makefile.objs|   1 +
 6 files changed, 213 insertions(+)
 create mode 100644 hw/rx/rx-gdbsim.c

diff --git a/default-configs/rx-softmmu.mak b/default-configs/rx-softmmu.mak
index 7c4eb2c1a0..df2b4e4f42 100644
--- a/default-configs/rx-softmmu.mak
+++ b/default-configs/rx-softmmu.mak
@@ -1,2 +1,3 @@
 # Default configuration for rx-softmmu
 
+CONFIG_RX_GDBSIM=y
diff --git a/include/hw/rx/rx62n.h b/include/hw/rx/rx62n.h
index 1d3e6a5cad..aa94758c27 100644
--- a/include/hw/rx/rx62n.h
+++ b/include/hw/rx/rx62n.h
@@ -37,6 +37,10 @@
 #define TYPE_R5F562N7_MCU "r5f562n7-mcu"
 #define TYPE_R5F562N8_MCU "r5f562n8-mcu"
 
+#define EXT_CS_BASE 0x0100
+#define VECTOR_TABLE_BASE   0xff80
+#define RX62N_CFLASH_BASE   0xfff8
+
 #define RX62N_NR_TMR2
 #define RX62N_NR_CMT2
 #define RX62N_NR_SCI6
diff --git a/hw/rx/rx-gdbsim.c b/hw/rx/rx-gdbsim.c
new file mode 100644
index 00..8cd7a438f2
--- /dev/null
+++ b/hw/rx/rx-gdbsim.c
@@ -0,0 +1,196 @@
+/*
+ * RX QEMU GDB simulator
+ *
+ * Copyright (c) 2019 Yoshinori Sato
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "cpu.h"
+#include "hw/hw.h"
+#include "hw/sysbus.h"
+#include "hw/loader.h"
+#include "hw/rx/rx62n.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
+#include "sysemu/device_tree.h"
+#include "hw/boards.h"
+
+/* Same address of GDB integrated simulator */
+#define SDRAM_BASE  EXT_CS_BASE
+
+typedef struct RxGdbSimMachineClass {
+/*< private >*/
+MachineClass parent_class;
+/*< public >*/
+const char *mcu_name;
+uint32_t xtal_freq_hz;
+} RxGdbSimMachineClass;
+
+typedef struct RxGdbSimMachineState {
+/*< private >*/
+MachineState parent_obj;
+/*< public >*/
+RX62NState mcu;
+} RxGdbSimMachineState;
+
+#define TYPE_RX_GDBSIM_MACHINE MACHINE_TYPE_NAME("rx62n-common")
+
+#define RX_GDBSIM_MACHINE(obj) \
+OBJECT_CHECK(RxGdbSimMachineState, (obj), TYPE_RX_GDBSIM_MACHINE)
+
+#define RX_GDBSIM_MACHINE_CLASS(klass) \
+OBJECT_CLASS_CHECK(RxGdbSimMachineClass, (klass), TYPE_RX_GDBSIM_MACHINE)
+#define RX_GDBSIM_MACHINE_GET_CLASS(obj) \
+OBJECT_GET_CLASS(RxGdbSimMachineClass, (obj), TYPE_RX_GDBSIM_MACHINE)
+
+static void rx_load_image(RXCPU *cpu, const char *filename,
+  uint32_t start, uint32_t size)
+{
+static uint32_t extable[32];
+long kernel_size;
+int i;
+
+kernel_size = load_image_targphys(filename, start, size);
+if (kernel_size < 0) {
+fprintf(stderr, "qemu: could not load kernel '%s'\n", filename);
+exit(1);
+}
+cpu->env.pc = start;
+
+/* setup exception trap trampoline */
+/* linux kernel only works little-endian mode */
+for (i = 0; i < ARRAY_SIZE(extable); i++) {
+extable[i] = cpu_to_le32(0x10 + i * 4);
+}
+rom_add_blob_fixed("extable", extable, sizeof(extable), VECTOR_TABLE_BASE);
+}
+
+static void rx_gdbsim_init(MachineState *machine)
+{
+MachineClass *mc = MACHINE_GET_CLASS(machine);
+RxGdbSimMachineState *s = RX_GDBSIM_MACHINE(machine);
+RxGdbSimMachineClass *rxc = RX_GDBSIM_MACHINE_GET_CLASS(machine);
+MemoryRegion *sysmem = get_system_memory();
+const char *kernel_filename = machine->kernel_filename;
+const char *dtb_filename = machine->dtb;
+
+if (machine->ram_size < mc->default_ram_size) {
+error_report("Invalid RAM size, should be more than %" PRIi64 " Bytes",
+ mc->default_ram_size);
+}
+
+/* Allocate memory space */
+memory_region_add_subregion(sysmem, 

[PULL 07/15] hw/timer: RX62N 8-Bit timer (TMR)

[Philippe Mathieu-Daudé]

From: Yoshinori Sato 

renesas_tmr: 8bit timer modules.
This part use many renesas's CPU.
Hardware manual.
https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf

Signed-off-by: Yoshinori Sato 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20200224141923.82118-16-ys...@users.sourceforge.jp>
[PMD: Split from CMT, filled VMStateField for migration]
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/timer/renesas_tmr.h |  55 
 hw/timer/renesas_tmr.c | 477 +
 MAINTAINERS|   2 +
 hw/timer/Kconfig   |   3 +
 hw/timer/Makefile.objs |   1 +
 5 files changed, 538 insertions(+)
 create mode 100644 include/hw/timer/renesas_tmr.h
 create mode 100644 hw/timer/renesas_tmr.c

diff --git a/include/hw/timer/renesas_tmr.h b/include/hw/timer/renesas_tmr.h
new file mode 100644
index 00..cf3baa7a28
--- /dev/null
+++ b/include/hw/timer/renesas_tmr.h
@@ -0,0 +1,55 @@
+/*
+ * Renesas 8bit timer Object
+ *
+ * Copyright (c) 2018 Yoshinori Sato
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_TIMER_RENESAS_TMR_H
+#define HW_TIMER_RENESAS_TMR_H
+
+#include "qemu/timer.h"
+#include "hw/sysbus.h"
+
+#define TYPE_RENESAS_TMR "renesas-tmr"
+#define RTMR(obj) OBJECT_CHECK(RTMRState, (obj), TYPE_RENESAS_TMR)
+
+enum timer_event {
+cmia = 0,
+cmib = 1,
+ovi = 2,
+none = 3,
+TMR_NR_EVENTS = 4
+};
+
+enum {
+TMR_CH = 2,
+TMR_NR_IRQ = 3 * TMR_CH
+};
+
+typedef struct RTMRState {
+/*< private >*/
+SysBusDevice parent_obj;
+/*< public >*/
+
+uint64_t input_freq;
+MemoryRegion memory;
+
+int64_t tick;
+uint8_t tcnt[TMR_CH];
+uint8_t tcora[TMR_CH];
+uint8_t tcorb[TMR_CH];
+uint8_t tcr[TMR_CH];
+uint8_t tccr[TMR_CH];
+uint8_t tcor[TMR_CH];
+uint8_t tcsr[TMR_CH];
+int64_t div_round[TMR_CH];
+uint8_t next[TMR_CH];
+qemu_irq cmia[TMR_CH];
+qemu_irq cmib[TMR_CH];
+qemu_irq ovi[TMR_CH];
+QEMUTimer timer[TMR_CH];
+} RTMRState;
+
+#endif
diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c
new file mode 100644
index 00..446f2eacdd
--- /dev/null
+++ b/hw/timer/renesas_tmr.c
@@ -0,0 +1,477 @@
+/*
+ * Renesas 8bit timer
+ *
+ * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
+ *(Rev.1.40 R01UH0033EJ0140)
+ *
+ * Copyright (c) 2019 Yoshinori Sato
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/irq.h"
+#include "hw/registerfields.h"
+#include "hw/qdev-properties.h"
+#include "hw/timer/renesas_tmr.h"
+#include "migration/vmstate.h"
+
+REG8(TCR, 0)
+  FIELD(TCR, CCLR,  3, 2)
+  FIELD(TCR, OVIE,  5, 1)
+  FIELD(TCR, CMIEA, 6, 1)
+  FIELD(TCR, CMIEB, 7, 1)
+REG8(TCSR, 2)
+  FIELD(TCSR, OSA,  0, 2)
+  FIELD(TCSR, OSB,  2, 2)
+  FIELD(TCSR, ADTE, 4, 2)
+REG8(TCORA, 4)
+REG8(TCORB, 6)
+REG8(TCNT, 8)
+REG8(TCCR, 10)
+  FIELD(TCCR, CKS,   0, 3)
+  FIELD(TCCR, CSS,   3, 2)
+  FIELD(TCCR, TMRIS, 7, 1)
+
+#define INTERNAL  0x01
+#define CASCADING 0x03
+#define CCLR_A0x01
+#define CCLR_B0x02
+
+static const int clkdiv[] = {0, 1, 2, 8, 32, 64, 1024, 8192};
+
+static uint8_t concat_reg(uint8_t *reg)
+{
+return (reg[0] << 8) | reg[1];
+}
+
+static void update_events(RTMRState *tmr, int ch)
+{
+uint16_t diff[TMR_NR_EVENTS], min;
+int64_t next_time;
+int i, event;
+
+if (tmr->tccr[ch] == 0) {
+return ;
+}
+if (FIELD_EX8(tmr->tccr[ch], TCCR, CSS) == 0) {
+/* external clock mode */
+/* event not happened */
+return ;
+}
+if (FIELD_EX8(tmr->tccr[0], TCCR, CSS) == CASCADING) {
+/* cascading mode */
+if (ch == 1) {
+tmr->next[ch] = none;
+return ;
+}
+diff[cmia] = concat_reg(tmr->tcora) - concat_reg(tmr->tcnt);
+diff[cmib] = concat_reg(tmr->tcorb) - concat_reg(tmr->tcnt);
+diff[ovi] = 0x1 - concat_reg(tmr->tcnt);
+} else {
+/* separate mode */
+diff[cmia] = tmr->tcora[ch] - tmr->tcnt[ch];
+diff[cmib] = tmr->tcorb[ch] - tmr->tcnt[ch];
+diff[ovi] = 0x100 - tmr->tcnt[ch];
+}
+/* Search for the most recently occurring event. */
+   

[PULL 10/15] hw/rx: RX62N microcontroller (MCU)

[Philippe Mathieu-Daudé]

From: Yoshinori Sato 

rx62n - RX62N cpu.

Signed-off-by: Yoshinori Sato 
Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
[PMD: Use TYPE_RX62N_CPU, use #define for RX62N_NR_TMR/CMT/SCI,
 renamed CPU -> MCU, device -> microcontroller]
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20200224141923.82118-18-ys...@users.sourceforge.jp>
[PMD: Rebased on b77b5b3dc7, split of machine, use _abort]
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/rx/rx62n.h |  75 +
 hw/rx/rx62n.c | 254 ++
 MAINTAINERS   |   2 +
 hw/Kconfig|   1 +
 hw/rx/Kconfig |   6 +
 hw/rx/Makefile.objs   |   1 +
 6 files changed, 339 insertions(+)
 create mode 100644 include/hw/rx/rx62n.h
 create mode 100644 hw/rx/rx62n.c
 create mode 100644 hw/rx/Kconfig
 create mode 100644 hw/rx/Makefile.objs

diff --git a/include/hw/rx/rx62n.h b/include/hw/rx/rx62n.h
new file mode 100644
index 00..7c6023bcd6
--- /dev/null
+++ b/include/hw/rx/rx62n.h
@@ -0,0 +1,75 @@
+/*
+ * RX62N MCU Object
+ *
+ * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
+ *(Rev.1.40 R01UH0033EJ0140)
+ *
+ * Copyright (c) 2019 Yoshinori Sato
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#ifndef HW_RX_RX62N_MCU_H
+#define HW_RX_RX62N_MCU_H
+
+#include "target/rx/cpu.h"
+#include "hw/intc/rx_icu.h"
+#include "hw/timer/renesas_tmr.h"
+#include "hw/timer/renesas_cmt.h"
+#include "hw/char/renesas_sci.h"
+#include "qemu/units.h"
+
+#define TYPE_RX62N_MCU "rx62n-mcu"
+#define RX62N_MCU(obj) OBJECT_CHECK(RX62NState, (obj), TYPE_RX62N_MCU)
+
+#define RX62N_NR_TMR2
+#define RX62N_NR_CMT2
+#define RX62N_NR_SCI6
+
+typedef struct RX62NState {
+/*< private >*/
+DeviceState parent_obj;
+/*< public >*/
+
+RXCPU cpu;
+RXICUState icu;
+RTMRState tmr[RX62N_NR_TMR];
+RCMTState cmt[RX62N_NR_CMT];
+RSCIState sci[RX62N_NR_SCI];
+
+MemoryRegion *sysmem;
+bool kernel;
+
+MemoryRegion iram;
+MemoryRegion iomem1;
+MemoryRegion d_flash;
+MemoryRegion iomem2;
+MemoryRegion iomem3;
+MemoryRegion c_flash;
+qemu_irq irq[NR_IRQS];
+} RX62NState;
+
+/*
+ * RX62N Internal Memory
+ * It is the value of R5F562N8.
+ * Please change the size for R5F562N7.
+ */
+#define RX62N_IRAM_SIZE (96 * KiB)
+#define RX62N_DFLASH_SIZE (32 * KiB)
+#define RX62N_CFLASH_SIZE (512 * KiB)
+
+#define RX62N_PCLK (48 * 1000 * 1000)
+
+#endif
diff --git a/hw/rx/rx62n.c b/hw/rx/rx62n.c
new file mode 100644
index 00..85b7770023
--- /dev/null
+++ b/hw/rx/rx62n.c
@@ -0,0 +1,254 @@
+/*
+ * RX62N Microcontroller
+ *
+ * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
+ * (Rev.1.40 R01UH0033EJ0140)
+ *
+ * Copyright (c) 2019 Yoshinori Sato
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/hw.h"
+#include "hw/rx/rx62n.h"
+#include "hw/loader.h"
+#include "hw/sysbus.h"
+#include "hw/qdev-properties.h"
+#include "sysemu/sysemu.h"
+#include "cpu.h"
+
+/*
+ * RX62N Internal Memory
+ */
+#define RX62N_IRAM_BASE 0x
+#define RX62N_DFLASH_BASE   0x0010
+#define RX62N_CFLASH_BASE   0xfff8
+
+/*
+ * RX62N Peripheral Address
+ * See users manual section 5
+ */
+#define RX62N_ICU_BASE  0x00087000
+#define RX62N_TMR_BASE  0x00088200
+#define RX62N_CMT_BASE  0x00088000
+#define RX62N_SCI_BASE  0x00088240
+
+/*
+ * RX62N Peripheral IRQ
+ * See users manual section 11
+ */
+#define RX62N_TMR_IRQ   174
+#define RX62N_CMT_IRQ   28
+#define RX62N_SCI_IRQ   214
+
+/*
+ * IRQ -> IPR mapping table
+ * 0x00 - 0x91: IPR no (IPR00 to IPR91)
+ * 0xff: IPR not assigned
+ * See "11.3.1 Interrupt Vector Table" in hardware manual.
+ 

[PULL 02/15] MAINTAINERS: Add an entry for common Renesas peripherals

[Philippe Mathieu-Daudé]

Renesas peripherals are common to SH4/RX based MCUs. Their
datasheets share common sections. It makes sense to maintain
them altogether.
Add the uncovered UART SCI peripheral.
The current names are misleading (see the 'sh_' prefix).
In another series we will remove these peripherals with
the 'renesas_' prefix. Out of the scope of this change in
MAINTAINERS.

Cc: Magnus Damm 
Cc: Yoshinori Sato 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
 MAINTAINERS | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 67c495e841..f1ae0775f4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1260,7 +1260,6 @@ R: Magnus Damm 
 S: Maintained
 F: hw/sh4/r2d.c
 F: hw/intc/sh_intc.c
-F: hw/timer/sh_timer.c
 F: include/hw/sh4/sh_intc.h
 
 Shix
@@ -1964,6 +1963,14 @@ F: hw/*/*xive*
 F: include/hw/*/*xive*
 F: docs/*/*xive*
 
+Renesas peripherals
+M: Yoshinori Sato 
+R: Magnus Damm 
+S: Maintained
+F: hw/char/sh_serial.c
+F: hw/timer/sh_timer.c
+F: include/hw/sh4/sh.h
+
 Subsystems
 --
 Audio
-- 
2.21.3

[PULL 05/15] hw/timer/sh_timer: Remove unused 'qemu/timer.h' include

[Philippe Mathieu-Daudé]

Remove unused "qemu/timer.h" include.

Reviewed-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
 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

[PULL 06/15] hw/intc: RX62N interrupt controller (ICUa)

[Philippe Mathieu-Daudé]

From: Yoshinori Sato 

This implementation supported only ICUa.
Hardware manual.
https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf

Signed-off-by: Yoshinori Sato 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20200224141923.82118-15-ys...@users.sourceforge.jp>
[PMD: Fill VMStateField for migration, cover files in MAINTAINERS]
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/intc/rx_icu.h |  76 
 hw/intc/rx_icu.c | 397 +++
 MAINTAINERS  |   6 +
 hw/intc/Kconfig  |   3 +
 hw/intc/Makefile.objs|   1 +
 5 files changed, 483 insertions(+)
 create mode 100644 include/hw/intc/rx_icu.h
 create mode 100644 hw/intc/rx_icu.c

diff --git a/include/hw/intc/rx_icu.h b/include/hw/intc/rx_icu.h
new file mode 100644
index 00..7176015cd9
--- /dev/null
+++ b/include/hw/intc/rx_icu.h
@@ -0,0 +1,76 @@
+/*
+ * RX Interrupt Control Unit
+ *
+ * Copyright (c) 2019 Yoshinori Sato
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#ifndef HW_INTC_RX_ICU_H
+#define HW_INTC_RX_ICU_H
+
+#include "hw/sysbus.h"
+
+enum TRG_MODE {
+TRG_LEVEL = 0,
+TRG_NEDGE = 1,  /* Falling */
+TRG_PEDGE = 2,  /* Raising */
+TRG_BEDGE = 3,  /* Both */
+};
+
+struct IRQSource {
+enum TRG_MODE sense;
+int level;
+};
+
+enum {
+/* Software interrupt request */
+SWI = 27,
+NR_IRQS = 256
+};
+
+struct RXICUState {
+/*< private >*/
+SysBusDevice parent_obj;
+/*< public >*/
+
+MemoryRegion memory;
+struct IRQSource src[NR_IRQS];
+uint32_t nr_irqs;
+uint8_t *map;
+uint32_t nr_sense;
+uint8_t *init_sense;
+
+uint8_t ir[NR_IRQS];
+uint8_t dtcer[NR_IRQS];
+uint8_t ier[NR_IRQS / 8];
+uint8_t ipr[142];
+uint8_t dmasr[4];
+uint16_t fir;
+uint8_t nmisr;
+uint8_t nmier;
+uint8_t nmiclr;
+uint8_t nmicr;
+int16_t req_irq;
+qemu_irq _irq;
+qemu_irq _fir;
+qemu_irq _swi;
+};
+typedef struct RXICUState RXICUState;
+
+#define TYPE_RX_ICU "rx-icu"
+#define RX_ICU(obj) OBJECT_CHECK(RXICUState, (obj), TYPE_RX_ICU)
+
+#endif /* RX_ICU_H */
diff --git a/hw/intc/rx_icu.c b/hw/intc/rx_icu.c
new file mode 100644
index 00..df4b6a8d22
--- /dev/null
+++ b/hw/intc/rx_icu.c
@@ -0,0 +1,397 @@
+/*
+ * RX Interrupt Control Unit
+ *
+ * Warning: Only ICUa is supported.
+ *
+ * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
+ *(Rev.1.40 R01UH0033EJ0140)
+ *
+ * Copyright (c) 2019 Yoshinori Sato
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "hw/irq.h"
+#include "hw/registerfields.h"
+#include "hw/qdev-properties.h"
+#include "hw/intc/rx_icu.h"
+#include "migration/vmstate.h"
+
+REG8(IR, 0)
+  FIELD(IR, IR,  0, 1)
+REG8(DTCER, 0x100)
+  FIELD(DTCER, DTCE,  0, 1)
+REG8(IER, 0x200)
+REG8(SWINTR, 0x2e0)
+  FIELD(SWINTR, SWINT, 0, 1)
+REG16(FIR, 0x2f0)
+  FIELD(FIR, FVCT, 0,  8)
+  FIELD(FIR, FIEN, 15, 1)
+REG8(IPR, 0x300)
+  FIELD(IPR, IPR, 0, 4)
+REG8(DMRSR, 0x400)
+REG8(IRQCR, 0x500)
+  FIELD(IRQCR, IRQMD, 2, 2)
+REG8(NMISR, 0x580)
+  FIELD(NMISR, NMIST, 0, 1)
+  FIELD(NMISR, LVDST, 1, 1)
+  FIELD(NMISR, OSTST, 2, 1)
+REG8(NMIER, 0x581)
+  FIELD(NMIER, NMIEN, 0, 1)
+  FIELD(NMIER, LVDEN, 1, 1)
+  FIELD(NMIER, OSTEN, 2, 1)
+REG8(NMICLR, 0x582)
+  FIELD(NMICLR, NMICLR, 0, 1)
+  FIELD(NMICLR, OSTCLR, 2, 1)
+REG8(NMICR, 0x583)
+  FIELD(NMICR, NMIMD, 3, 1)
+
+static void set_irq(RXICUState *icu, int n_IRQ, int req)
+{
+if ((icu->fir & R_FIR_FIEN_MASK) &&
+(icu->fir & 

[PULL 09/15] hw/char: RX62N serial communication interface (SCI)

[Philippe Mathieu-Daudé]

From: Yoshinori Sato 

This module supported only non FIFO type.
Hardware manual.
https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf

Signed-off-by: Yoshinori Sato 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20200224141923.82118-17-ys...@users.sourceforge.jp>
[PMD: Filled VMStateField for migration]
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/char/renesas_sci.h |  51 +
 hw/char/renesas_sci.c | 350 ++
 MAINTAINERS   |   2 +
 hw/char/Kconfig   |   3 +
 hw/char/Makefile.objs |   1 +
 5 files changed, 407 insertions(+)
 create mode 100644 include/hw/char/renesas_sci.h
 create mode 100644 hw/char/renesas_sci.c

diff --git a/include/hw/char/renesas_sci.h b/include/hw/char/renesas_sci.h
new file mode 100644
index 00..efdebc620a
--- /dev/null
+++ b/include/hw/char/renesas_sci.h
@@ -0,0 +1,51 @@
+/*
+ * Renesas Serial Communication Interface
+ *
+ * Copyright (c) 2018 Yoshinori Sato
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_CHAR_RENESAS_SCI_H
+#define HW_CHAR_RENESAS_SCI_H
+
+#include "chardev/char-fe.h"
+#include "hw/sysbus.h"
+
+#define TYPE_RENESAS_SCI "renesas-sci"
+#define RSCI(obj) OBJECT_CHECK(RSCIState, (obj), TYPE_RENESAS_SCI)
+
+enum {
+ERI = 0,
+RXI = 1,
+TXI = 2,
+TEI = 3,
+SCI_NR_IRQ = 4
+};
+
+typedef struct {
+/*< private >*/
+SysBusDevice parent_obj;
+/*< public >*/
+
+MemoryRegion memory;
+QEMUTimer timer;
+CharBackend chr;
+qemu_irq irq[SCI_NR_IRQ];
+
+uint8_t smr;
+uint8_t brr;
+uint8_t scr;
+uint8_t tdr;
+uint8_t ssr;
+uint8_t rdr;
+uint8_t scmr;
+uint8_t semr;
+
+uint8_t read_ssr;
+int64_t trtime;
+int64_t rx_next;
+uint64_t input_freq;
+} RSCIState;
+
+#endif
diff --git a/hw/char/renesas_sci.c b/hw/char/renesas_sci.c
new file mode 100644
index 00..5d7c6e6523
--- /dev/null
+++ b/hw/char/renesas_sci.c
@@ -0,0 +1,350 @@
+/*
+ * Renesas Serial Communication Interface
+ *
+ * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
+ *(Rev.1.40 R01UH0033EJ0140)
+ *
+ * Copyright (c) 2019 Yoshinori Sato
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/irq.h"
+#include "hw/registerfields.h"
+#include "hw/qdev-properties.h"
+#include "hw/char/renesas_sci.h"
+#include "migration/vmstate.h"
+
+/* SCI register map */
+REG8(SMR, 0)
+  FIELD(SMR, CKS,  0, 2)
+  FIELD(SMR, MP,   2, 1)
+  FIELD(SMR, STOP, 3, 1)
+  FIELD(SMR, PM,   4, 1)
+  FIELD(SMR, PE,   5, 1)
+  FIELD(SMR, CHR,  6, 1)
+  FIELD(SMR, CM,   7, 1)
+REG8(BRR, 1)
+REG8(SCR, 2)
+  FIELD(SCR, CKE,  0, 2)
+  FIELD(SCR, TEIE, 2, 1)
+  FIELD(SCR, MPIE, 3, 1)
+  FIELD(SCR, RE,   4, 1)
+  FIELD(SCR, TE,   5, 1)
+  FIELD(SCR, RIE,  6, 1)
+  FIELD(SCR, TIE,  7, 1)
+REG8(TDR, 3)
+REG8(SSR, 4)
+  FIELD(SSR, MPBT, 0, 1)
+  FIELD(SSR, MPB,  1, 1)
+  FIELD(SSR, TEND, 2, 1)
+  FIELD(SSR, ERR,  3, 3)
+FIELD(SSR, PER,  3, 1)
+FIELD(SSR, FER,  4, 1)
+FIELD(SSR, ORER, 5, 1)
+  FIELD(SSR, RDRF, 6, 1)
+  FIELD(SSR, TDRE, 7, 1)
+REG8(RDR, 5)
+REG8(SCMR, 6)
+  FIELD(SCMR, SMIF, 0, 1)
+  FIELD(SCMR, SINV, 2, 1)
+  FIELD(SCMR, SDIR, 3, 1)
+  FIELD(SCMR, BCP2, 7, 1)
+REG8(SEMR, 7)
+  FIELD(SEMR, ACS0, 0, 1)
+  FIELD(SEMR, ABCS, 4, 1)
+
+static int can_receive(void *opaque)
+{
+RSCIState *sci = RSCI(opaque);
+if (sci->rx_next > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
+return 0;
+} else {
+return FIELD_EX8(sci->scr, SCR, RE);
+}
+}
+
+static void receive(void *opaque, const uint8_t *buf, int size)
+{
+RSCIState *sci = RSCI(opaque);
+sci->rx_next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + sci->trtime;
+if (FIELD_EX8(sci->ssr, SSR, RDRF) || size > 1) {
+sci->ssr = FIELD_DP8(sci->ssr, SSR, ORER, 1);
+if (FIELD_EX8(sci->scr, SCR, RIE)) {
+qemu_set_irq(sci->irq[ERI], 1);
+}
+} else {
+sci->rdr = buf[0];
+sci->ssr = FIELD_DP8(sci->ssr, SSR, RDRF, 1);
+if (FIELD_EX8(sci->scr, SCR, RIE)) {
+qemu_irq_pulse(sci->irq[RXI]);
+}
+}
+}
+
+static void send_byte(RSCIState *sci)
+{
+if 

[PULL 00/15] Renesas hardware patches for 2020-06-21

[Philippe Mathieu-Daudé]

The following changes since commit 06c4cc3660b366278bdc7bc8b6677032d7b1118c:

  qht: Fix threshold rate calculation (2020-06-19 18:29:11 +0100)

are available in the Git repository at:

  https://gitlab.com/philmd/qemu.git tags/renesas-hw-20200621

for you to fetch changes up to 730101266e4026fc19808c740ee4b8118eeaaafe:

  docs: Document the RX target (2020-06-21 01:21:47 +0200)


Renesas hardware patches

- Add a common entry for Renesas hardware in MAINTAINERS
- Trivial SH4 cleanups
- Add RX GDB simulator from Yoshinori Sato

The Renesas RX target emulation was added in commit c8c35e5f51,
these patches complete the target by adding the hardware emulation.

Thank you Yoshinori for adding this code to QEMU, and your patience
during the review process. Now your port is fully integrated.

Travis-CI:
https://travis-ci.org/github/philmd/qemu/builds/700461815


Philippe Mathieu-Daud=C3=A9 (7):
  MAINTAINERS: Cover sh_intc files in the R2D/Shix machine sections
  MAINTAINERS: Add an entry for common Renesas peripherals
  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
  hw/rx: Register R5F562N7 and R5F562N8 MCUs
  BootLinuxConsoleTest: Test the RX GDB simulator

Richard Henderson (1):
  hw/rx: Honor -accel qtest

Yoshinori Sato (7):
  hw/intc: RX62N interrupt controller (ICUa)
  hw/timer: RX62N 8-Bit timer (TMR)
  hw/timer: RX62N compare match timer (CMT)
  hw/char: RX62N serial communication interface (SCI)
  hw/rx: RX62N microcontroller (MCU)
  hw/rx: Add RX GDB simulator
  docs: Document the RX target

 docs/system/target-rx.rst |  36 ++
 docs/system/targets.rst   |   1 +
 default-configs/rx-softmmu.mak|   1 +
 include/hw/char/renesas_sci.h |  51 +++
 include/hw/intc/rx_icu.h  |  76 
 include/hw/rx/rx62n.h |  76 
 include/hw/sh4/sh.h   |  12 +-
 include/hw/timer/renesas_cmt.h|  40 +++
 include/hw/timer/renesas_tmr.h|  55 +++
 include/hw/timer/tmu012.h |  23 ++
 hw/char/renesas_sci.c | 350 +++
 hw/intc/rx_icu.c  | 397 +
 hw/rx/rx-gdbsim.c | 196 +++
 hw/rx/rx62n.c | 323 +
 hw/sh4/sh7750.c   |   1 +
 hw/timer/renesas_cmt.c| 283 +++
 hw/timer/renesas_tmr.c| 477 ++
 hw/timer/sh_timer.c   |   3 +-
 MAINTAINERS   |  33 +-
 hw/Kconfig|   1 +
 hw/char/Kconfig   |   3 +
 hw/char/Makefile.objs |   1 +
 hw/intc/Kconfig   |   3 +
 hw/intc/Makefile.objs |   1 +
 hw/rx/Kconfig |  10 +
 hw/rx/Makefile.objs   |   2 +
 hw/timer/Kconfig  |   6 +
 hw/timer/Makefile.objs|   2 +
 tests/acceptance/machine_rx_gdbsim.py |  68 
 29 files changed, 2518 insertions(+), 13 deletions(-)
 create mode 100644 docs/system/target-rx.rst
 create mode 100644 include/hw/char/renesas_sci.h
 create mode 100644 include/hw/intc/rx_icu.h
 create mode 100644 include/hw/rx/rx62n.h
 create mode 100644 include/hw/timer/renesas_cmt.h
 create mode 100644 include/hw/timer/renesas_tmr.h
 create mode 100644 include/hw/timer/tmu012.h
 create mode 100644 hw/char/renesas_sci.c
 create mode 100644 hw/intc/rx_icu.c
 create mode 100644 hw/rx/rx-gdbsim.c
 create mode 100644 hw/rx/rx62n.c
 create mode 100644 hw/timer/renesas_cmt.c
 create mode 100644 hw/timer/renesas_tmr.c
 create mode 100644 hw/rx/Kconfig
 create mode 100644 hw/rx/Makefile.objs
 create mode 100644 tests/acceptance/machine_rx_gdbsim.py

--=20
2.21.3

[PULL 03/15] hw/sh4: Use MemoryRegion typedef

[Philippe Mathieu-Daudé]

Use the MemoryRegion type defined in "qemu/typedefs.h",
to keep the repository style consistent.

Reviewed-by: Richard Henderson 
Reviewed-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
---
 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

[PULL 01/15] MAINTAINERS: Cover sh_intc files in the R2D/Shix machine sections

[Philippe Mathieu-Daudé]

Commit 81527b94ad added hw/intc/sh_intc.c, but only to the R2D
machine (it is also used by the Shix machine). Complete the
previous commit by adding the header to the R2D section, and
both source + header to the Shix section.

Suggested-by: Thomas Huth 
Reviewed-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 955cc8dd5c..67c495e841 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1261,12 +1261,15 @@ S: Maintained
 F: hw/sh4/r2d.c
 F: hw/intc/sh_intc.c
 F: hw/timer/sh_timer.c
+F: include/hw/sh4/sh_intc.h
 
 Shix
 M: Yoshinori Sato 
 R: Magnus Damm 
 S: Odd Fixes
 F: hw/sh4/shix.c
+F: hw/intc/sh_intc.c
+F: include/hw/sh4/sh_intc.h
 
 SPARC Machines
 --
-- 
2.21.3

[PULL 04/15] hw/sh4: Extract timer definitions to 'hw/timer/tmu012.h'

[Philippe Mathieu-Daudé]

Extract timer definitions to 'hw/timer/tmu012.h'.

Reviewed-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
 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

Re: [PATCH] .travis.yml: Build acceptance tests with -O2 compiler optimization

[Alex Bennée]

Philippe Mathieu-Daudé  writes:

> On 6/21/20 1:29 AM, Philippe Mathieu-Daudé wrote:
>> As we just want the tests to succeed, build them with compiler
>> optimizations enabled to run the tests faster.
>
> Maybe it is a good opportunity to test -O3 instead...
> Since this configuration is not covered.

Don't know if -O3 is worth it - even Gentoo developers warn against
cranking it up too much.

In fact I'm surprised we don't build -O2 by default.

>
>> 
>> 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 74158f741b..61b247db9f 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,sh4-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
>> +- CONFIG="--extra-cflags=-O2 --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:
>> 


-- 
Alex Bennée

Re: [PATCH] chardev/tcp: fix error message double free error

[Marc-André Lureau]

Hi

On Sun, Jun 21, 2020 at 10:54 AM lichun  wrote:
>
> Signed-off-by: lichun 
> ---
>  chardev/char-socket.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index afebeec5c3..3b6c1c5848 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -1086,7 +1086,10 @@ static void qemu_chr_socket_connected(QIOTask *task, 
> void *opaque)
>  if (qio_task_propagate_error(task, )) {
>  tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
>  check_report_connect_error(chr, err);
> -error_free(err);
> +/* If connect_err_reported is true, it means err is already freed */
> +if (!s->connect_err_reported) {
> +error_free(err);
> +}

Good catch (did you find it with a static analysis tool?).

Instead of checking connect_err_reported here, I would rather let
check_report_connect_error() handle error_free(). Can you update the
patch?

thanks

>  goto cleanup;
>  }
>
> --
> 2.18.4
>

Re: [PATCH] .travis.yml: Build acceptance tests with -O2 compiler optimization

[Philippe Mathieu-Daudé]

On 6/21/20 1:29 AM, Philippe Mathieu-Daudé wrote:
> As we just want the tests to succeed, build them with compiler
> optimizations enabled to run the tests faster.

Maybe it is a good opportunity to test -O3 instead...
Since this configuration is not covered.

> 
> 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 74158f741b..61b247db9f 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,sh4-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
> +- CONFIG="--extra-cflags=-O2 --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:
> 

Re: [PATCH v3 09/11] tests/acceptance: record/replay tests with advcal images

[Philippe Mathieu-Daudé]

Hi Pavel,

On 5/29/20 9:05 AM, Pavel Dovgalyuk wrote:
> This patch adds more record/replay tests with kernel images.
> 
> Signed-off-by: Pavel Dovgalyuk 
> 
> --
> 
> v2:
>  - make download path fixed to allow pre-test downloading (suggested by 
> Willian Rampazzo)
> ---
>  0 files changed
> 
> diff --git a/tests/acceptance/replay_kernel.py 
> b/tests/acceptance/replay_kernel.py
> index c1ec002db6..bc21ddf082 100644
> --- a/tests/acceptance/replay_kernel.py
> +++ b/tests/acceptance/replay_kernel.py
> @@ -186,3 +186,108 @@ class ReplayKernel(LinuxKernelTest):
> 'console=ttyS0 vga=off')
>  console_pattern = 'No filesystem could mount root'
>  self.run_rr(kernel_path, kernel_command_line, console_pattern)
> +
> +def do_test_advcal_2018(self, file_path, kernel_name, args=None):
> +archive.extract(file_path, self.workdir)
> +
> +for entry in os.scandir(self.workdir):
> +if entry.name.startswith('day') and entry.is_dir():
> +kernel_path = entry.path + '/' + kernel_name
> +break
> +
> +kernel_command_line = ''
> +console_pattern = 'QEMU advent calendar'
> +self.run_rr(kernel_path, kernel_command_line, console_pattern,
> +args=args)
> +
> +def test_arm_vexpressa9(self):
> +"""
> +:avocado: tags=arch:arm
> +:avocado: tags=machine:vexpress-a9
> +"""
> +tar_hash = '32b7677ce8b6f1471fb0059865f451169934245b'
> +tar_url = ('https://www.qemu-advent-calendar.org'
> +   '/2018/download/day16.tar.xz')
> +file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
> +self.do_test_advcal_2018(file_path, 'winter.zImage',
> +('-dtb', self.workdir + '/day16/vexpress-v2p-ca9.dtb'))
> +
> +def test_m68k_mcf5208evb(self):
> +"""
> +:avocado: tags=arch:m68k
> +:avocado: tags=machine:mcf5208evb
> +"""
> +tar_hash = 'ac688fd00561a2b6ce1359f9ff6aa2b98c9a570c'
> +tar_url = ('https://www.qemu-advent-calendar.org'
> +   '/2018/download/day07.tar.xz')
> +file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
> +self.do_test_advcal_2018(file_path, 'sanity-clause.elf')
> +
> +def test_microblaze_s3adsp1800(self):
> +"""
> +:avocado: tags=arch:microblaze
> +:avocado: tags=machine:petalogix-s3adsp1800
> +"""
> +tar_hash = '08bf3e3bfb6b6c7ce1e54ab65d54e189f2caf13f'
> +tar_url = ('https://www.qemu-advent-calendar.org'
> +   '/2018/download/day17.tar.xz')
> +file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
> +self.do_test_advcal_2018(file_path, 'ballerina.bin')
> +
> +def test_ppc64_e500(self):
> +"""
> +:avocado: tags=arch:ppc64
> +:avocado: tags=machine:ppce500
> +"""
> +tar_hash = '6951d86d644b302898da2fd701739c9406527fe1'
> +tar_url = ('https://www.qemu-advent-calendar.org'
> +   '/2018/download/day19.tar.xz')
> +file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
> +self.do_test_advcal_2018(file_path, 'uImage', ('-cpu', 'e5500'))
> +
> +def test_ppc_g3beige(self):
> +"""
> +:avocado: tags=arch:ppc
> +:avocado: tags=machine:g3beige
> +"""
> +tar_hash = 'e0b872a5eb8fdc5bed19bd43ffe863900ebcedfc'
> +tar_url = ('https://www.qemu-advent-calendar.org'
> +   '/2018/download/day15.tar.xz')
> +file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
> +self.do_test_advcal_2018(file_path, 'invaders.elf',
> +('-M', 'graphics=off'))
> +
> +def test_ppc_mac99(self):
> +"""
> +:avocado: tags=arch:ppc
> +:avocado: tags=machine:mac99
> +"""
> +tar_hash = 'e0b872a5eb8fdc5bed19bd43ffe863900ebcedfc'
> +tar_url = ('https://www.qemu-advent-calendar.org'
> +   '/2018/download/day15.tar.xz')
> +file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
> +self.do_test_advcal_2018(file_path, 'invaders.elf',
> +('-M', 'graphics=off'))

Using QEMU built with -O3, I get:

 (4/4) tests/acceptance/replay_kernel.py:ReplayKernel.test_ppc_mac99:
replay: recording the execution...
replay: finished the recording with log size 21781169 bytes
replay: elapsed time 17.03 sec
replay: replaying the execution...
replay: successfully finished the replay
replay: elapsed time 57.04 sec
replay: replay overhead 234.93%
PASS (74.48 s)

Any idea why there is so much overhead here?

> +
> +def test_sparc_ss20(self):
> +"""
> +:avocado: tags=arch:sparc
> +:avocado: tags=machine:SS-20
> +"""
> +tar_hash = 'b18550d5d61c7615d989a06edace051017726a9f'
> +tar_url = ('https://www.qemu-advent-calendar.org'
> +   

[Bug 1884425] [NEW] MIPS64EL emu hangs at reboot

[Seal Sealy]

Public bug reported:

QEMU Release version: 5.0.50 (v5.0.0-1411-g26bf4a2921-dirty)

Full command line: qemu-system-mips64el -hda nt4svr.qcow2 -M magnum -L .
-global ds1225y.filename=nvram  -global ds1225y.size=8200 -net nic -net
user -cdrom en_winnt_4.0_svr.iso

Host machine: Windows 10 1909 64-bit, QEMU running under WSL with the
latest Kali distro and the latest Xming.

Guest machine: MIPS64EL Magnum machine, no OS needs to be installed to
reproduce - just change some stuff in the Setup program and try to exit

Note: Custom ROM with Windows NT support used, NTPROM.RAW used from
http://hpoussineau.free.fr/qemu/firmware/magnum-4000/setup.zip

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  MIPS64EL emu hangs at reboot

Status in QEMU:
  New

Bug description:
  QEMU Release version: 5.0.50 (v5.0.0-1411-g26bf4a2921-dirty)

  Full command line: qemu-system-mips64el -hda nt4svr.qcow2 -M magnum -L
  . -global ds1225y.filename=nvram  -global ds1225y.size=8200 -net nic
  -net user -cdrom en_winnt_4.0_svr.iso

  Host machine: Windows 10 1909 64-bit, QEMU running under WSL with the
  latest Kali distro and the latest Xming.

  Guest machine: MIPS64EL Magnum machine, no OS needs to be installed to
  reproduce - just change some stuff in the Setup program and try to
  exit

  Note: Custom ROM with Windows NT support used, NTPROM.RAW used from
  http://hpoussineau.free.fr/qemu/firmware/magnum-4000/setup.zip

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

Re: Is roms/vgabios/config.mak still needed?

[BALATON Zoltan]

On Wed, 17 Jun 2020, Philippe Mathieu-Daudé wrote:

On 6/17/20 8:08 PM, BALATON Zoltan wrote:

Hello,

I've noticed that configure creates roms/vgabios/config.mak but commit
91b8eba9ec3f5af7dd48927811eb7ff69fc4617f seems to have removed vgabios
so should this be dropped from configure now as well? If it's still
needed it should be added to .gitignore.


You are right, it is a left-over from 91b8eba9ec3f and should be dropped
from configure.


Will you or Gerd do something about it? I'd rather not touch configure 
without completely understading it.


Regards,
BALATON Zoltan

[PATCH 2/3] ati-vga: Do not assert on error

[BALATON Zoltan]

Do not abort on unsupported value just print log and continue. While
display will likely be broken this prevents malicious guest to crash
QEMU causing denial of service.

Signed-off-by: BALATON Zoltan 
---
 hw/display/ati.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index 21ae36c535..42755cffbb 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -86,8 +86,8 @@ static void ati_vga_switch_mode(ATIVGAState *s)
 break;
 default:
 qemu_log_mask(LOG_UNIMP, "Unsupported bpp value\n");
+return;
 }
-assert(bpp != 0);
 DPRINTF("Switching to %dx%d %d %d @ %x\n", h, v, stride, bpp, 
offs);
 vbe_ioport_write_index(>vga, 0, VBE_DISPI_INDEX_ENABLE);
 vbe_ioport_write_data(>vga, 0, VBE_DISPI_DISABLED);
-- 
2.21.3

[PATCH 0/3] Misc ati-vga fixes

[BALATON Zoltan]

These are some patches I had lying around in my tree, maybe it's time
to merge them.

BALATON Zoltan (3):
  ati-vga: Support unaligned access to hardware cursor registers
  ati-vga: Do not assert on error
  ati-vga: Add dummy MEM_SDRAM_MODE_REG

 hw/display/ati.c  | 92 +--
 hw/display/ati_dbg.c  |  1 +
 hw/display/ati_regs.h |  1 +
 3 files changed, 65 insertions(+), 29 deletions(-)

-- 
2.21.3

[PATCH 1/3] ati-vga: Support unaligned access to hardware cursor registers

[BALATON Zoltan]

This fixes horizontal mouse movement and pointer color with MacOS that
writes these registers with access size less than 4 so previously only
the last portion of access was effective overwriting previous partial
writes.

Signed-off-by: BALATON Zoltan 
---
 hw/display/ati.c | 87 
 1 file changed, 58 insertions(+), 29 deletions(-)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index d45127a976..21ae36c535 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -389,22 +389,28 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, 
unsigned int size)
 case 0xf00 ... 0xfff:
 val = pci_default_read_config(>dev, addr - 0xf00, size);
 break;
-case CUR_OFFSET:
-val = s->regs.cur_offset;
-break;
-case CUR_HORZ_VERT_POSN:
-val = s->regs.cur_hv_pos;
-val |= s->regs.cur_offset & BIT(31);
+case CUR_OFFSET ... CUR_OFFSET + 3:
+val = ati_reg_read_offs(s->regs.cur_offset, addr - CUR_OFFSET, size);
+break;
+case CUR_HORZ_VERT_POSN ... CUR_HORZ_VERT_POSN + 3:
+val = ati_reg_read_offs(s->regs.cur_hv_pos,
+addr - CUR_HORZ_VERT_POSN, size);
+if (addr + size > CUR_HORZ_VERT_POSN + 3) {
+val |= (s->regs.cur_offset & BIT(31)) >> (4 - size);
+}
 break;
-case CUR_HORZ_VERT_OFF:
-val = s->regs.cur_hv_offs;
-val |= s->regs.cur_offset & BIT(31);
+case CUR_HORZ_VERT_OFF ... CUR_HORZ_VERT_OFF + 3:
+val = ati_reg_read_offs(s->regs.cur_hv_offs,
+addr - CUR_HORZ_VERT_OFF, size);
+if (addr + size > CUR_HORZ_VERT_OFF + 3) {
+val |= (s->regs.cur_offset & BIT(31)) >> (4 - size);
+}
 break;
-case CUR_CLR0:
-val = s->regs.cur_color0;
+case CUR_CLR0 ... CUR_CLR0 + 3:
+val = ati_reg_read_offs(s->regs.cur_color0, addr - CUR_CLR0, size);
 break;
-case CUR_CLR1:
-val = s->regs.cur_color1;
+case CUR_CLR1 ... CUR_CLR1 + 3:
+val = ati_reg_read_offs(s->regs.cur_color1, addr - CUR_CLR1, size);
 break;
 case DST_OFFSET:
 val = s->regs.dst_offset;
@@ -693,48 +699,71 @@ static void ati_mm_write(void *opaque, hwaddr addr,
 case 0xf00 ... 0xfff:
 /* read-only copy of PCI config space so ignore writes */
 break;
-case CUR_OFFSET:
-if (s->regs.cur_offset != (data & 0x87f0)) {
-s->regs.cur_offset = data & 0x87f0;
+case CUR_OFFSET ... CUR_OFFSET + 3:
+{
+uint32_t t = s->regs.cur_offset;
+
+ati_reg_write_offs(, addr - CUR_OFFSET, data, size);
+t &= 0x87f0;
+if (s->regs.cur_offset != t) {
+s->regs.cur_offset = t;
 ati_cursor_define(s);
 }
 break;
-case CUR_HORZ_VERT_POSN:
-s->regs.cur_hv_pos = data & 0x3fff0fff;
-if (data & BIT(31)) {
-s->regs.cur_offset |= data & BIT(31);
+}
+case CUR_HORZ_VERT_POSN ... CUR_HORZ_VERT_POSN + 3:
+{
+uint32_t t = s->regs.cur_hv_pos | (s->regs.cur_offset & BIT(31));
+
+ati_reg_write_offs(, addr - CUR_HORZ_VERT_POSN, data, size);
+s->regs.cur_hv_pos = t & 0x3fff0fff;
+if (t & BIT(31)) {
+s->regs.cur_offset |= t & BIT(31);
 } else if (s->regs.cur_offset & BIT(31)) {
 s->regs.cur_offset &= ~BIT(31);
 ati_cursor_define(s);
 }
 if (!s->cursor_guest_mode &&
-(s->regs.crtc_gen_cntl & CRTC2_CUR_EN) && !(data & BIT(31))) {
+(s->regs.crtc_gen_cntl & CRTC2_CUR_EN) && !(t & BIT(31))) {
 dpy_mouse_set(s->vga.con, s->regs.cur_hv_pos >> 16,
   s->regs.cur_hv_pos & 0x, 1);
 }
 break;
+}
 case CUR_HORZ_VERT_OFF:
-s->regs.cur_hv_offs = data & 0x3f003f;
-if (data & BIT(31)) {
-s->regs.cur_offset |= data & BIT(31);
+{
+uint32_t t = s->regs.cur_hv_offs | (s->regs.cur_offset & BIT(31));
+
+ati_reg_write_offs(, addr - CUR_HORZ_VERT_OFF, data, size);
+s->regs.cur_hv_offs = t & 0x3f003f;
+if (t & BIT(31)) {
+s->regs.cur_offset |= t & BIT(31);
 } else if (s->regs.cur_offset & BIT(31)) {
 s->regs.cur_offset &= ~BIT(31);
 ati_cursor_define(s);
 }
 break;
-case CUR_CLR0:
-if (s->regs.cur_color0 != (data & 0xff)) {
-s->regs.cur_color0 = data & 0xff;
+}
+case CUR_CLR0 ... CUR_CLR0 + 3:
+{
+uint32_t t = s->regs.cur_color0;
+
+ati_reg_write_offs(, addr - CUR_CLR0, data, size);
+t &= 0xff;
+if (s->regs.cur_color0 != t) {
+s->regs.cur_color0 = t;
 ati_cursor_define(s);
 }
 break;
-case CUR_CLR1:
+}
+case CUR_CLR1 ... CUR_CLR1 + 3:
 /*
  * Update cursor 

[PATCH 3/3] ati-vga: Add dummy MEM_SDRAM_MODE_REG

[BALATON Zoltan]

Radeon chips have an SDRAM mode reg that is accessed by some drivers.
We don't emulate the memory controller but provide some default value
to prevent drivers getting unexpected 0.

Signed-off-by: BALATON Zoltan 
---
 hw/display/ati.c  | 5 +
 hw/display/ati_dbg.c  | 1 +
 hw/display/ati_regs.h | 1 +
 3 files changed, 7 insertions(+)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index 42755cffbb..944f9f420f 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -361,6 +361,11 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, 
unsigned int size)
 case MC_STATUS:
 val = 5;
 break;
+case MEM_SDRAM_MODE_REG:
+if (s->dev_id != PCI_DEVICE_ID_ATI_RAGE128_PF) {
+val = BIT(28) | BIT(20);
+}
+break;
 case RBBM_STATUS:
 case GUI_STAT:
 val = 64; /* free CMDFIFO entries */
diff --git a/hw/display/ati_dbg.c b/hw/display/ati_dbg.c
index 0ebbd36f14..bd0ecd48c7 100644
--- a/hw/display/ati_dbg.c
+++ b/hw/display/ati_dbg.c
@@ -42,6 +42,7 @@ static struct ati_regdesc ati_reg_names[] = {
 {"MC_FB_LOCATION", 0x0148},
 {"MC_AGP_LOCATION", 0x014C},
 {"MC_STATUS", 0x0150},
+{"MEM_SDRAM_MODE_REG", 0x0158},
 {"MEM_POWER_MISC", 0x015c},
 {"AGP_BASE", 0x0170},
 {"AGP_CNTL", 0x0174},
diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
index ebd37ee30d..d6282b2ef2 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -60,6 +60,7 @@
 #define MC_FB_LOCATION  0x0148
 #define MC_AGP_LOCATION 0x014C
 #define MC_STATUS   0x0150
+#define MEM_SDRAM_MODE_REG  0x0158
 #define MEM_POWER_MISC  0x015c
 #define AGP_BASE0x0170
 #define AGP_CNTL0x0174
-- 
2.21.3

[PATCH] chardev/tcp: fix error message double free error

[lichun]

Signed-off-by: lichun 
---
 chardev/char-socket.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index afebeec5c3..3b6c1c5848 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1086,7 +1086,10 @@ static void qemu_chr_socket_connected(QIOTask *task, 
void *opaque)
 if (qio_task_propagate_error(task, )) {
 tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
 check_report_connect_error(chr, err);
-error_free(err);
+/* If connect_err_reported is true, it means err is already freed */
+if (!s->connect_err_reported) {
+error_free(err);
+}
 goto cleanup;
 }
 
-- 
2.18.4