Re: [Qemu-devel] [Bug 1091115] [NEW] qemu-1.3.0 crashes when installing windows xp

[Konrad Frederic]

On 24/12/2012 10:48, 楼正伟 wrote:

2012/12/17 LazySid:

Bug description:
   These are the commands:
   $git checkout v1.3.0
   $./configure --prefix=/home/user/tmp --target-list=i386-softmmu --enable-sdl 
--disable-curses --disable-vnc --enable-kvm --disable-docs
   $make
   $make install
   In /home/user/tmp directory:
   $./bin/qemu-img create imgs/winxp.img 4G
   $./bin/qemu-system-i386 imgs/winxp.img -cdrom 
~/Downloads/zh-hans_windows_xp_professional_with_service_pack_3_x86_cd_x14-80404.iso

   then it show a bluescreen after a few seconds.
   See the attachment for more information, please.

   It works well when checking out v1.2.0.


I met the same problem when creating winxp guest by using
qemu-system-x86_64 in qemu-1.3.0.
I'm confused.



Hi,

Does that help : qemu-system-x86_64 -no-acpi ... ?

Fred

Re: [Qemu-devel] [PATCH 1/2] virtio-net: pass host features to virtio_net_init

[Konrad Frederic]

On 03/02/2013 00:06, Anthony Liguori wrote:

Signed-off-by: Anthony Liguori
---
  hw/s390x/s390-virtio-bus.c | 3 ++-
  hw/s390x/virtio-ccw.c  | 3 ++-
  hw/virtio-pci.c| 3 ++-
  hw/virtio.h| 3 ++-
  4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index d467781..089ed92 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -153,7 +153,8 @@ static int s390_virtio_net_init(VirtIOS390Device *dev)
  {
  VirtIODevice *vdev;

-vdev = virtio_net_init((DeviceState *)dev,&dev->nic,&dev->net);
+vdev = virtio_net_init((DeviceState *)dev,&dev->nic,&dev->net,
+   dev->host_features);
  if (!vdev) {
  return -1;
  }
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 231f81e..d92e427 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -555,7 +555,8 @@ static int virtio_ccw_net_init(VirtioCcwDevice *dev)
  {
  VirtIODevice *vdev;

-vdev = virtio_net_init((DeviceState *)dev,&dev->nic,&dev->net);
+vdev = virtio_net_init((DeviceState *)dev,&dev->nic,&dev->net,
+   dev->host_features[0]);
  if (!vdev) {
  return -1;
  }
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 9abbcdf..a869f53 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -997,7 +997,8 @@ static int virtio_net_init_pci(PCIDevice *pci_dev)
  VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
  VirtIODevice *vdev;

-vdev = virtio_net_init(&pci_dev->qdev,&proxy->nic,&proxy->net);
+vdev = virtio_net_init(&pci_dev->qdev,&proxy->nic,&proxy->net,
+   proxy->host_features);

Can't you pass the multiqueue enabled flags through proxy->net?

It would be better for me, as I only pass virtio net conf to device and
never host_features field.



  vdev->nvectors = proxy->nvectors;
  virtio_init_pci(proxy, vdev);
diff --git a/hw/virtio.h b/hw/virtio.h
index a29a54d..1e206b8 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -243,7 +243,8 @@ typedef struct VirtIOBlkConf VirtIOBlkConf;
  VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk);
  struct virtio_net_conf;
  VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
-  struct virtio_net_conf *net);
+  struct virtio_net_conf *net,
+  uint32_t host_features);
  typedef struct virtio_serial_conf virtio_serial_conf;
  VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf 
*serial);
  VirtIODevice *virtio_balloon_init(DeviceState *dev);

Re: [Qemu-devel] [PATCH v2 02/16] register: Add Register API

[KONRAD Frederic]

Hi,

Le 19/01/2016 23:34, Alistair Francis a écrit :

From: Peter Crosthwaite 

This API provides some encapsulation of registers and factors our some
common functionality to common code. Bits of device state (usually MMIO
registers), often have all sorts of access restrictions and semantics
associated with them. This API allow you to define what those
restrictions are on a bit-by-bit basis.

Helper functions are then used to access the register which observe the
semantics defined by the RegisterAccessInfo struct.

Some features:
Bits can be marked as read_only (ro field)
Bits can be marked as write-1-clear (w1c field)
Bits can be marked as reserved (rsvd field)
Reset values can be defined (reset)
Bits can throw guest errors when written certain values (ge0, ge1)
Bits can throw unimp errors when written certain values (ui0, ui1)
Bits can be marked clear on read (cor)
Pre and post action callbacks can be added to read and write ops
Verbose debugging info can be enabled/disabled

Useful for defining device register spaces in a data driven way. Cuts
down on a lot of the verbosity and repetition in the switch-case blocks
in the standard foo_mmio_read/write functions.

Also useful for automated generation of device models from hardware
design sources.

Signed-off-by: Peter Crosthwaite 
Signed-off-by: Alistair Francis 
---
changed from v2:
Simplified! Removed pre-read, nwx, wo
Removed byte loops (Gerd Review)
Made data pointer optional
Added fast paths for simple registers
Moved into hw/core and include/hw (Paolo Review)
changed from v1:
Rebranded as the "Register API" - I think thats probably what it is.
Near total rewrite of implementation.
De-arrayified reset (this is client/Memory APIs job).
Moved out of bitops into its own file (Blue review)
Added debug, the register pointer, and prefix to a struct (Blue Review)
Made 64-bit to play friendlier with memory API (Blue review)
Made backend storage uint8_t (MST review)
Added read/write callbacks (Blue review)
Added ui0, ui1 (Blue review)
Moved re-purposed width (now byte width defining actual storage size)
Arrayified ge0, ge1 (ui0, ui1 too) and added .reason
Added wo field (not an April fools joke - this has genuine meaning here)
Added we mask to write accessor

  hw/core/Makefile.objs |   1 +
  hw/core/register.c| 186 ++
  include/hw/register.h | 132 +++
  3 files changed, 319 insertions(+)
  create mode 100644 hw/core/register.c
  create mode 100644 include/hw/register.h

diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index abb3560..bf95db5 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -14,4 +14,5 @@ common-obj-$(CONFIG_SOFTMMU) += machine.o
  common-obj-$(CONFIG_SOFTMMU) += null-machine.o
  common-obj-$(CONFIG_SOFTMMU) += loader.o
  common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
+common-obj-$(CONFIG_SOFTMMU) += register.o
  common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
diff --git a/hw/core/register.c b/hw/core/register.c
new file mode 100644
index 000..02a4376
--- /dev/null
+++ b/hw/core/register.c
@@ -0,0 +1,186 @@
+/*
+ * Register Definition API
+ *
+ * Copyright (c) 2013 Xilinx Inc.
+ * Copyright (c) 2013 Peter Crosthwaite 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "hw/register.h"
+#include "qemu/log.h"
+
+static inline void register_write_log(RegisterInfo *reg, int dir, uint64_t val,
+  int mask, const char *msg,
+  const char *reason)
+{
+qemu_log_mask(mask, "%s:%s bits %#" PRIx64 " %s write of %d%s%s\n",
+  reg->prefix, reg->access->name, val, msg, dir,
+  reason ? ": " : "", reason ? reason : "");
+}
+
+static inline void register_write_val(RegisterInfo *reg, uint64_t val)
+{
+if (!reg->data) {
+return;
+}
+switch (reg->data_size) {
+case 1:
+*(uint8_t *)reg->data = val;
+break;
+case 2:
+*(uint16_t *)reg->data = val;
+break;
+case 4:
+*(uint32_t *)reg->data = val;
+break;
+case 8:
+*(uint64_t *)reg->data = val;
+break;
+default:
+abort();
+}
+}
+
+static inline uint64_t register_read_val(RegisterInfo *reg)
+{
+switch (reg->data_size) {
+case 1:
+return *(uint8_t *)reg->data;
+case 2:
+return *(uint16_t *)reg->data;
+case 4:
+return *(uint32_t *)reg->data;
+case 8:
+return *(uint64_t *)reg->data;
+default:
+abort();
+}
+return 0; /* unreachable */
+}
+
+void register_write(RegisterInfo *reg, uint64_t val, uint64_t we)
+{
+uint64_t old_val, new_val, test, no_w_mask;
+const RegisterAccessInfo *ac;
+const RegisterAccessError *rae;
+
+assert(reg);
+
+ac = reg->access;
+old_val = reg->data ? register_read_val(

Re: [Qemu-devel] [PATCH v2 03/16] register: Add Memory API glue

[KONRAD Frederic]

Le 19/01/2016 23:35, Alistair Francis a écrit :

From: Peter Crosthwaite 

Add memory io handlers that glue the register API to the memory API.
Just translation functions at this stage. Although it does allow for
devices to be created without all-in-one mmio r/w handlers.

Signed-off-by: Peter Crosthwaite 
Signed-off-by: Alistair Francis 
---
changed from v2:
Added fast path to register_write_memory to skip endianness bitbashing

  hw/core/register.c| 48 
  include/hw/register.h | 30 ++
  2 files changed, 78 insertions(+)

diff --git a/hw/core/register.c b/hw/core/register.c
index 02a4376..ca10cff 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -184,3 +184,51 @@ void register_reset(RegisterInfo *reg)
  
  register_write_val(reg, reg->access->reset);

  }
+
+static inline void register_write_memory(void *opaque, hwaddr addr,
+ uint64_t value, unsigned size, bool 
be)
+{
+RegisterInfo *reg = opaque;

Doesn't that need the QOM REGISTER(obj) conversion defined in patch 6?

Fred

+uint64_t we = ~0;
+int shift = 0;
+
+if (reg->data_size != size) {
+we = (size == 8) ? ~0ull : (1ull << size * 8) - 1;
+shift = 8 * (be ? reg->data_size - size - addr : addr);
+}
+
+assert(size + addr <= reg->data_size);
+register_write(reg, value << shift, we << shift);
+}
+
+void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value,
+  unsigned size)
+{
+register_write_memory(opaque, addr, value, size, true);
+}
+
+
+void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value,
+  unsigned size)
+{
+register_write_memory(opaque, addr, value, size, false);
+}
+
+static inline uint64_t register_read_memory(void *opaque, hwaddr addr,
+unsigned size, bool be)
+{
+RegisterInfo *reg = opaque;
+int shift = 8 * (be ? reg->data_size - size - addr : addr);
+
+return register_read(reg) >> shift;
+}
+
+uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size)
+{
+return register_read_memory(opaque, addr, size, true);
+}
+
+uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size)
+{
+return register_read_memory(opaque, addr, size, false);
+}
diff --git a/include/hw/register.h b/include/hw/register.h
index 249f458..a3c41db 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -86,6 +86,8 @@ struct RegisterAccessInfo {
   * @prefix: String prefix for log and debug messages
   *
   * @opaque: Opaque data for the register
+ *
+ * @mem: optional Memory region for the register
   */
  
  struct RegisterInfo {

@@ -103,6 +105,8 @@ struct RegisterInfo {
  
  bool read_lite;

  bool write_lite;
+
+MemoryRegion mem;
  };
  
  /**

@@ -129,4 +133,30 @@ uint64_t register_read(RegisterInfo *reg);
  
  void register_reset(RegisterInfo *reg);
  
+/**

+ * Memory API MMIO write handler that will write to a Register API register.
+ *  _be for big endian variant and _le for little endian.
+ * @opaque: RegisterInfo to write to
+ * @addr: Address to write
+ * @value: Value to write
+ * @size: Number of bytes to write
+ */
+
+void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value,
+  unsigned size);
+void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value,
+  unsigned size);
+
+/**
+ * Memory API MMIO read handler that will read from a Register API register.
+ *  _be for big endian variant and _le for little endian.
+ * @opaque: RegisterInfo to read from
+ * @addr: Address to read
+ * @size: Number of bytes to read
+ * returns: Value read from register
+ */
+
+uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size);
+uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size);
+
  #endif

Re: [Qemu-devel] [PATCH v2 01/16] memory: Allow subregions to not be printed by info mtree

[KONRAD Frederic]

Le 19/01/2016 23:34, Alistair Francis a écrit :

Add a function called memory_region_add_subregion_no_print() that
creates memory subregions that won't be printed when running
the 'info mtree' command.

Signed-off-by: Alistair Francis 
---

  include/exec/memory.h | 17 +
  memory.c  | 10 +-
  2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 01f1004..eff2a89 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -186,6 +186,7 @@ struct MemoryRegion {
  bool skip_dump;
  bool enabled;
  bool warning_printed; /* For reservations */
+bool do_not_print;
  uint8_t vga_logging_count;
  MemoryRegion *alias;
  hwaddr alias_offset;
@@ -952,6 +953,22 @@ void memory_region_del_eventfd(MemoryRegion *mr,
  void memory_region_add_subregion(MemoryRegion *mr,
   hwaddr offset,
   MemoryRegion *subregion);
+
+/**
+ * memory_region_add_subregion_no_print: Add a subregion to a container.
+ *
+ * The same functionality as memory_region_add_subregion except that any
+ * memory regions added by this function are not printed by 'info mtree'.
+ *
+ * @mr: the region to contain the new subregion; must be a container
+ *  initialized with memory_region_init().
+ * @offset: the offset relative to @mr where @subregion is added.
+ * @subregion: the subregion to be added.
+ */
+void memory_region_add_subregion_no_print(MemoryRegion *mr,
+  hwaddr offset,
+  MemoryRegion *subregion);
+
  /**
   * memory_region_add_subregion_overlap: Add a subregion to a container
   *  with overlap.
diff --git a/memory.c b/memory.c
index 93bd8ed..ee90682 100644
--- a/memory.c
+++ b/memory.c
@@ -1827,6 +1827,14 @@ void memory_region_add_subregion(MemoryRegion *mr,
  memory_region_add_subregion_common(mr, offset, subregion);
  }
  
+void memory_region_add_subregion_no_print(MemoryRegion *mr,

+  hwaddr offset,
+  MemoryRegion *subregion)
+{
+memory_region_add_subregion(mr, offset, subregion);
+subregion->do_not_print = true;
+}
+
  void memory_region_add_subregion_overlap(MemoryRegion *mr,
   hwaddr offset,
   MemoryRegion *subregion,
@@ -2190,7 +2198,7 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
  const MemoryRegion *submr;
  unsigned int i;
  
-if (!mr) {

+if (!mr || mr->do_not_print) {
  return;
  }
  

Seems OK to me.
Reviewed-by: KONRAD Frederic 

Re: [Qemu-devel] [PATCH v2 04/16] register: Add support for decoding information

[KONRAD Frederic]

Le 19/01/2016 23:35, Alistair Francis a écrit :

From: Peter Crosthwaite 

Allow defining of optional address decoding information in register
definitions. This is useful for clients that want to associate
registers with specific addresses.

Signed-off-by: Peter Crosthwaite 
Signed-off-by: Alistair Francis 
---
changed since v4:
Remove extraneous unused defintions.

  include/hw/register.h | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/include/hw/register.h b/include/hw/register.h
index a3c41db..90c0185 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -54,6 +54,11 @@ typedef struct RegisterAccessError {
   * allowing this function to modify the value before return to the client.
   */
  
+#define REG_DECODE_READ (1 << 0)

+#define REG_DECODE_WRITE (1 << 1)
+#define REG_DECODE_EXECUTE (1 << 2)
+#define REG_DECODE_RW (REG_DECODE_READ | REG_DECODE_WRITE)
+
  struct RegisterAccessInfo {
  const char *name;
  uint64_t ro;
@@ -71,6 +76,11 @@ struct RegisterAccessInfo {
  void (*post_write)(RegisterInfo *reg, uint64_t val);
  
  uint64_t (*post_read)(RegisterInfo *reg, uint64_t val);

+
+struct {
+hwaddr addr;
+uint8_t flags;
+} decode;

is that used somewhere?

Fred

  };
  
  /**

Re: [Qemu-devel] [PATCH v2 06/16] register: QOMify

[KONRAD Frederic]

Le 19/01/2016 23:35, Alistair Francis a écrit :

From: Peter Crosthwaite 

QOMify registers as a child of TYPE_DEVICE. This allows registers to
define GPIOs.

Define an init helper that will do QOM initialisation as well as setup
the r/w fast paths.

Signed-off-by: Peter Crosthwaite 
Signed-off-by: Alistair Francis 
---

  hw/core/register.c| 34 ++
  include/hw/register.h | 17 +
  2 files changed, 51 insertions(+)

diff --git a/hw/core/register.c b/hw/core/register.c
index ca10cff..000b87f 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -185,6 +185,28 @@ void register_reset(RegisterInfo *reg)
  register_write_val(reg, reg->access->reset);
  }
  
+void register_init(RegisterInfo *reg)

+{
+assert(reg);
+const RegisterAccessInfo *ac;
+
+if (!reg->data || !reg->access) {
+return;
+}
+
+object_initialize((void *)reg, sizeof(*reg), TYPE_REGISTER);
+
+ac = reg->access;
+
+/* if there are no debug msgs and no RMW requirement, mark for fast write 
*/
+reg->write_lite = reg->debug || ac->ro || ac->w1c || ac->pre_write ||
+((ac->ge0 || ac->ge1) && qemu_loglevel_mask(LOG_GUEST_ERROR)) ||
+((ac->ui0 || ac->ui1) && qemu_loglevel_mask(LOG_UNIMP))
+ ? false : true;
+/* no debug and no clear-on-read is a fast read */
+reg->read_lite = reg->debug || ac->cor ? false : true;
+}
+
  static inline void register_write_memory(void *opaque, hwaddr addr,
   uint64_t value, unsigned size, bool 
be)
  {
@@ -232,3 +254,15 @@ uint64_t register_read_memory_le(void *opaque, hwaddr 
addr, unsigned size)
  {
  return register_read_memory(opaque, addr, size, false);
  }
+
+static const TypeInfo register_info = {
+.name  = TYPE_REGISTER,
+.parent = TYPE_DEVICE,
+};
+
+static void register_register_types(void)
+{
+type_register_static(®ister_info);
+}
+
+type_init(register_register_types)
diff --git a/include/hw/register.h b/include/hw/register.h
index 0c6f03d..6677dee 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -11,6 +11,7 @@
  #ifndef REGISTER_H
  #define REGISTER_H
  
+#include "hw/qdev-core.h"

  #include "exec/memory.h"
  
  typedef struct RegisterInfo RegisterInfo;

@@ -101,6 +102,11 @@ struct RegisterAccessInfo {
   */
  
  struct RegisterInfo {

+/*< private >*/
+DeviceState parent_obj;
+
+/*< public >*/
+
  void *data;
  int data_size;
  
@@ -119,6 +125,9 @@ struct RegisterInfo {

  MemoryRegion mem;
  };
  
+#define TYPE_REGISTER "qemu,register"

+#define REGISTER(obj) OBJECT_CHECK(RegisterInfo, (obj), TYPE_REGISTER)
+
  /**
   * write a value to a register, subject to its restrictions
   * @reg: register to write to
@@ -144,6 +153,14 @@ uint64_t register_read(RegisterInfo *reg);
  void register_reset(RegisterInfo *reg);
  
  /**

+ * Initialize a register. GPIO's are setup as IOs to the specified device.
+ * Fast paths for eligible registers are enabled.
+ * @reg: Register to initialize
+ */
+
+void register_init(RegisterInfo *reg);
+
+/**
   * Memory API MMIO write handler that will write to a Register API register.
   *  _be for big endian variant and _le for little endian.
   * @opaque: RegisterInfo to write to

seems ok to me
Reviewed-by: KONRAD Frederic 

Fred

Re: [Qemu-devel] [PATCH v2 07/16] register: Add block initialise helper

[KONRAD Frederic]

Le 19/01/2016 23:35, Alistair Francis a écrit :

From: Peter Crosthwaite 

Add a helper that will scan a static RegisterAccessInfo Array
and populate a container MemoryRegion with registers as defined.

Signed-off-by: Peter Crosthwaite 
Signed-off-by: Alistair Francis 
---
V2:
  - Use memory_region_add_subregion_no_print()

  hw/core/register.c| 29 +
  include/hw/register.h | 21 +
  2 files changed, 50 insertions(+)

diff --git a/hw/core/register.c b/hw/core/register.c
index 000b87f..116fd0b 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -255,6 +255,35 @@ uint64_t register_read_memory_le(void *opaque, hwaddr 
addr, unsigned size)
  return register_read_memory(opaque, addr, size, false);
  }
  
+void register_init_block32(DeviceState *owner, const RegisterAccessInfo *rae,

+   int num, RegisterInfo *ri, uint32_t *data,
+   MemoryRegion *container, const MemoryRegionOps *ops,
+   bool debug_enabled)
+{
+const char *debug_prefix = object_get_typename(OBJECT(owner));
+int i;
+
+for (i = 0; i < num; i++) {
+int index = rae[i].decode.addr / 4;
+RegisterInfo *r = &ri[index];
+
+*r = (RegisterInfo) {
+.data = &data[index],
+.data_size = sizeof(uint32_t),
+.access = &rae[i],
+.debug = debug_enabled,
+.prefix = debug_prefix,
+.opaque = owner,
+};
+register_init(r);
+
+memory_region_init_io(&r->mem, OBJECT(owner), ops, r, r->access->name,
+  sizeof(uint32_t));
+memory_region_add_subregion_no_print(container,
+ r->access->decode.addr, &r->mem);
+}
+}
+
  static const TypeInfo register_info = {
  .name  = TYPE_REGISTER,
  .parent = TYPE_DEVICE,
diff --git a/include/hw/register.h b/include/hw/register.h
index 6677dee..f3e4c2c 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -186,6 +186,27 @@ void register_write_memory_le(void *opaque, hwaddr addr, 
uint64_t value,
  uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size);
  uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size);
  
+/**

+ * Init a block of consecutive registers into a container MemoryRegion. A
+ * number of constant register definitions are parsed to create a corresponding
+ * array of RegisterInfo's.
+ *
+ * @owner: device owning the registers
+ * @rae: Register definitions to init
+ * @num: number of registers to init (length of @rae)
+ * @ri: Register array to init
+ * @data: Array to use for register data
+ * @container: Memory region to contain new registers
+ * @ops: Memory region ops to use to access registers. Opaque data of handler
+ * with be a RegisterInfo * (from @ri)

typo here

Fred


+ * @debug enabled: turn on/off verbose debug information
+ */
+
+void register_init_block32(DeviceState *owner, const RegisterAccessInfo *rae,
+   int num, RegisterInfo *ri, uint32_t *data,
+   MemoryRegion *container, const MemoryRegionOps *ops,
+   bool debug_enabled);
+
  /* Define constants for a 32 bit register */
  #define REG32(reg, addr)  \
  enum { A_ ## reg = (addr) };  \

Re: [Qemu-devel] [PATCH V5 7/8] introduce xlnx-dp

[KONRAD Frederic]

Le 20/11/2015 11:06, Alistair Francis a écrit :

On Fri, Oct 16, 2015 at 7:11 PM,   wrote:

From: KONRAD Frederic 

This is the implementation of the DisplayPort.
It has an aux-bus to access dpcd and edid.

Graphic plane is connected to the channel 3.
Video plane is connected to the channel 0.
Audio stream are connected to the channels 4 and 5.

This patch doesn't pass checkpatch (I didn't test any others).
Can you run the series through checkpatch?
Yes but I might have introduced some coding style issues during the last 
changes.

Sorry for that will fix it!

Thanks,
Fred


Also a super small nit pick, some of your line splitting doesn't line up,
can you make sure that the function arguments and if statement conditions
line up with the previous line.


Signed-off-by: KONRAD Frederic 
Tested-By: Hyun Kwon 
---
  hw/display/Makefile.objs |1 +
  hw/display/xlnx_dp.c | 1370 ++
  include/hw/display/xlnx_dp.h |  110 
  3 files changed, 1481 insertions(+)
  create mode 100644 hw/display/xlnx_dp.c
  create mode 100644 include/hw/display/xlnx_dp.h

diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 250a43f..3625ab2 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -43,3 +43,4 @@ virtio-gpu.o-libs += $(VIRGL_LIBS)
  virtio-gpu-3d.o-cflags := $(VIRGL_CFLAGS)
  virtio-gpu-3d.o-libs += $(VIRGL_LIBS)
  obj-$(CONFIG_DPCD) += dpcd.o
+obj-$(CONFIG_XLNX_ZYNQMP) += xlnx_dp.o
diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
new file mode 100644
index 000..e91221c
--- /dev/null
+++ b/hw/display/xlnx_dp.c
@@ -0,0 +1,1370 @@
+/*
+ * xlnx_dp.c
+ *
+ *  Copyright (C) 2015 : GreenSocs Ltd
+ *  http://www.greensocs.com/ , email: i...@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option)any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU 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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "hw/display/xlnx_dp.h"
+
+#ifndef DEBUG_DP
+#define DEBUG_DP 0
+#endif
+
+#define DPRINTF(fmt, ...) do { 
\
+if (DEBUG_DP) {
\
+qemu_log("xlnx_dp: " fmt , ## __VA_ARGS__);
\
+}  
\
+} while (0);
+
+/*
+ * Register offset for DP.
+ */
+#define DP_LINK_BW_SET  (0x >> 2)
+#define DP_LANE_COUNT_SET   (0x0004 >> 2)
+#define DP_ENHANCED_FRAME_EN(0x0008 >> 2)
+#define DP_TRAINING_PATTERN_SET (0x000C >> 2)
+#define DP_LINK_QUAL_PATTERN_SET(0x0010 >> 2)
+#define DP_SCRAMBLING_DISABLE   (0x0014 >> 2)
+#define DP_DOWNSPREAD_CTRL  (0x0018 >> 2)
+#define DP_SOFTWARE_RESET   (0x001C >> 2)
+#define DP_TRANSMITTER_ENABLE   (0x0080 >> 2)
+#define DP_MAIN_STREAM_ENABLE   (0x0084 >> 2)
+#define DP_FORCE_SCRAMBLER_RESET(0x00C0 >> 2)
+#define DP_VERSION_REGISTER (0x00F8 >> 2)
+#define DP_CORE_ID  (0x00FC >> 2)
+
+#define DP_AUX_COMMAND_REGISTER (0x0100 >> 2)
+#define AUX_ADDR_ONLY_MASK  (0x1000)
+#define AUX_COMMAND_MASK(0x0F00)
+#define AUX_COMMAND_SHIFT   (8)
+#define AUX_COMMAND_NBYTES  (0x000F)
+
+#define DP_AUX_WRITE_FIFO   (0x0104 >> 2)
+#define DP_AUX_ADDRESS  (0x0108 >> 2)
+#define DP_AUX_CLOCK_DIVIDER(0x010C >> 2)
+#define DP_TX_USER_FIFO_OVERFLOW(0x0110 >> 2)
+#define DP_INTERRUPT_SIGNAL_STATE   (0x0130 >> 2)
+#define DP_AUX_REPLY_DATA   (0x0134 >> 2)
+#define DP_AUX_REPLY_CODE   (0x0138 >> 2)
+#define DP_AUX_REPLY_COUNT  (0x013C >> 2)
+#define DP_REPLY_DATA_COUNT (0x0148 >> 2)
+#define DP_REPLY_STATUS (0x014C >> 2)
+#define DP_HPD_DURATION (0x0150 >> 2)
+#define DP_MAIN_STREAM_HTOTAL   (0x0180 >> 2)
+#define DP_MAIN_STREAM_VTOTAL   (0x0184 >> 2)
+#define DP_MAIN_STREAM_PO

Re: [Qemu-devel] [PATCH V5 8/8] arm: xlnx-zynqmp: Add xlnx-dp and xlnx-dpdma

[KONRAD Frederic]

Le 20/11/2015 13:21, Alistair Francis a écrit :

On Fri, Oct 16, 2015 at 7:11 PM,   wrote:

From: KONRAD Frederic 

This adds the DP and the DPDMA to the Zynq MP platform.

Signed-off-by: KONRAD Frederic 
Reviewed-by: Peter Crosthwaite 
Tested-By: Hyun Kwon 
---
  hw/arm/xlnx-zynqmp.c | 20 
  include/hw/arm/xlnx-zynqmp.h |  5 +
  2 files changed, 25 insertions(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index b36ca3d..dfed5cd 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -32,6 +32,12 @@
  #define SATA_ADDR   0xFD0C
  #define SATA_NUM_PORTS  2

+#define DP_ADDR 0xfd4a
+#define DP_IRQ  113
+
+#define DPDMA_ADDR  0xfd4c
+#define DPDMA_IRQ   116
+
  static const uint64_t gem_addr[XLNX_ZYNQMP_NUM_GEMS] = {
  0xFF0B, 0xFF0C, 0xFF0D, 0xFF0E,
  };
@@ -97,6 +103,11 @@ static void xlnx_zynqmp_init(Object *obj)

  object_initialize(&s->sata, sizeof(s->sata), TYPE_SYSBUS_AHCI);
  qdev_set_parent_bus(DEVICE(&s->sata), sysbus_get_default());
+
+object_initialize(&s->dp, sizeof(s->dp), TYPE_XLNX_DP);
+qdev_set_parent_bus(DEVICE(&s->dp), sysbus_get_default());

New line


+object_initialize(&s->dpdma, sizeof(s->dpdma), TYPE_XLNX_DPDMA);
+qdev_set_parent_bus(DEVICE(&s->dpdma), sysbus_get_default());
  }

  static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -258,6 +269,15 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)

  sysbus_mmio_map(SYS_BUS_DEVICE(&s->sata), 0, SATA_ADDR);
  sysbus_connect_irq(SYS_BUS_DEVICE(&s->sata), 0, gic_spi[SATA_INTR]);
+
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->dp), 0, DP_ADDR);
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->dp), 0, gic_spi[DP_IRQ]);

New line


+sysbus_mmio_map(SYS_BUS_DEVICE(&s->dpdma), 0, DPDMA_ADDR);
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->dpdma), 0, gic_spi[DPDMA_IRQ]);
+object_property_set_bool(OBJECT(&s->dp), true, "realized", &err);
+object_property_set_bool(OBJECT(&s->dpdma), true, "realized", &err);

Can you add something to check these errors?


Ok.
BTW I'll move the I2C and AUX device out of xlnx-dp and put that here.
Is that ok with you?

Thanks,
Fred


Thanks,

Alistair


+object_property_set_link(OBJECT(&s->dp), OBJECT(&s->dpdma), "dpdma",
+ &error_abort);
  }

  static Property xlnx_zynqmp_props[] = {
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 4005a99..5a4d6cc 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -24,6 +24,8 @@
  #include "hw/char/cadence_uart.h"
  #include "hw/ide/pci.h"
  #include "hw/ide/ahci.h"
+#include "hw/dma/xlnx_dpdma.h"
+#include "hw/display/xlnx_dp.h"

  #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
  #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
@@ -66,6 +68,9 @@ typedef struct XlnxZynqMPState {

  char *boot_cpu;
  ARMCPU *boot_cpu_ptr;
+
+XlnxDPState dp;
+XlnxDPDMAState dpdma;
  }  XlnxZynqMPState;

  #define XLNX_ZYNQMP_H
--
1.9.0

Re: [Qemu-devel] [PATCH V6 2/8] introduce aux-bus

[KONRAD Frederic]

Le 13/01/2016 22:02, Peter Crosthwaite a écrit :

On Mon, Jan 4, 2016 at 10:25 AM,   wrote:

From: KONRAD Frederic 

This introduces a new bus: aux-bus.

It contains an address space for aux slaves devices and a bridge to an I2C bus
for I2C through AUX transactions.

Signed-off-by: KONRAD Frederic 
Tested-By: Hyun Kwon 
---
  default-configs/aarch64-softmmu.mak |   1 +
  hw/misc/Makefile.objs   |   1 +
  hw/misc/aux.c   | 348 
  include/hw/misc/aux.h   | 124 +
  4 files changed, 474 insertions(+)
  create mode 100644 hw/misc/aux.c
  create mode 100644 include/hw/misc/aux.h

diff --git a/default-configs/aarch64-softmmu.mak 
b/default-configs/aarch64-softmmu.mak
index 96dd994..d3a2665 100644
--- a/default-configs/aarch64-softmmu.mak
+++ b/default-configs/aarch64-softmmu.mak
@@ -3,4 +3,5 @@
  # We support all the 32 bit boards so need all their config
  include arm-softmmu.mak

+CONFIG_AUX=y
  CONFIG_XLNX_ZYNQMP=y
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index d4765c2..4af5fbe 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -44,3 +44,4 @@ obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o
  obj-$(CONFIG_PVPANIC) += pvpanic.o
  obj-$(CONFIG_EDU) += edu.o
  obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
+obj-$(CONFIG_AUX) += aux.o
diff --git a/hw/misc/aux.c b/hw/misc/aux.c
new file mode 100644
index 000..cdfec67
--- /dev/null
+++ b/hw/misc/aux.c
@@ -0,0 +1,348 @@
+/*
+ * aux.c
+ *
+ *  Copyright 2015 : GreenSocs Ltd
+ *  http://www.greensocs.com/ , email: i...@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option)any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU 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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/*
+ * This is an implementation of the AUX bus for VESA Display Port v1.1a.
+ */
+
+#include "hw/misc/aux.h"
+#include "hw/i2c/i2c.h"
+#include "monitor/monitor.h"
+
+#ifndef DEBUG_AUX
+#define DEBUG_AUX 0
+#endif
+
+#define DPRINTF(fmt, ...) do { 
\
+if (DEBUG_AUX) {   
\
+qemu_log("aux: " fmt , ## __VA_ARGS__);
\
+}  
\
+} while (0);
+
+#define TYPE_AUXTOI2C "aux-to-i2c-bridge"
+#define AUXTOI2C(obj) OBJECT_CHECK(AUXTOI2CState, (obj), TYPE_AUXTOI2C)
+
+#define TYPE_AUX_BUS "aux-bus"
+#define AUX_BUS(obj) OBJECT_CHECK(AUXBus, (obj), TYPE_AUX_BUS)
+
+static void aux_slave_dev_print(Monitor *mon, DeviceState *dev, int indent);
+
+static void aux_bus_class_init(ObjectClass *klass, void *data)
+{
+BusClass *k = BUS_CLASS(klass);
+
+/* AUXSlave has an MMIO so we need to change the way we print information
+ * in monitor.
+ */
+k->print_dev = aux_slave_dev_print;
+}
+
+static const TypeInfo aux_bus_info = {
+.name = TYPE_AUX_BUS,
+.parent = TYPE_BUS,
+.instance_size = sizeof(AUXBus),
+.class_init = aux_bus_class_init
+};
+
+AUXBus *aux_init_bus(DeviceState *parent, const char *name)
+{
+AUXBus *bus;
+
+bus = AUX_BUS(qbus_create(TYPE_AUX_BUS, parent, name));
+bus->bridge = AUXTOI2C(qdev_create(BUS(bus), TYPE_AUXTOI2C));
+
+/* Memory related. */
+bus->aux_io = g_malloc(sizeof(*bus->aux_io));
+memory_region_init(bus->aux_io, OBJECT(bus), "aux-io", (1 << 20));
+address_space_init(&bus->aux_addr_space, bus->aux_io, "aux-io");
+return bus;
+}
+
+static void aux_bus_map_device(AUXBus *bus, AUXSlave *dev, hwaddr addr)
+{
+memory_region_add_subregion(bus->aux_io, addr, dev->mmio);
+}
+
+static bool aux_bus_is_bridge(AUXBus *bus, DeviceState *dev)
+{
+return (dev == DEVICE(bus->bridge));
+}
+
+AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
+  uint8_t len, uint8_t *data)
+{
+int temp;
+AUXReply ret = AUX_NACK;
+I2CBus *i2c_bus = aux_get_i2c_bus(bus);
+size_t i;
+bool is_write = false;
+
+DPRINTF("request at address 0x%" PRIX32 ", command %u, len %u\n", address,
+cmd, len);
+
+switch (cmd) {
+/*
+ * Forward the request on the AUX bus..
+ */
+case WRITE_AUX:
+is_write =

Re: [Qemu-devel] Status of my hacks on the MTTCG WIP branch

[KONRAD Frederic]

Le 14/01/2016 14:10, Alex Bennée a écrit :

Alex Bennée  writes:


Pranith Kumar  writes:


Hi Alex,

On Tue, Jan 12, 2016 at 12:29 PM, Alex Bennée 
wrote:

https://github.com/stsquad/qemu/tree/mttcg/multi_tcg_v8_wip_ajb_fix_locks
I built this branch and ran an arm64 guest. It seems to be failing
similarly to what I reported earlier:

#0  0x72211cc9 in __GI_raise (sig=sig@entry=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x722150d8 in __GI_abort () at abort.c:89
#2  0x5572014c in qemu_ram_addr_from_host_nofail
(ptr=0xffc000187863) at /home/pranith/devops/code/qemu/cputlb.c:357
#3  0x557209dd in get_page_addr_code (env1=0x56702058,
addr=18446743798833248356) at /home/pranith/devops/code/qemu/cputlb.c:568
#4  0x556db98c in tb_find_physical (cpu=0x566f9dd0,
pc=18446743798833248356, cs_base=0, flags=18446744071830503424) at
/home/pranith/devops/code/qemu/cpu-exec.c:224
#5  0x556dbaf4 in tb_find_slow (cpu=0x566f9dd0,
pc=18446743798833248356, cs_base=0, flags=18446744071830503424) at
/home/pranith/devops/code/qemu/cpu-exec.c:268
#6  0x556dbc77 in tb_find_fast (cpu=0x566f9dd0) at
/home/pranith/devops/code/qemu/cpu-exec.c:311
#7  0x556dc0f1 in cpu_arm_exec (cpu=0x566f9dd0) at
/home/pranith/devops/code/qemu/cpu-exec.c:492
#8  0x557050ee in tcg_cpu_exec (cpu=0x566f9dd0) at
/home/pranith/devops/code/qemu/cpus.c:1486
#9  0x557051af in tcg_exec_all (cpu=0x566f9dd0) at
/home/pranith/devops/code/qemu/cpus.c:1515
#10 0x55704800 in qemu_tcg_cpu_thread_fn (arg=0x566f9dd0) at
/home/pranith/devops/code/qemu/cpus.c:1187
#11 0x725a8182 in start_thread (arg=0x7fffd20c8700) at
pthread_create.c:312
#12 0x722d547d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:111



Having seen a backtrace of a crash while the other thread was flushing
the TLB entries I sprinkled a bunch of:

   g_assert(cpu == current_cpu);

In all public functions in cputlb that took a CPU. There are a bunch of
cases that don't defer actions across CPUs which need to be fixed up. I
suspect they don't hit in the arm case because the type of TLB flushing
pattern is different. In aarch64 it my backtrace it was triggered by
tlbi_aa64_vae1is_write:

   7Thread 0x7ffe777fe700 (LWP 32705) "worker" sem_timedwait () at 
../nptl/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S:101
   6Thread 0x7ffe77fff700 (LWP 32704) "worker" sem_timedwait () at 
../nptl/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S:101
   5Thread 0x7fff8d9d0700 (LWP 32703) "CPU 1/TCG" 0x5572cc18 in memcpy 
(__len=8, __src=, __dest=)
 at /usr/include/x86_64-linux-gnu/bits/string3.h:51
* 4Thread 0x7fff8e1d1700 (LWP 32702) "CPU 0/TCG" memset () at 
../sysdeps/x86_64/memset.S:94
   3Thread 0x7fff8f1cb700 (LWP 32701) "worker" sem_timedwait () at 
../nptl/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S:101
   2Thread 0x7fffe45c8700 (LWP 32700) "qemu-system-aar" syscall () at 
../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
   1Thread 0x77f98c00 (LWP 32696) "qemu-system-aar" 0x70ba01ef in 
__GI_ppoll (fds=0x575cb5b0, nfds=8, timeout=,
 timeout@entry=0x7fffdf60, sigmask=sigmask@entry=0x0) at 
../sysdeps/unix/sysv/linux/ppoll.c:56
#0  memset () at ../sysdeps/x86_64/memset.S:94
#1  0x55728bee in memset (__len=32768, __ch=0, __dest=0x56632568) 
at /usr/include/x86_64-linux-gnu/bits/string3.h:84
#2  v_tlb_flush_by_mmuidx (argp=0x7fff8e1d0430, cpu=0x56632380) at 
/home/alex/lsrc/qemu/qemu.git/cputlb.c:136
#3  tlb_flush_page_by_mmuidx (cpu=cpu@entry=0x56632380, 
addr=addr@entry=547976253440) at /home/alex/lsrc/qemu/qemu.git/cputlb.c:243
#4  0x557fcb4a in tlbi_aa64_vae1is_write (env=, ri=, value=)
 at /home/alex/lsrc/qemu/qemu.git/target-arm/helper.c:2757
#5  0x7fffa441dac5 in code_gen_buffer ()
#6  0x556eef4b in cpu_tb_exec (tb_ptr=, 
cpu=0x565eddd0) at /home/alex/lsrc/qemu/qemu.git/cpu-exec.c:157
#7  cpu_arm_exec (cpu=cpu@entry=0x565eddd0) at 
/home/alex/lsrc/qemu/qemu.git/cpu-exec.c:520
#8  0x557108e8 in tcg_cpu_exec (cpu=0x565eddd0) at 
/home/alex/lsrc/qemu/qemu.git/cpus.c:1486
#9  tcg_exec_all (cpu=0x565eddd0) at 
/home/alex/lsrc/qemu/qemu.git/cpus.c:1515
#10 qemu_tcg_cpu_thread_fn (arg=0x565eddd0) at 
/home/alex/lsrc/qemu/qemu.git/cpus.c:1187
#11 0x70e80182 in start_thread (arg=0x7fff8e1d1700) at 
pthread_create.c:312
#12 0x70bad47d in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:111
[Switching to thread 5 (Thread 0x7fff8d9d0700 (LWP 32703))]
#0  0x5572cc18 in memcpy (__len=8, __src=, 
__dest=) at /usr/include/x86_64-linux-gnu/bits/string3.h:51
51return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
#0  0x5572cc18 in memcpy (__len=8, __src=, 
__dest=) at /usr/include/x86_64-linux-gnu/bits/string3.h:51
#1  stq_he_p (v=, ptr=) at 
/home/alex/lsrc/qemu/qemu.gi

Re: [Qemu-devel] Status of my hacks on the MTTCG WIP branch

[KONRAD Frederic]

Le 15/01/2016 15:24, Pranith Kumar a écrit :

Hi Alex,

On Fri, Jan 15, 2016 at 8:53 AM, Alex Bennée > wrote:

> Can you try this branch:
>
> 
https://github.com/stsquad/qemu/tree/mttcg/multi_tcg_v8_wip_ajb_fix_locks-r1

>
> I think I've caught all the things likely to screw up addressing.
>

I tried this branch and the boot hangs like follows:

[2.001083] random: systemd-udevd urandom read with 1 bits of 
entropy available

main-loop: WARNING: I/O thread spun for 1000 iterations
[   23.778970] INFO: rcu_sched detected stalls on CPUs/tasks: {} 
(detected by 0, t=2102 jiffies, g=-165, c=-166, q=83)
[   23.780265] All QSes seen, last rcu_sched kthread activity 2101 
(4294939656-4294937555), jiffies_till_next_fqs=1, root ->qsmask 0x0
[   23.781228] swapper/0   R  running task0 0  0 
0x0080

[   23.781977] Call trace:
[   23.782375] [] dump_backtrace+0x0/0x170
[   23.782852] [] show_stack+0x20/0x2c
[   23.783279] [] sched_show_task+0x9c/0xf0
[   23.783746] [] rcu_check_callbacks+0x7b8/0x828
[   23.784230] [] update_process_times+0x40/0x74
[   23.784723] [] tick_sched_handle.isra.15+0x38/0x7c
[   23.785247] [] tick_sched_timer+0x48/0x84
[   23.785705] [] __run_hrtimer+0x90/0x200
[   23.786148] [] hrtimer_interrupt+0xec/0x268
[   23.786612] [] arch_timer_handler_virt+0x38/0x48
[   23.787120] [] handle_percpu_devid_irq+0x90/0x12c
[   23.787621] [] generic_handle_irq+0x38/0x54
[   23.788093] [] __handle_domain_irq+0x68/0xc4
[   23.788578] [] gic_handle_irq+0x38/0x84
[   23.789035] Exception stack(0xffc00073bde0 to 0xffc00073bf00)
[   23.789650] bde0: 00738000 ffc0 0073e71c ffc0 0073bf20 
ffc0 00086948 ffc0
[   23.790356] be00: 000d848c ffc0   3ffcdb0c 
ffc0  0100
[   23.791030] be20: 38b97100 ffc0 0073bea0 ffc0 67f6e000 
0005 567f1c33 
[   23.791744] be40: 00748cf0 ffc0 0073be70 ffc0 c1e2e4a0 
ffbd 3a801148 ffc0
[   23.792406] be60:  0040 0073e000 ffc0 3a801168 
ffc0 97bbb588 007f
[   23.793055] be80: 0021d7e8 ffc0 97b3d6ec 007f c37184d0 
007f 00738000 ffc0
[   23.793720] bea0: 0073e71c ffc0 006ff7e8 ffc0 007c8000 
ffc0 0073e680 ffc0
[   23.794373] bec0: 0072fac0 ffc0 0001  0073bf30 
ffc0 0050e9e8 ffc0
[   23.795025] bee0:   0073bf20 ffc0 00086944 
ffc0 0073bf20 ffc0

[   23.795721] [] el1_irq+0x64/0xc0
[   23.796131] [] cpu_startup_entry+0x130/0x204
[   23.796605] [] rest_init+0x78/0x84
[   23.797028] [] start_kernel+0x3a0/0x3b8
[   23.797528] rcu_sched kthread starved for 2101 jiffies!


I will try to debug and see where it is hanging.

Thanks!
--
Pranith


Hi Pranith,

I don't have time today to look into that.

But I missed a tb_find_physical which happen during tb_lock not held..
This hack should fix that (and probably slow things down):

diff --git a/cpu-exec.c b/cpu-exec.c
index 903126f..25a005a 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -252,9 +252,9 @@ static TranslationBlock *tb_find_physical(CPUState *cpu,
 }

 /* Move the TB to the head of the list */
-*ptb1 = tb->phys_hash_next;
-tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h];
-tcg_ctx.tb_ctx.tb_phys_hash[h] = tb;
+//*ptb1 = tb->phys_hash_next;
+//tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h];
+//tcg_ctx.tb_ctx.tb_phys_hash[h] = tb;
 return tb;
 }

Fred

Re: [Qemu-devel] Status of my hacks on the MTTCG WIP branch

[KONRAD Frederic]

Le 15/01/2016 15:46, Alex Bennée a écrit :

KONRAD Frederic  writes:


Le 15/01/2016 15:24, Pranith Kumar a écrit :

Hi Alex,

On Fri, Jan 15, 2016 at 8:53 AM, Alex Bennée mailto:alex.ben...@linaro.org>> wrote:

Can you try this branch:



https://github.com/stsquad/qemu/tree/mttcg/multi_tcg_v8_wip_ajb_fix_locks-r1

I think I've caught all the things likely to screw up addressing.


I tried this branch and the boot hangs like follows:

[2.001083] random: systemd-udevd urandom read with 1 bits of
entropy available
main-loop: WARNING: I/O thread spun for 1000 iterations
[   23.778970] INFO: rcu_sched detected stalls on CPUs/tasks: {}
(detected by 0, t=2102 jiffies, g=-165, c=-166, q=83)
[   23.780265] All QSes seen, last rcu_sched kthread activity 2101
(4294939656-4294937555), jiffies_till_next_fqs=1, root ->qsmask 0x0
[   23.781228] swapper/0   R  running task0 0  0
0x0080
[   23.781977] Call trace:
[   23.782375] [] dump_backtrace+0x0/0x170
[   23.782852] [] show_stack+0x20/0x2c
[   23.783279] [] sched_show_task+0x9c/0xf0
[   23.783746] [] rcu_check_callbacks+0x7b8/0x828
[   23.784230] [] update_process_times+0x40/0x74
[   23.784723] [] tick_sched_handle.isra.15+0x38/0x7c
[   23.785247] [] tick_sched_timer+0x48/0x84
[   23.785705] [] __run_hrtimer+0x90/0x200
[   23.786148] [] hrtimer_interrupt+0xec/0x268
[   23.786612] [] arch_timer_handler_virt+0x38/0x48
[   23.787120] [] handle_percpu_devid_irq+0x90/0x12c
[   23.787621] [] generic_handle_irq+0x38/0x54
[   23.788093] [] __handle_domain_irq+0x68/0xc4
[   23.788578] [] gic_handle_irq+0x38/0x84
[   23.789035] Exception stack(0xffc00073bde0 to 0xffc00073bf00)
[   23.789650] bde0: 00738000 ffc0 0073e71c ffc0 0073bf20
ffc0 00086948 ffc0
[   23.790356] be00: 000d848c ffc0   3ffcdb0c
ffc0  0100
[   23.791030] be20: 38b97100 ffc0 0073bea0 ffc0 67f6e000
0005 567f1c33 
[   23.791744] be40: 00748cf0 ffc0 0073be70 ffc0 c1e2e4a0
ffbd 3a801148 ffc0
[   23.792406] be60:  0040 0073e000 ffc0 3a801168
ffc0 97bbb588 007f
[   23.793055] be80: 0021d7e8 ffc0 97b3d6ec 007f c37184d0
007f 00738000 ffc0
[   23.793720] bea0: 0073e71c ffc0 006ff7e8 ffc0 007c8000
ffc0 0073e680 ffc0
[   23.794373] bec0: 0072fac0 ffc0 0001  0073bf30
ffc0 0050e9e8 ffc0
[   23.795025] bee0:   0073bf20 ffc0 00086944
ffc0 0073bf20 ffc0
[   23.795721] [] el1_irq+0x64/0xc0
[   23.796131] [] cpu_startup_entry+0x130/0x204
[   23.796605] [] rest_init+0x78/0x84
[   23.797028] [] start_kernel+0x3a0/0x3b8
[   23.797528] rcu_sched kthread starved for 2101 jiffies!


I will try to debug and see where it is hanging.

Thanks!
--
Pranith

Hi Pranith,

I don't have time today to look into that.

But I missed a tb_find_physical which happen during tb_lock not held..
This hack should fix that (and probably slow things down):

diff --git a/cpu-exec.c b/cpu-exec.c
index 903126f..25a005a 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -252,9 +252,9 @@ static TranslationBlock *tb_find_physical(CPUState *cpu,
   }

   /* Move the TB to the head of the list */
-*ptb1 = tb->phys_hash_next;
-tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h];
-tcg_ctx.tb_ctx.tb_phys_hash[h] = tb;
+//*ptb1 = tb->phys_hash_next;
+//tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h];
+//tcg_ctx.tb_ctx.tb_phys_hash[h] = tb;
   return tb;
   }

Hmm not in my build cpu_exec:

 ...
 tb_lock();
 tb = tb_find_fast(cpu);
 ...

Which I think is right. I mean I can see if it wasn't then breakage
could occur when you manipulate the lookup but I think we should keep
the lock there and if it proves to be a performance hit come up with a
safe optimisation. I think Paolo talked about using RCU type locks.

That's definitely a performance hit.
Ok we should talk about that Monday.

Fred


--
Alex Bennée

Re: [Qemu-devel] Using directory as initrd

[KONRAD Frederic]

Hi Kasper,

The normal approach is to use git send email to send your patch on the list
instead of attaching the patch to the email so people can do inline comment.

You can use scripts/checkpatch.pl to check the coding style before sending.
(eg: you miss the signed off line and the description in the patch).

And better use the qemu master branch (you seems to be using qemu-2.0?)

Fred



Le 17/01/2016 22:04, Kasper Dupont a écrit :

I would like to use a directory as initrd file without
having to write it to an initrd file each time I have
changed anything in that directory.

I have written code to pipe an initrd directly from cpio
to qemu. Do you have any feedback on the attached patch?

Re: [Qemu-devel] Integrating a Memory Simulator

[KONRAD Frederic]

Hi,

Why do you target the user mode for that?
I think it might be easier with the softmmu.
But depending on what you want to do it won't be so easy.

I don't think there are any transaction to read the code basically the 
RAM MemoryRegion works with pointer.
You can create a MemoryRegion which will just create a transaction and 
passes that to your model but you won't

be able to execute code from it. So you would need to modify that.
But at the end you won't have the same access like in the real HW 
because cache are not modelled in QEMU and

it will be really slow :).

Fred

Le 22/02/2016 01:26, Hao Bai a écrit :

Hi All,

I was trying to integrate the DRAMSim2 memory simulator [1] into QEMU. 
Basically I wanted to modify the current memory interface of QEMU so 
that all memory accesses will be directed to DRAMSim2. Can anyone give 
me hints/comments/thoughts on how to do this? I am targeting x86-64 
architecture in user mode.


[1] Repository: https://github.com/dramninjasUMD/DRAMSim2
Paper: https://www.ece.umd.edu/~blj/papers/cal10-1.pdf 



Cheers

Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus.

[Konrad Frederic]

On 23/11/2012 13:08, Cornelia Huck wrote:

On Thu, 22 Nov 2012 15:50:50 +0100
fred.kon...@greensocs.com wrote:



+/* Create a virtio bus.  */
+VirtioBus *virtio_bus_new(DeviceState *host, const VirtioBusInfo *info)
+{
+/*
+ * This is needed, as we want to have different names for each virtio-bus.
+ * If we don't do that, we can't add more than one VirtIODevice.
+ */
+static int next_virtio_bus;
+char *bus_name = g_strdup_printf("virtio-bus.%d", next_virtio_bus++);

This still has the overflow/id-reuse problem, hasn't it?

What do you mean by overflow problem ?




+
+BusState *qbus = qbus_create(TYPE_VIRTIO_BUS, host, bus_name);
+VirtioBus *bus = VIRTIO_BUS(qbus);
+bus->info = info;
+qbus->allow_hotplug = 0;
+bus->bus_in_use = false;
+DPRINTF("%s bus created\n", bus_name);
+return bus;
+}

Don't you need a way to destroy the bus again when the proxy device is
hotunplugged?

Yes, you're right I must add a way to destroy the bus, and the devices 
when the proxy is hot unplugged!


Thanks,

Fred

Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus.

[Konrad Frederic]

On 23/11/2012 13:23, Stefan Hajnoczi wrote:

On Thu, Nov 22, 2012 at 03:50:50PM +0100, fred.kon...@greensocs.com wrote:

+/* Bind the VirtIODevice to the VirtioBus. */
+void virtio_bus_bind_device(VirtioBus *bus)
+{
+BusState *qbus = BUS(bus);
+assert(bus != NULL);
+assert(bus->vdev != NULL);
+virtio_bind_device(bus->vdev,&(bus->info->virtio_bindings), qbus->parent);
+}

Should plug and bind be together in a single function?  Binding the
device seems like an internal step that the bus takes when you plug the
device.
Maybe we can, but we must init the "transport" device before bind the 
device.





+struct VirtioBusInfo {

This is defining an ad-hoc interface.  QOM has support for interfaces so
that a virtio-pci adapter brovides a VirtioBindingInterface which
VirtioBus can talk to intead of using VirtioBusInfo.
Can you point me to example in the tree to see how this QOM interfaces 
work ?



+void (*init_cb)(DeviceState *dev);
+void (*exit_cb)(DeviceState *dev);

Can _cb be dropped from the name?  Structs with function pointers always
provide "callbacks" so the _cb is unnecessary.  For example, QOM methods
like BusClass->reset() don't include _cb either.

Ok.

+VirtIOBindings virtio_bindings;
+};
+
+struct VirtioBus {
+BusState qbus;
+bool bus_in_use;

Should bus_in_use basically be bus->vdev != NULL?

Yes I can do that. :).

Thanks,

Fred

Re: [Qemu-devel] [RFC PATCH v2 2/3] virtio-pci : add a virtio-bus interface

[Konrad Frederic]

On 23/11/2012 13:34, Peter Maydell wrote:

On 23 November 2012 12:29, Stefan Hajnoczi  wrote:

On Thu, Nov 22, 2012 at 03:50:51PM +0100, fred.kon...@greensocs.com wrote:

+static const struct VirtioBusInfo virtio_bus_info = {
+.init_cb = virtio_pci_init_cb,
+.exit_cb = virtio_pci_exit_cb,
+
+.virtio_bindings = {

Eventually VirtIOBindings can probably be inlined into VirtioBusInfo.  I
don't see a need for separate structs.

I agree. It might (or might not) be convenient to retain it
temporarily while converting all the transports, but
VirtIOBindings is part of the old code which we're trying
to refactor here, and I'd expect it to go away when we're done.

-- PMM
Yes, for the moment, I didn't refactor this VirtIOBindings, so it is 
better to separate struct to keep the virtiodevice binding function.

Re: [Qemu-devel] [RFC PATCH v2 0/3] Virtio-refactoring.

[Konrad Frederic]

On 23/11/2012 13:38, Stefan Hajnoczi wrote:

On Thu, Nov 22, 2012 at 03:50:49PM +0100, fred.kon...@greensocs.com wrote:

From: KONRAD Frederic
I made the changes you suggest in the last RFC.

There are still two issues with the command line :

 * When I use ./qemu* -device virtio-blk -device virtio-pci
   It is said that no virtio-bus are present.
 * The virtio-blk is plugged in the last created virtio-bus if no "bus="
   option is present. It's an issue as we can only plug one virtio-device.

The first problem is a more general issue as it is the case for the SCSI bus and
can be fixed later.

Thanks for sharing virtio refactoring progress.

I think the challenge will be truly converting existing code over to the
new approach.  This RFC series adds a new layer on top of the existing
code but doesn't actually replace it.

Would be interesting to see the complete picture, even if you need to
leave some TODOs in the middle when sending RFC patches.

Stefan

Yes, sure.

So the next would be :
* use QOM interface in place of VirtioBusInfo ?
* refactor the VirtIODevice to remove VirtIOBinding ?

I though modifying the less I can the VirtIODevice as it could break all 
the s390 devices.


Fred

Re: [Qemu-devel] [RFC PATCH v2 2/3] virtio-pci : add a virtio-bus interface

[Konrad Frederic]

On 23/11/2012 15:26, Peter Maydell wrote:

On 23 November 2012 14:23, Konrad Frederic  wrote:

On 23/11/2012 13:34, Peter Maydell wrote:

On 23 November 2012 12:29, Stefan Hajnoczi   wrote:

Eventually VirtIOBindings can probably be inlined into VirtioBusInfo.  I
don't see a need for separate structs.

I agree. It might (or might not) be convenient to retain it
temporarily while converting all the transports, but
VirtIOBindings is part of the old code which we're trying
to refactor here, and I'd expect it to go away when we're done.

Yes, for the moment, I didn't refactor this VirtIOBindings, so it
is better to separate struct to keep the virtiodevice binding function.

Where you're deliberately not changing something as a temporary
step you need to comment it to make that clear. Otherwise
people trying to review the code won't be able to tell...

-- PMM

Ok, sorry for that.

Fred

Re: [Qemu-devel] [RFC PATCH v2 0/3] Virtio-refactoring.

[Konrad Frederic]

On 23/11/2012 17:18, Stefan Hajnoczi wrote:

On Fri, Nov 23, 2012 at 03:29:52PM +0100, Konrad Frederic wrote:

On 23/11/2012 13:38, Stefan Hajnoczi wrote:

On Thu, Nov 22, 2012 at 03:50:49PM +0100, fred.kon...@greensocs.com wrote:

From: KONRAD Frederic
I made the changes you suggest in the last RFC.

There are still two issues with the command line :

 * When I use ./qemu* -device virtio-blk -device virtio-pci
   It is said that no virtio-bus are present.
 * The virtio-blk is plugged in the last created virtio-bus if no "bus="
   option is present. It's an issue as we can only plug one virtio-device.

The first problem is a more general issue as it is the case for the SCSI bus and
can be fixed later.

Thanks for sharing virtio refactoring progress.

I think the challenge will be truly converting existing code over to the
new approach.  This RFC series adds a new layer on top of the existing
code but doesn't actually replace it.

Would be interesting to see the complete picture, even if you need to
leave some TODOs in the middle when sending RFC patches.

Stefan

Yes, sure.

So the next would be :
 * use QOM interface in place of VirtioBusInfo ?

This is probably a detail.  It probably doesn't make a big difference in
the RFC series.


 * refactor the VirtIODevice to remove VirtIOBinding ?

Yes, it would be nice to make your design the core and push the
compatibility stuff ("virtio-blk-pci", etc) to a layer on top, instead
of leaving the existing code as the core and putting QOM on top.

Ok, I'll do that.

I though modifying the less I can the VirtIODevice as it could break
all the s390 devices.

Sure and I think you've done a good job at showing incremental patches
vs a big bang approach that replaces everything in one patch.

Stefan

Thanks,

Fred

Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus.

[Konrad Frederic]

On 23/11/2012 13:08, Cornelia Huck wrote:

On Thu, 22 Nov 2012 15:50:50 +0100
fred.kon...@greensocs.com wrote:



+/* Create a virtio bus.  */
+VirtioBus *virtio_bus_new(DeviceState *host, const VirtioBusInfo *info)
+{
+/*
+ * This is needed, as we want to have different names for each virtio-bus.
+ * If we don't do that, we can't add more than one VirtIODevice.
+ */
+static int next_virtio_bus;
+char *bus_name = g_strdup_printf("virtio-bus.%d", next_virtio_bus++);

This still has the overflow/id-reuse problem, hasn't it?


+
+BusState *qbus = qbus_create(TYPE_VIRTIO_BUS, host, bus_name);
+VirtioBus *bus = VIRTIO_BUS(qbus);
+bus->info = info;
+qbus->allow_hotplug = 0;
+bus->bus_in_use = false;
+DPRINTF("%s bus created\n", bus_name);
+return bus;
+}

Don't you need a way to destroy the bus again when the proxy device is
hotunplugged?


Is the virtio-pci-* proxy currently supporting hotunplugging ?

Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus.

[Konrad Frederic]

On 26/11/2012 15:33, Anthony Liguori wrote:

fred.kon...@greensocs.com writes:


From: KONRAD Frederic

This patch create a new VirtioBus, which can be added to Virtio transports like
virtio-pci, virtio-mmio,...

One VirtIODevice can be connected to this device, like virtio-blk in the 3rd
patch.

The VirtioBus shares through a VirtioBusInfo structure :

 * two callbacks with the transport : init_cb and exit_cb, which must be
   called by the VirtIODevice, after the initialization and before the
   destruction, to put the right PCI IDs and/or stop the event fd.

 * a VirtIOBindings structure.

Signed-off-by: KONRAD Frederic
---
  hw/Makefile.objs |   1 +
  hw/virtio-bus.c  | 148 +++
  hw/virtio-bus.h  |  58 ++
  3 files changed, 207 insertions(+)
  create mode 100644 hw/virtio-bus.c
  create mode 100644 hw/virtio-bus.h

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index ea46f81..bd14d1b 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -2,6 +2,7 @@ common-obj-y = usb/ ide/
  common-obj-y += loader.o
  common-obj-$(CONFIG_VIRTIO) += virtio-console.o
  common-obj-$(CONFIG_VIRTIO) += virtio-rng.o
+common-obj-$(CONFIG_VIRTIO) += virtio-bus.o
  common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
  common-obj-y += fw_cfg.o
  common-obj-$(CONFIG_PCI) += pci.o pci_bridge.o pci_bridge_dev.o
diff --git a/hw/virtio-bus.c b/hw/virtio-bus.c
new file mode 100644
index 000..991b6f5
--- /dev/null
+++ b/hw/virtio-bus.c
@@ -0,0 +1,148 @@
+/*
+ * VirtioBus
+ *
+ *  Copyright (C) 2012 : GreenSocs Ltd
+ *  http://www.greensocs.com/ , email: i...@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU 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<http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "hw.h"
+#include "qemu-error.h"
+#include "qdev.h"
+#include "virtio-bus.h"
+#include "virtio.h"
+
+#define DEBUG_VIRTIO_BUS 1
+
+#define DPRINTF(fmt, ...) if (DEBUG_VIRTIO_BUS) {\
+printf("virtio_bus: " fmt , ## __VA_ARGS__); \
+  }

#ifdef DEBUG_VIRTIO_BUS
#define DPRINTF(fmt, ...) ...
#else
#define DPRINTF(fmt, ...) do { } while (0)
#endif

You're leaving a dangling if clause which can do very strange things if
used before an else statement.


+
+static void virtio_bus_init_cb(VirtioBus *bus);
+static int virtio_bus_reset(BusState *qbus);

You should rearrange the code to avoid forward declarations of static functions.


+
+static void virtio_bus_class_init(ObjectClass *klass, void *data)
+{
+BusClass *k = BUS_CLASS(klass);
+k->reset = virtio_bus_reset;
+}
+
+static TypeInfo virtio_bus_info = {
+.name = TYPE_VIRTIO_BUS,
+.parent = TYPE_BUS,
+.instance_size = sizeof(VirtioBus),
+.class_init = virtio_bus_class_init,
+};
+
+/* Reset the bus */
+static int virtio_bus_reset(BusState *qbus)
+{
+VirtioBus *bus = VIRTIO_BUS(qbus);
+if (bus->bus_in_use) {
+virtio_reset(bus->vdev);
+}
+return 1;
+}
+
+/* Plug the VirtIODevice */
+int virtio_bus_plug_device(VirtioBus *bus, VirtIODevice *vdev)
+{
+BusState *qbus = BUS(bus);
+/*
+ * This is a problem, when bus= option is not set, the last created
+ * virtio-bus is used. So it give the following error.
+ */
+DPRINTF("plug device into %s.\n", qbus->name);
+if (bus->bus_in_use) {
+error_report("%s in use.\n", qbus->name);
+return -1;
+}
+bus->bus_in_use = true;
+
+/* keep the VirtIODevice in the VirtioBus. */
+bus->vdev = vdev;
+
+/* call the "transport" callback. */
+virtio_bus_init_cb(bus);
+return 0;
+}

I think the desired semantics here could be achieved by modifying
qbus_find_recursive() to take an optional argument which then causes it
to only return a "!full" bus.  You can then add a member to BusState to
indicate whether the bus is full or not.

You can then set the parameter qdev_device_add() and it should Just Work.
Ok, so I need to modify qdev_monitor.c to add an option ( in command 
line ) to set this optional argument ?



+
+/* Create a virtio bus.  */
+VirtioBus *virtio_bus_new(DeviceState *host, const VirtioBusInfo *info)
+{
+/*
+ * This is needed, as we want to h

Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus.

[Konrad Frederic]

On 26/11/2012 17:59, Anthony Liguori wrote:

Peter Maydell  writes:


On 26 November 2012 14:33, Anthony Liguori  wrote:

VirtioBusInfo is not a great name.  This is a proxy class that allows
for a device to implement the virtio bus interface.

This could be done as an interface but since nothing else uses
interfaces, I'm okay with something like this.  But the first argument
ought to be an opaque for all methods.

We have at least one user of Interface in the tree IIRC.
I'd much rather we did this the right way -- the only reason
it's the way Fred has coded it is that there's no obvious
body of code in the tree to copy, so we're thrashing around
a bit. If you tell us what the correct set of structs/classes/
interfaces/etc is then we can implement it :-)

I really think extending virtio-bus to a virtio-pci-bus and then
initializing it with a link to the PCI device is the best approach.

It's by far the simpliest approach in terms of coding.

Did I explain it adequately?  To recap:

virtio-bus extends bus-state
  - implements everything that VirtIOBindings implements as methods

virtio-pci-bus extends virtio-bus
  - is constructed with a pointer to a PCIDevice
  - implements the methods necessary to be a virtio bus

I still have trouble with that ^^.

virtio-pci-bus extends virtio-bus so I put something like that :

static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
{
BusClass *bc = BUS_CLASS(klass);
VirtioBusClass *k = VIRTIO_BUS_CLASS(klass);
/* VirtIOBindings */
k->notify = virtio_pci_notify;
k->save_config = virtio_pci_save_config;
k->load_config = virtio_pci_load_config;
k->save_queue = virtio_pci_save_queue;
k->load_queue = virtio_pci_load_queue;
k->get_features = virtio_pci_get_features;
k->query_guest_notifiers = virtio_pci_query_guest_notifiers;
k->set_host_notifier = virtio_pci_set_host_notifier;
k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
k->vmstate_change = virtio_pci_vmstate_change;
/*
 * TODO : Init and exit function.
 * void (*init)(void *opaque);
 * void (*exit)(void *opaque);
 */
}

static TypeInfo virtio_pci_bus_info = {
.name  = TYPE_VIRTIO_PCI_BUS,
.parent= TYPE_VIRTIO_BUS,
.class_init= virtio_pci_bus_class_init,
};

and I have VirtioDevice which extends DeviceState like that :

static void virtio_device_class_init(ObjectClass *klass, void *data)
{
/* Set the default value here. */
DeviceClass *dc = DEVICE_CLASS(klass);
dc->bus_type = TYPE_VIRTIO_BUS;
dc->init = virtio_device_init;
}

static TypeInfo virtio_device_info = {
.name = TYPE_VIRTIO_DEVICE,
.parent = TYPE_DEVICE,
.instance_size = sizeof(VirtioDeviceState),
/* Abstract the virtio-device */
.class_init = virtio_device_class_init,
.abstract = true,
.class_size = sizeof(VirtioDeviceClass),
};

The problem is that the virtio devices can't be connected to the 
virtio-pci-bus even if it extends virtio-bus because TYPE_VIRTIO_BUS != 
TYPE_VIRTIO_PCI_BUS.


Did I miss something ?

Thanks,

Fred



virtio-device extends device-state
  - implements methods used by virtio-bus

Regards,

Anthony Liguori


-- PMM

Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus.

[Konrad Frederic]

On 29/11/2012 14:09, Peter Maydell wrote:

On 29 November 2012 12:37, Konrad Frederic  wrote:

On 26/11/2012 17:59, Anthony Liguori wrote:

virtio-pci-bus extends virtio-bus
   - is constructed with a pointer to a PCIDevice
   - implements the methods necessary to be a virtio bus

I still have trouble with that ^^.
The problem is that the virtio devices can't be connected to the
virtio-pci-bus even if it extends virtio-bus because TYPE_VIRTIO_BUS !=
TYPE_VIRTIO_PCI_BUS.

Conceptually it ought to work I think: if the bus is-a TYPE_VIRTIO_BUS
then we should permit plugging in even if the actual bus object happens
to be an instance of a subclass.

Yes, is my implementation doing the right thing ?
I mean is the virtio-pci-bus a virtio-bus ?



I suspect that qbus_find_recursive should be doing an
object_class_dynamic_cast() to check that the bus is of a suitable
type, rather than the
 (strcmp(object_get_typename(OBJECT(bus)), bus_typename) != 0)
which it does at the moment.

Yes, but we can cast VIRTIO_BUS in BUS no ?
So in this case we could plug VirtioDevice in BUS and that's not what we 
want ?




-- PMM

Re: [Qemu-devel] [PATCH] qdev: relax bus type check in qdev_device_add()

[Konrad Frederic]

On 29/11/2012 14:56, Peter Maydell wrote:

On 29 November 2012 13:49, Anthony Liguori  wrote:

We are currently checking for an exact type match.  Use QOM dynamic_cast to
check for a compatible type instead.

I think this only catches the case where a bus was explicitly
specified via bus=. For the default case you also need to change
the similar code for checking the bus type in qbus_find_recursive(),
right?

-- PMM

Right, it's working only with the "bus=" command line.

Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus.

[Konrad Frederic]

On 29/11/2012 14:55, Andreas Färber wrote:

Am 29.11.2012 14:47, schrieb Konrad Frederic:

On 29/11/2012 14:09, Peter Maydell wrote:

On 29 November 2012 12:37, Konrad Frederic
wrote:

On 26/11/2012 17:59, Anthony Liguori wrote:

virtio-pci-bus extends virtio-bus
- is constructed with a pointer to a PCIDevice
- implements the methods necessary to be a virtio bus

I still have trouble with that ^^.
The problem is that the virtio devices can't be connected to the
virtio-pci-bus even if it extends virtio-bus because TYPE_VIRTIO_BUS !=
TYPE_VIRTIO_PCI_BUS.

Conceptually it ought to work I think: if the bus is-a TYPE_VIRTIO_BUS
then we should permit plugging in even if the actual bus object happens
to be an instance of a subclass.

Yes, is my implementation doing the right thing ?
I mean is the virtio-pci-bus a virtio-bus ?

In your v3 patchset no. In the inline code yes, via
virtio_pci_bus_info's .parent.


I suspect that qbus_find_recursive should be doing an
object_class_dynamic_cast() to check that the bus is of a suitable
type, rather than the
  (strcmp(object_get_typename(OBJECT(bus)), bus_typename) != 0)
which it does at the moment.

Yes, but we can cast VIRTIO_BUS in BUS no ?
So in this case we could plug VirtioDevice in BUS and that's not what we
want ?

You would want to check whether object_class_dynamic_cast(OBJECT(bus),
TYPE_VIRTIO_BUS) == NULL. This should evaluate to true if the bus is not
a virtio-bus.

Yes, ok I got it!

Thanks,

Fred

Re: [Qemu-devel] [PATCH] qdev: relax bus type check in qdev_device_add() (v2)

[Konrad Frederic]

On 29/11/2012 16:12, Anthony Liguori wrote:

We are currently checking for an exact type match.  Use QOM dynamic_cast to
check for a compatible type instead.

Cc: Konrad Frederic
Cc: Peter Maydell
Signed-off-by: Anthony Liguori
---
v1 ->  v2:
  - also add cast to qbus_find_recursive (Peter)
  - simplify by doing object_dynamic_cast instead of messing with classes
---
  hw/qdev-monitor.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 479eecd..a1b4d6a 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -289,8 +289,7 @@ static BusState *qbus_find_recursive(BusState *bus, const 
char *name,
  if (name&&  (strcmp(bus->name, name) != 0)) {
  match = 0;
  }
-if (bus_typename&&
-(strcmp(object_get_typename(OBJECT(bus)), bus_typename) != 0)) {
+if (bus_typename&&  !object_dynamic_cast(OBJECT(bus), bus_typename)) {
  match = 0;
  }
  if (match) {
@@ -435,7 +434,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
  if (!bus) {
  return NULL;
  }
-if (strcmp(object_get_typename(OBJECT(bus)), k->bus_type) != 0) {
+if (!object_dynamic_cast(OBJECT(bus), k->bus_type)) {
  qerror_report(QERR_BAD_BUS_FOR_DEVICE,
driver, object_get_typename(OBJECT(bus)));
  return NULL;

That seems to work.

Thanks,

Fred.

Re: [Qemu-devel] [RFC] [tcg] Idea on refactoring target code generation loop (gen_intermediate_code)

[KONRAD Frederic]

Hi,

Le 09/03/2016 16:52, Richard Henderson a écrit :

On 03/09/2016 09:38 AM, Lluís Vilanova wrote:

Hi,

NOTE: I won't be throwing patches anytime soon, I just want to know 
if there's

   interest in this for the future.

While adding events for tracing guest instructions, I've found that the
per-target "gen_intermediate_code()" function is very similar but not 
exactly
the same for each of the targets. This makes architecture-agnostic 
features
harder to maintain across targets, specially when it comes to their 
relative

order.

So, would it be worth it if I generalized part of that code into an
architecture-agnostic function that calls into target-specific hooks 
wherever it

needs extending? There are many ways to do it that we can discuss later.


It's worth talking about, since I do believe it would make long-term 
maintenance across the targets easier.


These "target-specific hooks" probably ought not be "hooks" in the
traditional sense of attaching them to CPUState.  I'd be more 
comfortable with a refactoring that used include files -- maybe .h or 
maybe .inc.c.  If we do the normal sort of hook, then we've got to 
either expose DisasContext in places we shouldn't, or dynamically 
allocate it.  Neither seems particularly appealing.



r~



On the other side I think attaching them to CPUState would make 
heterogenous system emulation easier?


Fred

Re: [Qemu-devel] [RFC v2 08/11] tcg: add options for enabling MTTCG

[KONRAD Frederic]

Le 12/04/2016 14:42, Sergey Fedorov a écrit :

On 12/04/16 14:59, Peter Maydell wrote:

On 12 April 2016 at 12:48, Alex Bennée  wrote:

Sergey Fedorov  writes:

Maybe we'd better use existing qemu accelerator framework and introduce
"-machine accel=mttcg" option?

My worry would be breaking existing code which assumes kvm | tcg. We
will want to enable mttcg by default for combos that work without having
to tweak tooling that already uses the -machine options.

Comments from the peanut gallery:
  * We definitely want to just default to MTTCG where it works:
users shouldn't have to add new command line switches to get the
better performance etc, because in practice that won't happen
  * Anything that adds a new command line option at all has to
be supported practically forever, so options that are only
useful for a transitional period are worth thinking twice about


Yes, but users may like to have an option to disable MTTCG for some
reason. I'm also concerned about icount mode: not sure how to account
virtual time when all vCPUs run in parallel.



Hi,

I'm thinking the same, we don't have a solution for icount yet.
The reverse execution support is probably badly broken as well.

Fred


Kind regards,
Sergey

Re: [Qemu-devel] [PATCH v3 04/11] include/processor.h: define cpu_relax()

[KONRAD Frederic]

Le 20/04/2016 01:07, Emilio G. Cota a écrit :

Taken from the linux kernel.

Reviewed-by: Richard Henderson 
Signed-off-by: Emilio G. Cota 
---
  include/qemu/processor.h | 28 
  1 file changed, 28 insertions(+)
  create mode 100644 include/qemu/processor.h

diff --git a/include/qemu/processor.h b/include/qemu/processor.h
new file mode 100644
index 000..675a00a
--- /dev/null
+++ b/include/qemu/processor.h
@@ -0,0 +1,28 @@


Hi,

Does this file need a license?

Fred


+#ifndef QEMU_PROCESSOR_H
+#define QEMU_PROCESSOR_H
+
+#include "qemu/atomic.h"
+
+#if defined(__i386__) || defined(__x86_64__)
+#define cpu_relax() asm volatile("rep; nop" ::: "memory")
+#endif
+
+#ifdef __ia64__
+#define cpu_relax() asm volatile("hint @pause" ::: "memory")
+#endif
+
+#ifdef __aarch64__
+#define cpu_relax() asm volatile("yield" ::: "memory")
+#endif
+
+#if defined(__powerpc64__)
+/* set Hardware Multi-Threading (HMT) priority to low; then back to medium */
+#define cpu_relax() asm volatile("or 1, 1, 1;"
+ "or 2, 2, 2;" ::: "memory")
+#endif
+
+#ifndef cpu_relax
+#define cpu_relax() barrier()
+#endif
+
+#endif /* QEMU_PROCESSOR_H */

Re: [Qemu-devel] run qemu with x86_64 arch (host arch)

[KONRAD Frederic]

Le 19/04/2016 23:10, Marwa Hamza a écrit :

hello
i'm trying to run qemu with x86_64 arch
i prepared file system with busybox
i got that elf file
_install/bin/busybox: format de fichier elf64-x86-64
architecture: i386:x86-64, fanions 0x0102:
EXEC_P, D_PAGED
adresse de départ 0x00522024
and i compiled linux kernel
make ARCH=x86_64 x86_64_defconfig
make ARCH=x86_64
and i configured qemu
./configure --target-list=x86_64-softmmu --disable-vnc --enable-sdl
--disable-kvm --enable-debug
make
then when i run qemu i got a kernel panic _ not syncing vfs :unable to
mount rootfs on unknown block(1,0)
./x86_64-softmmu/qemu-system-x86_64  -M pc -kernel
/linux-4.1.18/arch/x86_64/boot/bzImage -initrd
/busybox-1.21.0/rootfs.img.gz -append "root=/dev/ram rdinit=/bin/sh"
thanks


Hi,

The complete end of the log would help, but I might have two clues for you:

Do you have enabled ramdisk support in your kernel configuration?
What is the format of rootfs.img.gz? Is that supported by your kernel?

Fred

Re: [Qemu-devel] MTTCG Sync-up call today?

[KONRAD Frederic]

Hi Alex,

I can't do the call today, sorry.

Thanks,
Fred

Le 09/05/2016 13:56, Alex Bennée a écrit :


Hi,

Do we have anything we want to discuss today?

--
Alex Bennée

Re: [Qemu-devel] [RFC v1 09/11] tcg: drop global lock during TCG code execution

[KONRAD Frederic]

Hi Alex,

Thanks for having pulling all that together!
About this patch the original author was Jan Kiszka 
(https://lists.gnu.org/archive/html/qemu-devel/2011-09/msg03323.html)


This has probably been dropped during a rebase.

Thanks,
Fred

Le 18/03/2016 17:18, Alex Bennée a écrit :

From: KONRAD Frederic 

This finally allows TCG to benefit from the iothread introduction: Drop
the global mutex while running pure TCG CPU code. Reacquire the lock
when entering MMIO or PIO emulation, or when leaving the TCG loop.

We have to revert a few optimization for the current TCG threading
model, namely kicking the TCG thread in qemu_mutex_lock_iothread and not
kicking it in qemu_cpu_kick. We also need to disable RAM block
reordering until we have a more efficient locking mechanism at hand.

Still, a Linux x86 UP guest and my Musicpal ARM model boot fine here.
These numbers demonstrate where we gain something:

20338 jan   20   0  331m  75m 6904 R   99  0.9   0:50.95 qemu-system-arm
20337 jan   20   0  331m  75m 6904 S   20  0.9   0:26.50 qemu-system-arm

The guest CPU was fully loaded, but the iothread could still run mostly
independent on a second core. Without the patch we don't get beyond

32206 jan   20   0  330m  73m 7036 R   82  0.9   1:06.00 qemu-system-arm
32204 jan   20   0  330m  73m 7036 S   21  0.9   0:17.03 qemu-system-arm

We don't benefit significantly, though, when the guest is not fully
loading a host CPU.

Message-Id: <1439220437-23957-10-git-send-email-fred.kon...@greensocs.com>
Signed-off-by: KONRAD Frederic 
[AJB: -smp single-threaded fix, rm old info from commit msg]
Signed-off-by: Alex Bennée 

---
v1 (ajb):
   - SMP failure now fixed by previous commit
Changes from Fred Konrad (mttcg-v7 via paolo):
   * Rebase on the current HEAD.
   * Fixes a deadlock in qemu_devices_reset().
   * Remove the mutex in address_space_*
---
  cpu-exec.c | 10 ++
  cpus.c | 26 +++---
  cputlb.c   |  4 
  exec.c | 12 
  hw/i386/kvmvapic.c |  3 +++
  softmmu_template.h | 17 +
  translate-all.c| 11 +--
  7 files changed, 58 insertions(+), 25 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 3572256..76891fd 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -28,6 +28,7 @@
  #include "qemu/rcu.h"
  #include "exec/tb-hash.h"
  #include "exec/log.h"
+#include "qemu/main-loop.h"
  #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
  #include "hw/i386/apic.h"
  #endif
@@ -449,6 +450,13 @@ int cpu_exec(CPUState *cpu)
  for(;;) {
  interrupt_request = cpu->interrupt_request;
  if (unlikely(interrupt_request)) {
+/* FIXME: this needs to take the iothread lock.
+ * For this we need to find all places in
+ * cc->cpu_exec_interrupt that can call cpu_loop_exit,
+ * and call qemu_unlock_iothread_mutex() there.  Else,
+ * add a flag telling cpu_loop_exit() to unlock it.
+ */
+
  if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
  /* Mask out external interrupts for this step. */
  interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
@@ -501,6 +509,7 @@ int cpu_exec(CPUState *cpu)
 the program flow was changed */
  next_tb = 0;
  }
+
  }
  if (unlikely(cpu->exit_request
   || replay_has_interrupt())) {
@@ -618,6 +627,7 @@ int cpu_exec(CPUState *cpu)
  g_assert(env == &x86_cpu->env);
  #endif
  #endif /* buggy compiler */
+
  cpu->can_do_io = 1;
  tb_lock_reset();
  }
diff --git a/cpus.c b/cpus.c
index a87fbf9..0e3d5f9 100644
--- a/cpus.c
+++ b/cpus.c
@@ -911,8 +911,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *cpu)
  #endif /* _WIN32 */
  
  static QemuMutex qemu_global_mutex;

-static QemuCond qemu_io_proceeded_cond;
-static unsigned iothread_requesting_mutex;
  
  static QemuThread io_thread;
  
@@ -928,7 +926,6 @@ void qemu_init_cpu_loop(void)

  qemu_cond_init(&qemu_cpu_cond);
  qemu_cond_init(&qemu_pause_cond);
  qemu_cond_init(&qemu_work_cond);
-qemu_cond_init(&qemu_io_proceeded_cond);
  qemu_mutex_init(&qemu_global_mutex);
  
  qemu_thread_get_self(&io_thread);

@@ -1043,10 +1040,6 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)
  qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex);
  }
  
-while (iothread_requesting_mutex) {

-qemu_cond_wait(&qemu_io_proceeded_cond, &qemu_global_mutex);
-}
-
  CPU_FOREACH(cpu) {
  qemu_wait_io_event_common(cpu);
  }
@@ -1314,22 +1307,7 @@ 

[Qemu-devel] [RFC] A clock framework in QEMU.

[KONRAD Frederic]

Hi,

We would like to have a way to have a clock tree inside QEMU:
  * models can have clock outputs and/or clock inputs.
  * changing the clock rate of propagates in the clock tree through
callbacks which will be implemented in the model (eg: like qemu_irq)
  * would be nice to be able to visualize the rate of a clock in the
monitor.

There is already an implementation in QEMU (in omap*) but:
  * it's not generic/usable in the whole QEMU tree.
  * it's not using QOM.

So the proposition are either to construct one new framework or to 
extract and reuse the old one:

  * new types must be created eg: qemu_clk_in, qemu_clk_out.
  * I think it shouldn't use qemu_irq (because this is confusing) but
maybe use a simple qom link to bound them.
  * The model which have the clock input will need to implement the
clock update/enable/disable callback.

So for example PLL or some clock gate units will just have one input and 
some outputs.

Then the outputs can be controlled by the output callbacks (for example
the input rate change or the clock is gated).

Does that makes sense?
Do you have any opinion about that?

Thanks,
Fred

Re: [Qemu-devel] GUI for peripherals

[KONRAD Frederic]

Hi Karthik,

I think Liviu did something about this:
https://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg03825.html

Fred

Le 30/05/2016 à 08:25, Karthik a écrit :

Hello,

What would be the best way to provide GUI for peripheral control?
This is not for linux rather than embedded system running home brew rtos.

For example an GPIO module, I would need an UI to toggle an GPIO and see
the output, like press an button and simulate it turning on an LED.

Would it be possible to write the GUI separately and use QMP to communicate
with my custom driver to control peripherals? or is there an better way to
do this?

Best regards,
Karthik

Re: [Qemu-devel] [RFC] A clock framework in QEMU.

[KONRAD Frederic]

Hi Peter,

Do you agree with this? I think you are the hw/*/omap* maintainer?

Thanks,
Fred

Le 31/05/2016 à 21:08, KONRAD Frederic a écrit :

Hi,

We would like to have a way to have a clock tree inside QEMU:
  * models can have clock outputs and/or clock inputs.
  * changing the clock rate of propagates in the clock tree through
callbacks which will be implemented in the model (eg: like qemu_irq)
  * would be nice to be able to visualize the rate of a clock in the
monitor.

There is already an implementation in QEMU (in omap*) but:
  * it's not generic/usable in the whole QEMU tree.
  * it's not using QOM.

So the proposition are either to construct one new framework or to
extract and reuse the old one:
  * new types must be created eg: qemu_clk_in, qemu_clk_out.
  * I think it shouldn't use qemu_irq (because this is confusing) but
maybe use a simple qom link to bound them.
  * The model which have the clock input will need to implement the
clock update/enable/disable callback.

So for example PLL or some clock gate units will just have one input and
some outputs.
Then the outputs can be controlled by the output callbacks (for example
the input rate change or the clock is gated).

Does that makes sense?
Do you have any opinion about that?

Thanks,
Fred

Re: [Qemu-devel] [RFC] A clock framework in QEMU.

[KONRAD Frederic]

Le 02/06/2016 à 12:01, Edgar E. Iglesias a écrit :

On Tue, May 31, 2016 at 09:08:14PM +0200, KONRAD Frederic wrote:

Hi,


Hi Fred,



We would like to have a way to have a clock tree inside QEMU:
  * models can have clock outputs and/or clock inputs.
  * changing the clock rate of propagates in the clock tree through
callbacks which will be implemented in the model (eg: like qemu_irq)
  * would be nice to be able to visualize the rate of a clock in the
monitor.


This sounds good to me. Allthough it may be obvious, you may want
to be explicit (in future docs) in that clock pin toggles are not
going to be modelled. Only meta-information about frequencies.


Yes I'll explicit that.





There is already an implementation in QEMU (in omap*) but:
  * it's not generic/usable in the whole QEMU tree.
  * it's not using QOM.

So the proposition are either to construct one new framework or to extract
and reuse the old one:
  * new types must be created eg: qemu_clk_in, qemu_clk_out.
  * I think it shouldn't use qemu_irq (because this is confusing) but
maybe use a simple qom link to bound them.
  * The model which have the clock input will need to implement the
clock update/enable/disable callback.


It'll be easier to comment on this when you have some RFC code but it may
be enough with a clk_update callback. A freq of 0 is a disabled clock.
I may be stating the obvious again but note that devices may have
multiple clk inputs and multiple clock outputs. It would be nice if
these are named aswell (and not just indexed).


The omap clock structure have some kind of disabled member. But I agree 
it makes sense to say that a zero frequence is a disabled clock.


Thanks,
Fred



Thanks and best regards,
Edgar



So for example PLL or some clock gate units will just have one input and
some outputs.
Then the outputs can be controlled by the output callbacks (for example
the input rate change or the clock is gated).

Does that makes sense?
Do you have any opinion about that?

Re: [Qemu-devel] [PATCH V8 4/9] introduce aux-bus

[KONRAD Frederic]

Le 06/06/2016 à 20:41, Alistair Francis a écrit :

On Mon, Jun 6, 2016 at 7:21 AM,   wrote:

From: KONRAD Frederic 

This introduces a new bus: aux-bus.

It contains an address space for aux slaves devices and a bridge to an I2C bus
for I2C through AUX transactions.

Signed-off-by: KONRAD Frederic 
Tested-By: Hyun Kwon 
---
 default-configs/aarch64-softmmu.mak |   1 +
 hw/misc/Makefile.objs   |   1 +
 hw/misc/aux.c   | 297 
 include/hw/misc/aux.h   | 125 +++
 4 files changed, 424 insertions(+)
 create mode 100644 hw/misc/aux.c
 create mode 100644 include/hw/misc/aux.h

diff --git a/default-configs/aarch64-softmmu.mak 
b/default-configs/aarch64-softmmu.mak
index 96dd994..d3a2665 100644
--- a/default-configs/aarch64-softmmu.mak
+++ b/default-configs/aarch64-softmmu.mak
@@ -3,4 +3,5 @@
 # We support all the 32 bit boards so need all their config
 include arm-softmmu.mak

+CONFIG_AUX=y
 CONFIG_XLNX_ZYNQMP=y
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index bc0dd2c..ffb49c1 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -51,3 +51,4 @@ obj-$(CONFIG_MIPS_ITU) += mips_itu.o
 obj-$(CONFIG_PVPANIC) += pvpanic.o
 obj-$(CONFIG_EDU) += edu.o
 obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
+obj-$(CONFIG_AUX) += aux.o
diff --git a/hw/misc/aux.c b/hw/misc/aux.c
new file mode 100644
index 000..6605224
--- /dev/null
+++ b/hw/misc/aux.c
@@ -0,0 +1,297 @@
+/*
+ * aux.c
+ *
+ *  Copyright 2015 : GreenSocs Ltd
+ *  http://www.greensocs.com/ , email: i...@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option)any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU 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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/*
+ * This is an implementation of the AUX bus for VESA Display Port v1.1a.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/misc/aux.h"
+#include "hw/i2c/i2c.h"
+#include "monitor/monitor.h"
+
+#ifndef DEBUG_AUX
+#define DEBUG_AUX 0
+#endif
+
+#define DPRINTF(fmt, ...) do { 
\
+if (DEBUG_AUX) {   
\
+qemu_log("aux: " fmt , ## __VA_ARGS__);
\
+}  
\
+} while (0);
+
+#define TYPE_AUXTOI2C "aux-to-i2c-bridge"
+#define AUXTOI2C(obj) OBJECT_CHECK(AUXTOI2CState, (obj), TYPE_AUXTOI2C)
+
+#define TYPE_AUX_BUS "aux-bus"
+#define AUX_BUS(obj) OBJECT_CHECK(AUXBus, (obj), TYPE_AUX_BUS)


This should be in the header file where the struct is.


Ok



+
+static void aux_slave_dev_print(Monitor *mon, DeviceState *dev, int indent);
+static inline I2CBus *aux_bridge_get_i2c_bus(AUXTOI2CState *bridge);
+
+/* aux-bus implementation (internal not public) */
+static void aux_bus_class_init(ObjectClass *klass, void *data)
+{
+BusClass *k = BUS_CLASS(klass);
+
+/* AUXSlave has an MMIO so we need to change the way we print information
+ * in monitor.
+ */
+k->print_dev = aux_slave_dev_print;
+}
+
+AUXBus *aux_init_bus(DeviceState *parent, const char *name)
+{
+AUXBus *bus;
+
+bus = AUX_BUS(qbus_create(TYPE_AUX_BUS, parent, name));
+bus->bridge = AUXTOI2C(qdev_create(BUS(bus), TYPE_AUXTOI2C));
+
+/* Memory related. */
+bus->aux_io = g_malloc(sizeof(*bus->aux_io));
+memory_region_init(bus->aux_io, OBJECT(bus), "aux-io", (1 << 20));
+address_space_init(&bus->aux_addr_space, bus->aux_io, "aux-io");
+return bus;
+}
+
+static void aux_bus_map_device(AUXBus *bus, AUXSlave *dev, hwaddr addr)
+{
+memory_region_add_subregion(bus->aux_io, addr, dev->mmio);
+}
+
+static bool aux_bus_is_bridge(AUXBus *bus, DeviceState *dev)
+{
+return (dev == DEVICE(bus->bridge));
+}
+
+I2CBus *aux_get_i2c_bus(AUXBus *bus)
+{
+return aux_bridge_get_i2c_bus(bus->bridge);
+}
+
+AUXReply aux_request(AUXBus *bus, AUXCommand cmd, uint32_t address,
+  uint8_t len, uint8_t *data)
+{
+AUXReply ret = AUX_NACK;
+I2CBus *i2c_bus = aux_get_i2c_bus(bus);
+size_t i;
+bool is_write = false;
+
+DPRINTF("request at address 0x%" PRIX32 ", command %u, len %u\n", address,
+

Re: [Qemu-devel] [PATCH v6 03/13] register: Add Memory API glue

[KONRAD Frederic]

Hi Alistair,

Le 13/05/2016 à 00:45, Alistair Francis a écrit :

Add memory io handlers that glue the register API to the memory API.
Just translation functions at this stage. Although it does allow for
devices to be created without all-in-one mmio r/w handlers.

This patch also adds the RegisterInfoArray struct, which allows all of
the individual RegisterInfo structs to be grouped into a single memory
region.

Signed-off-by: Peter Crosthwaite 
Signed-off-by: Alistair Francis 
---
V6:
 - Add the memory region later
V5:
 - Convert to using only one memory region

 hw/core/register.c| 72 +++
 include/hw/register.h | 50 +++
 2 files changed, 122 insertions(+)

diff --git a/hw/core/register.c b/hw/core/register.c
index 5e6f621..25196e6 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -147,3 +147,75 @@ void register_reset(RegisterInfo *reg)

 register_write_val(reg, reg->access->reset);
 }
+
+static inline void register_write_memory(void *opaque, hwaddr addr,
+ uint64_t value, unsigned size, bool 
be)
+{
+RegisterInfoArray *reg_array = opaque;
+RegisterInfo *reg = NULL;
+uint64_t we = ~0;
+int i, shift = 0;
+
+for (i = 0; i < reg_array->num_elements; i++) {
+if (reg_array->r[i]->access->decode.addr == addr) {
+reg = reg_array->r[i];
+break;
+}
+}
+assert(reg);
+
+/* Generate appropriate write enable mask and shift values */
+if (reg->data_size < size) {
+we = MAKE_64BIT_MASK(0, reg->data_size * 8);
+shift = 8 * (be ? reg->data_size - size : 0);
+} else if (reg->data_size >= size) {
+we = MAKE_64BIT_MASK(0, size * 8);
+}
+
+register_write(reg, value << shift, we << shift, reg_array->prefix,
+   reg_array->debug);
+}
+
+void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value,
+  unsigned size)
+{
+register_write_memory(opaque, addr, value, size, true);
+}
+
+
+void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value,
+  unsigned size)
+{
+register_write_memory(opaque, addr, value, size, false);
+}
+
+static inline uint64_t register_read_memory(void *opaque, hwaddr addr,
+unsigned size, bool be)
+{
+RegisterInfoArray *reg_array = opaque;
+RegisterInfo *reg = NULL;
+int i, shift;
+
+for (i = 0; i < reg_array->num_elements; i++) {
+if (reg_array->r[i]->access->decode.addr == addr) {
+reg = reg_array->r[i];
+break;
+}
+}
+assert(reg);
+
+shift = 8 * (be ? reg->data_size - size : 0);
+
+return (register_read(reg, reg_array->prefix, reg_array->debug) >> shift) &
+   MAKE_64BIT_MASK(0, size * 8);
+}
+
+uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size)
+{
+return register_read_memory(opaque, addr, size, true);
+}
+
+uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size)
+{
+return register_read_memory(opaque, addr, size, false);
+}
diff --git a/include/hw/register.h b/include/hw/register.h
index 07d0616..786707b 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -15,6 +15,7 @@

 typedef struct RegisterInfo RegisterInfo;
 typedef struct RegisterAccessInfo RegisterAccessInfo;
+typedef struct RegisterInfoArray RegisterInfoArray;

 /**
  * Access description for a register that is part of guest accessible device
@@ -51,6 +52,10 @@ struct RegisterAccessInfo {
 void (*post_write)(RegisterInfo *reg, uint64_t val);

 uint64_t (*post_read)(RegisterInfo *reg, uint64_t val);
+
+struct {
+hwaddr addr;
+} decode;


Is there any reason why there is a struct here?

Fred


 };

 /**
@@ -79,6 +84,25 @@ struct RegisterInfo {
 };

 /**
+ * This structure is used to group all of the individual registers which are
+ * modeled using the RegisterInfo strucutre.
+ *
+ * @r is an aray containing of all the relevent RegisterInfo structures.
+ *
+ * @num_elements is the number of elements in the array r
+ *
+ * @mem: optional Memory region for the register
+ */
+
+struct RegisterInfoArray {
+int num_elements;
+RegisterInfo **r;
+
+bool debug;
+const char *prefix;
+};
+
+/**
  * write a value to a register, subject to its restrictions
  * @reg: register to write to
  * @val: value to write
@@ -107,4 +131,30 @@ uint64_t register_read(RegisterInfo *reg, const char* 
prefix, bool debug);

 void register_reset(RegisterInfo *reg);

+/**
+ * Memory API MMIO write handler that will write to a Register API register.
+ *  _be for big endian variant and _le for little endian.
+ * @opaque: RegisterInfo to write to
+ * @addr: Address to write
+ * @value: Value to write
+ * @size: Number of bytes to write
+ */
+
+void register_write_memory_be(void *opaque, hwaddr addr, uint64_t

Re: [Qemu-devel] [PATCH V9 0/9] Xilinx DisplayPort.

[KONRAD Frederic]

Le 09/06/2016 à 20:05, Peter Maydell a écrit :

On 7 June 2016 at 22:39, Alistair Francis  wrote:

On Tue, Jun 7, 2016 at 1:30 PM,   wrote:

From: KONRAD Frederic 


Hey Peter,

These are all reviewed by Xilinx, this is ready to merge from our point of view.


This series breaks 'make check'. Specifically, the patch
"i2c: implement broadcast write" causes:

(cd build/x86 && QTEST_QEMU_BINARY=arm-softmmu/qemu-system-arm
QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM %
255 + 1))} gtester -k --verbose -m=quick tests/ds1338-test)
TEST: tests/ds1338-test... (pid=22635)
  /arm/ds1338/tx-rx:
Broken pipe
FAIL
GTester: last random seed: R02Se51ed4e024b0f59ef23e564a2a28bb83
(pid=22641)
FAIL: tests/ds1338-test

thanks
-- PMM



Hi Peter,

Yeah sorry I found the bug I'll resend.
BTW this doesn't happen with `make check` aarch64, is that normal?

Thanks,
Fred

Re: [Qemu-devel] [PATCH V10 0/9] Xilinx DisplayPort.

[KONRAD Frederic]

Le 14/06/2016 à 15:59, Peter Maydell a écrit :

On 13 June 2016 at 16:50,   wrote:

From: KONRAD Frederic 

This is the 10th version of this patch-set of the implementation of the Xilinx
DisplayPort and DPDMA.

This 10th version fixes one bug with the I2C broadcast patch.




Applied to target-arm.next, thanks.

-- PMM



Thanks!
Fred

Re: [Qemu-devel] [PATCH V10 0/9] Xilinx DisplayPort.

[KONRAD Frederic]

Le 14/06/2016 à 16:58, Peter Maydell a écrit :

On 14 June 2016 at 14:59, Peter Maydell  wrote:

On 13 June 2016 at 16:50,   wrote:

From: KONRAD Frederic 

This is the 10th version of this patch-set of the implementation of the Xilinx
DisplayPort and DPDMA.

This 10th version fixes one bug with the I2C broadcast patch.


I found some minor build issues in this series, which I'm just going
to fix up in my tree rather than making you respin this:


Format string issues which mean it doesn't build on OSX or 32-bit hosts:

/home/petmay01/linaro/qemu-for-merges/hw/display/xlnx_dp.c:700:5:
error: format ‘%lX’ expects argument of type ‘long unsigned int’, but
argument 3 has type ‘uint64_t’ [-Werror=format=]
/home/petmay01/linaro/qemu-for-merges/hw/display/xlnx_dp.c:709:5:
error: format ‘%lX’ expects argument of type ‘long unsigned int’, but
argument 3 has type ‘uint64_t’ [-Werror=format=]

Clang build issues:

/Users/pm215/src/qemu-for-merges/hw/i2c/i2c-ddc.c:284:33: warning:
unused variable 'vmstate_i2c_ddc' [-Wunused-const-variable]

(vmstate wasn't actually wired up to dc->vmsd)


Hi Peter,

oops be carefull there is a typo in your diff (see below)



/Users/pm215/src/qemu-for-merges/hw/dma/xlnx_dpdma.c:307:20: warning:
unused function 'xlnx_dpdma_set_desc_next_address' [-Wunused-function]


Full diff of fixups:

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index 552955f..be53b75 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -697,7 +697,7 @@ static uint64_t xlnx_dp_read(void *opaque, hwaddr
offset, unsigned size)
 break;
 }

-DPRINTF("core read @%" PRIx64 " = 0x%8.8lX\n", offset << 2, ret);
+DPRINTF("core read @%" PRIx64 " = 0x%8.8" PRIX64 "\n", offset << 2, ret);
 return ret;
 }

@@ -706,7 +706,7 @@ static void xlnx_dp_write(void *opaque, hwaddr
offset, uint64_t value,
 {
 XlnxDPState *s = XLNX_DP(opaque);

-DPRINTF("core write @%" PRIx64 " = 0x%8.8lX\n", offset, value);
+DPRINTF("core write @%" PRIx64 " = 0x%8.8" PRIX64 "\n", offset, value);

 offset = offset >> 2;

diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
index 97a5da7..8ceb21d 100644
--- a/hw/dma/xlnx_dpdma.c
+++ b/hw/dma/xlnx_dpdma.c
@@ -304,14 +304,6 @@ static uint64_t
xlnx_dpdma_descriptor_next_address(XlnxDPDMAState *s,
+ s->registers[DPDMA_DSCR_NEXT_ADDR_CH(channel)];
 }

-static inline void xlnx_dpdma_set_desc_next_address(XlnxDPDMAState *s,
-  uint8_t channel,
-  uint64_t addr)
-{
-s->registers[DPDMA_DSCR_NEXT_ADDRE_CH(channel)] = extract64(addr, 32, 32);
-s->registers[DPDMA_DSCR_NEXT_ADDR_CH(channel)] = extract64(addr, 0, 32);
-}
-
 static bool xlnx_dpdma_is_channel_enabled(XlnxDPDMAState *s,
 uint8_t channel)
 {
diff --git a/hw/i2c/i2c-ddc.c b/hw/i2c/i2c-ddc.c
index 02cd374..b47ec9a 100644
--- a/hw/i2c/i2c-ddc.c
+++ b/hw/i2c/i2c-ddc.c
@@ -270,27 +270,28 @@ static void i2c_ddc_init(Object *obj)
 build_edid_blob(&lcd_edid, s->edid_blob);
 }

+static const VMStateDescription vmstate_i2c_ddc = {
+.name = TYPE_I2CDDC,
+.version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_BOOL(firstbyte, I2CDDCState),
+VMSTATE_UINT8(reg, I2CDDCState),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static void i2c_ddc_class_init(ObjectClass *oc, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(oc);
 I2CSlaveClass *isc = I2C_SLAVE_CLASS(oc);

 dc->reset = i2c_ddc_reset;
+dc->vmsd = &vmstate_ic2_ddc;

vmstate_i2c_ddc*

Thanks,
Fred

 isc->event = i2c_ddc_event;
 isc->recv = i2c_ddc_rx;
 isc->send = i2c_ddc_tx;
 }

-static const VMStateDescription vmstate_i2c_ddc = {
-.name = TYPE_I2CDDC,
-.version_id = 1,
-.fields = (VMStateField[]) {
-VMSTATE_BOOL(firstbyte, I2CDDCState),
-VMSTATE_UINT8(reg, I2CDDCState),
-VMSTATE_END_OF_LIST()
-}
-};
-
 static TypeInfo i2c_ddc_info = {
 .name = TYPE_I2CDDC,
 .parent = TYPE_I2C_SLAVE,



thanks
-- PMM

Re: [Qemu-devel] [PATCH] xlnx_dp: fix iffy xlnx_dp_aux_push_tx_fifo

[KONRAD Frederic]

Le 21/06/2016 à 15:16, Paolo Bonzini a écrit :

xlnx_dp_aux_push_tx_fifo takes an immediate uint8_t and a buffer length,
which must be 1 because that is how many uint8_t's fit in a uint8_t.
Sure enough, that is what xlnx_dp_write passes to it, but the function
is just weird.  Therefore, make xlnx_dp_aux_push_tx_fifo look like
xlnx_dp_aux_push_rx_fifo, taking a pointer to the buffer.

Reported by Coverity.


Hi Paolo,

Seems ok to me.



Signed-off-by: Paolo Bonzini 
---
 hw/display/xlnx_dp.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index be53b75..f43eb09 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -438,10 +438,10 @@ static void xlnx_dp_aux_clear_tx_fifo(XlnxDPState *s)
 fifo8_reset(&s->tx_fifo);
 }

-static void xlnx_dp_aux_push_tx_fifo(XlnxDPState *s, uint8_t val, size_t len)
+static void xlnx_dp_aux_push_tx_fifo(XlnxDPState *s, uint8_t *buf, size_t len)
 {
 DPRINTF("Push %u data in tx_fifo\n", (unsigned)len);
-fifo8_push_all(&s->tx_fifo, &val, len);
+fifo8_push_all(&s->tx_fifo, buf, len);
 }

 static uint8_t xlnx_dp_aux_pop_tx_fifo(XlnxDPState *s)
@@ -806,9 +806,11 @@ static void xlnx_dp_write(void *opaque, hwaddr offset, 
uint64_t value,
  * TODO: Power down things?
  */
 break;
-case DP_AUX_WRITE_FIFO:
-xlnx_dp_aux_push_tx_fifo(s, value, 1);
+case DP_AUX_WRITE_FIFO: {
+uint8_t c = value;
+xlnx_dp_aux_push_tx_fifo(s, &c, 1);
 break;
+}


BTW do you need those braces here?

Thanks,
Fred


 case DP_AUX_CLOCK_DIVIDER:
 break;
 case DP_AUX_REPLY_COUNT:

Re: [Qemu-devel] [PATCH] xlnx_dp: fix iffy xlnx_dp_aux_push_tx_fifo

[KONRAD Frederic]

Le 22/06/2016 à 00:14, Eric Blake a écrit :

On 06/21/2016 08:09 AM, KONRAD Frederic wrote:



Le 21/06/2016 à 15:16, Paolo Bonzini a écrit :

xlnx_dp_aux_push_tx_fifo takes an immediate uint8_t and a buffer length,
which must be 1 because that is how many uint8_t's fit in a uint8_t.
Sure enough, that is what xlnx_dp_write passes to it, but the function
is just weird.  Therefore, make xlnx_dp_aux_push_tx_fifo look like
xlnx_dp_aux_push_rx_fifo, taking a pointer to the buffer.

Reported by Coverity.





+case DP_AUX_WRITE_FIFO: {
+uint8_t c = value;
+xlnx_dp_aux_push_tx_fifo(s, &c, 1);
 break;
+}


BTW do you need those braces here?


Yes. The declaration of 'c' inside a case label causes (at least some
versions of) gcc to gripe, if it is not in a {} scope.



Hi Eric,

Makes sense!

Thanks,
Fred

RE: REG: TTC Timer

[Konrad, Frederic]

Hi Philippe,
Hi Gowri,

The zcu102 has a zynqmp soc object (hw/arm/xlnx-zcu102.c:125):

static void xlnx_zcu102_init(MachineState *machine)
{
...
object_initialize_child(OBJECT(machine), "soc", &s->soc, TYPE_XLNX_ZYNQMP);

So the TTCs should work in the ZCU102.

Best Regards,
Fred

-Original Message-
From: Philippe Mathieu-Daudé  
Sent: 05 December 2022 09:24
To: Gowri Shankar ; QEMU Developers 
; qemu-arm 
Cc: qemu-disc...@nongnu.org; Konrad, Frederic ; 
Iglesias, Francisco ; Alistair Francis 

Subject: Re: REG: TTC Timer

On 22/11/22 12:27, Gowri Shankar wrote:
> Hi Team,
> 
> Advance Thanks for Your support.
> 
> Could you please clarify one point here?
> I am using a Xilinx ZCU102 machine with QEMU7.1.0.
> 
> I have seen QEMU 7.1.0 release has TTC timers for the Xilinx-zynqmp 
> SoC model.
> url: https://wiki.qemu.org/ChangeLog/7.1
> <https://wiki.qemu.org/ChangeLog/7.1>
> 
> In this case, can the ZCU102 machine also use the TTC feature?
> If yes and possible, Could you please share the example code snippet?
> --
> Thanks & Regards,
> P. Gowrishankar.
> +919944802490

Cc'ing qemu-arm@ mailing list and Xilinx ZCU102 machine developers.

<>

RE: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16 bit accesses

[Konrad, Frederic]

Hi Peter,

CC'ing Philippe.

> -Original Message-
> From: Qemu-devel  bounces+fkonrad=amd@nongnu.org> On Behalf Of Peter Maydell
> Sent: 02 August 2022 14:19
> To: qemu-devel@nongnu.org
> Cc: Fabien Chouteau ; Frederic Konrad
> 
> Subject: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16 bit
> accesses
> 
> In real hardware, the APB and AHB PNP data tables can be accessed
> with byte and halfword reads as well as word reads.  Our
> implementation currently only handles word reads.  Add support for
> the 8 and 16 bit accesses.  Note that we only need to handle aligned
> accesses -- unaligned accesses should continue to trap, as happens on
> hardware.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1132
> Signed-off-by: Peter Maydell 
> ---
> It would be nice if we could just set the .valid.min_access_size in
> the MemoryRegionOps to 1 and have the memory system core synthesize
> the 1 and 2 byte accesses from a 4 byte read, but currently that
> doesn't work (see various past mailing list threads).

That looks good to me but I thought this was fixed by 1a5a5570 and 0fbe394a
because RTEMS do bytes accesses?

Did that break at some point?

Regards,
Fred

> ---
>  hw/misc/grlib_ahb_apb_pnp.c | 10 ++
>  hw/misc/trace-events|  4 ++--
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/misc/grlib_ahb_apb_pnp.c b/hw/misc/grlib_ahb_apb_pnp.c
> index 43e001c3c7b..5b05f158592 100644
> --- a/hw/misc/grlib_ahb_apb_pnp.c
> +++ b/hw/misc/grlib_ahb_apb_pnp.c
> @@ -136,7 +136,8 @@ static uint64_t grlib_ahb_pnp_read(void *opaque,
> hwaddr offset, unsigned size)
>  uint32_t val;
> 
>  val = ahb_pnp->regs[offset >> 2];
> -trace_grlib_ahb_pnp_read(offset, val);
> +val = extract32(val, (4 - (offset & 3) - size) * 8, size * 8);
> +trace_grlib_ahb_pnp_read(offset, size, val);
> 
>  return val;
>  }
> @@ -152,7 +153,7 @@ static const MemoryRegionOps grlib_ahb_pnp_ops =
> {
>  .write  = grlib_ahb_pnp_write,
>  .endianness = DEVICE_BIG_ENDIAN,
>  .impl = {
> -.min_access_size = 4,
> +.min_access_size = 1,
>  .max_access_size = 4,
>  },
>  };
> @@ -247,7 +248,8 @@ static uint64_t grlib_apb_pnp_read(void *opaque,
> hwaddr offset, unsigned size)
>  uint32_t val;
> 
>  val = apb_pnp->regs[offset >> 2];
> -trace_grlib_apb_pnp_read(offset, val);
> +val = extract32(val, (4 - (offset & 3) - size) * 8, size * 8);
> +trace_grlib_apb_pnp_read(offset, size, val);
> 
>  return val;
>  }
> @@ -263,7 +265,7 @@ static const MemoryRegionOps grlib_apb_pnp_ops =
> {
>  .write  = grlib_apb_pnp_write,
>  .endianness = DEVICE_BIG_ENDIAN,
>  .impl = {
> -.min_access_size = 4,
> +.min_access_size = 1,
>  .max_access_size = 4,
>  },
>  };
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 4d51a80de1d..c18bc0605e8 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -247,8 +247,8 @@ via1_adb_poll(uint8_t data, const char *vadbint, int
> status, int index, int size
>  via1_auxmode(int mode) "setting auxmode to %d"
> 
>  # grlib_ahb_apb_pnp.c
> -grlib_ahb_pnp_read(uint64_t addr, uint32_t value) "AHB PnP read
> addr:0x%03"PRIx64" data:0x%08x"
> -grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read
> addr:0x%03"PRIx64" data:0x%08x"
> +grlib_ahb_pnp_read(uint64_t addr, unsigned size, uint32_t value) "AHB PnP
> read addr:0x%03"PRIx64" size:%u data:0x%08x"
> +grlib_apb_pnp_read(uint64_t addr, unsigned size, uint32_t value) "APB PnP
> read addr:0x%03"PRIx64" size:%u data:0x%08x"
> 
>  # led.c
>  led_set_intensity(const char *color, const char *desc, uint8_t
> intensity_percent) "LED desc:'%s' color:%s intensity: %u%%"
> --
> 2.25.1
> 

RE: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16 bit accesses

[Konrad, Frederic]

> -Original Message-
> From: Peter Maydell 
> Sent: 02 August 2022 15:34
> To: Konrad, Frederic 
> Cc: qemu-devel@nongnu.org; Fabien Chouteau ;
> Frederic Konrad ; f4...@amsat.org
> Subject: Re: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16
> bit accesses
> 
> On Tue, 2 Aug 2022 at 15:20, Konrad, Frederic 
> wrote:
> >
> > Hi Peter,
> >
> > CC'ing Philippe.
> >
> > > -Original Message-
> > > From: Qemu-devel  > > bounces+fkonrad=amd@nongnu.org> On Behalf Of Peter Maydell
> > > Sent: 02 August 2022 14:19
> > > To: qemu-devel@nongnu.org
> > > Cc: Fabien Chouteau ; Frederic Konrad
> > > 
> > > Subject: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16
> bit
> > > accesses
> > >
> > > In real hardware, the APB and AHB PNP data tables can be accessed
> > > with byte and halfword reads as well as word reads.  Our
> > > implementation currently only handles word reads.  Add support for
> > > the 8 and 16 bit accesses.  Note that we only need to handle aligned
> > > accesses -- unaligned accesses should continue to trap, as happens on
> > > hardware.
> > >
> > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1132
> > > Signed-off-by: Peter Maydell 
> > > ---
> > > It would be nice if we could just set the .valid.min_access_size in
> > > the MemoryRegionOps to 1 and have the memory system core
> synthesize
> > > the 1 and 2 byte accesses from a 4 byte read, but currently that
> > > doesn't work (see various past mailing list threads).
> >
> > That looks good to me but I thought this was fixed by 1a5a5570 and
> 0fbe394a
> > because RTEMS do bytes accesses?
> >
> > Did that break at some point?
> 
> I definitely tried letting the .impl vs .valid settings handle this,
> but the access_with_adjusted_size() code doesn't do the right thing.
> (In particular, the test case ELF in the bug report works with
> this patch, and doesn't work without it...)
> 
> I'm pretty sure the problem with access_with_adjusted_size() is a
> long-standing bug -- I found a couple of mailing list threads about
> it. We really ought to fix that properly, but that's definitely not
> for-7.1 material.

Ok got it, thanks.

Reviewed-by: Frederic Konrad 

> 
> -- PMM

RE: [PATCH] xlnx_dp: drop unsupported AUXCommand in xlnx_dp_aux_set_command

[Konrad, Frederic]

> -Original Message-
> From: Qemu-devel  bounces+fkonrad=amd@nongnu.org> On Behalf Of Qiang Liu
> Sent: 08 August 2022 08:55
> To: qemu-devel@nongnu.org
> Cc: Qiang Liu ; Thomas Huth ;
> Alistair Francis ; Edgar E. Iglesias
> ; Peter Maydell ;
> open list:Xilinx ZynqMP and... 
> Subject: [PATCH] xlnx_dp: drop unsupported AUXCommand in
> xlnx_dp_aux_set_command
> 
> In xlnx_dp_aux_set_command, when the command leads to the default
> branch, xlxn-dp will abort and then crash.
> 
> This patch removes this abort and drops this operation.
> 
> Fixes: 58ac482 ("introduce xlnx-dp")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/411
> Reported-by: Qiang Liu 
> Tested-by: Qiang Liu 
> Suggested-by: Thomas Huth 
> Signed-off-by: Qiang Liu 
> ---
>  hw/display/xlnx_dp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index a071c81..b0828d6 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -532,8 +532,8 @@ static void xlnx_dp_aux_set_command(XlnxDPState
> *s, uint32_t value)
>  qemu_log_mask(LOG_UNIMP, "xlnx_dp: Write i2c status not
> implemented\n");
>  break;
>  default:
> -error_report("%s: invalid command: %u", __func__, cmd);
> -abort();
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid command: %u",
> __func__, cmd);
> +return;
>  }
> 
>  s->core_registers[DP_INTERRUPT_SIGNAL_STATE] |= 0x04;
> --
> 2.25.1
> 

Looks good to me.

Reviewed-by: Frederic Konrad 

RE: [PATCH RFC] prevent overflow in xlnx_dpdma_desc_get_source_address()

[Konrad, Frederic]

Hi,

> -Original Message-
> From: qemu-devel-bounces+fkonrad=amd@nongnu.org 
>  On Behalf Of
> Peter Maydell
> Sent: Friday, April 12, 2024 12:07 PM
> To: Alexandra Diupina 
> Cc: Alistair Francis ; Edgar E. Iglesias 
> ; qemu-...@nongnu.org; qemu-
> de...@nongnu.org; sdl.q...@linuxtesting.org
> Subject: Re: [PATCH RFC] prevent overflow in 
> xlnx_dpdma_desc_get_source_address()
> 
> On Fri, 12 Apr 2024 at 09:13, Alexandra Diupina  
> wrote:
> >
> > Overflow can occur in a situation where desc->source_address
> > has a maximum value (pow(2, 32) - 1), so add a cast to a
> > larger type before the assignment.
> >
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> >
> > Fixes: d3c6369a96 ("introduce xlnx-dpdma")
> > Signed-off-by: Alexandra Diupina 
> > ---
> >  hw/dma/xlnx_dpdma.c | 20 ++--
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
> > index 1f5cd64ed1..224259225c 100644
> > --- a/hw/dma/xlnx_dpdma.c
> > +++ b/hw/dma/xlnx_dpdma.c
> > @@ -175,24 +175,24 @@ static uint64_t 
> > xlnx_dpdma_desc_get_source_address(DPDMADescriptor *desc,
> >
> >  switch (frag) {
> >  case 0:
> > -addr = desc->source_address
> > -+ (extract32(desc->address_extension, 16, 12) << 20);
> > +addr = (uint64_t)(desc->source_address
> > ++ (extract32(desc->address_extension, 16, 12) << 20));
> 
> Unless I'm confused, this cast doesn't help, because we
> will have already done a 32-bit addition of desc->source_address
> and the value from the address_extension part, so it doesn't
> change the result.
> 
> If we want to do the addition at 64 bits then using extract64()
> would be the simplest way to arrange for that.
> 
> However, I can't figure out what this code is trying to do and
> make that line up with the data sheet; maybe this isn't the
> right datasheet for this device?
> 
> https://docs.amd.com/r/en-US/ug1085-zynq-ultrascale-trm/ADDR_EXT-Field
> 
> The datasheet suggests that we should take 32 bits of the address
> from one field (here desc->source_address) and 16 bits of the
> address from another (here desc->address_extension's high bits)
> and combine them to make a 48 bit address. But this code is only
> looking at 12 bits of the high 16 in address_extension, and it
> doesn't shift them right enough to put them into bits [47:32]
> of the final address.
> 
> Xilinx folks: what hardware is this modelling, and is it
> really the right behaviour?

Looks like this is the right documentation.  Most probably the descriptor field 
changed
since I did that model, or I got really confused.

> 
> Also, this device looks like it has a host-endianness bug: the
> DPDMADescriptor struct is read directly from guest memory in
> dma_memory_read(), but the device never does anything to swap
> the fields from guest memory order to host memory order. So
> this is likely broken on big-endian hosts.
> 

Yes indeed.

Best Regards,
Fred

> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v2 1/3] leon3: add a little bootloader

[KONRAD Frederic]

Hi Mark,


  }


I think this patch is basically okay, however if you don't supply both a kernel 
and
bios then you get the slightly enigmatic message below:

$ ./qemu-system-sparc -M leon3_generic
qemu-system-sparc: Can't read bios image (null)

Perhaps add a define for LEON3_BIOS_FILENAME and return that if filename == 
NULL to
give a better error message?


Okay I see there is already a PROM_FILENAME that exists and can be used here.


I think we already have this behavior without this patch. Should this be fixed
in an other patch?

Regars,
Fred




ATB,

Mark.

Re: [Qemu-devel] [PATCH v2 2/3] leon3: introduce the plug and play mecanism

[KONRAD Frederic]

Le 5/3/19 à 10:09 AM, Mark Cave-Ayland a écrit :

On 25/04/2019 13:18, KONRAD Frederic wrote:


This adds the AHB and APB plug and play devices.
They are scanned during the linux boot to discover the various peripheral.

Reviewed-by: Fabien Chouteau 
Signed-off-by: KONRAD Frederic 
---
  hw/misc/Makefile.objs   |   2 +
  hw/misc/grlib_ahb_apb_pnp.c | 269 
  hw/sparc/leon3.c|  34 -
  include/hw/misc/grlib_ahb_apb_pnp.h |  60 
  include/hw/sparc/grlib.h|  35 +++--
  5 files changed, 382 insertions(+), 18 deletions(-)
  create mode 100644 hw/misc/grlib_ahb_apb_pnp.c
  create mode 100644 include/hw/misc/grlib_ahb_apb_pnp.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index c71e07a..77b9df9 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -77,3 +77,5 @@ obj-$(CONFIG_AUX) += auxbus.o
  obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
  obj-$(CONFIG_MSF2) += msf2-sysreg.o
  obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
+
+obj-$(CONFIG_GRLIB) += grlib_ahb_apb_pnp.o
diff --git a/hw/misc/grlib_ahb_apb_pnp.c b/hw/misc/grlib_ahb_apb_pnp.c
new file mode 100644
index 000..90d5f6e
--- /dev/null
+++ b/hw/misc/grlib_ahb_apb_pnp.c
@@ -0,0 +1,269 @@
+/*
+ * GRLIB AHB APB PNP
+ *
+ *  Copyright (C) 2019 AdaCore
+ *
+ *  Developed by :
+ *  Frederic Konrad   
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU 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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/misc/grlib_ahb_apb_pnp.h"
+
+#define GRLIB_PNP_VENDOR_SHIFT (24)
+#define GRLIB_PNP_VENDOR_SIZE   (8)
+#define GRLIB_PNP_DEV_SHIFT(12)
+#define GRLIB_PNP_DEV_SIZE (12)
+#define GRLIB_PNP_VER_SHIFT (5)
+#define GRLIB_PNP_VER_SIZE  (5)
+#define GRLIB_PNP_IRQ_SHIFT (0)
+#define GRLIB_PNP_IRQ_SIZE  (5)
+#define GRLIB_PNP_ADDR_SHIFT   (20)
+#define GRLIB_PNP_ADDR_SIZE(12)
+#define GRLIB_PNP_MASK_SHIFT(4)
+#define GRLIB_PNP_MASK_SIZE(12)
+
+#define GRLIB_AHB_DEV_ADDR_SHIFT   (20)
+#define GRLIB_AHB_DEV_ADDR_SIZE(12)
+#define GRLIB_AHB_ENTRY_SIZE   (0x20)
+#define GRLIB_AHB_MAX_DEV  (64)
+#define GRLIB_AHB_SLAVE_OFFSET (0x800)
+
+#define GRLIB_APB_DEV_ADDR_SHIFT   (8)
+#define GRLIB_APB_DEV_ADDR_SIZE(12)
+#define GRLIB_APB_ENTRY_SIZE   (0x08)
+#define GRLIB_APB_MAX_DEV  (512)
+
+#define GRLIB_PNP_MAX_REGS (0x1000)
+
+typedef struct AHBPnp {
+SysBusDevice parent_obj;
+MemoryRegion iomem;
+
+uint32_t regs[GRLIB_PNP_MAX_REGS >> 2];
+uint8_t master_count;
+uint8_t slave_count;
+} AHBPnp;
+
+void grlib_ahb_pnp_add_entry(AHBPnp *dev, uint32_t address, uint32_t mask,
+ uint8_t vendor, uint16_t device, int slave,
+ int type)
+{
+unsigned int reg_start;
+
+/*
+ * AHB entries look like this:
+ *
+ * 31  23  11 - 9  4 --- 0
+ *  | VENDOR ID | DEVICE ID | IRQ ? | VERSION  | IRQ |
+ *  --
+ *  |  USER  |
+ *  --
+ *  |  USER  |
+ *  --
+ *  |  USER  |
+ *  --
+ *  |  USER  |
+ *  --
+ * 31 --- 20 --- 15 - 3  0
+ *  | ADDR[31..12] | 00PC |MASK   | TYPE |
+ *  --
+ * 31 --- 20 --- 15 - 3  0
+ *  | ADDR[31..12] | 00PC |MASK   | TYPE |
+ *  --
+ * 31 --- 20 --- 15 - 3  0
+ *  | ADDR[31..12] | 00PC |MASK   | TYPE |
+ *  --
+ * 31 --- 20 --- 15 - 3  0
+ *  | ADDR[31..12] | 00PC |MASK   | TYPE |
+ *  --
+ */
+
+if (slave) {
+assert(dev->slave_c

Re: [Qemu-devel] [PATCH v2 0/3] Leon3 patches

[KONRAD Frederic]

Le 5/3/19 à 10:19 AM, Mark Cave-Ayland a écrit :

On 25/04/2019 13:18, KONRAD Frederic wrote:


Hi all,

Those are some little fixes for the leon3 machine:
   * The first part initializes the uart and the timer when no bios are
 provided.
   * The second part adds AHB and APB plug and play devices to allow to boot
 linux.
   * The third part adds myself to the MAINTAINERS for this board.

The test images are available here: https://www.gaisler.com/anonftp/linux/lin
ux-2.6/images/leon-linux-4.9/leon-linux-4.9-1.0/up/

Tested with:
   qemu-system-sparc -M leon3_generic --nographic --kernel image.ram

V1 -> V2:
   * minor fixes in the first patch suggested by Philippe.

Regards,
Fred

KONRAD Frederic (3):
   leon3: add a little bootloader
   leon3: introduce the plug and play mecanism
   MAINTAINERS: add myself for leon3

  MAINTAINERS |   1 +
  hw/misc/Makefile.objs   |   2 +
  hw/misc/grlib_ahb_apb_pnp.c | 269 
  hw/sparc/leon3.c| 114 +--
  include/hw/misc/grlib_ahb_apb_pnp.h |  60 
  include/hw/sparc/grlib.h|  35 +++--
  6 files changed, 455 insertions(+), 26 deletions(-)
  create mode 100644 hw/misc/grlib_ahb_apb_pnp.c
  create mode 100644 include/hw/misc/grlib_ahb_apb_pnp.h


Hi Frederic,

I've now had a chance to review this - I think it basically looks good, it just 
needs
a few minor tweaks. I can also confirm that using the test images at the URL 
above, I
can boot to a userspace shell.

I'm happy to take this through my qemu-sparc branch once everything is ready to 
go.


ATB,

Mark.



Hi Mark,

Thanks for the review.

Regards,
Fred

Re: [PATCH v2] tests/acceptance/machine_sparc_leon3: Do not run HelenOS test by default

[KONRAD Frederic]

Le 2/12/20 à 9:36 PM, Philippe Mathieu-Daudé a écrit :

The www.helenos.org server is slow and downloading the Leon3 binary
takes too long [*]. Do not include this test in the default suite.

Similarly to commit 471c97a69b:

   Currently the Avocado framework does not distinct the time spent
   downloading assets vs. the time spent running a test. With big
   assets (like a full VM image) the tests likely fail.

   This is a limitation known by the Avocado team.
   Until this issue get fixed, do not run this tests automatically.

   Tests can still be run setting the AVOCADO_TIMEOUT_EXPECTED
   environment variable.

[*] https://travis-ci.org/stsquad/qemu/jobs/649599880#L4198

Reported-by: Alex Bennée 
Signed-off-by: Philippe Mathieu-Daudé 
---
v2: Add missing staged hunk...
---
  tests/acceptance/machine_sparc_leon3.py | 4 
  1 file changed, 4 insertions(+)

diff --git a/tests/acceptance/machine_sparc_leon3.py 
b/tests/acceptance/machine_sparc_leon3.py
index f77e210ccb..27e4717a51 100644
--- a/tests/acceptance/machine_sparc_leon3.py
+++ b/tests/acceptance/machine_sparc_leon3.py
@@ -5,6 +5,9 @@
  # This work is licensed under the terms of the GNU GPL, version 2 or
  # later. See the COPYING file in the top-level directory.
  
+import os

+
+from avocado import skipUnless
  from avocado_qemu import Test
  from avocado_qemu import wait_for_console_pattern
  
@@ -13,6 +16,7 @@ class Leon3Machine(Test):
  
  timeout = 60
  
+@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')

  def test_leon3_helenos_uimage(self):
  """
  :avocado: tags=arch:sparc



Reviewed-by: KONRAD Frederic 

Thanks Philippe!

Re: [PATCH 00/10] Add Allwinner H3 SoC and Orange Pi PC Machine

[KONRAD Frederic]

Le 12/2/19 à 10:09 PM, Niek Linnenbank a écrit :

Dear QEMU developers,

Hereby I would like to contribute the following set of patches to QEMU
which add support for the Allwinner H3 System on Chip and the
Orange Pi PC machine. The following features and devices are supported:

  * SMP (Quad Core Cortex A7)
  * Generic Interrupt Controller configuration
  * SRAM mappings
  * Timer device (re-used from Allwinner A10)
  * UART
  * SD/MMC storage controller
  * EMAC ethernet connectivity
  * USB 2.0 interfaces
  * Clock Control Unit
  * System Control module
  * Security Identifier device

Functionality related to graphical output such as HDMI, GPU,
Display Engine and audio are not included. Recently released
mainline Linux kernels (4.19 up to latest master) and mainline U-Boot
are known to work. The SD/MMC code is tested using bonnie++ and
various tools such as fsck, dd and fdisk. The EMAC is verified with iperf3
using -netdev socket.

To build a Linux mainline kernel that can be booted by the Orange Pi PC
machine, simply configure the kernel using the sunxi_defconfig configuration:
  $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- make mrproper
  $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- make sunxi_defconfig

To be able to use USB storage, you need to manually enable the corresponding
configuration item. Start the kconfig configuration tool:
  $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- make menuconfig

Navigate to the following item, enable it and save your configuration:
  Device Drivers > USB support > USB Mass Storage support

Build the Linux kernel with:
  $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- make -j5

To boot the newly build linux kernel in QEMU with the Orange Pi PC machine, use:
  $ qemu-system-arm -M orangepi -m 512 -nic user -nographic \
  -kernel /path/to/linux/arch/arm/boot/zImage \
  -append 'console=ttyS0,115200' \
  -dtb /path/to/linux/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dtb

Note that this kernel does not have a root filesystem. You may provide it
with an official Orange Pi PC image [1] either as an SD card or as
USB mass storage. To boot using the Orange Pi PC Debian image on SD card,
simply add the -sd argument and provide the proper root= kernel parameter:
  $ qemu-system-arm -M orangepi -m 512 -nic user -nographic \
  -kernel /path/to/linux/arch/arm/boot/zImage \
  -append 'console=ttyS0,115200 root=/dev/mmcblk0p2' \
  -dtb /path/to/linux/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dtb \
  -sd OrangePi_pc_debian_stretch_server_linux5.3.5_v1.0.img

Alternatively, you can also choose to build and boot a recent buildroot [2]
using the orangepi_pc_defconfig or Armbian image [3] for Orange Pi PC.
To attach an USB mass storage device to the machine, simply append to the 
command:
  -drive if=none,id=stick,file=myimage.img \
  -device usb-storage,bus=usb-bus.0,drive=stick

U-Boot mainline can be build and configured using the orangepi_pc_defconfig
using similar commands as describe above for Linux. To start U-Boot using
the Orange Pi PC machine, provide the u-boot binary to the -kernel argument:
  $ qemu-system-arm -M orangepi -m 512 -nic user -nographic \
  -kernel /path/to/uboot/u-boot -sd disk.img

Use the following U-boot commands to load and boot a Linux kernel from SD card:
  -> setenv bootargs console=ttyS0,115200
  -> ext2load mmc 0 0x4200 zImage
  -> ext2load mmc 0 0x4300 sun8i-h2-plus-orangepi-zero.dtb
  -> bootz 0x4200 - 0x4300

Looking forward to your review comments. I will do my best
to update the patches where needed.

With kind regards,

Niek Linnenbank

[1] http://www.orangepi.org/downloadresources/
[2] https://buildroot.org/download.html
[3] https://www.armbian.com/orange-pi-pc/


Works well on my side with vanilla linux-4.9.13 built with gcc-8.3.0 + busybox
and sun8i-h3-orangepi-one.dtb.

Tested-by: KONRAD Frederic 



Niek Linnenbank (10):
   hw: arm: add Allwinner H3 System-on-Chip
   hw: arm: add Xunlong Orange Pi PC machine
   arm: allwinner-h3: add Clock Control Unit
   arm: allwinner-h3: add USB host controller
   arm: allwinner-h3: add System Control module
   arm/arm-powerctl: set NSACR.{CP11,CP10} bits in arm_set_cpu_on()
   arm: allwinner-h3: add CPU Configuration module
   arm: allwinner-h3: add Security Identifier device
   arm: allwinner-h3: add SD/MMC host controller
   arm: allwinner-h3: add EMAC ethernet device

  MAINTAINERS   |   8 +
  default-configs/arm-softmmu.mak   |   1 +
  hw/arm/Kconfig|   9 +
  hw/arm/Makefile.objs  |   1 +
  hw/arm/allwinner-h3.c | 316 ++
  hw/arm/orangepi.c | 114 
  hw/misc/Makefile.objs |   4 +
  hw/misc/allwinner-h3-clk.c| 227 
  hw/misc/allwinner-h3-cpucfg.c | 280 +
  hw/misc/allwinner-h3-sid.c| 162 ++
  hw/misc/allwinner-h3-syscon.c 

Re: [PATCH 10/10] arm: allwinner-h3: add EMAC ethernet device

[KONRAD Frederic]

Le 12/2/19 à 10:09 PM, Niek Linnenbank a écrit :

The Allwinner H3 System on Chip includes an Ethernet MAC (EMAC)
which provides 10M/100M/1000M Ethernet connectivity. This commit
adds support for the Allwinner H3 EMAC, including emulation for
the following functionality:

  * DMA transfers
  * MII interface
  * Transmit CRC calculation

Signed-off-by: Niek Linnenbank 
---
  hw/arm/Kconfig |   1 +
  hw/arm/allwinner-h3.c  |  17 +
  hw/arm/orangepi.c  |   7 +
  hw/net/Kconfig |   3 +
  hw/net/Makefile.objs   |   1 +
  hw/net/allwinner-h3-emac.c | 786 +
  hw/net/trace-events|  10 +
  include/hw/arm/allwinner-h3.h  |   2 +
  include/hw/net/allwinner-h3-emac.h |  69 +++
  9 files changed, 896 insertions(+)
  create mode 100644 hw/net/allwinner-h3-emac.c
  create mode 100644 include/hw/net/allwinner-h3-emac.h

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index ebf8d2325f..551cff3442 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -294,6 +294,7 @@ config ALLWINNER_A10
  config ALLWINNER_H3
  bool
  select ALLWINNER_A10_PIT
+select ALLWINNER_H3_EMAC
  select SERIAL
  select ARM_TIMER
  select ARM_GIC
diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index c2972caf88..274b8548c0 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -53,6 +53,9 @@ static void aw_h3_init(Object *obj)
  
  sysbus_init_child_obj(obj, "mmc0", &s->mmc0, sizeof(s->mmc0),

TYPE_AW_H3_SDHOST);
+
+sysbus_init_child_obj(obj, "emac", &s->emac, sizeof(s->emac),
+  TYPE_AW_H3_EMAC);
  }
  
  static void aw_h3_realize(DeviceState *dev, Error **errp)

@@ -237,6 +240,20 @@ static void aw_h3_realize(DeviceState *dev, Error **errp)
  return;
  }
  
+/* EMAC */

+if (nd_table[0].used) {
+qemu_check_nic_model(&nd_table[0], TYPE_AW_H3_EMAC);
+qdev_set_nic_properties(DEVICE(&s->emac), &nd_table[0]);
+}
+object_property_set_bool(OBJECT(&s->emac), true, "realized", &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
+sysbusdev = SYS_BUS_DEVICE(&s->emac);
+sysbus_mmio_map(sysbusdev, 0, AW_H3_EMAC_BASE);
+sysbus_connect_irq(sysbusdev, 0, s->irq[AW_H3_GIC_SPI_EMAC]);
+
  /* Universal Serial Bus */
  sysbus_create_simple(TYPE_AW_H3_EHCI, AW_H3_EHCI0_BASE,
   s->irq[AW_H3_GIC_SPI_EHCI0]);
diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
index dee3efaf08..8a61eb0e69 100644
--- a/hw/arm/orangepi.c
+++ b/hw/arm/orangepi.c
@@ -61,6 +61,13 @@ static void orangepi_init(MachineState *machine)
  exit(1);
  }
  
+/* Setup EMAC properties */

+object_property_set_int(OBJECT(&s->h3->emac), 1, "phy-addr", &err);
+if (err != NULL) {
+error_reportf_err(err, "Couldn't set phy address: ");
+exit(1);
+}
+
  /* Mark H3 object realized */
  object_property_set_bool(OBJECT(s->h3), true, "realized", &err);
  if (err != NULL) {
diff --git a/hw/net/Kconfig b/hw/net/Kconfig
index 3856417d42..36d3923992 100644
--- a/hw/net/Kconfig
+++ b/hw/net/Kconfig
@@ -74,6 +74,9 @@ config MIPSNET
  config ALLWINNER_EMAC
  bool
  
+config ALLWINNER_H3_EMAC

+bool
+
  config IMX_FEC
  bool
  
diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs

index 7907d2c199..5548deb07a 100644
--- a/hw/net/Makefile.objs
+++ b/hw/net/Makefile.objs
@@ -23,6 +23,7 @@ common-obj-$(CONFIG_XGMAC) += xgmac.o
  common-obj-$(CONFIG_MIPSNET) += mipsnet.o
  common-obj-$(CONFIG_XILINX_AXI) += xilinx_axienet.o
  common-obj-$(CONFIG_ALLWINNER_EMAC) += allwinner_emac.o
+common-obj-$(CONFIG_ALLWINNER_H3_EMAC) += allwinner-h3-emac.o
  common-obj-$(CONFIG_IMX_FEC) += imx_fec.o
  
  common-obj-$(CONFIG_CADENCE) += cadence_gem.o

diff --git a/hw/net/allwinner-h3-emac.c b/hw/net/allwinner-h3-emac.c
new file mode 100644
index 00..37f6f44406
--- /dev/null
+++ b/hw/net/allwinner-h3-emac.c
@@ -0,0 +1,786 @@
+/*
+ * Allwinner H3 EMAC emulation
+ *
+ * Copyright (C) 2019 Niek Linnenbank 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU 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 "hw/sysbus.h"
+#include "migration/vmstate.h"
+#include "net/net.h"
+#include "hw/irq.h"
+#include "hw/qdev-prope

Re: [PATCH 10/10] arm: allwinner-h3: add EMAC ethernet device

[KONRAD Frederic]

Le 12/4/19 à 4:14 PM, Philippe Mathieu-Daudé a écrit :

On 12/3/19 10:33 AM, KONRAD Frederic wrote:

Le 12/2/19 à 10:09 PM, Niek Linnenbank a écrit :

The Allwinner H3 System on Chip includes an Ethernet MAC (EMAC)
which provides 10M/100M/1000M Ethernet connectivity. This commit
adds support for the Allwinner H3 EMAC, including emulation for
the following functionality:

  * DMA transfers
  * MII interface
  * Transmit CRC calculation

Signed-off-by: Niek Linnenbank 
---
  hw/arm/Kconfig |   1 +
  hw/arm/allwinner-h3.c  |  17 +
  hw/arm/orangepi.c  |   7 +
  hw/net/Kconfig |   3 +
  hw/net/Makefile.objs   |   1 +
  hw/net/allwinner-h3-emac.c | 786 +
  hw/net/trace-events    |  10 +
  include/hw/arm/allwinner-h3.h  |   2 +
  include/hw/net/allwinner-h3-emac.h |  69 +++
  9 files changed, 896 insertions(+)
  create mode 100644 hw/net/allwinner-h3-emac.c
  create mode 100644 include/hw/net/allwinner-h3-emac.h

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index ebf8d2325f..551cff3442 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -294,6 +294,7 @@ config ALLWINNER_A10
  config ALLWINNER_H3
  bool
  select ALLWINNER_A10_PIT
+    select ALLWINNER_H3_EMAC
  select SERIAL
  select ARM_TIMER
  select ARM_GIC
diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index c2972caf88..274b8548c0 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -53,6 +53,9 @@ static void aw_h3_init(Object *obj)
  sysbus_init_child_obj(obj, "mmc0", &s->mmc0, sizeof(s->mmc0),
    TYPE_AW_H3_SDHOST);
+
+    sysbus_init_child_obj(obj, "emac", &s->emac, sizeof(s->emac),
+  TYPE_AW_H3_EMAC);
  }
  static void aw_h3_realize(DeviceState *dev, Error **errp)
@@ -237,6 +240,20 @@ static void aw_h3_realize(DeviceState *dev, Error **errp)
  return;
  }
+    /* EMAC */
+    if (nd_table[0].used) {
+    qemu_check_nic_model(&nd_table[0], TYPE_AW_H3_EMAC);
+    qdev_set_nic_properties(DEVICE(&s->emac), &nd_table[0]);
+    }
+    object_property_set_bool(OBJECT(&s->emac), true, "realized", &err);
+    if (err != NULL) {
+    error_propagate(errp, err);
+    return;
+    }
+    sysbusdev = SYS_BUS_DEVICE(&s->emac);
+    sysbus_mmio_map(sysbusdev, 0, AW_H3_EMAC_BASE);
+    sysbus_connect_irq(sysbusdev, 0, s->irq[AW_H3_GIC_SPI_EMAC]);
+
  /* Universal Serial Bus */
  sysbus_create_simple(TYPE_AW_H3_EHCI, AW_H3_EHCI0_BASE,
   s->irq[AW_H3_GIC_SPI_EHCI0]);
diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
index dee3efaf08..8a61eb0e69 100644
--- a/hw/arm/orangepi.c
+++ b/hw/arm/orangepi.c
@@ -61,6 +61,13 @@ static void orangepi_init(MachineState *machine)
  exit(1);
  }
+    /* Setup EMAC properties */
+    object_property_set_int(OBJECT(&s->h3->emac), 1, "phy-addr", &err);
+    if (err != NULL) {
+    error_reportf_err(err, "Couldn't set phy address: ");
+    exit(1);
+    }
+
  /* Mark H3 object realized */
  object_property_set_bool(OBJECT(s->h3), true, "realized", &err);
  if (err != NULL) {
diff --git a/hw/net/Kconfig b/hw/net/Kconfig
index 3856417d42..36d3923992 100644
--- a/hw/net/Kconfig
+++ b/hw/net/Kconfig
@@ -74,6 +74,9 @@ config MIPSNET
  config ALLWINNER_EMAC
  bool
+config ALLWINNER_H3_EMAC
+    bool
+
  config IMX_FEC
  bool
diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
index 7907d2c199..5548deb07a 100644
--- a/hw/net/Makefile.objs
+++ b/hw/net/Makefile.objs
@@ -23,6 +23,7 @@ common-obj-$(CONFIG_XGMAC) += xgmac.o
  common-obj-$(CONFIG_MIPSNET) += mipsnet.o
  common-obj-$(CONFIG_XILINX_AXI) += xilinx_axienet.o
  common-obj-$(CONFIG_ALLWINNER_EMAC) += allwinner_emac.o
+common-obj-$(CONFIG_ALLWINNER_H3_EMAC) += allwinner-h3-emac.o
  common-obj-$(CONFIG_IMX_FEC) += imx_fec.o
  common-obj-$(CONFIG_CADENCE) += cadence_gem.o
diff --git a/hw/net/allwinner-h3-emac.c b/hw/net/allwinner-h3-emac.c
new file mode 100644
index 00..37f6f44406
--- /dev/null
+++ b/hw/net/allwinner-h3-emac.c
@@ -0,0 +1,786 @@
+/*
+ * Allwinner H3 EMAC emulation
+ *
+ * Copyright (C) 2019 Niek Linnenbank 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * a

RE: [PATCH] hw/display/xlnx_dp: fix overflow in xlnx_dp_aux_push_tx_fifo()

[Konrad, Frederic]

Hi,

> -Original Message-
> From: qemu-devel-bounces+fkonrad=amd@nongnu.org 
>  On Behalf Of
> Qiang Liu
> Sent: 09 January 2023 07:00
> To: qemu-devel@nongnu.org
> Cc: Qiang Liu ; Alistair Francis 
> ; Edgar E. Iglesias ; Peter
> Maydell ; open list:Xilinx ZynqMP and... 
> 
> Subject: [PATCH] hw/display/xlnx_dp: fix overflow in 
> xlnx_dp_aux_push_tx_fifo()
> 
> This patch checks if the s->tx_fifo is full.
> 
> Fixes: 58ac482a66de ("introduce xlnx-dp")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1424
> Reported-by: Qiang Liu 
> Signed-off-by: Qiang Liu 
> ---
>  hw/display/xlnx_dp.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index 972473d94f..617b394af2 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -854,7 +854,11 @@ static void xlnx_dp_write(void *opaque, hwaddr offset, 
> uint64_t value,
>  break;
>  case DP_AUX_WRITE_FIFO: {
>  uint8_t c = value;
> -xlnx_dp_aux_push_tx_fifo(s, &c, 1);
> +if (fifo8_is_full(&s->tx_fifo)) {
> +qemu_log_mask(LOG_GUEST_ERROR, "xlnx_dp: TX fifo is full");
> +} else {
> +xlnx_dp_aux_push_tx_fifo(s, &c, 1);
> +}

I'd rather move the check in xlnx_dp_aux_push_tx_fifo, like 
xlnx_dp_aux_pop_tx_fifo.
Otherwise looks good to me.

Thanks,
Fred

>  break;
>  }
>  case DP_AUX_CLOCK_DIVIDER:
> --
> 2.25.1
> 

Re: [Qemu-devel] [PATCH v5 14/33] tcg: add kick timer for single-threaded vCPU emulation

[KONRAD Frederic]

Hi Alex,

This is nice! Do we actually need to do qemu_cpu_kick_no_halt() in the
timer handler?

Thanks,
Fred

Le 27/10/2016 à 17:10, Alex Bennée a écrit :

Currently we rely on the side effect of the main loop grabbing the
iothread_mutex to give any long running basic block chains a kick to
ensure the next vCPU is scheduled. As this code is being re-factored and
rationalised we now do it explicitly here.

Signed-off-by: Alex Bennée 

---
v2
  - re-base fixes
  - get_ticks_per_sec() -> NANOSECONDS_PER_SEC
v3
  - add define for TCG_KICK_FREQ
  - fix checkpatch warning
v4
  - wrap next calc in inline qemu_tcg_next_kick() instead of macro
v5
  - move all kick code into own section
  - use global for timer
  - add helper functions to start/stop timer
  - stop timer when all cores paused
---
 cpus.c | 60 
 1 file changed, 60 insertions(+)

diff --git a/cpus.c b/cpus.c
index aedec7c..ad4ab68 100644
--- a/cpus.c
+++ b/cpus.c
@@ -735,6 +735,52 @@ void configure_icount(QemuOpts *opts, Error **errp)
 }

 /***/
+/* TCG vCPU kick timer
+ *
+ * The kick timer is responsible for moving single threaded vCPU
+ * emulation on to the next vCPU. If more than one vCPU is running a
+ * timer event with force a cpu->exit so the next vCPU can get
+ * scheduled.
+ *
+ * The timer is removed if all vCPUs are idle and restarted again once
+ * idleness is complete.
+ */
+
+static QEMUTimer *tcg_kick_vcpu_timer;
+
+static void qemu_cpu_kick_no_halt(void);
+
+#define TCG_KICK_PERIOD (NANOSECONDS_PER_SECOND / 10)
+
+static inline int64_t qemu_tcg_next_kick(void)
+{
+return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + TCG_KICK_PERIOD;
+}
+
+static void kick_tcg_thread(void *opaque)
+{
+timer_mod(tcg_kick_vcpu_timer, qemu_tcg_next_kick());
+qemu_cpu_kick_no_halt();
+}
+
+static void start_tcg_kick_timer(void)
+{
+if (!tcg_kick_vcpu_timer && CPU_NEXT(first_cpu)) {
+tcg_kick_vcpu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,  
kick_tcg_thread, NULL);
+timer_mod(tcg_kick_vcpu_timer, qemu_tcg_next_kick());
+}
+}
+
+static void stop_tcg_kick_timer(void)
+{
+if (tcg_kick_vcpu_timer) {
+timer_del(tcg_kick_vcpu_timer);
+tcg_kick_vcpu_timer = NULL;
+}
+}
+
+
+/***/
 void hw_error(const char *fmt, ...)
 {
 va_list ap;
@@ -988,9 +1034,12 @@ static void qemu_wait_io_event_common(CPUState *cpu)
 static void qemu_tcg_wait_io_event(CPUState *cpu)
 {
 while (all_cpu_threads_idle()) {
+stop_tcg_kick_timer();
 qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex);
 }

+start_tcg_kick_timer();
+
 while (iothread_requesting_mutex) {
 qemu_cond_wait(&qemu_io_proceeded_cond, &qemu_global_mutex);
 }
@@ -1178,6 +1227,15 @@ static void deal_with_unplugged_cpus(void)
 }
 }

+/* Single-threaded TCG
+ *
+ * In the single-threaded case each vCPU is simulated in turn. If
+ * there is more than a single vCPU we create a simple timer to kick
+ * the vCPU and ensure we don't get stuck in a tight loop in one vCPU.
+ * This is done explicitly rather than relying on side-effects
+ * elsewhere.
+ */
+
 static void *qemu_tcg_cpu_thread_fn(void *arg)
 {
 CPUState *cpu = arg;
@@ -1204,6 +1262,8 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 }
 }

+start_tcg_kick_timer();
+
 /* process any pending work */
 atomic_mb_set(&exit_request, 1);

Re: [Qemu-devel] [PATCH v1 1/2] arm_generic_timer: Add the ARM Generic Timer

[KONRAD Frederic]

Le 02/11/2016 à 17:41, Alistair Francis a écrit :

Add the ARM generic timer. This allows the guest to poll the timer for
values and also supports secure writes only.

Signed-off-by: Alistair Francis 
---

 hw/timer/Makefile.objs   |   1 +
 hw/timer/arm_generic_timer.c | 216 +++
 include/hw/timer/arm_generic_timer.h |  60 ++
 3 files changed, 277 insertions(+)
 create mode 100644 hw/timer/arm_generic_timer.c
 create mode 100644 include/hw/timer/arm_generic_timer.h

diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 7ba8c23..f88c468 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -17,6 +17,7 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
 common-obj-$(CONFIG_IMX) += imx_gpt.o
 common-obj-$(CONFIG_LM32) += lm32_timer.o
 common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
+common-obj-$(CONFIG_XLNX_ZYNQMP) += arm_generic_timer.o

 obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o
diff --git a/hw/timer/arm_generic_timer.c b/hw/timer/arm_generic_timer.c
new file mode 100644
index 000..8341e06
--- /dev/null
+++ b/hw/timer/arm_generic_timer.c
@@ -0,0 +1,216 @@
+/*
+ * QEMU model of the ARM Generic Timer
+ *
+ * Copyright (c) 2016 Xilinx Inc.
+ * Written by Alistair Francis 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/timer/arm_generic_timer.h"
+#include "qemu/timer.h"
+#include "qemu/log.h"
+
+#ifndef ARM_GEN_TIMER_ERR_DEBUG
+#define ARM_GEN_TIMER_ERR_DEBUG 0
+#endif
+
+static void counter_control_postw(RegisterInfo *reg, uint64_t val64)
+{
+ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
+bool new_status = extract32(s->regs[R_COUNTER_CONTROL_REGISTER],
+R_COUNTER_CONTROL_REGISTER_EN_SHIFT,
+R_COUNTER_CONTROL_REGISTER_EN_LENGTH);
+uint64_t current_ticks;
+
+current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ NANOSECONDS_PER_SECOND, 100);
+
+if ((s->enabled && !new_status) ||
+(!s->enabled && new_status)) {
+/* The timer is being disabled or enabled */
+s->tick_offset = current_ticks - s->tick_offset;
+}
+
+s->enabled = new_status;
+}
+
+static uint64_t couter_low_value_postr(RegisterInfo *reg, uint64_t val64)


s/couter/counter ?


+{
+ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
+uint64_t current_ticks, total_ticks;
+uint32_t low_ticks;
+
+if (s->enabled) {
+current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ NANOSECONDS_PER_SECOND, 100);
+total_ticks = current_ticks - s->tick_offset;
+low_ticks = (uint32_t) total_ticks;
+} else {
+/* Timer is disabled, return the time when it was disabled */
+low_ticks = (uint32_t) s->tick_offset;
+}
+
+return low_ticks;
+}
+
+static uint64_t couter_high_value_postr(RegisterInfo *reg, uint64_t val64)


same here?

Fred


+{
+ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
+uint64_t current_ticks, total_ticks;
+uint32_t high_ticks;
+
+if (s->enabled) {
+current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ NANOSECONDS_PER_SECOND, 100);
+total_ticks = current_ticks - s->tick_offset;
+high_ticks = (uint32_t) (total_ticks >> 32);
+} else {
+/* Timer is disabled, return the time when it was disabled */
+high_ticks = (uint32_t) (s->tick_offset >> 32);
+}
+
+return high_ticks;
+}
+
+
+static RegisterAccessInfo arm_gen_timer_regs_info[] = {
+{   .name = "COUNTER_CONTROL_REGISTER",
+.addr = A_COUNTER_CONTROL_REGISTER,
+.rsvd = 0xfffc,
+.post_write = counter_control_postw,
+},{ .name = "COUNTER_STATUS_REGISTER",
+.addr = A_COUNTER_STATUS_REGISTER,
+   

Re: [Qemu-devel] Mentor Summit notes

[KONRAD Frederic]

Hey Stefan,

Thanks for the feedback.

Le 01/11/2016 à 10:31, Stefan Hajnoczi a écrit :

Valentine and I attended Google Summer of Code Mentor Summit on behalf
of QEMU.  The event brings together GSoC mentors from all
participating organizations.

A lot of people benefit from QEMU and told me so at the summit.  There
are also many who hadn't heard of QEMU before.

I gave a lightning talk about Pranith Kumar's MTTCG project since it's
a good example of a QEMU project:
https://docs.google.com/presentation/d/1kidkMp4k-ak180aI-eZQfWuIEpQoeY-0N-oqt9eZo20/edit?usp=sharing

Feedback from Mentor Summit:

1. Pavel Pisa wants to upstream CAN bus support.  I asked him to
rebase and submit the patches.  Please take a look when patches are
posted.


I'm interested in this and can help for the review.

Thanks,
Fred



2. RTEMS mentioned they sometimes rely on out-of-tree QEMU targets and
want to encourage authors to upstream their code.  Xilinx was
mentioned in particular.

3. Wine brainstormed x86 Windows applications on ARM using
qemu-user-x86 + wine (x86).  Probably requires multithreaded TCG.
Work required to get OpenGL and other native features passed through.
Talk to Stefan Dösinger (Wine) if interested.

Stefan

Re: [Qemu-devel] [PATCH V1 03/10] qemu-clk: allow to bind two clocks together

[KONRAD Frederic]

Le 17/10/2016 à 20:19, Peter Maydell a écrit :

On 5 October 2016 at 23:10,   wrote:

From: KONRAD Frederic 

This introduces the clock binding and the update part.
When the qemu_clk_rate_update(qemu_clk, int) function is called:
  * The clock callback is called on the qemu_clk so it can change the rate.
  * The qemu_clk_rate_update function is called on all the driven clock.

Signed-off-by: KONRAD Frederic 
---
 include/qemu/qemu-clock.h | 66 +++
 qemu-clock.c  | 56 
 2 files changed, 122 insertions(+)

diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
index 1d56a2e..d575566 100644
--- a/include/qemu/qemu-clock.h
+++ b/include/qemu/qemu-clock.h
@@ -27,15 +27,29 @@
 #include "qemu/osdep.h"
 #include "qom/object.h"

+typedef uint64_t (*qemu_clk_on_rate_update_cb)(void *opaque, uint64_t rate);


I think it's more readable to have the typedef define
the function type, not the pointer-to-function type. (See for
instance CPReadFn, CPWriteFn in target-arm/cpu.h.) That way
your function pointers in structs and so on have type
"MyFnType *fn;" and are more obviously pointers.

(Also QEMU style says camelcase for types. QEMUClkRateUpdateCallback?)


Ok I'll fix that!




+
 #define TYPE_CLOCK "qemu-clk"
 #define QEMU_CLOCK(obj) OBJECT_CHECK(struct qemu_clk, (obj), TYPE_CLOCK)

+typedef struct ClkList ClkList;
+
 typedef struct qemu_clk {
 /*< private >*/
 Object parent_obj;
 char *name;/* name of this clock in the device. */
+uint64_t in_rate;  /* rate of the clock which drive this pin. */
+uint64_t out_rate; /* rate of this clock pin. */
+void *opaque;
+qemu_clk_on_rate_update_cb cb;
+QLIST_HEAD(, ClkList) bound;
 } *qemu_clk;

+struct ClkList {
+qemu_clk clk;
+QLIST_ENTRY(ClkList) node;
+};
+
 /**
  * qemu_clk_attach_to_device:
  * @dev: the device on which the clock need to be attached.
@@ -59,4 +73,56 @@ void qemu_clk_attach_to_device(DeviceState *dev, qemu_clk 
clk,
  */
 qemu_clk qemu_clk_get_pin(DeviceState *dev, const char *name);

+/**
+ * qemu_clk_bind_clock:
+ * @out: the clock output.
+ * @in: the clock input.
+ *
+ * Connect the clock together. This is unidirectional so a
+ * qemu_clk_update_rate will go from @out to @in.
+ *
+ */
+void qemu_clk_bind_clock(qemu_clk out, qemu_clk in);


Hang on, I thought that passing a clock to
qemu_clk_attach_to_device() was going to be the thing
that connected the clock up...



qemu_clk_attach_to_device() adds the clock to the device so it can be
found later to allow eg: qtree to show the clock tree.

qemu_clk_bind_clock do the actual bind between the clock.


+
+/**
+ * qemu_clk_unbound:
+ * @out: the clock output.
+ * @in: the clock input.
+ *
+ * Disconnect the clocks if they were bound together.
+ *
+ */
+void qemu_clk_unbind(qemu_clk out, qemu_clk in);


Function prototype and comment don't match...


+
+/**
+ * qemu_clk_update_rate:
+ * @clk: the clock to update.
+ * @rate: the new rate.
+ *
+ * Update the @clk to the new @rate.
+ *
+ */
+void qemu_clk_update_rate(qemu_clk clk, uint64_t rate);


What units is this 'rate' in ?


It's Hz, will add that to the comment.



+
+/**
+ * qemu_clk_refresh:
+ * @clk: the clock to be refreshed.
+ *
+ * If a model alters the topology of a clock tree, it must call this function
+ * to refresh the clock tree.
+ *
+ */
+void qemu_clk_refresh(qemu_clk clk);


...for which clock in the tree does it have to call the function?
All of them? Any one of them at random?

Ok I will precise that.

Thanks,
Fred




+
+/**
+ * qemu_clk_set_callback:
+ * @clk: the clock where to set the callback.
+ * @cb: the callback to associate to the callback.
+ * @opaque: the opaque data passed to the calback.
+ *
+ */
+void qemu_clk_set_callback(qemu_clk clk,
+   qemu_clk_on_rate_update_cb cb,
+   void *opaque);
+
 #endif /* QEMU_CLOCK_H */
diff --git a/qemu-clock.c b/qemu-clock.c
index 0ba6caf..541f615 100644
--- a/qemu-clock.c
+++ b/qemu-clock.c
@@ -37,6 +37,62 @@
 }\
 } while (0);

+void qemu_clk_refresh(qemu_clk clk)
+{
+qemu_clk_update_rate(clk, clk->in_rate);
+}
+
+void qemu_clk_update_rate(qemu_clk clk, uint64_t rate)
+{
+ClkList *child;
+
+clk->in_rate = rate;
+clk->out_rate = rate;
+
+if (clk->cb) {
+clk->out_rate = clk->cb(clk->opaque, rate);
+}
+
+DPRINTF("%s output rate updated to %" PRIu64 "\n",
+object_get_canonical_path(OBJECT(clk)),
+clk->out_rate);
+
+QLIST_FOREACH(child, &clk->bound, node) {
+qemu_clk_update_rate(child->clk, clk->out_rate);
+}
+}
+
+void qemu_clk_bind_clock(qemu_clk out, qemu_clk in)
+{
+ClkList *chi

Re: [Qemu-devel] [PATCH V1 06/10] docs: add qemu-clock documentation

[KONRAD Frederic]

Le 17/10/2016 à 20:31, Peter Maydell a écrit :

On 5 October 2016 at 23:11,   wrote:

From: KONRAD Frederic 

This adds the qemu-clock documentation.

Signed-off-by: KONRAD Frederic 
---
 docs/clock.txt | 109 +
 1 file changed, 109 insertions(+)
 create mode 100644 docs/clock.txt

diff --git a/docs/clock.txt b/docs/clock.txt
new file mode 100644
index 000..b25c5ab
--- /dev/null
+++ b/docs/clock.txt
@@ -0,0 +1,109 @@
+
+What is a QEMU_CLOCK
+
+
+A QEMU_CLOCK is a QOM Object developed for the purpose of modeling a clock tree
+with QEMU.


This doesn't match the type name you use in the code.


+It only simulates the clock by keeping a copy of the current frequency and
+doesn't model the signal itself such as pin toggle or duty cycle.
+
+It allows to model the impact of badly configured PLL, clock source selection
+or disabled clock on the models.


So is a QEMU_CLOCK a clock source?
How does this scheme model "I'm a device, and I have a clock
input (ie a clock sink)" ?


A qemu-clk is a clock pin. It can be either input or output.

So for a clock source device:

typedef struct {
DeviceState parent_obj;

uint32_t rate;
struct qemu_clk out;
} FixedClock;

out is actually a clock output.

During instance init:

object_initialize(&s->out, sizeof(s->out), TYPE_CLOCK);
qemu_clk_attach_to_device(DEVICE(obj), &s->out, "clk_out");

So after that: any device which wants to get the clock pin can do it
like this (fixed is a FixedClock):

qemu_clk_get_pin(DEVICE(fixed), "clk_out");

Or like this: fixed->out.

For a clock sink device:

typedef struct CRF_APB {
< ... >
/* input clocks */
qemu_clk pss_ref_clk;
qemu_clk video_clk;
qemu_clk pss_alt_ref_clk;
qemu_clk aux_refclk;
qemu_clk gt_crx_ref_clk;
< ... >
} CRF_APB;

Those are actually clock input. But this is the same object.

It works the same way for instance init:
object_initialize(&s->pss_ref_clk,
  sizeof(s->pss_ref_clk), TYPE_CLOCK);
qemu_clk_attach_to_device(DEVICE(obj),
  &s->pss_ref_clk, "pss_ref_clk");

But you might want to register a callback when the rate is updated:
qemu_clk_set_callback(s->pss_ref_clk, cb, s);

cb will be called when the clock tree is refreshed.
When the callback is called it will take the opaque passed to
qemu_clk_set_callback() and the rate of the previous clock in the chain.
This callback need to return the rate which will be passed to the next
clock in the chain.

So if we connect those two previous models together:

qemu_clk_bind_clock(qemu_clk_get_pin(DEVICE(s->fixed),
 "clk_out"),
qemu_clk_get_pin(DEVICE(s->crf),
 "pss_ref_clk"));

When the clock source is refreshed for example during realize for
FixedClock:
qemu_clk_update_rate(&s->out, s->rate);

It will call refresh on all the connected clocks with the rate modified
by it's own callback if there is a callback defined, so here:
qemu_clk_update_rate(pss_ref_clk, (rate of FixedClock));
As we defined cb as a update_rate callback it will be notified with the
rate of the previous clock.




+Binding the clock together to create a tree
+===
+
+In order to create a clock tree with QEMU_CLOCK two or more clock must be bound
+together. Let's say there are two clocks clk_a and clk_b:
+Using qemu_clk_bind(clk_a, clk_b) will bind clk_a and clk_b.
+
+Binding two qemu-clk together creates a unidirectional link which means that
+changing the rate of clk_a will propagate to clk_b and not the opposite.
+The binding process automatically refreshes clk_b rate.
+
+Clock can be bound and unbound during execution for modeling eg: a clock
+selector.
+
+A clock can drive more than one other clock. eg with this code:
+qemu_clk_bind(clk_a, clk_b);
+qemu_clk_bind(clk_a, clk_c);
+
+A clock rate change one clk_a will propagate to clk_b and clk_c.


What's the rate of a clock initially? Do you need to call
qemu_clk_update_rate() during board construction for a
fixed clock (or is it forbidden to call it during board
construction? during device reset?)


The rate is initially 0. The clock tree is refreshed from the sources to 
the device. And the fixed-clock model calls qemu_clk_update_rate()

in realize.


Presumably it's a programming error to bind a bunch of clocks
in a loop...


Yes and connecting a qemu-clock which is actually considered as an
input to an output as well.



+
+Implementing a callback on a rate change
+
+
+The function prototype is the following:
+typedef uint64_t (*qemu_clk_rate_change_cb)(void *opaque, uint64_t rate);
+
+It's main

Re: [Qemu-devel] [PATCH V1 02/10] qemu-clk: allow to attach a clock to a device

[KONRAD Frederic]

Le 17/10/2016 à 20:13, Peter Maydell a écrit :

On 5 October 2016 at 23:10,   wrote:

From: KONRAD Frederic 

This allows to attach a clock to a DeviceState.
Contrary to gpios, the clock pins are not contained in the DeviceState but
with the child property so they can appears in the qom-tree.


Is this API patterned along the same lines as some other
API we already have ? If so, it would be helpful to mention
it in the commit message.

(Also this series would probably benefit from somebody who
actually knows how the QOM APIs are supposed to work having
a look at it.)


I CC'ed Andreas maybe he can help.




Signed-off-by: KONRAD Frederic 
---
 include/qemu/qemu-clock.h | 24 +++-
 qemu-clock.c  | 23 +++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
index e7acd68..1d56a2e 100644
--- a/include/qemu/qemu-clock.h
+++ b/include/qemu/qemu-clock.h
@@ -33,8 +33,30 @@
 typedef struct qemu_clk {
 /*< private >*/
 Object parent_obj;
+char *name;/* name of this clock in the device. */
 } *qemu_clk;

-#endif /* QEMU_CLOCK_H */
+/**
+ * qemu_clk_attach_to_device:
+ * @dev: the device on which the clock need to be attached.
+ * @clk: the clock which need to be attached.
+ * @name: the name of the clock can't be NULL.
+ *
+ * Attach @clk named @name to the device @dev.


I'm not quite sure what this function does, but it looks like
what it actually does is attach @clk to device @dev as a
clock named @name.


+ *
+ */
+void qemu_clk_attach_to_device(DeviceState *dev, qemu_clk clk,
+   const char *name);

+/**
+ * qemu_clk_get_pin:
+ * @dev: the device which contain the clock.
+ * @name: the name of the clock.
+ *
+ * Get the clock named @name located in the device @dev, or NULL if not found.
+ *
+ * Returns the clock named @name contained in @dev.
+ */
+qemu_clk qemu_clk_get_pin(DeviceState *dev, const char *name);


I don't understand the name of this function. It says "get_pin"
but it returns a clock, not a pin ?


Yes actually you're right. Only we only use this function outside the
model. In the machine when you want to connects the clock model
together. That's why I called it like that. But I can change if you
want.




+#endif /* QEMU_CLOCK_H */
diff --git a/qemu-clock.c b/qemu-clock.c
index ceea98d..0ba6caf 100644
--- a/qemu-clock.c
+++ b/qemu-clock.c
@@ -25,6 +25,7 @@
 #include "qemu/qemu-clock.h"
 #include "hw/hw.h"
 #include "qemu/log.h"
+#include "qapi/error.h"

 #ifndef DEBUG_QEMU_CLOCK
 #define DEBUG_QEMU_CLOCK 0
@@ -36,6 +37,28 @@
 }\
 } while (0);

+void qemu_clk_attach_to_device(DeviceState *dev, qemu_clk clk,
+   const char *name)
+{
+assert(name);
+assert(!clk->name);


This is really checking that the clock hasn't already been attached
to something else, right?


Yes.




+object_property_add_child(OBJECT(dev), name, OBJECT(clk), &error_abort);
+clk->name = g_strdup(name);


When does this string get freed?


Nowhere actually! I'll fix that.




+}
+
+qemu_clk qemu_clk_get_pin(DeviceState *dev, const char *name)
+{
+gchar *path = NULL;
+Object *clk;
+bool ambiguous;
+
+path = g_strdup_printf("%s/%s", object_get_canonical_path(OBJECT(dev)),
+   name);
+clk = object_resolve_path(path, &ambiguous);


Needs a QOM API expert, but I'm about 90% certain that you
don't need to go via the canonical path to get hold of
an object that's a child of some other object you already
have. (Also object_resolve_path() accepts NULL if you
don't care about the return value in 'ambiguous', but I
think you'll end up not needing to call it at all.)
Possibly you want object_resolve_path_component(OBJECT(dev), name) ?


+g_free(path);
+return QEMU_CLOCK(clk);
+}
+
 static const TypeInfo qemu_clk_info = {
 .name  = TYPE_CLOCK,
 .parent= TYPE_OBJECT,
--
2.5.5



thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 1/4] fdc: Add a floppy qbus

[KONRAD Frederic]

Hi,

Le 20/10/2016 à 09:55, Kevin Wolf a écrit :

This adds a qbus to the floppy controller that should contain the floppy
drives eventually. At the moment it just exists and is empty.

Signed-off-by: Kevin Wolf 
Reviewed-by: John Snow 
---
 hw/block/fdc.c | 40 +++-
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index b79873a..a3afb62 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -52,6 +52,33 @@
 }   \
 } while (0)

+
+//
+/* qdev floppy bus  */
+
+#define TYPE_FLOPPY_BUS "Floppy"


I don't remember of any TYPE_* starting with a uppercase. Maybe better 
using floppy? or floppy-bus?


Fred


+#define FLOPPY_BUS(obj) OBJECT_CHECK(FloppyBus, (obj), TYPE_FLOPPY_BUS)
+
+typedef struct FDCtrl FDCtrl;
+
+typedef struct FloppyBus {
+BusState bus;
+FDCtrl *fdc;
+} FloppyBus;
+
+static const TypeInfo floppy_bus_info = {
+.name = TYPE_FLOPPY_BUS,
+.parent = TYPE_BUS,
+.instance_size = sizeof(FloppyBus),
+};
+
+static void floppy_bus_create(FDCtrl *fdc, FloppyBus *bus, DeviceState *dev)
+{
+qbus_create_inplace(bus, sizeof(FloppyBus), TYPE_FLOPPY_BUS, dev, NULL);
+bus->fdc = fdc;
+}
+
+
 //
 /* Floppy drive emulation   */

@@ -148,8 +175,6 @@ static FDriveSize drive_size(FloppyDriveType drive)
 #define FD_SECTOR_SC   2   /* Sector size code */
 #define FD_RESET_SENSEI_COUNT  4   /* Number of sense interrupts on RESET */

-typedef struct FDCtrl FDCtrl;
-
 /* Floppy disk drive emulation */
 typedef enum FDiskFlags {
 FDISK_DBL_SIDES  = 0x01,
@@ -684,6 +709,7 @@ struct FDCtrl {
 /* Power down config (also with status regB access mode */
 uint8_t pwrd;
 /* Floppy drives */
+FloppyBus bus;
 uint8_t num_floppies;
 FDrive drives[MAX_FD];
 int reset_sensei;
@@ -2442,7 +2468,8 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
 *fdc_tc = qdev_get_gpio_in(dev, 0);
 }

-static void fdctrl_realize_common(FDCtrl *fdctrl, Error **errp)
+static void fdctrl_realize_common(DeviceState *dev, FDCtrl *fdctrl,
+  Error **errp)
 {
 int i, j;
 static int command_tables_inited = 0;
@@ -2480,6 +2507,8 @@ static void fdctrl_realize_common(FDCtrl *fdctrl, Error 
**errp)
 k->register_channel(fdctrl->dma, fdctrl->dma_chann,
 &fdctrl_transfer_handler, fdctrl);
 }
+
+floppy_bus_create(fdctrl, &fdctrl->bus, dev);
 fdctrl_connect_drives(fdctrl, errp);
 }

@@ -2508,7 +2537,7 @@ static void isabus_fdc_realize(DeviceState *dev, Error 
**errp)
 }

 qdev_set_legacy_instance_id(dev, isa->iobase, 2);
-fdctrl_realize_common(fdctrl, &err);
+fdctrl_realize_common(dev, fdctrl, &err);
 if (err != NULL) {
 error_propagate(errp, err);
 return;
@@ -2559,7 +2588,7 @@ static void sysbus_fdc_common_realize(DeviceState *dev, 
Error **errp)
 FDCtrlSysBus *sys = SYSBUS_FDC(dev);
 FDCtrl *fdctrl = &sys->state;

-fdctrl_realize_common(fdctrl, errp);
+fdctrl_realize_common(dev, fdctrl, errp);
 }

 FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
@@ -2744,6 +2773,7 @@ static void fdc_register_types(void)
 type_register_static(&sysbus_fdc_type_info);
 type_register_static(&sysbus_fdc_info);
 type_register_static(&sun4m_fdc_info);
+type_register_static(&floppy_bus_info);
 }

 type_init(fdc_register_types)

Re: [Qemu-devel] [PATCH v4 2/4] fdc: Add a floppy drive qdev

[KONRAD Frederic]

Le 20/10/2016 à 09:55, Kevin Wolf a écrit :

Floppy controllers automatically create two floppy drive devices in qdev
now. (They always created two drives, but managed them only internally.)

Signed-off-by: Kevin Wolf 
Reviewed-by: John Snow 
---
 hw/block/fdc.c | 151 +
 1 file changed, 120 insertions(+), 31 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index a3afb62..5aa8e52 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -60,6 +60,8 @@
 #define FLOPPY_BUS(obj) OBJECT_CHECK(FloppyBus, (obj), TYPE_FLOPPY_BUS)

 typedef struct FDCtrl FDCtrl;
+typedef struct FDrive FDrive;
+static FDrive *get_drv(FDCtrl *fdctrl, int unit);

 typedef struct FloppyBus {
 BusState bus;
@@ -180,7 +182,7 @@ typedef enum FDiskFlags {
 FDISK_DBL_SIDES  = 0x01,
 } FDiskFlags;

-typedef struct FDrive {
+struct FDrive {
 FDCtrl *fdctrl;
 BlockBackend *blk;
 /* Drive status */
@@ -201,7 +203,7 @@ typedef struct FDrive {
 uint8_t media_rate;   /* Data rate of medium*/

 bool media_validated; /* Have we validated the media? */
-} FDrive;
+};


 static FloppyDriveType get_fallback_drive_type(FDrive *drv);
@@ -466,6 +468,100 @@ static void fd_revalidate(FDrive *drv)
 }
 }

+static void fd_change_cb(void *opaque, bool load)
+{
+FDrive *drive = opaque;
+
+drive->media_changed = 1;
+drive->media_validated = false;
+fd_revalidate(drive);
+}
+
+static const BlockDevOps fd_block_ops = {
+.change_media_cb = fd_change_cb,
+};
+
+
+#define TYPE_FLOPPY_DRIVE "floppy"
+#define FLOPPY_DRIVE(obj) \
+ OBJECT_CHECK(FloppyDrive, (obj), TYPE_FLOPPY_DRIVE)
+
+typedef struct FloppyDrive {
+DeviceState qdev;
+uint32_tunit;
+} FloppyDrive;
+
+static Property floppy_drive_properties[] = {
+DEFINE_PROP_UINT32("unit", FloppyDrive, unit, -1),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static int floppy_drive_init(DeviceState *qdev)
+{
+FloppyDrive *dev = FLOPPY_DRIVE(qdev);
+FloppyBus *bus = DO_UPCAST(FloppyBus, bus, dev->qdev.parent_bus);


Can't you use FLOPPY_BUS() introduced in the previous patch instead?
or I missed something?

Fred


+FDrive *drive;
+
+if (dev->unit == -1) {
+for (dev->unit = 0; dev->unit < MAX_FD; dev->unit++) {
+drive = get_drv(bus->fdc, dev->unit);
+if (!drive->blk) {
+break;
+}
+}
+}
+
+if (dev->unit >= MAX_FD) {
+error_report("Can't create floppy unit %d, bus supports only %d units",
+ dev->unit, MAX_FD);
+return -1;
+}
+
+/* TODO Check whether unit is in use */
+
+drive = get_drv(bus->fdc, dev->unit);
+
+if (drive->blk) {
+if (blk_get_on_error(drive->blk, 0) != BLOCKDEV_ON_ERROR_ENOSPC) {
+error_report("fdc doesn't support drive option werror");
+return -1;
+}
+if (blk_get_on_error(drive->blk, 1) != BLOCKDEV_ON_ERROR_REPORT) {
+error_report("fdc doesn't support drive option rerror");
+return -1;
+}
+} else {
+/* Anonymous BlockBackend for an empty drive */
+drive->blk = blk_new();
+}
+
+fd_init(drive);
+if (drive->blk) {
+blk_set_dev_ops(drive->blk, &fd_block_ops, drive);
+pick_drive_type(drive);
+}
+fd_revalidate(drive);
+
+return 0;
+}
+
+static void floppy_drive_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *k = DEVICE_CLASS(klass);
+k->init = floppy_drive_init;
+set_bit(DEVICE_CATEGORY_STORAGE, k->categories);
+k->bus_type = TYPE_FLOPPY_BUS;
+k->props = floppy_drive_properties;
+k->desc = "virtual floppy drive";
+}
+
+static const TypeInfo floppy_drive_info = {
+.name = TYPE_FLOPPY_DRIVE,
+.parent = TYPE_DEVICE,
+.instance_size = sizeof(FloppyDrive),
+.class_init = floppy_drive_class_init,
+};
+
 //
 /* Intel 82078 floppy disk controller emulation  */

@@ -1185,9 +1281,9 @@ static inline FDrive *drv3(FDCtrl *fdctrl)
 }
 #endif

-static FDrive *get_cur_drv(FDCtrl *fdctrl)
+static FDrive *get_drv(FDCtrl *fdctrl, int unit)
 {
-switch (fdctrl->cur_drv) {
+switch (unit) {
 case 0: return drv0(fdctrl);
 case 1: return drv1(fdctrl);
 #if MAX_FD == 4
@@ -1198,6 +1294,11 @@ static FDrive *get_cur_drv(FDCtrl *fdctrl)
 }
 }

+static FDrive *get_cur_drv(FDCtrl *fdctrl)
+{
+return get_drv(fdctrl, fdctrl->cur_drv);
+}
+
 /* Status A register : 0x00 (read-only) */
 static uint32_t fdctrl_read_statusA(FDCtrl *fdctrl)
 {
@@ -2357,46 +2458,33 @@ static void fdctrl_result_timer(void *opaque)
 }
 }

-static void fdctrl_change_cb(void *opaque, bool load)
-{
-FDrive *drive = opaque;
-
-drive->media_changed = 1;
-drive->media_validated = false;
-fd_revalidate(drive);
-}
-
-static const BlockDevOps fdctrl_block_ops = {
-.change_media_cb = fdctrl_change_c

Re: [Qemu-devel] [RFC PATCH 00/11] Clock framework API.

[KONRAD Frederic]

Le 29/07/2016 à 15:59, Peter Maydell a écrit :

On 13 June 2016 at 17:27,   wrote:

From: KONRAD Frederic 

Hi,

This is a first draft of the clock framework API it contains:

   * The first 5 patches which introduce the framework.
   * The 6th patch which introduces a fixed-clock model.
   * The rest which gives an example how to model a PLL from the existing
 zynqmp-crf extracted from the qemu xilinx tree.

No specific behavior is expected yet when the CRF register set is accessed but
the user can see for example the dp_video_ref and vpll_to_lpd rate changing in
the monitor with the "info qtree" command when the vpll_ctrl register is
modified.

Apologies for the 6-week-late review. I think mostly this is
along the right lines but I've given comments where I have
specific issues.

It might also be useful to sketch out how a simple clock
tree would look in the documentation file.


Hi Peter,

Thanks for having looked into this.

Fred



thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 02/11] qemu-clk: allow to attach a clock to a device

[KONRAD Frederic]

Le 29/06/2016 à 02:15, Alistair Francis a écrit :

On Mon, Jun 13, 2016 at 9:27 AM,   wrote:

From: KONRAD Frederic 

This allows to attach a clock to a DeviceState.
Contrary to gpios, the clock pins are not contained in the DeviceState but
with the child property so they can appears in the qom-tree.

Signed-off-by: KONRAD Frederic 
---
  include/qemu/qemu-clock.h | 24 +++-
  qemu-clock.c  | 22 ++
  2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
index e7acd68..a2ba105 100644
--- a/include/qemu/qemu-clock.h
+++ b/include/qemu/qemu-clock.h
@@ -33,8 +33,30 @@
  typedef struct qemu_clk {
  /*< private >*/
  Object parent_obj;
+char *name;/* name of this clock in the device. */
  } *qemu_clk;

-#endif /* QEMU_CLOCK_H */
+/**
+ * qemu_clk_attach_to_device:
+ * @d: the device on which the clock need to be attached.
+ * @clk: the clock which need to be attached.
+ * @name: the name of the clock can't be NULL.
+ *
+ * Attach @clk named @name to the device @d.
+ *
+ */
+void qemu_clk_attach_to_device(DeviceState *d, qemu_clk clk,

dev instead of just d


+   const char *name);

+/**
+ * qemu_clk_get_pin:
+ * @d: the device which contain the clock.
+ * @name: the name of the clock.
+ *
+ * Get the clock named @name located in the device @d, or NULL if not found.
+ *
+ * Returns the clock named @name contained in @d.
+ */
+qemu_clk qemu_clk_get_pin(DeviceState *d, const char *name);

+#endif /* QEMU_CLOCK_H */
diff --git a/qemu-clock.c b/qemu-clock.c
index 4a47fb4..81f2852 100644
--- a/qemu-clock.c
+++ b/qemu-clock.c
@@ -23,6 +23,7 @@

  #include "qemu/qemu-clock.h"
  #include "hw/hw.h"
+#include "qapi/error.h"

  /* #define DEBUG_QEMU_CLOCK */

@@ -33,6 +34,27 @@ do { printf("qemu-clock: " fmt , ## __VA_ARGS__); } while (0)
  #define DPRINTF(fmt, ...) do { } while (0)
  #endif

+void qemu_clk_attach_to_device(DeviceState *d, qemu_clk clk, const char *name)
+{
+assert(name);
+assert(!clk->name);
+object_property_add_child(OBJECT(d), name, OBJECT(clk), &error_abort);
+clk->name = g_strdup(name);
+}
+
+qemu_clk qemu_clk_get_pin(DeviceState *d, const char *name)
+{
+gchar *path = NULL;
+Object *clk;
+bool ambiguous;
+
+path = g_strdup_printf("%s/%s", object_get_canonical_path(OBJECT(d)),
+   name);
+clk = object_resolve_path(path, &ambiguous);

Should ambiguous be passed back to the caller?


Up to you, I don't see the use case in the machine where we want to get 
the clock?





+g_free(path);
+return QEMU_CLOCK(clk);

Shouldn't you check to see if you got something valid before casting?


Yes true, I was relying on the fact that QEMU_CLOCK is in the end:
object_dynamic_cast_assert(..) which according to the doc:

 * If an invalid object is passed to this function, a run time assert 
will be

 * generated.

but it seems not to be the case in reality if CONFIG_QOM_CAST_DEBUG is 
disabled:


Object *object_dynamic_cast_assert(Object *obj, const char *typename,
   const char *file, int line, const 
char *func)

{
trace_object_dynamic_cast_assert(obj ? obj->class->type->name : 
"(null)",

 typename, file, line, func);

#ifdef CONFIG_QOM_CAST_DEBUG
int i;
Object *inst;

for (i = 0; obj && i < OBJECT_CLASS_CAST_CACHE; i++) {
if (obj->class->object_cast_cache[i] == typename) {
goto out;
}
}

inst = object_dynamic_cast(obj, typename);

if (!inst && obj) {
fprintf(stderr, "%s:%d:%s: Object %p is not an instance of type 
%s\n",

file, line, func, obj, typename);
abort();
}

assert(obj == inst);

if (obj && obj == inst) {
for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) {
obj->class->object_cast_cache[i - 1] =
obj->class->object_cast_cache[i];
}
obj->class->object_cast_cache[i - 1] = typename;
}

out:
#endif
return obj;
}

Is that normal?

Thanks,
Fred



Thanks,

Alistair


+}
+
  static const TypeInfo qemu_clk_info = {
  .name  = TYPE_CLOCK,
  .parent= TYPE_OBJECT,
--
2.5.5

Re: [Qemu-devel] [RFC PATCH 05/11] docs: add qemu-clock documentation

[KONRAD Frederic]

Le 29/06/2016 à 02:38, Alistair Francis a écrit :

On Mon, Jun 13, 2016 at 9:27 AM,   wrote:

From: KONRAD Frederic 

This adds the qemu-clock documentation.

Signed-off-by: KONRAD Frederic 
---
  docs/clock.txt | 112 +
  1 file changed, 112 insertions(+)
  create mode 100644 docs/clock.txt

diff --git a/docs/clock.txt b/docs/clock.txt
new file mode 100644
index 000..f4ad4c8
--- /dev/null
+++ b/docs/clock.txt
@@ -0,0 +1,112 @@
+
+What is a QEMU_CLOCK
+
+
+A QEMU_CLOCK is a QOM Object developed for the purpose of modeling a clock tree
+with QEMU.
+
+It only simulates the clock by keeping a copy of the current frequency and
+doesn't model the signal itself such as pin toggle or duty cycle.
+
+It allows to model the impact of badly configured PLL, clock source selection
+or disabled clock on the models.
+
+Bounding the clock together to create a tree
+
+
+In order to create a clock tree with QEMU_CLOCK two or more clock must be bound
+together. Let's say there are two clocks clk_a and clk_b:
+Using qemu_clk_bound(clk_a, clk_b) will bound clk_a and clk_b.
+
+Binding two qemu-clk together is a unidirectional link which means that 
changing
+the rate of clk_a will propagate to clk_b and not the opposite. The bound
+process automatically refresh clk_b rate.
+
+Clock can be bound and unbound during execution for modeling eg: a clock
+selector.
+
+A clock can drive more than one other clock. eg with this code:
+qemu_clk_bound(clk_a, clk_b);
+qemu_clk_bound(clk_a, clk_c);
+
+A clock rate change one clk_a will propagate to clk_b and clk_c.
+
+Implementing a callback on a rate change
+
+
+The function prototype is the following:
+typedef float (*qemu_clk_rate_change_cb)(void *opaque, float rate);
+
+It's main goal is to modify the rate before it's passed to the next clocks in
+the tree.
+
+eg: for a 4x PLL the function will be:
+float qemu_clk_rate_change_cb(void *opaque, float rate)
+{
+return 4.0 * rate;
+}
+
+To set the callback for the clock:
+void qemu_clk_set_callback(qemu_clk clk, qemu_clk_on_rate_update_cb cb,
+   void *opaque);
+can be called.
+
+NOTE: It's not recommended that the clock is driven by more than one clock as 
it
+would mean that we don't know which clock trigger the callback.

Would this not be something worth knowing?


I think it shouldn't be the case as connecting two inputs on an output 
doesn't mean anything?
Maybe worth adding something to ensure it can't happen and modify 
qemu_clk_bind_clock to automatically unbind the old clock?


Fred


Thanks,

Alistair


+The rate update process
+===
+
+The rate update happen in this way:
+When a model wants to update a clock frequency (eg: based on a register change
+or something similar) it will call qemu_clk_update_rate(..) on the clock:
+  * The callback associated to the clock is called with the new rate.
+  * qemu_clk_update_rate(..) is then called on all bound clock with the
+value returned by the callback.
+
+NOTE: When no callback is attached the clock qemu_clk_update_rate(..) is called
+with the unmodified rate.
+
+Attaching a QEMU_CLOCK to a DeviceState
+===
+
+Attaching a qemu-clk to a DeviceState is required to be able to get the clock
+outside the model through qemu_clk_get_pin(..).
+
+It is also required to be able to print the clock and its rate with info qtree.
+For example:
+
+  type System
+  dev: xlnx.zynqmp_crf, id ""
+gpio-out "sysbus-irq" 1
+gpio-out "RST_A9" 4
+qemu-clk "dbg_trace" 0.0
+qemu-clk "vpll_to_lpd" 62500.0
+qemu-clk "dp_stc_ref" 0.0
+qemu-clk "dpll_to_lpd" 1250.0
+qemu-clk "acpu_clk" 0.0
+qemu-clk "pcie_ref" 0.0
+qemu-clk "topsw_main" 0.0
+qemu-clk "topsw_lsbus" 0.0
+qemu-clk "dp_audio_ref" 0.0
+qemu-clk "sata_ref" 0.0
+qemu-clk "dp_video_ref" 71428568.0
+qemu-clk "vpll_clk" 25.0
+qemu-clk "apll_to_lpd" 1250.0
+qemu-clk "dpll_clk" 5000.0
+qemu-clk "gpu_ref" 0.0
+qemu-clk "aux_refclk" 0.0
+qemu-clk "video_clk" 2700.0
+qemu-clk "gdma_ref" 0.0
+qemu-clk "gt_crx_ref_clk" 0.0
+qemu-clk "dbg_fdp" 0.0
+qemu-clk "apll_clk" 5000.0
+qemu-clk "pss_alt_ref_clk" 0.0
+qemu-clk "ddr" 0.0
+qemu-clk "pss_ref_clk" 5000.0
+qemu-clk "dpdma_ref" 0.0
+qemu-clk "dbg_tstmp" 0.0
+mmio fd1a/010c
+
+This way a DeviceState can have multiple clock input or output.
+
--
2.5.5

Re: [Qemu-devel] [RFC PATCH 06/11] introduce fixed-clock

[KONRAD Frederic]

Le 02/07/2016 à 01:07, Alistair Francis a écrit :

On Mon, Jun 13, 2016 at 9:27 AM,   wrote:

From: KONRAD Frederic 

This is a fixed clock device.
It justs behave as an empty device with a parametrable output rate.

Signed-off-by: KONRAD Frederic 
---
  hw/misc/Makefile.objs |  2 +
  hw/misc/fixed-clock.c | 87 +++
  include/hw/misc/fixed-clock.h | 30 +++
  3 files changed, 119 insertions(+)
  create mode 100644 hw/misc/fixed-clock.c
  create mode 100644 include/hw/misc/fixed-clock.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index e504463..e8b8855 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -52,3 +52,5 @@ obj-$(CONFIG_MIPS_ITU) += mips_itu.o
  obj-$(CONFIG_PVPANIC) += pvpanic.o
  obj-$(CONFIG_EDU) += edu.o
  obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
+
+obj-y += fixed-clock.o
diff --git a/hw/misc/fixed-clock.c b/hw/misc/fixed-clock.c
new file mode 100644
index 000..c273a91
--- /dev/null
+++ b/hw/misc/fixed-clock.c
@@ -0,0 +1,87 @@
+/*
+ * Fixed clock
+ *
+ *  Copyright (C) 2016 : GreenSocs Ltd
+ *  http://www.greensocs.com/ , email: i...@greensocs.com
+ *
+ *  Frederic Konrad   
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU 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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+#include "hw/misc/fixed-clock.h"
+#include "qemu/qemu-clock.h"
+#include "qapi/error.h"
+
+/* #define DEBUG_FIXED_CLOCK */

Don't include this.


+
+#ifdef DEBUG_FIXED_CLOCK
+#define DPRINTF(fmt, ...) \
+do { printf("fixed-clock: " fmt , ## __VA_ARGS__); } while (0)

It might be better to use __func__ here.

It should also be qemu_log instead of printf().


+#else
+#define DPRINTF(fmt, ...) do { } while (0)
+#endif
+
+typedef struct {
+DeviceState parent_obj;
+
+uint32_t rate;
+struct qemu_clk out;
+} FixedClock;

Doesn't this need to be in the header file?


I think it's not necessary as we get the clock through the API?



+
+static Property fixed_clock_properties[] = {
+DEFINE_PROP_UINT32("rate", FixedClock, rate, 0),
+DEFINE_PROP_END_OF_LIST()
+};
+
+static void fixed_clock_realizefn(DeviceState *d, Error **errp)

dev instead of d

Thanks,

Alistair


+{
+FixedClock *s = FIXED_CLOCK(d);
+
+qemu_clk_update_rate(&s->out, s->rate);
+}
+
+static void fixed_clock_instance_init(Object *obj)
+{
+FixedClock *s = FIXED_CLOCK(obj);
+
+object_initialize(&s->out, sizeof(s->out), TYPE_CLOCK);
+qemu_clk_attach_to_device(DEVICE(obj), &s->out, "clk_out");
+}
+
+static void fixed_clock_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->realize = fixed_clock_realizefn;
+dc->props = fixed_clock_properties;
+}
+
+static const TypeInfo fixed_clock_info = {
+.name  = TYPE_FIXED_CLOCK,
+.parent= TYPE_DEVICE,
+.instance_size = sizeof(FixedClock),
+.instance_init = fixed_clock_instance_init,
+.class_init= fixed_clock_class_init,
+};
+
+static void fixed_clock_register_types(void)
+{
+type_register_static(&fixed_clock_info);
+}
+
+type_init(fixed_clock_register_types);
diff --git a/include/hw/misc/fixed-clock.h b/include/hw/misc/fixed-clock.h
new file mode 100644
index 000..1376444
--- /dev/null
+++ b/include/hw/misc/fixed-clock.h
@@ -0,0 +1,30 @@
+/*
+ * Fixed clock
+ *
+ *  Copyright (C) 2016 : GreenSocs Ltd
+ *  http://www.greensocs.com/ , email: i...@greensocs.com
+ *
+ *  Frederic Konrad   
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU 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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef FIXED_CLOCK_H
+#define FIXED_CLOCK_H
+
+#define TYPE_FIXED_CLOCK "fixed-clock"
+#define FIXED_CLOCK(obj) OBJECT_CHECK(FixedClock, (obj), TYPE_FIXED_CLOCK)
+
+#endif /* FIXED_CLOCK_H */
--
2.5.5

Re: [Qemu-devel] [RFC PATCH 03/11] qemu-clk: allow to bound two clocks together

[KONRAD Frederic]

Le 29/07/2016 à 15:39, Peter Maydell a écrit :

On 13 June 2016 at 17:27,   wrote:

From: KONRAD Frederic 

This introduces the clock binding and the update part.
When the qemu_clk_rate_update(qemu_clk, int) function is called:
   * The clock callback is called on the qemu_clk so it can change the rate.
   * The qemu_clk_rate_update function is called on all the driven clock.

Signed-off-by: KONRAD Frederic 
---
  include/qemu/qemu-clock.h | 65 +++
  qemu-clock.c  | 56 
  2 files changed, 121 insertions(+)

diff --git a/include/qemu/qemu-clock.h b/include/qemu/qemu-clock.h
index a2ba105..677de9a 100644
--- a/include/qemu/qemu-clock.h
+++ b/include/qemu/qemu-clock.h
@@ -27,15 +27,29 @@
  #include "qemu/osdep.h"
  #include "qom/object.h"

+typedef float (*qemu_clk_on_rate_update_cb)(void *opaque, float rate);
+
  #define TYPE_CLOCK "qemu-clk"
  #define QEMU_CLOCK(obj) OBJECT_CHECK(struct qemu_clk, (obj), TYPE_CLOCK)

+typedef struct ClkList ClkList;
+
  typedef struct qemu_clk {
  /*< private >*/
  Object parent_obj;
  char *name;/* name of this clock in the device. */
+float in_rate; /* rate of the clock which drive this pin. */
+float out_rate;/* rate of this clock pin. */

I'm rather dubious that we should be using floats here.
Almost nothing in our hardware emulation uses float or double.


We probably can use uint64_t here?

Thanks,
Fred



+void *opaque;
+qemu_clk_on_rate_update_cb cb;
+QLIST_HEAD(, ClkList) bound;
  } *qemu_clk;

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 09/11] zynqmp_crf: add the clock mechanism

[KONRAD Frederic]

Le 02/07/2016 à 01:23, Alistair Francis a écrit :

On Mon, Jun 13, 2016 at 9:27 AM,   wrote:

From: KONRAD Frederic 

This adds the pll to the zynqmp_crf and the dp_video clock output.

Signed-off-by: KONRAD Frederic 
---
  hw/misc/xilinx_zynqmp_crf.c | 440 
  1 file changed, 440 insertions(+)

diff --git a/hw/misc/xilinx_zynqmp_crf.c b/hw/misc/xilinx_zynqmp_crf.c
index 4c670a0..2097534 100644
--- a/hw/misc/xilinx_zynqmp_crf.c
+++ b/hw/misc/xilinx_zynqmp_crf.c
@@ -30,6 +30,7 @@
  #include "hw/register.h"
  #include "qemu/bitops.h"
  #include "qemu/log.h"
+#include "qemu/qemu-clock.h"

  #ifndef XILINX_CRF_APB_ERR_DEBUG
  #define XILINX_CRF_APB_ERR_DEBUG 0
@@ -281,6 +282,38 @@ typedef struct CRF_APB {

  uint32_t regs[R_MAX];
  RegisterInfo regs_info[R_MAX];
+
+/* input clocks */
+qemu_clk pss_ref_clk;
+qemu_clk video_clk;
+qemu_clk pss_alt_ref_clk;
+qemu_clk aux_refclk;
+qemu_clk gt_crx_ref_clk;
+
+/* internal clocks */
+qemu_clk apll_clk;
+qemu_clk dpll_clk;
+qemu_clk vpll_clk;
+
+/* output clocks */
+qemu_clk acpu_clk;
+qemu_clk dbg_trace;
+qemu_clk dbg_fdp;
+qemu_clk dp_video_ref;
+qemu_clk dp_audio_ref;
+qemu_clk dp_stc_ref;
+qemu_clk ddr;
+qemu_clk gpu_ref;
+qemu_clk sata_ref;
+qemu_clk pcie_ref;
+qemu_clk gdma_ref;
+qemu_clk dpdma_ref;
+qemu_clk topsw_main;
+qemu_clk topsw_lsbus;
+qemu_clk dbg_tstmp;
+qemu_clk apll_to_lpd;
+qemu_clk dpll_to_lpd;
+qemu_clk vpll_to_lpd;
  } CRF_APB;

  static const MemoryRegionOps crf_apb_ops = {
@@ -325,6 +358,318 @@ static uint64_t ir_disable_prew(RegisterInfo *reg, 
uint64_t val64)
  return 0;
  }

+enum clk_src {
+VIDEO_CLK = 4,
+PSS_ALT_REF_CLK = 5,
+AUX_REF_CLK = 6,
+GT_CRX_REF_CLK = 7,
+PSS_REF_CLK = 0
+};
+
+static void apll_to_lpd_postw(RegisterInfo *reg, uint64_t val64)
+{
+CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+
+qemu_clk_refresh(s->apll_to_lpd);
+}
+
+static float apll_to_lpd_update_rate(void *opaque, float input_rate)
+{
+CRF_APB *s = XILINX_CRF_APB(opaque);
+uint32_t divisor = AF_EX32(s->regs, APLL_TO_LPD_CTRL, DIVISOR0);
+
+if (!divisor) {
+return 0.0f;
+} else {
+return input_rate / (float)divisor;
+}
+}
+
+static void dpll_to_lpd_postw(RegisterInfo *reg, uint64_t val64)
+{
+CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+
+qemu_clk_refresh(s->dpll_to_lpd);
+}
+
+static float dpll_to_lpd_update_rate(void *opaque, float input_rate)
+{
+CRF_APB *s = XILINX_CRF_APB(opaque);
+uint32_t divisor = AF_EX32(s->regs, DPLL_TO_LPD_CTRL, DIVISOR0);
+
+if (!divisor) {
+return 0.0f;
+} else {
+return input_rate / (float)divisor;
+}
+}
+
+static void vpll_to_lpd_postw(RegisterInfo *reg, uint64_t val64)
+{
+CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+
+qemu_clk_refresh(s->vpll_to_lpd);
+}
+
+static float vpll_to_lpd_update_rate(void *opaque, float input_rate)
+{
+CRF_APB *s = XILINX_CRF_APB(opaque);
+uint32_t divisor = AF_EX32(s->regs, VPLL_TO_LPD_CTRL, DIVISOR0);
+
+if (!divisor) {
+return 0.0f;
+} else {
+return input_rate / (float)divisor;
+}
+}
+
+static void apll_ctrl_postw(RegisterInfo *reg, uint64_t val64)
+{
+CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+uint32_t source = AF_EX32(s->regs, APLL_CTRL, BYPASS)
+? AF_EX32(s->regs, APLL_CTRL, POST_SRC)
+: AF_EX32(s->regs, APLL_CTRL, PRE_SRC);
+
+/*
+ * We must ensure that only one clock is bound to the apll internal clock.
+ */
+qemu_clk_unbound(s->pss_ref_clk, s->apll_clk);
+qemu_clk_unbound(s->video_clk, s->apll_clk);
+qemu_clk_unbound(s->pss_alt_ref_clk, s->apll_clk);
+qemu_clk_unbound(s->aux_refclk, s->apll_clk);
+qemu_clk_unbound(s->gt_crx_ref_clk, s->apll_clk);
+
+switch (source) {
+case VIDEO_CLK:
+qemu_clk_bound_clock(s->video_clk, s->apll_clk);
+break;
+case PSS_ALT_REF_CLK:
+qemu_clk_bound_clock(s->pss_alt_ref_clk, s->apll_clk);
+break;
+case AUX_REF_CLK:
+qemu_clk_bound_clock(s->aux_refclk, s->apll_clk);
+break;
+case GT_CRX_REF_CLK:
+qemu_clk_bound_clock(s->gt_crx_ref_clk, s->apll_clk);
+break;
+default:
+qemu_clk_bound_clock(s->pss_ref_clk, s->apll_clk);
+break;
+}
+}
+
+static void dpll_ctrl_postw(RegisterInfo *reg, uint64_t val64)
+{
+CRF_APB *s = XILINX_CRF_APB(reg->opaque);
+uint32_t source = AF_EX32(s->regs, DPLL_CTRL, BYPASS)
+? AF_EX32(s->regs, DPLL_CTRL, POST_SRC)
+: AF_EX32(s->regs, DPLL_CTRL, PRE_SRC);
+
+/*
+ * We must ensure that only one clock is bound to the dpll internal clock.
+  

Re: [Qemu-devel] [RFC PATCH 08/11] zynqmp_crf: fix against AF_EX32 changes

[KONRAD Frederic]

Le 29/07/2016 à 15:48, Peter Maydell a écrit :

On 13 June 2016 at 17:27,   wrote:

From: KONRAD Frederic 

This seems to be due to a difference between the AF_EX32 define.

Signed-off-by: KONRAD Frederic 
---
  hw/misc/xilinx_zynqmp_crf.c | 354 ++--
  1 file changed, 177 insertions(+), 177 deletions(-)

This should just be squashed into the previous patch.
Yes, my intention was to keep that separate in a first time as I'm not 
sure if this is a bug in the original file.


Thanks,
Fred

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 10/11] zynqmp: add the zynqmp_crf to the platform

[KONRAD Frederic]

Le 02/07/2016 à 01:11, Alistair Francis a écrit :

On Mon, Jun 13, 2016 at 9:27 AM,   wrote:

From: KONRAD Frederic 

This adds the zynqmp_crf to the zynqmp platform.

Signed-off-by: KONRAD Frederic 
---
  hw/arm/xlnx-zynqmp.c | 7 +++
  include/hw/arm/xlnx-zynqmp.h | 1 +
  2 files changed, 8 insertions(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 4d504da..a8b7669 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -135,6 +135,11 @@ static void xlnx_zynqmp_init(Object *obj)
TYPE_XILINX_SPIPS);
  qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
  }
+
+s->crf = object_new("xlnx.zynqmp_crf");
+qdev_set_parent_bus(DEVICE(s->crf), sysbus_get_default());
+object_property_add_child(obj, "xlnx.zynqmp_crf", OBJECT(s->crf),
+  &error_abort);
  }

  static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -366,6 +371,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
&error_abort);
 g_free(bus_name);
  }
+
+sysbus_mmio_map(SYS_BUS_DEVICE(s->crf), 0, 0xFD1A);

Shouldn't this be realised?

Also macro for the address.


  }

  static Property xlnx_zynqmp_props[] = {
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 2332596..78fed6e 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -81,6 +81,7 @@ typedef struct XlnxZynqMPState {
  SysbusAHCIState sata;
  SDHCIState sdhci[XLNX_ZYNQMP_NUM_SDHCI];
  XilinxSPIPS spi[XLNX_ZYNQMP_NUM_SPIS];
+Object *crf;

Can we follow the same way as the other devices?

Yes I can do that.



You'll need to split a header file out for the device then.

Thanks,

Alistair


  char *boot_cpu;
  ARMCPU *boot_cpu_ptr;
--
2.5.5

Re: [Qemu-devel] [RFC PATCH 09/11] zynqmp_crf: add the clock mechanism

[KONRAD Frederic]

Le 29/07/2016 à 15:51, Peter Maydell a écrit :

On 13 June 2016 at 17:27,   wrote:

From: KONRAD Frederic 

This adds the pll to the zynqmp_crf and the dp_video clock output.

Signed-off-by: KONRAD Frederic 
---
 hw/misc/xilinx_zynqmp_crf.c | 440 
 1 file changed, 440 insertions(+)

diff --git a/hw/misc/xilinx_zynqmp_crf.c b/hw/misc/xilinx_zynqmp_crf.c
index 4c670a0..2097534 100644
--- a/hw/misc/xilinx_zynqmp_crf.c
+++ b/hw/misc/xilinx_zynqmp_crf.c
@@ -30,6 +30,7 @@
 #include "hw/register.h"
 #include "qemu/bitops.h"
 #include "qemu/log.h"
+#include "qemu/qemu-clock.h"

 #ifndef XILINX_CRF_APB_ERR_DEBUG
 #define XILINX_CRF_APB_ERR_DEBUG 0
@@ -281,6 +282,38 @@ typedef struct CRF_APB {

 uint32_t regs[R_MAX];
 RegisterInfo regs_info[R_MAX];
+
+/* input clocks */
+qemu_clk pss_ref_clk;
+qemu_clk video_clk;
+qemu_clk pss_alt_ref_clk;
+qemu_clk aux_refclk;
+qemu_clk gt_crx_ref_clk;
+
+/* internal clocks */
+qemu_clk apll_clk;
+qemu_clk dpll_clk;
+qemu_clk vpll_clk;
+
+/* output clocks */
+qemu_clk acpu_clk;
+qemu_clk dbg_trace;
+qemu_clk dbg_fdp;
+qemu_clk dp_video_ref;
+qemu_clk dp_audio_ref;
+qemu_clk dp_stc_ref;
+qemu_clk ddr;
+qemu_clk gpu_ref;
+qemu_clk sata_ref;
+qemu_clk pcie_ref;
+qemu_clk gdma_ref;
+qemu_clk dpdma_ref;
+qemu_clk topsw_main;
+qemu_clk topsw_lsbus;
+qemu_clk dbg_tstmp;
+qemu_clk apll_to_lpd;
+qemu_clk dpll_to_lpd;
+qemu_clk vpll_to_lpd;
 } CRF_APB;


This looks a bit weird. Why are the input clocks and the output
clocks the same type? I was expecting that an output clock would
be "owned" by this device (and so a qemu_clk), whereas an input
clock would just be a reference to a clock owned by the device
on the other end of it.



Hi Peter,

Yes this is a choice I had to make.
Basically there is nothing different between what we call output and 
input: They both allow to set a callback before the frequency is 
transported to the next clock in the clock tree.

And I use a name to get the clock, eg: for a simple clock divider:

  
in --o| ClockDivider |o-- out
  

will be modeled like this in terms of qemu_clock:

freq_update_cb()
  return rate/4;
  |
  |
 \_/

 --o| qemu_clk |o-- bindings --o| qemu_clk |o--
|in||out   |


Then to bind that to a fixed clock we have in the machine model:
  qemu_clk_bind_clock(
qemu_clk_get_pin(FixedClock, "out"),
qemu_clk_get_pin(ClockDivider, "in"));

Thanks,
Fred


thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 14/06/2017 à 13:54, Paolo Bonzini a écrit :

On 13/06/2017 12:33, Peter Maydell wrote:

For the migration maybe we can refresh the whole clock tree at the end
of the migration. Is that a good idea?

That seems kind of awkward -- where would this code that did a
clock tree refresh be? Also you're then reliant on all the
callback functions registered to not actually do anything that
affects device state when the refresh happens. It would be
cleaner if the qemu-clk objects managed their own internal
state migration (but can we do this without having to make
them be Device objects rather than just objects ?).

Cc'd Paolo who might have an opinion on these -- my opinion currently
is mostly "this doesn't really look right" rather than knowing what
the right approach is.


Same here. :)

I think the various bindings and rates could be refreshed as devices are
migrated.

> "This assumes that the device migration order is okay"

I think it's the real problem here?


according to the clock tree, that is if you have three devices X/Y/Z and
five clocks a/b/c/d/e/f:

  fixed-clock
| |
   X:a   X:b
| |\
   Y:c   Y:d  Z:e
  |
 Z:f

you could do this:

- migrate X
  - retrieve the PLL ratios for a and b's bound clocks (if the ratio
is variable, otherwise no need for this)
  - in the post_load callback, bind a and b to the fixed-clock
(if the binding is variable, otherwise no need for this)
- migrate Y
  - retrieve the PLL ratio for d's bound clocks (if the ratio
is variable, otherwise no need for this)
  - in the post_load callback, bind c and d to a and b respectively
(if the binding is variable, otherwise no need for this)
- migrate Z
  - in the post_load callback, bind e and f to b and d respectively
(if the binding is variable, otherwise no need for this)



So for you the migration of the clock tree will be done by the device?
I think this is OK :).

Fred


Paolo

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 15/06/2017 à 17:15, Edgar E. Iglesias a écrit :

On Thu, Jun 15, 2017 at 04:04:56PM +0100, Peter Maydell wrote:

On 15 June 2017 at 15:57, Edgar E. Iglesias  wrote:

On Thu, Jun 15, 2017 at 03:40:40PM +0100, Peter Maydell wrote:

Unfortunately we make no guarantees at all about migration order
for devices as far as I'm aware, so devices have to cope regardless.



How does this work for interrupts/gpios?


Interrupts/gpios (qemu_irqs) don't have any internal state,
so all that is needed is for both devices to correctly migrate
their idea of their internal state, and it doesn't matter which
order that happens in. (Typically in QEMU devices track the state
of their inbound interrupt lines even if in real hardware there's
no flop doing that.)

The difference here is that the clock objects themselves have
internal state. That's not necessarily a bad idea, but it does
mean that something's got to migrate that state or otherwise
regenerate it. (Anthony once proposed that we should change qemu_irq
objects to have internal state, because that's effectively what real
hardware is and it would save the need for each device to track its
input line state and be notified if the line didn't actually change
state. It would just have been an enormous upheaval and migration
compat break...)


Thanks. I didn't realize that the internal clock state would be used in a
way that is not deriveable from other device state & input clocks.
I'm not sure this is a use-case we need to support, thoughts?

Maybe there's some value in keeping interrupt and clock handling alike,
e.g removing the internal state from clocks.

I need to have another look at the series before I comment too much
since I don't remember the details...



The use case for that is essentially knowing the rate value when
needed:
  * for example in the monitor.
  * if don't want to refresh the whole tree.

For the rest.. yes we can just keep the value in the device state..

Fred


Best regards,
Edgar

[Qemu-devel] Changing email address.

[KONRAD Frederic]

Hi guys,

I recently changed company, so I won't be available on the old
email address.

I will still be working on QEMU, publishing patches on the list
and keeping an eye on the on going clock series.

So better using my new address if you want to reach or CC me :).

Thanks,
Fred

Re: [Qemu-devel] [PULL v1 0/7] MMIO Exec pull request

[KONRAD Frederic]

Le 06/23/2017 à 12:54 PM, Peter Maydell a écrit :

On 14 June 2017 at 18:45, Edgar E. Iglesias  wrote:

From: "Edgar E. Iglesias" 

Hi,

Paolo suggested offline that we send a pull request for this series.
Here it is, I've run it through my testsuite + tested the LQSPI testcase
on Zynq.

Cheers,
Edgar

The following changes since commit 3f0602927b120a480b35dcf58cf6f95435b3ae91:

   Merge remote-tracking branch 
'remotes/pmaydell/tags/pull-target-arm-20170613' into staging (2017-06-13 
15:49:07 +0100)

are available in the git repository at:

   g...@github.com:edgarigl/qemu.git tags/edgar/mmio-exec.for-upstream

for you to fetch changes up to 63ef40dd6bc6cfdae5fa67ccac1cb11e7a5161b0:

   xilinx_spips: allow mmio execution (2017-06-14 17:31:08 +0200)


mmio-exec.for-upstream

----
KONRAD Frederic (7):
   cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT
   cputlb: move get_page_addr_code
   cputlb: fix the way get_page_addr_code fills the tlb
   qdev: add MemoryRegion property
   introduce mmio_interface
   exec: allow to get a pointer for some mmio memory region
   xilinx_spips: allow mmio execution


Hi Edgar -- since these patches have come via your tree they should
have your signed-off-by line, not just Fred's. They also seem to
still have all the V1->V2->V3 revision notes in the commit logs.

I'd be happier with a reviewed-by: from somebody for the exec:
patch, if possible...

thanks
-- PMM



Hi Peter,

Actually if I remember right Edgar reviewed them but put the
review-by tag in the cover letter.

Thanks,
Fred

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 06/23/2017 à 11:51 AM, Peter Maydell a écrit :

On 15 June 2017 at 16:15, Edgar E. Iglesias  wrote:

On Thu, Jun 15, 2017 at 04:04:56PM +0100, Peter Maydell wrote:

The difference here is that the clock objects themselves have
internal state. That's not necessarily a bad idea, but it does
mean that something's got to migrate that state or otherwise
regenerate it. (Anthony once proposed that we should change qemu_irq
objects to have internal state, because that's effectively what real
hardware is and it would save the need for each device to track its
input line state and be notified if the line didn't actually change
state. It would just have been an enormous upheaval and migration
compat break...)


Thanks. I didn't realize that the internal clock state would be used in a
way that is not deriveable from other device state & input clocks.
I'm not sure this is a use-case we need to support, thoughts?

Maybe there's some value in keeping interrupt and clock handling alike,
e.g removing the internal state from clocks.


It occurs to me that it might be possible to split the
difference, ie the clock object still has internal state,
but the device that owns it is responsible for that state's
migration, so you would have
   VMSTATE_CLOCK(myclock, MyDeviceState),


Yes I think that makes sense.



in the vmstate for every clock you owned. (Not sure whether
that should be "every input clock" or "every output clock"
or even both?)

As an aside, I still find it very odd that you get a clock
object for both an input clock and an output clock. I feel
like we should have one end owns the clock object and the
other just has a reference to it of some kind.


The point is beeing able to bind/unbind clocks inside the device
to model a clock selector for example. So you just bind/unbind
the input and output clocks.

Secondly you can print the clock input rate in the mtree command
so you can eg: debug it.

Thanks,
Fred



thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 06/23/2017 à 02:47 PM, Peter Maydell a écrit :

On 23 June 2017 at 13:38, KONRAD Frederic  wrote:

Le 06/23/2017 à 11:51 AM, Peter Maydell a écrit :

As an aside, I still find it very odd that you get a clock
object for both an input clock and an output clock. I feel
like we should have one end owns the clock object and the
other just has a reference to it of some kind.



The point is beeing able to bind/unbind clocks inside the device
to model a clock selector for example. So you just bind/unbind
the input and output clocks.


I don't see why this needs both 'input clock' and 'output
clock' to be separate objects. What I have in mind is:

Device A:
   has-a clock C1

Device B:
   has-a clock C2

Device C: (a clock selector)
   clock inputs CI1, CI2
   has-a clock C3

Each device "owns" its output clock objects, but input
clocks are just pointers to the clock object owned by the
device at the other end. In the board you wire up CI1 to C1,
and CI2 to C2 (using link properties I guess).
Then in device C you can implement the clock switching by
some kind of bind(s->CI1, &s->C3) call because you have
pointers to all the relevant clock objects.

As I understand it your current implementation makes not
just the output clocks C1 C2 C2 be clock objects, but also
the inputs CI1 CI2, so effectively each link from a clock
source to a clock sink has two objects involved.


Yes that makes sense but you won't have the name.

And one other thing I wanted to have is beeing able to refresh
the clock tree without refreshing everything. If you start a
refresh from the "pointer" and it is bind to other things you
will refresh the other devices as well. Maybe we don't care or we
can workaround that but..

Fred



thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 06/23/2017 à 03:58 PM, Peter Maydell a écrit :

On 23 June 2017 at 14:07, KONRAD Frederic  wrote:

Le 06/23/2017 à 02:47 PM, Peter Maydell a écrit :

Each device "owns" its output clock objects, but input
clocks are just pointers to the clock object owned by the
device at the other end. In the board you wire up CI1 to C1,
and CI2 to C2 (using link properties I guess).
Then in device C you can implement the clock switching by
some kind of bind(s->CI1, &s->C3) call because you have
pointers to all the relevant clock objects.

As I understand it your current implementation makes not
just the output clocks C1 C2 C2 be clock objects, but also
the inputs CI1 CI2, so effectively each link from a clock
source to a clock sink has two objects involved.



Yes that makes sense but you won't have the name.


I don't think that's a big deal.


And one other thing I wanted to have is beeing able to refresh
the clock tree without refreshing everything. If you start a
refresh from the "pointer" and it is bind to other things you
will refresh the other devices as well. Maybe we don't care or we
can workaround that but..


The pointer is for your clock inputs -- when would you
want to start a refresh from that? I would expect
refreshes to only ever go downstream -- you update
the config of your clock outputs and things downstream
of them will update in turn.


I started with the goal in mind that the binding + the callback
can refresh themself without user intervention.

So actually the user only have to change the binding in case of a
clock selector.. Everything else is done by the framework.

For example if we want to change a multiplier through a register:

void register_write(..)
{
  /* Refresh related clock input. */
}

void clock_cb(..)
{
  return register_value * rate_in;
}

If we drop the clock first possibility:

void register_write(..)
{
  /* refresh all depending clock with
   * rate_in * register value.
   * Either this can be tricky as we need to know exactly which
   * clock need to be refreshed or we need to refresh everybody.
   * On the device example in the patch-set this can become a
   * mess.
   */
}

void clock_cb(..)
{
  return register_value * rate_in;
}

Second possibility:

void register_write(..)
{
  /* refresh the clock which is referenced as input.
   * This is easy BUT it will refresh all other devices bound to
   * this clock.
   */
}

void clock_cb(..)
{
  return register_value * rate_in;
}

Thanks,
Fred



thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Le 06/06/2017 à 17:18, Peter Maydell a écrit :

On 24 May 2017 at 08:35, KONRAD Frederic  wrote:

Le 28/02/2017 à 11:02, fred.kon...@greensocs.com a écrit :

This is the third version of the clock framework API it contains:

  * The first 6 patches which introduce the framework.
  * The 7th patch which introduces a fixed-clock model.
  * The rest which gives an example how to model a PLL from the existing
zynqmp-crf extracted from the qemu xilinx tree.

No specific behavior is expected yet when the CRF register set is accessed
but
the user can see for example the dp_video_ref and vpll_to_lpd rate
changing in
the monitor with the "info qtree" command when the vpll_ctrl register is
modified.


Some top-level review:

* I like the documentation, this is very helpful
* consider tracepoints rather than DPRINTF macro
* qemu_clk_device_add_clock() does a g_strdup, but when is this freed?
  (consider devices which are hot-unpluggable)
* similarly, what if a device with a clock with a lot of child nodes
  is destroyed? how are the ClkList structs freed?
* interaction with migration -- how is the "this clock is at this rate"
  state intended to be migrated?
* I'll leave the review of the xilinx patches to the xilinx folk
* the 'introduce zynqmp_crf' patch is missing any signoffs
  (in particular if it's from the xilinx tree it will need
  signoff from them)

thanks
-- PMM



Nice thanks for the feedback!

Actually the hot unpluggable is a good question what can we do for that?
Is that reasonable for a device which produce a clock to be hot
unpluggable?

For the migration maybe we can refresh the whole clock tree at the end
of the migration. Is that a good idea?

Fred

Re: [Qemu-devel] [PATCH V2 0/7] execute code from mmio area

[KONRAD Frederic]

Ping!

Would be nice for us if we can get this into 2.9.

Thanks,
Fred

Le 17/02/2017 à 21:17, fred.kon...@greensocs.com a écrit :

From: KONRAD Frederic 

This series allows to execute code from mmio areas.
The main goal of this is to be able to run code for example from an SPI device.

The three first patch fixes the way get_page_addr_code fills the TLB.

The fourth patch implements the mmio execution helpers: the device must
implement the request_ptr callback of the MemoryRegion and will be notified when
the guest wants to execute code from it.

The fifth patch introduces mmio_interface device which allows to dynamically
map a host pointer somewhere into the memory.

The sixth patch implements the execution from the SPI memories in the
xilinx_spips model.

Thanks,
Fred

V1 -> V2:
  * Fix the DPRINTF error.
RFC -> V1:
  * Use an interface (mmio-interface) to fix any reference leak issue.

KONRAD Frederic (7):
  cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT
  cputlb: move get_page_addr_code
  cputlb: fix the way get_page_addr_code fills the tlb
  exec: allow to get a pointer for some mmio memory region
  qdev: add MemoryRegion property
  introduce mmio_interface
  xilinx_spips: allow mmio execution

 cputlb.c |  81 ++---
 hw/misc/Makefile.objs|   1 +
 hw/misc/mmio_interface.c | 128 +++
 hw/ssi/xilinx_spips.c|  74 --
 include/exec/memory.h|  35 +++
 include/hw/misc/mmio_interface.h |  49 +++
 include/hw/qdev-properties.h |   2 +
 memory.c |  57 +
 8 files changed, 372 insertions(+), 55 deletions(-)
 create mode 100644 hw/misc/mmio_interface.c
 create mode 100644 include/hw/misc/mmio_interface.h

Re: [Qemu-devel] [PULL v1 0/7] MMIO Exec pull request

[KONRAD Frederic]

On 07/17/2017 07:27 PM, Edgar E. Iglesias wrote:

On Mon, Jul 17, 2017 at 11:33 PM, Peter Maydell 
wrote:


On 14 June 2017 at 18:45, Edgar E. Iglesias 
wrote:

From: "Edgar E. Iglesias" 
Paolo suggested offline that we send a pull request for this series.
Here it is, I've run it through my testsuite + tested the LQSPI testcase
on Zynq.




mmio-exec.for-upstream

--------
KONRAD Frederic (7):
   cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT
   cputlb: move get_page_addr_code
   cputlb: fix the way get_page_addr_code fills the tlb
   qdev: add MemoryRegion property
   introduce mmio_interface
   exec: allow to get a pointer for some mmio memory region
   xilinx_spips: allow mmio execution


Hi Edgar -- can you or Fred explain how this code interacts with
VM migration? The mmio-interface device creates a RAM memory
region with memory_region_init_ram_ptr(), but it doesn't call
vmstate_register_ram(). On the other hand the core migration code
will try to migrate the contents of the RAMBlock anyway, just
without a name.

It's not clear to me how this works, and it would be nice to
get it clear so that we can make any necessary fixes before the
2.10 release hits and we lose the opportunity to make any
migration-compatibility-breaking changes.

thanks
-- PMM



Hi Peter,

These temporary regions should be read-only and treated as temporary caches
AFAIU things.
I would say that they don't need to be migrated. After migration, the new
VM will recreate the ram areas from device backing.

Is there a way we can prevent migration of the RAMBlock?

Cheers,
Edgar



Hi All,

Yes Edgar is right, they don't need to be migrated (as the old
Xilinx SPI cache).
And it will be the case for all the other stuff using this as
well.

Maybe we can simply drop the region before the migration?

Fred

[Qemu-devel] [PATCH v1 1/3] add memory_region_get_offset_within_address_space

[KONRAD Frederic]

This is helpful in the next patch to know if a rom is pointed by an alias.

Signed-off-by: KONRAD Frederic 
---
 include/exec/memory.h | 10 ++
 memory.c  | 22 --
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8503685..e342412 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1270,6 +1270,16 @@ void memory_region_set_size(MemoryRegion *mr, uint64_t 
size);
 void memory_region_set_alias_offset(MemoryRegion *mr,
 hwaddr offset);
 
+/*
+ * memory_region_get_offset_within_address_space: get the offset of a region
+ *
+ * Returns the offset of a region within its address space. @mr must be mapped
+ * to an #AddressSpace.
+ *
+ * @mr: the #MemoryRegion to check.
+ */
+hwaddr memory_region_get_offset_within_address_space(MemoryRegion *mr);
+
 /**
  * memory_region_present: checks if an address relative to a @container
  * translates into #MemoryRegion within @container
diff --git a/memory.c b/memory.c
index 1044bba..2b7439b 100644
--- a/memory.c
+++ b/memory.c
@@ -598,11 +598,18 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
 return r;
 }
 
-static AddressSpace *memory_region_to_address_space(MemoryRegion *mr)
+static AddressSpace *memory_region_to_address_space(MemoryRegion *mr,
+hwaddr *offset)
 {
 AddressSpace *as;
 
+if (offset) {
+*offset = 0;
+}
 while (mr->container) {
+if (offset) {
+*offset += mr->addr;
+}
 mr = mr->container;
 }
 QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
@@ -613,6 +620,17 @@ static AddressSpace 
*memory_region_to_address_space(MemoryRegion *mr)
 return NULL;
 }
 
+hwaddr memory_region_get_offset_within_address_space(MemoryRegion *mr)
+{
+hwaddr offset;
+AddressSpace *as;
+
+as = memory_region_to_address_space(mr, &offset);
+assert(as);
+
+return offset;
+}
+
 /* Render a memory region into the global view.  Ranges in @view obscure
  * ranges in @mr.
  */
@@ -2251,7 +2269,7 @@ static MemoryRegionSection 
memory_region_find_rcu(MemoryRegion *mr,
 addr += root->addr;
 }
 
-as = memory_region_to_address_space(root);
+as = memory_region_to_address_space(root, NULL);
 if (!as) {
 return ret;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v1 3/3] armv7m_systick: abort instead of locking on a bad rate

[KONRAD Frederic]

This helps the board developer by asserting that system_clock_rate is not
null. Using systick with a zero rate will lead to a deadlock so better showing
the error.

Signed-off-by: KONRAD Frederic 
---
 hw/timer/armv7m_systick.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
index df8d280..745efb7 100644
--- a/hw/timer/armv7m_systick.c
+++ b/hw/timer/armv7m_systick.c
@@ -54,6 +54,9 @@ static void systick_reload(SysTickState *s, int reset)
 s->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 }
 s->tick += (s->reload + 1) * systick_scale(s);
+
+/* system_clock_scale = 0 leads to a nasty deadlock, better aborting */
+assert(systick_scale(s));
 timer_mod(s->timer, s->tick);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v1 0/3] Some armv7m fixes

[KONRAD Frederic]

Hi,

While playing with armv7m, I found two little bugs:
  - When there is an alias @0x to a flash memory the cpu state isn't
reset correctly which leads later to an exception as ARM instruction-set is
used. Presumably this bug might be present with the netduino2 board.
  - If the developer omits to set system_clock_rate we later go in a livelock
when systick is triggered. Better aborting before to avoid the pain chasing
the livelock.

Thanks,
Fred

KONRAD Frederic (3):
  add memory_region_get_offset_within_address_space
  arm: fix the armv7m reset state
  armv7m_systick: abort instead of locking on a bad rate

 hw/timer/armv7m_systick.c |  3 +++
 include/exec/memory.h | 10 ++
 memory.c  | 22 --
 target/arm/cpu.c  | 14 ++
 4 files changed, 47 insertions(+), 2 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v1 2/3] arm: fix the armv7m reset state

[KONRAD Frederic]

This fixes an odd bug when a ROM is present somewhere and an alias @0x
is pointing to the ROM. The "if (rom)" test fails and we don't get a valid reset
state. QEMU later crashes with an exception because the ARMv7-M starts with the
ARM instruction set. (eg: PC & 0x01 is 0).

This patch uses memory_region_get_offset_within_address_space introduced before
to check if an alias doesn't point to a flash somewhere.

Signed-off-by: KONRAD Frederic 
---
 target/arm/cpu.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 28a9141..b8afd97 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -201,6 +201,20 @@ static void arm_cpu_reset(CPUState *s)
 
 /* Load the initial SP and PC from the vector table at address 0 */
 rom = rom_ptr(0);
+
+if (!rom) {
+/* Sometimes address 0x is an alias to a flash which
+ * actually have a ROM.
+ */
+MemoryRegionSection section;
+hwaddr offset = 0;
+
+section = memory_region_find(s->as->root, 0, 8);
+offset = memory_region_get_offset_within_address_space(section.mr);
+memory_region_unref(section.mr);
+rom = rom_ptr(offset);
+}
+
 if (rom) {
 /* Address zero is covered by ROM which hasn't yet been
  * copied into physical memory.
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v1 3/3] armv7m_systick: abort instead of locking on a bad rate

[KONRAD Frederic]

On 06/29/2017 02:43 PM, Peter Maydell wrote:

On 29 June 2017 at 13:35, Philippe Mathieu-Daudé  wrote:

This is true it is better to abort here than risking a deadlock.

However it seems to me they are 3 issues here:
- the deadlock pattern is caused by using a global variable,
- stellaris:ssys_calculate_system_clock() no checking RCC.SYSDIV and
RCC2.SYSDIV2 values <= 2 are reserved (GUEST_ERROR)
- stellaris:ssys_write(RCC2) not overrides correctly RCC


Stellaris works fine. It's other ARMv7M boards (which might forget
to set system_clock_scale) which don't work.


Yes actually the issue is with new boards when you forgot to set
system_clock_scale (as it happened to me :)).

Peter you suggest we fix that later when the clock is in place?

Fred




I'd rather take this opportunity to fix the deadlock pattern using a
getter/setter on system_clock_scale, doing the zero check in the setter and
eventually aborting in the getter, what do you think?


We should be using a clock property on the CPU instead of system_clock_scale.
Unfortunately Konrad's series attempting to add that infrastructure
is still in the "trying to sort out the right API in code review"
stage. I don't think it's worth trying to fiddle with the API
for it before we have the right eventual infrastructure in place.

(What system_clock_scale is actually doing is setting the
emulated frequency of the CPU, since that affects the
frequency of the timer.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 3/3] armv7m_systick: abort instead of locking on a bad rate

[KONRAD Frederic]

On 06/29/2017 03:02 PM, Philippe Mathieu-Daudé wrote:

On 06/29/2017 09:43 AM, Peter Maydell wrote:
On 29 June 2017 at 13:35, Philippe Mathieu-Daudé 
 wrote:

This is true it is better to abort here than risking a deadlock.

However it seems to me they are 3 issues here:
- the deadlock pattern is caused by using a global variable,
- stellaris:ssys_calculate_system_clock() no checking 
RCC.SYSDIV and

RCC2.SYSDIV2 values <= 2 are reserved (GUEST_ERROR)
- stellaris:ssys_write(RCC2) not overrides correctly RCC


Stellaris works fine. It's other ARMv7M boards (which might forget
to set system_clock_scale) which don't work.


Oh I misread ssys_calculate_system_clock(), indeed 
system_clock_scale can not get below 5 so no deadlock on Stellaris.


I'd rather take this opportunity to fix the deadlock pattern 
using a
getter/setter on system_clock_scale, doing the zero check in 
the setter and

eventually aborting in the getter, what do you think?


We should be using a clock property on the CPU instead of 
system_clock_scale.
Unfortunately Konrad's series attempting to add that 
infrastructure

is still in the "trying to sort out the right API in code review"
stage. I don't think it's worth trying to fiddle with the API
for it before we have the right eventual infrastructure in place.


I see. I'd rather move the comment and assert() in systick_scale().


Makes sense, I missed the potential divide-by-zero exception
which might happen in SysTick Current Value:

val = ((s->tick - (t + 1)) / systick_scale(s)) + 1;

I'll do the change and re-post a V2 when there will be some input
on the two first patches.

Thanks,
Fred




(What system_clock_scale is actually doing is setting the
emulated frequency of the CPU, since that affects the
frequency of the timer.)

Re: [Qemu-devel] [PATCH v1 2/3] arm: fix the armv7m reset state

[KONRAD Frederic]

On 06/29/2017 05:14 PM, Peter Maydell wrote:

On 29 June 2017 at 10:28, KONRAD Frederic  wrote:

This fixes an odd bug when a ROM is present somewhere and an alias @0x
is pointing to the ROM. The "if (rom)" test fails and we don't get a valid reset
state. QEMU later crashes with an exception because the ARMv7-M starts with the
ARM instruction set. (eg: PC & 0x01 is 0).

This patch uses memory_region_get_offset_within_address_space introduced before
to check if an alias doesn't point to a flash somewhere.

Signed-off-by: KONRAD Frederic 


This is awkward, because in the "we have a ROM but it's not been
copied into memory yet" case, the only thing we have is the
rom->addr, which is the address which the user's ROM blob said
it ought to be loaded in at. If the user didn't actually provide
a ROM blob that loads at 0 that seems a bit like a user error,
and I don't think this patch will catch all the cases of that
sort of mistake.


I don't think it's really a user mistake because on the real HW
the alias is configurable.. at least in my case.

There is a "jumper" setting to mirror either the Flash or the
SRAM, etc. So the binaries isn't located at 0 but at the flash
address 0x800 or some such. That's the case with u-boot and
the precompiled examples I found for this stm32f board.

 For instance if address 0 is real flash and the

high address alias is modelled by having the high address be the
alias, then if the user passes us an ELF file saying "load to
the high address" then this change won't catch that I think
(because doing the memory_region_find/get_offset_within_address_space
will return 0, which has already been tried). You'd need to
somehow have a way to say "find all the addresses within this
AS where this MR is mapped" and try them all...


This is more likely to be a user error :). Maybe we can load
the ROM before the reset but that seems a lot more invasive..

BTW isn't there a trick with the ELF entry somewhere? Or is that
for the Cortex-A?

Thanks,
Fred



thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 2/3] arm: fix the armv7m reset state

[KONRAD Frederic]

On 06/29/2017 06:45 PM, Peter Maydell wrote:

On 29 June 2017 at 17:41, KONRAD Frederic  wrote:

On 06/29/2017 05:14 PM, Peter Maydell wrote:

This is awkward, because in the "we have a ROM but it's not been
copied into memory yet" case, the only thing we have is the
rom->addr, which is the address which the user's ROM blob said
it ought to be loaded in at. If the user didn't actually provide
a ROM blob that loads at 0 that seems a bit like a user error,
and I don't think this patch will catch all the cases of that
sort of mistake.



I don't think it's really a user mistake because on the real HW
the alias is configurable.. at least in my case.

There is a "jumper" setting to mirror either the Flash or the
SRAM, etc. So the binaries isn't located at 0 but at the flash
address 0x800 or some such. That's the case with u-boot and
the precompiled examples I found for this stm32f board.


  For instance if address 0 is real flash and the
high address alias is modelled by having the high address be the
alias, then if the user passes us an ELF file saying "load to
the high address" then this change won't catch that I think
(because doing the memory_region_find/get_offset_within_address_space
will return 0, which has already been tried). You'd need to
somehow have a way to say "find all the addresses within this
AS where this MR is mapped" and try them all...



This is more likely to be a user error :). Maybe we can load
the ROM before the reset but that seems a lot more invasive..


It's the same thing, though, right? If the user's ELF file
says "vector table is at 0x800" then we should either
(a) say that's a user error, or
(b) handle it right, whether we implemented the QEMU model with
the flash at 0 and the alias at 0x800, or with the flash
at 0x800 and the alias at 0.


Hi Peter,

Fondamentaly yes, it is the same.. but it seems really strange to
me to load the elf in the alias.

If I choose (a) I'll need to objcpy all the elf to 0 or modify
the build script which should work on the real board.. This seems
not really a good option to me.

If I choose (b) I won't be able to load it to SRAM and it is
basically the same result I'll need to move or modify the config.

Thanks,
Fred




BTW isn't there a trick with the ELF entry somewhere? Or is that
for the Cortex-A?


We don't honour the ELF entry point on M profile (arguably
a bug) -- armv7m_load_kernel() ignores the entry point
returned by load_elf() in the 'entry' variable.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 1/1] xilinx-dp: Add support for the yuy2 video format

[KONRAD Frederic]

On 06/30/2017 03:55 PM, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Add support for the yuy2 video format.

Signed-off-by: Edgar E. Iglesias 
Acked-by: Sai Pavan Boddu 
---
  hw/display/xlnx_dp.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index f7b7b80..a77d7db 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -624,6 +624,9 @@ static void xlnx_dp_change_graphic_fmt(XlnxDPState *s)
  case 0:
  s->v_plane.format = PIXMAN_x8b8g8r8;
  break;
+case DP_NL_VID_Y0_CB_Y1_CR:
+s->v_plane.format = PIXMAN_yuy2;
+break;
  case DP_NL_VID_RGBA8880:
  s->v_plane.format = PIXMAN_x8b8g8r8;
  break;



Reviewed-by: KONRAD Frederic 

Fred

Re: [Qemu-devel] [PATCH v1 2/3] arm: fix the armv7m reset state

[KONRAD Frederic]

On 06/30/2017 11:06 AM, Peter Maydell wrote:

On 30 June 2017 at 09:24, KONRAD Frederic  wrote:

On 06/29/2017 06:45 PM, Peter Maydell wrote:

It's the same thing, though, right? If the user's ELF file
says "vector table is at 0x800" then we should either
(a) say that's a user error, or
(b) handle it right, whether we implemented the QEMU model with
the flash at 0 and the alias at 0x800, or with the flash
at 0x800 and the alias at 0.



Fondamentaly yes, it is the same.. but it seems really strange to
me to load the elf in the alias.


The user creating the ELF file has no idea whether we
modelled the flash at 0 and the alias at highmem or
the other way around -- that is an implementation detail
that should be completely invisible to the user.
My two suggestions are based on that point -- either we
should treat "ELF loaded at highmem" as always wrong, or
we should always support it.


If I choose (a) I'll need to objcpy all the elf to 0 or modify
the build script which should work on the real board.. This seems
not really a good option to me.


I agree that I don't like this.


If I choose (b) I won't be able to load it to SRAM and it is
basically the same result I'll need to move or modify the config.


I don't understand this, though. Option (b) is probably painful
to implement (I don't have a good idea of how to do it) but
it ought to mean that the ELF files that work on the board
also work for QEMU (regardless of how the board model
implemented the aliased flash).



Yes that's exactly what I want.

Basically the 0x alias can point to the SRAM or the ROM
during the reset depending on some boot config. The ELF is
directly loaded in the ROM or in the SRAM and my patch allows to
fetch the two first words in the reset handler to make it work
for any boot config.

For example this devices p69:
www.st.com/resource/en/reference_manual/DM00031020.pdf

Thanks,
Fred


thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 2/3] arm: fix the armv7m reset state

[KONRAD Frederic]

On 07/03/2017 10:51 AM, Peter Maydell wrote:

On 3 July 2017 at 08:31, KONRAD Frederic  wrote:

On 06/30/2017 11:06 AM, Peter Maydell wrote:

On 30 June 2017 at 09:24, KONRAD Frederic 
wrote:

If I choose (b) I won't be able to load it to SRAM and it is
basically the same result I'll need to move or modify the config.



I don't understand this, though. Option (b) is probably painful
to implement (I don't have a good idea of how to do it) but
it ought to mean that the ELF files that work on the board
also work for QEMU (regardless of how the board model
implemented the aliased flash).



Yes that's exactly what I want.

Basically the 0x alias can point to the SRAM or the ROM
during the reset depending on some boot config. The ELF is
directly loaded in the ROM or in the SRAM and my patch allows to
fetch the two first words in the reset handler to make it work
for any boot config.


Yes, but it only works if you implemented it that way
round, and not for board implementations which put the
real device at 0 and the alias at high memory. I'd like a fix
which deals with all of this, not just with the particular
arrangement your board implementation has.


Ok got it, I'll check if I can do something clean which can
handle both ways.

Fred



thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 00/10] Clock framework API.

[KONRAD Frederic]

Ping.

Thanks,
Fred

Le 28/02/2017 à 11:02, fred.kon...@greensocs.com a écrit :

From: KONRAD Frederic 

Hi,

This is the third version of the clock framework API it contains:

  * The first 6 patches which introduce the framework.
  * The 7th patch which introduces a fixed-clock model.
  * The rest which gives an example how to model a PLL from the existing
zynqmp-crf extracted from the qemu xilinx tree.

No specific behavior is expected yet when the CRF register set is accessed but
the user can see for example the dp_video_ref and vpll_to_lpd rate changing in
the monitor with the "info qtree" command when the vpll_ctrl register is
modified.

bus: main-system-bus
  type System
  dev: xlnx.zynqmp_crf, id ""
gpio-out "sysbus-irq" 1
qemu-clk "dbg_trace" 0
qemu-clk "dp_stc_ref" 0
qemu-clk "dpll_to_lpd" 1250
qemu-clk "acpu_clk" 0
qemu-clk "pcie_ref" 0
qemu-clk "topsw_main" 0
qemu-clk "topsw_lsbus" 0
qemu-clk "dp_audio_ref" 0
qemu-clk "sata_ref" 0
qemu-clk "dp_video_ref" 1428571
qemu-clk "vpll_clk" 5000
qemu-clk "apll_to_lpd" 1250
qemu-clk "dpll_clk" 5000
qemu-clk "gpu_ref" 0
qemu-clk "aux_refclk" 0
qemu-clk "video_clk" 2700
qemu-clk "gdma_ref" 0
qemu-clk "gt_crx_ref_clk" 0
qemu-clk "dbg_fdp" 0
qemu-clk "apll_clk" 5000
qemu-clk "pss_alt_ref_clk" 0
qemu-clk "ddr" 0
qemu-clk "dbg_tstmp" 0
qemu-clk "pss_ref_clk" 5000
qemu-clk "dpdma_ref" 0
qemu-clk "vpll_to_lpd" 1250
mmio fd1a/010c

This series is based on the current master
(d992f2f1368ceb92e6bfd8efece174110f4236ff).

Thanks,
Fred

V2 -> V3:
  * Rebased on current master.
  * Renamed qemu_clk / QEMUClock as suggested by Cédric.
  * Renamed in_rate to ref_rate and out_rate to rate.
  * Renamed qemu_clk_bind_clock to qemu_clk_bind.
  * Example added to the documentation as suggested by Peter.

V1 -> V2:
  * Rebased on current master.
  * Some function renamed and documentation fixed.

RFC -> V1:
  * Rebased on current master.
  * The docs has been fixed.
  * qemu_clk_init_device helper has been provided to ease the initialization
of the devices.

KONRAD Frederic (10):
  qemu-clk: introduce qemu-clk qom object
  qemu-clk: allow to add a clock to a device
  qemu-clk: allow to bind two clocks together
  qemu-clk: introduce an init array to help the device construction
  qdev-monitor: print the device's clock with info qtree
  docs: add qemu-clock documentation
  introduce fixed-clock
  introduce zynqmp_crf
  zynqmp: add the zynqmp_crf to the platform
  zynqmp: add reference clock

 Makefile.objs |   1 +
 docs/clock.txt| 278 
 hw/arm/xlnx-zynqmp.c  |  56 +++
 hw/misc/Makefile.objs |   2 +
 hw/misc/fixed-clock.c |  88 
 hw/misc/xilinx_zynqmp_crf.c   | 968 ++
 include/hw/arm/xlnx-zynqmp.h  |   8 +
 include/hw/misc/fixed-clock.h |  30 ++
 include/qemu/qemu-clock.h | 161 +++
 qdev-monitor.c|   2 +
 qemu-clock.c  | 176 
 11 files changed, 1770 insertions(+)
 create mode 100644 docs/clock.txt
 create mode 100644 hw/misc/fixed-clock.c
 create mode 100644 hw/misc/xilinx_zynqmp_crf.c
 create mode 100644 include/hw/misc/fixed-clock.h
 create mode 100644 include/qemu/qemu-clock.h
 create mode 100644 qemu-clock.c

Re: [Qemu-devel] [PULL 7/7] hw/misc/mmio_interface: Return after error_setg() to avoid crash

[KONRAD Frederic]

On 08/14/2017 01:55 PM, Peter Maydell wrote:

On 14 August 2017 at 12:52, Thomas Huth  wrote:

On 14.08.2017 13:45, Peter Maydell wrote:

It seems like it should be an error to permit this to be
created from the command line at all



That's also what thought first ... but the commit message of commit
7cc2298c46a6afa4f4ff7e5cd262809c782d701b says that it "can be
hotplugged/hotunplugged" ? ... that's confusing ...


It means that memory.c creates and deletes instances of the
device on demand, not that it's hotplugged in the "controlled by
the user simulating PCI or other hotplug" sense.

thanks
-- PMM



Sorry, missed that I changed my email address recently.
I'll add some docs and fix the things we mentioned for 2.11.

For the context:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg446748.html

Thanks,
Fred

Re: [Qemu-devel] BookE MMU question

[KONRAD Frederic]

Hi,

I think you need to go more in detail in what this map_region
function does.. eg: what is in the MAS registers before the tlbwe
happen (checking field by field) and what is the tlb which is
created / expected.

I got a pretty similar problem with a MAV V2 MMU and fixed size
tlb.. But I don't think it affects your device.. I'm not totally
sure though.

Fred


On 08/18/2017 03:48 PM, BALATON Zoltan wrote:

Hello,

While trying to get my recently posted Sam460ex emulation working 
(more details on that here: 
http://lists.nongnu.org/archive/html/qemu-ppc/2017-08/msg00112.html)
I'm stuck at a point with BookE MMU behaviour that seems to 
differ from real hardware but I don't know much about it so I 
hope someone with more knowledge can spot the problem or give 
some hints where to look for it.


When trying to boot AROS it currently fails when mmu_init() is 
run from 
https://github.com/ezrec/AROS-mirror/blob/ABI_V1/AROS/arch/ppc-sam440/kernel/mmu.c 


(around line 273 I think).

With a lot of debug enabled I see this:

[KRN] MMU Init
[KRN] lowest = 007f74e8, base = 0080, highest = 00c081f0
[KRN] Kernel size: 4128KB code, 34KB data
[KRN] Executing at ff841658, stack at ff7fd260, bss at ff7fd848, 
data at ff7fffb8
[KRN] TLB0f: -I---rwxrwx  - 0fff : : 
0:0290 1: 2:043f
[KRN] TLB02: -I-G-rw-rw- 8000 - 8fff : 8000: 
0:8290 1:800c 2:051b
[KRN] TLB03: -I-G-rw-rw- 9000 - 9fff : 9000: 
0:9290 1:900c 2:051b
[KRN] TLB04: -I-G-rw-rw- a000 - afff : a000: 
0:a290 1:a00d 2:051b
[KRN] TLB05: -I-G-rw-rw- b000 - bfff : b000: 
0:b290 1:b00d 2:051b
[KRN] TLB06: -I-G-rw-rw- c000 - cfff : c000: 
0:c290 1:c00d 2:051b
[KRN] TLB01: -I-G-rw-rw- d000 - dfff : : 
0:d290 1:000c 2:051b
[KRN] TLB07: -I-G-rw-rw- e000 - e0ff : : 
0:e270 1:000d 2:051b
[KRN] TLB08: -I-G-rw-rw- e100 - e1ff : 2000: 
0:e1000270 1:200d 2:051b
[KRN] TLB0e: -I-G-rwxrwx e200 - e20f : bff0: 
0:e2000250 1:bff4 2:053f
[KRN] TLB09: -I-G-rw-rw- e300 - e30003ff : 1000: 
0:e3000200 1:100d 2:051b
[KRN] TLB0a: -I-G-rw-rw- e3001000 - e30013ff : 3000: 
0:e3001200 1:300d 2:051b
[KRN] TLB0b: -I-G-rw-rw- e400 - e4003fff : 0801: 
0:e4000220 1:0801000c 2:051b
[KRN] TLB0c: -I---rwxrwx e500 - e50f : : 
0:e5000250 1:0004 2:043f
[KRN] TLB0d: -I-G-rwxrwx ef00 - efff : ef00: 
0:ef000270 1:ef04 2:053f
[KRN] TLB00: -I---rwxrwx ff00 -  : : 
0:ff000270 1: 2:043f

[KRN] map_region(007f7000, ff7f7000, 9000, 081b):
[KRN] TLB00: 007f7000 - 007f7fff : ff7f7000 - ff7f7fff:

helper_440_tlbwe word 0 entry 0 value ff7f7210
tlb_flush_nocheck: (count: 36)
helper_440_tlbwe word 1 entry 0 value 007f7000
tlb_flush_nocheck: (count: 37)
helper_440_tlbwe word 2 entry 0 value 081b
ppcemb_tlb_check: TLB 0 address ff7fd648 PID 0 <=> ff7f7000 
f000 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 1 address ff7fd648 PID 0 <=> d000 
f000 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 2 address ff7fd648 PID 0 <=> 8000 
f000 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 3 address ff7fd648 PID 0 <=> 9000 
f000 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 4 address ff7fd648 PID 0 <=> a000 
f000 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 5 address ff7fd648 PID 0 <=> b000 
f000 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 6 address ff7fd648 PID 0 <=> c000 
f000 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 7 address ff7fd648 PID 0 <=> e000 
ff00 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 8 address ff7fd648 PID 0 <=> e100 
ff00 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 9 address ff7fd648 PID 0 <=> e300 
fc00 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 10 address ff7fd648 PID 0 <=> e3001000 
fc00 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 11 address ff7fd648 PID 0 <=> e400 
c000 0 3b

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 12 address ff7fd648 PID 0 <=> e500 
fff0 0 7f

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 13 address ff7fd648 PID 0 <=> ef00 
ff00 0 7f

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 14 address ff7fd648 PID 0 <=> e200 
fff0 0 7f

mmubooke_check_tlb: TLB entry not found
ppcemb_tlb_check: TLB 15 address ff7fd648 PID 0 <=>  
f000 0 7f

mmubooke_check_tlb: TLB entry not found
mmubooke_check_tlb: TLB entry not found
mmubooke_check_tlb: TLB entry not found
mmubooke_check_tlb: TLB entry not found
mmubook