Re: [PATCH] meson.build: Support ncurses on MacOS

2021-06-13 Thread Philippe Mathieu-Daudé
On 6/13/21 8:33 AM, Stefan Weil wrote:
> Am 13.06.21 um 03:40 schrieb Brad Smith:
> 
>> This same problem also applies to OpenBSD as we have the same
>> version of ncurses with support for wide characters. I have a similar
>> patch in our QEMU port.
> 
> 
> Then we should either extend the conditional statement to handle OpenBSD
> as well, or simply define both macros unconditionally:
> 
>     # Newer versions of curses use NCURSES_WIDECHAR.
>     # Older versions (e. g. on MacOS, OpenBSD) still require
> _XOPEN_SOURCE_EXTENDED.
>     curses_compile_args = ['-DNCURSES_WIDECHAR=1',
> '-D_XOPEN_SOURCE_EXTENDED=1']
> 
> Defining only _XOPEN_SOURCE_EXTENDED would also work with old and new
> versions, so that's another option.

It is simpler to ask Brad to upstream the OpenBSD patch :)



Re: [PATCH] esp: fix migration version check in esp_is_version_5()

2021-06-13 Thread Philippe Mathieu-Daudé
On 6/13/21 12:26 PM, Mark Cave-Ayland wrote:
> Commit 4e78f3bf35 "esp: defer command completion interrupt on incoming data
> transfers" added a version check for use with VMSTATE_*_TEST macros to allow
> migration from older QEMU versions. Unfortunately the version check fails to
> work in its current form since if the VMStateDescription version_id is
> incremented, the test returns false and so the fields are not included in the
> outgoing migration stream.
> 
> Change the version check to use >= rather == to ensure that migration works
> correctly when the ESPState VMStateDescription has version_id > 5.
> 
> Signed-off-by: Mark Cave-Ayland 
> Fixes: 4e78f3bf35 ("esp: defer command completion interrupt on incoming data 
> transfers")
> ---
Well, it is not buggy yet :)

>  hw/scsi/esp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index bfdb94292b..39756ddd99 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -1120,7 +1120,7 @@ static bool esp_is_version_5(void *opaque, int 
> version_id)

Can you rename esp_is_at_least_version_5()?

>  ESPState *s = ESP(opaque);
>  
>  version_id = MIN(version_id, s->mig_version_id);
> -return version_id == 5;
> +return version_id >= 5;
>  }
>  
>  int esp_pre_save(void *opaque)
> 




Re: [PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM

2021-06-13 Thread Philippe Mathieu-Daudé
Cc'ing Finn & Laurent.

On 6/13/21 6:37 PM, Mark Cave-Ayland wrote:
> Here is the next set of patches from my attempts to boot MacOS under QEMU's
> Q800 machine related to the Sonic network adapter.
> 
> Patches 1 and 2 sort out checkpatch and convert from DPRINTF macros to
> trace-events.
> 
> Patch 3 fixes the PROM checksum and MAC address storage format as found by
> stepping through the MacOS toolbox.
> 
> Patch 4 ensures that the CPU loads/stores are correctly converted to 16-bit
> accesses for the network card and patch 5 fixes a bug when selecting the
> index specified for CAM entries.
> 
> NOTE TO MIPS MAINTAINERS:
> 
> - The Sonic network adapter is used as part of the MIPS jazz machine, however
>   I don't have a working kernel and system to test it with. Any pointers to
>   test images would be appreciated.
> 
> - The changes to the PROM checksum in patch 3 were determined by stepping
>   through the MacOS toolbox, and is different from the existing algorithm.
>   Has the current PROM checksum algorithm been validated on a MIPS guest or
>   was it just a guess? It might be that 2 different algorithms are needed for
>   the Q800 vs. Jazz machine.
> 
> - My current guess is the jazzsonic driver is broken since the last set of
>   dp8393x changes as the MIPS jazz machine does not set the "big_endian"
>   property on the dp8393x device. I'd expect that the following diff would
>   be needed, but I can't confirm this without a suitable test image.
> 
> diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
> index 1e1cf8154e..1df67035aa 100644
> --- a/hw/mips/jazz.c
> +++ b/hw/mips/jazz.c
> @@ -280,6 +280,7 @@ static void mips_jazz_init(MachineState *machine,
>  dev = qdev_new("dp8393x");
>  qdev_set_nic_properties(dev, nd);
>  qdev_prop_set_uint8(dev, "it_shift", 2);
> +qdev_prop_set_bit(dev, "big_endian", true);
>  object_property_set_link(OBJECT(dev), "dma_mr",
>   OBJECT(rc4030_dma_mr), _abort);
>  sysbus = SYS_BUS_DEVICE(dev);
> 
> Signed-off-by: Mark Cave-Ayland 
> 
> [q800-macos-upstream patchset series: 3]
> 
> Mark Cave-Ayland (5):
>   dp8393x: checkpatch fixes
>   dp8393x: convert to trace-events
>   dp8393x: fix PROM checksum and MAC address storage
>   dp8393x: don't force 32-bit register access
>   dp8393x: fix CAM descriptor entry index
> 
>  hw/net/dp8393x.c| 332 
>  hw/net/trace-events |  17 +++
>  2 files changed, 198 insertions(+), 151 deletions(-)
> 



[RFC PATCH] migration: Add missing dependency on GNUTLS

2021-06-13 Thread Philippe Mathieu-Daudé
Commit 7de2e856533 made migration/qemu-file-channel.c include
"io/channel-tls.h" but forgot to add the new GNUTLS dependency
on Meson, leading to build failure on OSX:

  [2/35] Compiling C object libmigration.fa.p/migration_qemu-file-channel.c.o
  FAILED: libmigration.fa.p/migration_qemu-file-channel.c.o
  cc -Ilibmigration.fa.p -I. -I.. -Iqapi [ ... ] -o 
libmigration.fa.p/migration_qemu-file-channel.c.o -c 
../migration/qemu-file-channel.c
  In file included from ../migration/qemu-file-channel.c:29:
  In file included from include/io/channel-tls.h:26:
  In file included from include/crypto/tlssession.h:24:
  include/crypto/tlscreds.h:28:10: fatal error: 'gnutls/gnutls.h' file not found
  #include 
   ^
  1 error generated.

Reported-by: Stefan Weil 
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/407
Fixes: 7de2e856533 ("yank: Unregister function when using TLS migration")
Signed-off-by: Philippe Mathieu-Daudé 
---
RFC: Not tested on OSX. Stefan, do you know why this isn't covered
 on Cirrus-CI?  https://cirrus-ci.com/build/4876003651616768
---
 migration/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/meson.build b/migration/meson.build
index f8714dcb154..5b5a3f7b337 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -8,7 +8,7 @@
   'qemu-file.c',
   'yank_functions.c',
 )
-softmmu_ss.add(migration_files)
+softmmu_ss.add(migration_files, gnutls)
 
 softmmu_ss.add(files(
   'block-dirty-bitmap.c',
-- 
2.31.1




Re: [PULL v2 00/34] tcg patch queue

2021-06-13 Thread Philippe Mathieu-Daudé
On 6/14/21 3:20 AM, Richard Henderson wrote:
> V2 fixes an error in patch 22 wrt MacOS.
> It's a shame we don't have public CI for that.

We do:

https://cirrus-ci.com/github/qemu/qemu

Maybe it is not documented well enough?

Apparently we could integrate it to gitlab pipeline:
https://potyarkin.ml/posts/2020/cirrus-ci-integration-for-gitlab-projects/

> 
> 
> r~



Re: [PULL 1/9] yank: Unregister function when using TLS migration

2021-06-13 Thread Philippe Mathieu-Daudé
Hi Leonardo,

On 6/9/21 4:45 PM, Dr. David Alan Gilbert (git) wrote:
> From: Leonardo Bras 
> 
> After yank feature was introduced in migration, whenever migration
> is started using TLS, the following error happens in both source and
> destination hosts:
> 
> (qemu) qemu-kvm: ../util/yank.c:107: yank_unregister_instance:
> Assertion `QLIST_EMPTY(>yankfns)' failed.
> 
> This happens because of a missing yank_unregister_function() when using
> qio-channel-tls.
> 
> Fix this by also allowing TYPE_QIO_CHANNEL_TLS object type to perform
> yank_unregister_function() in channel_close() and multifd_load_cleanup().
> 
> Also, inside migration_channel_connect() and
> migration_channel_process_incoming() move yank_register_function() so
> it only runs once on a TLS migration.
> 
> Fixes: b5eea99ec2f ("migration: Add yank feature", 2021-01-13)
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1964326
> Signed-off-by: Leonardo Bras 
> Reviewed-by: Lukas Straub 
> Reviewed-by: Peter Xu 
> 
> --

Please use the '---' separator, otherwise tools don't strip
out your comments and they end in the repository, adding
confusion.

> Changes since v2:
> - Dropped all references to ioc->master
> - yank_register_function() and yank_unregister_function() now only run
>   once in a TLS migration.
> 
> Changes since v1:
> - Cast p->c to QIOChannelTLS into multifd_load_cleanup()
> Message-Id: <20210601054030.1153249-1-leobra...@gmail.com>
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  migration/channel.c   | 26 ++
>  migration/multifd.c   |  3 ++-
>  migration/qemu-file-channel.c |  4 +++-
>  3 files changed, 19 insertions(+), 14 deletions(-)



Re: [PATCH v2 1/2] hw/nvme: fix endianess conversion and add controller list

2021-06-13 Thread Gollu Appalanaidu

On Wed, Jun 09, 2021 at 10:22:49PM +0200, Klaus Jensen wrote:

On Jun  1 20:32, Gollu Appalanaidu wrote:

Add the controller identifiers list CNS 0x13, available list of ctrls
in NVM Subsystem that may or may not be attached to namespaces.

In Identify Ctrl List of the CNS 0x12 and 0x13 no endian conversion
for the nsid field.

Signed-off-by: Gollu Appalanaidu 

-v2:
Fix the review comments from Klaus and squashed 2nd commit into
1st commit

---
hw/nvme/ctrl.c   | 26 --
hw/nvme/trace-events |  2 +-
include/block/nvme.h |  1 +
3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 2e7498a73e..813a72c655 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -4251,9 +4251,11 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, 
NvmeRequest *req, bool active)
   return NVME_INVALID_CMD_SET | NVME_DNR;
}

-static uint16_t nvme_identify_ns_attached_list(NvmeCtrl *n, NvmeRequest *req)
+static uint16_t nvme_identify_ctrl_list(NvmeCtrl *n, NvmeRequest *req,
+bool attached)
{
   NvmeIdentify *c = (NvmeIdentify *)>cmd;
+uint32_t nsid = le32_to_cpu(c->nsid);
   uint16_t min_id = le16_to_cpu(c->ctrlid);
   uint16_t list[NVME_CONTROLLER_LIST_SIZE] = {};
   uint16_t *ids = [1];
@@ -4261,15 +4263,17 @@ static uint16_t nvme_identify_ns_attached_list(NvmeCtrl 
*n, NvmeRequest *req)
   NvmeCtrl *ctrl;
   int cntlid, nr_ids = 0;

-trace_pci_nvme_identify_ns_attached_list(min_id);
+trace_pci_nvme_identify_ctrl_list(c->cns, min_id);

-if (c->nsid == NVME_NSID_BROADCAST) {
-return NVME_INVALID_FIELD | NVME_DNR;
-}
+if (attached) {
+if (nsid == NVME_NSID_BROADCAST) {
+return NVME_INVALID_FIELD | NVME_DNR;
+}

-ns = nvme_subsys_ns(n->subsys, c->nsid);
-if (!ns) {
-return NVME_INVALID_FIELD | NVME_DNR;
+ns = nvme_subsys_ns(n->subsys, nsid);
+if (!ns) {
+return NVME_INVALID_FIELD | NVME_DNR;
+}
   }

   for (cntlid = min_id; cntlid < ARRAY_SIZE(n->subsys->ctrls); cntlid++) {


Assume that `attached` is false and `n->subsys` is NULL.

KABM :)


This scenario has been tested but executed without any issue, since here
ARRAY_SIZE calculating size as per the "NVME_MAX_CONTROLLERS" defined.

These two CNS values shows affect when there exists a Subsystem. will add
check condition if there is no Subsystem will return invalid field in command.

if (!n->subsys) {
return NVME_INVALID_FIELD | NVME_DNR;
}



Re: [PATCH v3 1/2] hw/char: sifive_uart

2021-06-13 Thread Alistair Francis
On Mon, Jun 14, 2021 at 12:13 AM Lukas Jünger
 wrote:
>
> Make function names consistent
>
> Signed-off-by: Lukas Jünger 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/char/sifive_uart.c | 46 ++-
>  1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
> index fe12666789..5df8212961 100644
> --- a/hw/char/sifive_uart.c
> +++ b/hw/char/sifive_uart.c
> @@ -31,7 +31,7 @@
>   */
>
>  /* Returns the state of the IP (interrupt pending) register */
> -static uint64_t uart_ip(SiFiveUARTState *s)
> +static uint64_t sifive_uart_ip(SiFiveUARTState *s)
>  {
>  uint64_t ret = 0;
>
> @@ -48,7 +48,7 @@ static uint64_t uart_ip(SiFiveUARTState *s)
>  return ret;
>  }
>
> -static void update_irq(SiFiveUARTState *s)
> +static void sifive_uart_update_irq(SiFiveUARTState *s)
>  {
>  int cond = 0;
>  if ((s->ie & SIFIVE_UART_IE_TXWM) ||
> @@ -63,7 +63,7 @@ static void update_irq(SiFiveUARTState *s)
>  }
>
>  static uint64_t
> -uart_read(void *opaque, hwaddr addr, unsigned int size)
> +sifive_uart_read(void *opaque, hwaddr addr, unsigned int size)
>  {
>  SiFiveUARTState *s = opaque;
>  unsigned char r;
> @@ -74,7 +74,7 @@ uart_read(void *opaque, hwaddr addr, unsigned int size)
>  memmove(s->rx_fifo, s->rx_fifo + 1, s->rx_fifo_len - 1);
>  s->rx_fifo_len--;
>  qemu_chr_fe_accept_input(>chr);
> -update_irq(s);
> +sifive_uart_update_irq(s);
>  return r;
>  }
>  return 0x8000;
> @@ -84,7 +84,7 @@ uart_read(void *opaque, hwaddr addr, unsigned int size)
>  case SIFIVE_UART_IE:
>  return s->ie;
>  case SIFIVE_UART_IP:
> -return uart_ip(s);
> +return sifive_uart_ip(s);
>  case SIFIVE_UART_TXCTRL:
>  return s->txctrl;
>  case SIFIVE_UART_RXCTRL:
> @@ -99,8 +99,8 @@ uart_read(void *opaque, hwaddr addr, unsigned int size)
>  }
>
>  static void
> -uart_write(void *opaque, hwaddr addr,
> -   uint64_t val64, unsigned int size)
> +sifive_uart_write(void *opaque, hwaddr addr,
> +  uint64_t val64, unsigned int size)
>  {
>  SiFiveUARTState *s = opaque;
>  uint32_t value = val64;
> @@ -109,11 +109,11 @@ uart_write(void *opaque, hwaddr addr,
>  switch (addr) {
>  case SIFIVE_UART_TXFIFO:
>  qemu_chr_fe_write(>chr, , 1);
> -update_irq(s);
> +sifive_uart_update_irq(s);
>  return;
>  case SIFIVE_UART_IE:
>  s->ie = val64;
> -update_irq(s);
> +sifive_uart_update_irq(s);
>  return;
>  case SIFIVE_UART_TXCTRL:
>  s->txctrl = val64;
> @@ -129,9 +129,9 @@ uart_write(void *opaque, hwaddr addr,
>__func__, (int)addr, (int)value);
>  }
>
> -static const MemoryRegionOps uart_ops = {
> -.read = uart_read,
> -.write = uart_write,
> +static const MemoryRegionOps sifive_uart_ops = {
> +.read = sifive_uart_read,
> +.write = sifive_uart_write,
>  .endianness = DEVICE_NATIVE_ENDIAN,
>  .valid = {
>  .min_access_size = 4,
> @@ -139,7 +139,7 @@ static const MemoryRegionOps uart_ops = {
>  }
>  };
>
> -static void uart_rx(void *opaque, const uint8_t *buf, int size)
> +static void sifive_uart_rx(void *opaque, const uint8_t *buf, int size)
>  {
>  SiFiveUARTState *s = opaque;
>
> @@ -150,26 +150,27 @@ static void uart_rx(void *opaque, const uint8_t *buf, 
> int size)
>  }
>  s->rx_fifo[s->rx_fifo_len++] = *buf;
>
> -update_irq(s);
> +sifive_uart_update_irq(s);
>  }
>
> -static int uart_can_rx(void *opaque)
> +static int sifive_uart_can_rx(void *opaque)
>  {
>  SiFiveUARTState *s = opaque;
>
>  return s->rx_fifo_len < sizeof(s->rx_fifo);
>  }
>
> -static void uart_event(void *opaque, QEMUChrEvent event)
> +static void sifive_uart_event(void *opaque, QEMUChrEvent event)
>  {
>  }
>
> -static int uart_be_change(void *opaque)
> +static int sifive_uart_be_change(void *opaque)
>  {
>  SiFiveUARTState *s = opaque;
>
> -qemu_chr_fe_set_handlers(>chr, uart_can_rx, uart_rx, uart_event,
> -uart_be_change, s, NULL, true);
> +qemu_chr_fe_set_handlers(>chr, sifive_uart_can_rx, sifive_uart_rx,
> + sifive_uart_event, sifive_uart_be_change, s,
> + NULL, true);
>
>  return 0;
>  }
> @@ -183,9 +184,10 @@ SiFiveUARTState *sifive_uart_create(MemoryRegion 
> *address_space, hwaddr base,
>  SiFiveUARTState *s = g_malloc0(sizeof(SiFiveUARTState));
>  s->irq = irq;
>  qemu_chr_fe_init(>chr, chr, _abort);
> -qemu_chr_fe_set_handlers(>chr, uart_can_rx, uart_rx, uart_event,
> -uart_be_change, s, NULL, true);
> -memory_region_init_io(>mmio, NULL, _ops, s,
> +qemu_chr_fe_set_handlers(>chr, sifive_uart_can_rx, sifive_uart_rx,
> + sifive_uart_event, sifive_uart_be_change, s,
> +

Re: [PULL 00/34] tcg patch queue

2021-06-13 Thread Richard Henderson

On 6/13/21 10:10 AM, Peter Maydell wrote:

Also on x86-64 host, this failure in check-tcg:

make[2]: Leaving directory
'/home/petmay01/linaro/qemu-for-merges/build/all-linux-static/tests/tcg/hppa-linux-user'
make[2]: Entering directory
'/home/petmay01/linaro/qemu-for-merges/build/all-linux-static/tests/tcg/hppa-linux-user'
timeout --foreground 60
/home/petmay01/linaro/qemu-for-merges/build/all-linux-static/qemu-hppa
  signals >  signals.out
timeout: the monitored command dumped core
Illegal instruction
../Makefile.target:156: recipe for target 'run-signals' failed
make[2]: *** [run-signals] Error 132
make[2]: Leaving directory
'/home/petmay01/linaro/qemu-for-merges/build/all-linux-static/tests/tcg/hppa-linux-user'
/home/petmay01/linaro/qemu-for-merges/tests/tcg/Makefile.qemu:102:
recipe for target 'run-guest-tests' failed

but I think this is a pre-existing intermittent.


Interesting.  I've never seen this one before.
Do you recall if this is only intermittent with -static?


r~



[PULL v2 22/34] tcg: Allocate code_gen_buffer into struct tcg_region_state

2021-06-13 Thread Richard Henderson
Do not mess around with setting values within tcg_init_ctx.
Put the values into 'region' directly, which is where they
will live for the lifetime of the program.

Reviewed-by: Alex Bennée 
Reviewed-by: Luis Pires 
Signed-off-by: Richard Henderson 
---
 tcg/region.c | 64 ++--
 1 file changed, 27 insertions(+), 37 deletions(-)

diff --git a/tcg/region.c b/tcg/region.c
index 5beba41412..afa11ec5d7 100644
--- a/tcg/region.c
+++ b/tcg/region.c
@@ -70,13 +70,12 @@ static size_t tree_size;
 
 bool in_code_gen_buffer(const void *p)
 {
-const TCGContext *s = _init_ctx;
 /*
  * Much like it is valid to have a pointer to the byte past the
  * end of an array (so long as you don't dereference it), allow
  * a pointer to the byte past the end of the code gen buffer.
  */
-return (size_t)(p - s->code_gen_buffer) <= s->code_gen_buffer_size;
+return (size_t)(p - region.start_aligned) <= region.total_size;
 }
 
 #ifdef CONFIG_DEBUG_TCG
@@ -562,8 +561,8 @@ static bool alloc_code_gen_buffer(size_t tb_size, int 
splitwx, Error **errp)
 }
 qemu_madvise(buf, size, QEMU_MADV_HUGEPAGE);
 
-tcg_ctx->code_gen_buffer = buf;
-tcg_ctx->code_gen_buffer_size = size;
+region.start_aligned = buf;
+region.total_size = size;
 return true;
 }
 #elif defined(_WIN32)
@@ -584,8 +583,8 @@ static bool alloc_code_gen_buffer(size_t size, int splitwx, 
Error **errp)
 return false;
 }
 
-tcg_ctx->code_gen_buffer = buf;
-tcg_ctx->code_gen_buffer_size = size;
+region.start_aligned = buf;
+region.total_size = size;
 return true;
 }
 #else
@@ -637,8 +636,8 @@ static bool alloc_code_gen_buffer_anon(size_t size, int 
prot,
 /* Request large pages for the buffer.  */
 qemu_madvise(buf, size, QEMU_MADV_HUGEPAGE);
 
-tcg_ctx->code_gen_buffer = buf;
-tcg_ctx->code_gen_buffer_size = size;
+region.start_aligned = buf;
+region.total_size = size;
 return true;
 }
 
@@ -659,8 +658,8 @@ static bool alloc_code_gen_buffer_splitwx_memfd(size_t 
size, Error **errp)
 return false;
 }
 /* The size of the mapping may have been adjusted. */
-size = tcg_ctx->code_gen_buffer_size;
-buf_rx = tcg_ctx->code_gen_buffer;
+buf_rx = region.start_aligned;
+size = region.total_size;
 #endif
 
 buf_rw = qemu_memfd_alloc("tcg-jit", size, 0, , errp);
@@ -682,8 +681,8 @@ static bool alloc_code_gen_buffer_splitwx_memfd(size_t 
size, Error **errp)
 #endif
 
 close(fd);
-tcg_ctx->code_gen_buffer = buf_rw;
-tcg_ctx->code_gen_buffer_size = size;
+region.start_aligned = buf_rw;
+region.total_size = size;
 tcg_splitwx_diff = buf_rx - buf_rw;
 
 /* Request large pages for the buffer and the splitwx.  */
@@ -734,7 +733,7 @@ static bool alloc_code_gen_buffer_splitwx_vmremap(size_t 
size, Error **errp)
 return false;
 }
 
-buf_rw = (mach_vm_address_t)tcg_ctx->code_gen_buffer;
+buf_rw = (mach_vm_address_t)region.start_aligned;
 buf_rx = 0;
 ret = mach_vm_remap(mach_task_self(),
 _rx,
@@ -846,11 +845,8 @@ static bool alloc_code_gen_buffer(size_t size, int 
splitwx, Error **errp)
  */
 void tcg_region_init(size_t tb_size, int splitwx, unsigned max_cpus)
 {
-void *buf, *aligned, *end;
-size_t total_size;
 size_t page_size;
 size_t region_size;
-size_t n_regions;
 size_t i;
 bool ok;
 
@@ -858,39 +854,33 @@ void tcg_region_init(size_t tb_size, int splitwx, 
unsigned max_cpus)
splitwx, _fatal);
 assert(ok);
 
-buf = tcg_init_ctx.code_gen_buffer;
-total_size = tcg_init_ctx.code_gen_buffer_size;
-page_size = qemu_real_host_page_size;
-n_regions = tcg_n_regions(total_size, max_cpus);
-
-/* The first region will be 'aligned - buf' bytes larger than the others */
-aligned = QEMU_ALIGN_PTR_UP(buf, page_size);
-g_assert(aligned < tcg_init_ctx.code_gen_buffer + total_size);
-
 /*
  * Make region_size a multiple of page_size, using aligned as the start.
  * As a result of this we might end up with a few extra pages at the end of
  * the buffer; we will assign those to the last region.
  */
-region_size = (total_size - (aligned - buf)) / n_regions;
+region.n = tcg_n_regions(region.total_size, max_cpus);
+page_size = qemu_real_host_page_size;
+region_size = region.total_size / region.n;
 region_size = QEMU_ALIGN_DOWN(region_size, page_size);
 
 /* A region must have at least 2 pages; one code, one guard */
 g_assert(region_size >= 2 * page_size);
+region.stride = region_size;
+
+/* Reserve space for guard pages. */
+region.size = region_size - page_size;
+region.total_size -= page_size;
+
+/*
+ * The first region will be smaller than the others, via the prologue,
+ * which has yet to be allocated.  For now, the first region begins at
+ * the page boundary.
+ */
+  

[PULL v2 00/34] tcg patch queue

2021-06-13 Thread Richard Henderson
V2 fixes an error in patch 22 wrt MacOS.
It's a shame we don't have public CI for that.


r~


The following changes since commit 894fc4fd670aaf04a67dc7507739f914ff4bacf2:

  Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into 
staging (2021-06-11 09:21:48 +0100)

are available in the Git repository at:

  https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210613

for you to fetch changes up to a5a8b84772e13066c6c45f480cc5b5312bbde08e:

  docs/devel: Explain in more detail the TB chaining mechanisms (2021-06-13 
17:42:40 -0700)


Clean up code_gen_buffer allocation.
Add tcg_remove_ops_after.
Fix tcg_constant_* documentation.
Improve TB chaining documentation.
Fix float32_exp2.
Fix arm tcg_out_op function signature.


Jose R. Ziviani (1):
  tcg/arm: Fix tcg_out_op function signature

Luis Pires (1):
  docs/devel: Explain in more detail the TB chaining mechanisms

Richard Henderson (32):
  meson: Split out tcg/meson.build
  meson: Split out fpu/meson.build
  tcg: Re-order tcg_region_init vs tcg_prologue_init
  tcg: Remove error return from tcg_region_initial_alloc__locked
  tcg: Split out tcg_region_initial_alloc
  tcg: Split out tcg_region_prologue_set
  tcg: Split out region.c
  accel/tcg: Inline cpu_gen_init
  accel/tcg: Move alloc_code_gen_buffer to tcg/region.c
  accel/tcg: Rename tcg_init to tcg_init_machine
  tcg: Create tcg_init
  accel/tcg: Merge tcg_exec_init into tcg_init_machine
  accel/tcg: Use MiB in tcg_init_machine
  accel/tcg: Pass down max_cpus to tcg_init
  tcg: Introduce tcg_max_ctxs
  tcg: Move MAX_CODE_GEN_BUFFER_SIZE to tcg-target.h
  tcg: Replace region.end with region.total_size
  tcg: Rename region.start to region.after_prologue
  tcg: Tidy tcg_n_regions
  tcg: Tidy split_cross_256mb
  tcg: Move in_code_gen_buffer and tests to region.c
  tcg: Allocate code_gen_buffer into struct tcg_region_state
  tcg: Return the map protection from alloc_code_gen_buffer
  tcg: Sink qemu_madvise call to common code
  util/osdep: Add qemu_mprotect_rw
  tcg: Round the tb_size default from qemu_get_host_physmem
  tcg: Merge buffer protection and guard page protection
  tcg: When allocating for !splitwx, begin with PROT_NONE
  tcg: Move tcg_init_ctx and tcg_ctx from accel/tcg/
  tcg: Introduce tcg_remove_ops_after
  tcg: Fix documentation for tcg_constant_* vs tcg_temp_free_*
  softfloat: Fix tp init in float32_exp2

 docs/devel/tcg.rst| 101 -
 meson.build   |  12 +-
 accel/tcg/internal.h  |   2 +
 include/qemu/osdep.h  |   1 +
 include/sysemu/tcg.h  |   2 -
 include/tcg/tcg.h |  28 +-
 tcg/aarch64/tcg-target.h  |   1 +
 tcg/arm/tcg-target.h  |   1 +
 tcg/i386/tcg-target.h |   2 +
 tcg/mips/tcg-target.h |   6 +
 tcg/ppc/tcg-target.h  |   2 +
 tcg/riscv/tcg-target.h|   1 +
 tcg/s390/tcg-target.h |   3 +
 tcg/sparc/tcg-target.h|   1 +
 tcg/tcg-internal.h|  40 ++
 tcg/tci/tcg-target.h  |   1 +
 accel/tcg/tcg-all.c   |  32 +-
 accel/tcg/translate-all.c | 439 +---
 bsd-user/main.c   |   3 +-
 fpu/softfloat.c   |   2 +-
 linux-user/main.c |   1 -
 tcg/region.c  | 999 ++
 tcg/tcg.c | 649 +++---
 util/osdep.c  |   9 +
 tcg/arm/tcg-target.c.inc  |   3 +-
 fpu/meson.build   |   1 +
 tcg/meson.build   |  14 +
 27 files changed, 1266 insertions(+), 1090 deletions(-)
 create mode 100644 tcg/tcg-internal.h
 create mode 100644 tcg/region.c
 create mode 100644 fpu/meson.build
 create mode 100644 tcg/meson.build



Re: [PATCH 3/3] target/arm: Use acpi_ghes_present() to see if we report ACPI memory errors

2021-06-13 Thread Dongjiu Geng
On Fri, 4 Jun 2021 at 01:13, Peter Maydell  wrote:
>
> The virt_is_acpi_enabled() function is specific to the virt board, as
> is the check for its 'ras' property.  Use the new acpi_ghes_present()
> function to check whether we should report memory errors via
> acpi_ghes_record_errors().
>
> This avoids a link error if QEMU was built without support for the
> virt board, and provides a mechanism that can be used by any future
> board models that want to add ACPI memory error reporting support
> (they only need to call acpi_ghes_add_fw_cfg()).
>
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/kvm64.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 37ceadd9a9d..59982d470d3 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -1410,14 +1410,10 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, 
> void *addr)
>  {
>  ram_addr_t ram_addr;
>  hwaddr paddr;
> -Object *obj = qdev_get_machine();
> -VirtMachineState *vms = VIRT_MACHINE(obj);
> -bool acpi_enabled = virt_is_acpi_enabled(vms);
>
>  assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO);
>
> -if (acpi_enabled && addr &&
> -object_property_get_bool(obj, "ras", NULL)) {
> +if (acpi_ghes_present() && addr) {
>  ram_addr = qemu_ram_addr_from_host(addr);
>  if (ram_addr != RAM_ADDR_INVALID &&
>  kvm_physical_memory_addr_from_host(c->kvm_state, addr, )) {
> --
> 2.20.1
>

Reviewed-by: Dongjiu Geng 



Re: [PATCH 2/3] hw/acpi: Provide function acpi_ghes_present()

2021-06-13 Thread Dongjiu Geng
On Fri, 4 Jun 2021 at 01:13, Peter Maydell  wrote:
>
> Allow code elsewhere in the system to check whether the ACPI GHES
> table is present, so it can determine whether it is OK to try to
> record an error by calling acpi_ghes_record_errors().
>
> (We don't need to migrate the new 'present' field in AcpiGhesState,
> because it is set once at system initialization and doesn't change.)
>
> Signed-off-by: Peter Maydell 
> ---
>  include/hw/acpi/ghes.h |  9 +
>  hw/acpi/ghes-stub.c|  5 +
>  hw/acpi/ghes.c | 17 +
>  3 files changed, 31 insertions(+)
>
> diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
> index 2ae8bc1ded3..674f6958e90 100644
> --- a/include/hw/acpi/ghes.h
> +++ b/include/hw/acpi/ghes.h
> @@ -64,6 +64,7 @@ enum {
>
>  typedef struct AcpiGhesState {
>  uint64_t ghes_addr_le;
> +bool present; /* True if GHES is present at all on this board */
>  } AcpiGhesState;
>
>  void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker);
> @@ -72,4 +73,12 @@ void acpi_build_hest(GArray *table_data, BIOSLinker 
> *linker,
>  void acpi_ghes_add_fw_cfg(AcpiGhesState *vms, FWCfgState *s,
>GArray *hardware_errors);
>  int acpi_ghes_record_errors(uint8_t notify, uint64_t error_physical_addr);
> +
> +/**
> + * acpi_ghes_present: Report whether ACPI GHES table is present
> + *
> + * Returns: true if the system has an ACPI GHES table and it is
> + * safe to call acpi_ghes_record_errors() to record a memory error.
> + */
> +bool acpi_ghes_present(void);
>  #endif
> diff --git a/hw/acpi/ghes-stub.c b/hw/acpi/ghes-stub.c
> index 9faba043b85..c315de1802d 100644
> --- a/hw/acpi/ghes-stub.c
> +++ b/hw/acpi/ghes-stub.c
> @@ -15,3 +15,8 @@ int acpi_ghes_record_errors(uint8_t source_id, uint64_t 
> physical_address)
>  {
>  return -1;
>  }
> +
> +bool acpi_ghes_present(void)
> +{
> +return false;
> +}
> diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> index a4dac6bf15e..a749b84d624 100644
> --- a/hw/acpi/ghes.c
> +++ b/hw/acpi/ghes.c
> @@ -386,6 +386,8 @@ void acpi_ghes_add_fw_cfg(AcpiGhesState *ags, FWCfgState 
> *s,
>  /* Create a read-write fw_cfg file for Address */
>  fw_cfg_add_file_callback(s, ACPI_GHES_DATA_ADDR_FW_CFG_FILE, NULL, NULL,
>  NULL, &(ags->ghes_addr_le), sizeof(ags->ghes_addr_le), false);
> +
> +ags->present = true;
>  }
>
>  int acpi_ghes_record_errors(uint8_t source_id, uint64_t physical_address)
> @@ -443,3 +445,18 @@ int acpi_ghes_record_errors(uint8_t source_id, uint64_t 
> physical_address)
>
>  return ret;
>  }
> +
> +bool acpi_ghes_present(void)
> +{
> +AcpiGedState *acpi_ged_state;
> +AcpiGhesState *ags;
> +
> +acpi_ged_state = ACPI_GED(object_resolve_path_type("", TYPE_ACPI_GED,
> +   NULL));
> +
> +if (!acpi_ged_state) {
> +return false;
> +}
> +ags = _ged_state->ghes_state;
> +return ags->present;
> +}
> --
> 2.20.1
>

Reviewed-by: Dongjiu Geng 



Re: [PATCH 1/3] hw/acpi: Provide stub version of acpi_ghes_record_errors()

2021-06-13 Thread Dongjiu Geng
On Fri, 4 Jun 2021 at 01:13, Peter Maydell  wrote:
>
> Generic code in target/arm wants to call acpi_ghes_record_errors();
> provide a stub version so that we don't fail to link when
> CONFIG_ACPI_APEI is not set. This requires us to add a new
> ghes-stub.c file to contain it and the meson.build mechanics
> to use it when appropriate.
>
> Signed-off-by: Peter Maydell 
> ---
>  hw/acpi/ghes-stub.c | 17 +
>  hw/acpi/meson.build |  6 +++---
>  2 files changed, 20 insertions(+), 3 deletions(-)
>  create mode 100644 hw/acpi/ghes-stub.c
>
> diff --git a/hw/acpi/ghes-stub.c b/hw/acpi/ghes-stub.c
> new file mode 100644
> index 000..9faba043b85
> --- /dev/null
> +++ b/hw/acpi/ghes-stub.c
> @@ -0,0 +1,17 @@
> +/*
> + * Support for generating APEI tables and recording CPER for Guests:
> + * stub functions.
> + *
> + * Copyright (c) 2021 Linaro, Ltd
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/acpi/ghes.h"
> +
> +int acpi_ghes_record_errors(uint8_t source_id, uint64_t physical_address)
> +{
> +return -1;
> +}
> diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
> index dd69577212a..03ea43f8627 100644
> --- a/hw/acpi/meson.build
> +++ b/hw/acpi/meson.build
> @@ -13,13 +13,13 @@ acpi_ss.add(when: 'CONFIG_ACPI_PCI', if_true: 
> files('pci.c'))
>  acpi_ss.add(when: 'CONFIG_ACPI_VMGENID', if_true: files('vmgenid.c'))
>  acpi_ss.add(when: 'CONFIG_ACPI_HW_REDUCED', if_true: 
> files('generic_event_device.c'))
>  acpi_ss.add(when: 'CONFIG_ACPI_HMAT', if_true: files('hmat.c'))
> -acpi_ss.add(when: 'CONFIG_ACPI_APEI', if_true: files('ghes.c'))
> +acpi_ss.add(when: 'CONFIG_ACPI_APEI', if_true: files('ghes.c'), 
> if_false:('ghes-stub.c'))
>  acpi_ss.add(when: 'CONFIG_ACPI_X86', if_true: files('core.c', 'piix4.c', 
> 'pcihp.c'), if_false: files('acpi-stub.c'))
>  acpi_ss.add(when: 'CONFIG_ACPI_X86_ICH', if_true: files('ich9.c', 'tco.c'))
>  acpi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c'), if_false: 
> files('ipmi-stub.c'))
>  acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
>  acpi_ss.add(when: 'CONFIG_TPM', if_true: files('tpm.c'))
> -softmmu_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 
> 'aml-build-stub.c'))
> +softmmu_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 
> 'aml-build-stub.c', 'ghes-stub.c'))
>  softmmu_ss.add_all(when: 'CONFIG_ACPI', if_true: acpi_ss)
>  softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('acpi-stub.c', 
> 'aml-build-stub.c',
> -  'acpi-x86-stub.c', 
> 'ipmi-stub.c'))
> +  'acpi-x86-stub.c', 
> 'ipmi-stub.c', 'ghes-stub.c'))
> --
> 2.20.1
>

Reviewed-by: Dongjiu Geng 



[PATCH 4/4] alpha: Provide console information to the PALcode at start-up.

2021-06-13 Thread Jason Thorpe
Redefine the a2 register passed by Qemu at start-up to also include
some configuration flags, in addition to the CPU count, and define
a flag to mirror the "-nographics" option.

Signed-off-by: Jason Thorpe 
---
 hw/alpha/dp264.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index ac97104464..d86dcb1d81 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -72,9 +72,20 @@ static void clipper_init(MachineState *machine)
 cpus[i] = ALPHA_CPU(cpu_create(machine->cpu_type));
 }
 
+/* arg0 -> memory size
+   arg1 -> kernel entry point
+   arg2 -> config word
+
+   Config word: bits 0-5 -> ncpus
+bit  6   -> nographics option (for HWRPB CTB)
+
+   See init_hwrpb() in the PALcode.  */
+
 cpus[0]->env.trap_arg0 = ram_size;
 cpus[0]->env.trap_arg1 = 0;
 cpus[0]->env.trap_arg2 = smp_cpus;
+if (!machine->enable_graphics)
+  cpus[0]->env.trap_arg2 |= (1 << 6);
 
 /* Init the chipset.  Because we're using CLIPPER IRQ mappings,
the minimum PCI device IdSel is 1.  */
-- 
2.30.2




[PATCH 2/4] alpha: Set minimum PCI device ID to 1 to match Clipper IRQ mappings.

2021-06-13 Thread Jason Thorpe
Since we are emulating a Clipper device topology, we need to set the
minimum PCI device ID to 1, as there is no IRQ mapping for a device
at ID 0 (see sys_dp264.c:clipper_map_irq()).

- Add a 'devfn_min' argument to typhoon_init().  Pass that argument
  along to pci_register_root_bus().
- In clipper_init(), pass PCI_DEVFN(1, 0) as the minimum PCI device
  ID/function.

Signed-off-by: Jason Thorpe 
---
 hw/alpha/alpha_sys.h | 2 +-
 hw/alpha/dp264.c | 5 +++--
 hw/alpha/typhoon.c   | 5 +++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index e2c02e2bbe..4835b3d5ee 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -11,7 +11,7 @@
 
 
 PCIBus *typhoon_init(MemoryRegion *, ISABus **, qemu_irq *, AlphaCPU *[4],
- pci_map_irq_fn);
+ pci_map_irq_fn, uint8_t devfn_min);
 
 /* alpha_pci.c.  */
 extern const MemoryRegionOps alpha_pci_ignore_ops;
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 1017ecf330..ac97104464 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -76,9 +76,10 @@ static void clipper_init(MachineState *machine)
 cpus[0]->env.trap_arg1 = 0;
 cpus[0]->env.trap_arg2 = smp_cpus;
 
-/* Init the chipset.  */
+/* Init the chipset.  Because we're using CLIPPER IRQ mappings,
+   the minimum PCI device IdSel is 1.  */
 pci_bus = typhoon_init(machine->ram, _bus, _irq, cpus,
-   clipper_pci_map_irq);
+   clipper_pci_map_irq, PCI_DEVFN(1, 0));
 
 /* Since we have an SRM-compatible PALcode, use the SRM epoch.  */
 mc146818_rtc_init(isa_bus, 1900, rtc_irq);
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 87020cbe0d..fa31a2f286 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -815,7 +815,8 @@ static void typhoon_alarm_timer(void *opaque)
 }
 
 PCIBus *typhoon_init(MemoryRegion *ram, ISABus **isa_bus, qemu_irq *p_rtc_irq,
- AlphaCPU *cpus[4], pci_map_irq_fn sys_map_irq)
+ AlphaCPU *cpus[4], pci_map_irq_fn sys_map_irq,
+ uint8_t devfn_min)
 {
 MemoryRegion *addr_space = get_system_memory();
 DeviceState *dev;
@@ -885,7 +886,7 @@ PCIBus *typhoon_init(MemoryRegion *ram, ISABus **isa_bus, 
qemu_irq *p_rtc_irq,
 b = pci_register_root_bus(dev, "pci",
   typhoon_set_irq, sys_map_irq, s,
   >pchip.reg_mem, >pchip.reg_io,
-  0, 64, TYPE_PCI_BUS);
+  devfn_min, 64, TYPE_PCI_BUS);
 phb->bus = b;
 sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
 
-- 
2.30.2




[PATCH 3/4] alpha: Provide a PCI-ISA bridge device node for guest OS's that expect it

2021-06-13 Thread Jason Thorpe
Provide a PCI device node at ID 7 for the PCI-ISA bridge.  Even though
Tsunami/Typhoon systems would have used a different chip (Cypress or ALI),
for simplicity we model the Intel i82378, which was also used on several
Alpha models.  This is needed for some operating systems that only probe
ISA devices if a PCI-ISA or PCI-EISA bridge is found.

Signed-off-by: Jason Thorpe 
---
 hw/alpha/typhoon.c | 111 -
 1 file changed, 109 insertions(+), 2 deletions(-)

diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index fa31a2f286..de01828d23 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -919,11 +919,32 @@ PCIBus *typhoon_init(MemoryRegion *ram, ISABus **isa_bus, 
qemu_irq *p_rtc_irq,
 /* Pchip1 PCI I/O, 0x802.FC00., 32MB.  */
 /* Pchip1 PCI configuration, 0x802.FE00., 16MB.  */
 
-/* Init the ISA bus.  */
-/* ??? Technically there should be a cy82c693ub pci-isa bridge.  */
+/* Init the PCI-ISA bridge.  Technically, PCI-based Alphas shipped
+   with one of three different PCI-ISA bridges:
+
+   - Intel i82378 SIO
+   - Cypress CY82c693UB
+   - ALI M1533
+
+   (An Intel i82375 PCI-EISA bridge was also used on some models.)
+
+   For simplicity, we model an i82378 here, even though it wouldn't
+   have been on any Tsunami/Typhoon systems; it's close enough, and
+   we don't want to deal with modelling the CY82c693UB (which has
+   incompatible edge/level control registers, plus other peripherals
+   like IDE and USB) or the M1533 (which also has IDE and USB).
+
+   Importantly, we need to provide a PCI device node for it, otherwise
+   some operating systems won't notice there's an ISA bus to configure.
+
+   ??? We are using a private, stripped-down implementation of i82378
+   so that we can handle the way the ISA interrupts are wired up on
+   Tsunami/Typhoon systems.  */
 {
 qemu_irq *isa_irqs;
 
+pci_create_simple(b, PCI_DEVFN(7, 0), "i82378-typhoon");
+
 *isa_bus = isa_bus_new(NULL, get_system_memory(), >pchip.reg_io,
_abort);
 isa_irqs = i8259_init(*isa_bus,
@@ -954,10 +975,96 @@ static const TypeInfo typhoon_iommu_memory_region_info = {
 .class_init = typhoon_iommu_memory_region_class_init,
 };
 
+/* The following was copied from hw/isa/i82378.c and modified to provide
+   only the minimal PCI device node.  */
+
+/*
+ * QEMU Intel i82378 emulation (PCI to ISA bridge)
+ *
+ * Copyright (c) 2010-2011 Herv\xc3\xa9 Poussineau
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#include "migration/vmstate.h"
+
+#define TYPE_I82378 "i82378-typhoon"
+#define I82378(obj) \
+OBJECT_CHECK(I82378State, (obj), TYPE_I82378)
+
+typedef struct I82378State {
+PCIDevice parent_obj;
+} I82378State;
+
+static const VMStateDescription vmstate_i82378 = {
+.name = "pci-i82378-typhoon",
+.version_id = 0,
+.minimum_version_id = 0,
+.fields = (VMStateField[]) {
+VMSTATE_PCI_DEVICE(parent_obj, I82378State),
+VMSTATE_END_OF_LIST()
+},
+};
+
+static void i82378_realize(PCIDevice *pci, Error **errp)
+{
+uint8_t *pci_conf;
+
+pci_conf = pci->config;
+pci_set_word(pci_conf + PCI_COMMAND,
+ PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
+pci_set_word(pci_conf + PCI_STATUS,
+ PCI_STATUS_DEVSEL_MEDIUM);
+
+pci_config_set_interrupt_pin(pci_conf, 1); /* interrupt pin 0 */
+}
+
+static void i82378_init(Object *obj)
+{
+}
+
+static void i82378_class_init(ObjectClass *klass, void *data)
+{
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+k->realize = i82378_realize;
+k->vendor_id = PCI_VENDOR_ID_INTEL;
+k->device_id = PCI_DEVICE_ID_INTEL_82378;
+k->revision = 0x03;
+k->class_id = PCI_CLASS_BRIDGE_ISA;
+dc->vmsd = _i82378;
+set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+}
+
+static const TypeInfo i82378_typhoon_type_info = {
+.name = TYPE_I82378,
+.parent = TYPE_PCI_DEVICE,
+.instance_size = sizeof(I82378State),
+.instance_init = i82378_init,
+.class_init = i82378_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
+{ },
+},
+};
+
 static void 

[PATCH 0/4] Emulator fixes to enable running NetBSD/alpha

2021-06-13 Thread Jason Thorpe
The following are a set of fixes to device and system emulation that
allow NetBSD/alpha to run in qemu-system-alpha.

The first change fixes behavior in the mc146818rtc emulation to more
accurately reflect how the real hardware vis a vis the PF status bit
(specifically, that it is independent of the PIE control bit).  The
behavior of PF now matches the data sheet for the part.  This documented
behavior is relied upon by NetBSD/alpha to calibrate some timing loops.

The next two fix up a couple of aspects of the emulated device topology
for the "Clipper" system emulation.

The fourth passes console configuration information to the PALcode
at start-up, which in turn will reflect this information in the
Console Terminal Block to the guest operating system, and relies
on a corresponding PALcode change, although older PALcode will still
work if the "-nographics" option is not specified.

Jason Thorpe (4):
  mc146818rtc: Make PF independent of PIE
  alpha: Set minimum PCI device ID to 1 to match Clipper IRQ mappings.
  alpha: Provide a PCI-ISA bridge device node for guest OS's that expect
it
  alpha: Provide console information to the PALcode at start-up.

 hw/alpha/alpha_sys.h |   2 +-
 hw/alpha/dp264.c |  16 +-
 hw/alpha/typhoon.c   | 116 +--
 hw/rtc/mc146818rtc.c |   4 --
 4 files changed, 127 insertions(+), 11 deletions(-)

-- 
2.30.2




[PATCH 1/4] mc146818rtc: Make PF independent of PIE

2021-06-13 Thread Jason Thorpe
Make the PF flag behave like real hardware by always running the
periodic timer without regard to the setting of the PIE bit, so
that the PF will be set when the period expires even if an interrupt
will not be raised.  This behavior is documented on page 16 of the
MC146818A advance information datasheet.

Signed-off-by: Jason Thorpe 
---
 hw/rtc/mc146818rtc.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 4fbafddb22..366b8f13de 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -155,10 +155,6 @@ static uint32_t rtc_periodic_clock_ticks(RTCState *s)
 {
 int period_code;
 
-if (!(s->cmos_data[RTC_REG_B] & REG_B_PIE)) {
-return 0;
- }
-
 period_code = s->cmos_data[RTC_REG_A] & 0x0f;
 
 return periodic_period_to_clock(period_code);
-- 
2.30.2




[PATCH v2 1/1] Provide a minimal Console Terminal Block in the HWRPB.

2021-06-13 Thread Jason Thorpe
Provide a minimal Console Terminal Block in the HWRPB so that operating
systems that depend on it can correctly initialize the console device.
This is suffucient, at least, for the BSD operating systems, but may not
be sufficient for Digital UNIX.

In addition to defining and filling out the structures, there are a couple
of other key changes:

- Redefine the a2 register passed by Qemu at start-up to also include
  some configuration flags, in addition to the CPU count, and define
  a flag to mirror the "-nographics" option.

- We need to initialize the HWRPB *after* initializing VGA, so that
  we'll know if a VGA device is present and in which slot for filling
  out the CTB.

Signed-off-by: Jason Thorpe 
---
 hwrpb.h  | 54 ++
 init.c   | 36 +---
 protos.h |  2 ++
 vgaio.c  |  2 ++
 4 files changed, 91 insertions(+), 3 deletions(-)

diff --git a/hwrpb.h b/hwrpb.h
index 2166bad..7b6efc6 100644
--- a/hwrpb.h
+++ b/hwrpb.h
@@ -146,6 +146,60 @@ struct crb_struct {
struct vf_map_struct map[1];
 };
 
+struct ctb_struct {
+   unsigned long type;
+   unsigned long unit;
+   unsigned long res0;
+   unsigned long len;
+   unsigned long ipl;
+   unsigned long tintr_vec;
+   unsigned long rintr_vec;
+   unsigned long term_type;
+   unsigned long keybd_type;
+   unsigned long keybd_trans;
+   unsigned long keybd_map;
+   unsigned long keybd_state;
+   unsigned long keybd_last;
+   unsigned long font_us;
+   unsigned long font_mcs;
+   unsigned long font_width;
+   unsigned long font_height;
+   unsigned long mon_width;
+   unsigned long mon_height;
+   unsigned long dpi;
+   unsigned long planes;
+   unsigned long cur_width;
+   unsigned long cur_height;
+   unsigned long head_cnt;
+   unsigned long opwindow;
+   unsigned long head_offset;
+   unsigned long putchar;
+   unsigned long io_state;
+   unsigned long listen_state;
+   unsigned long xaddr;
+   unsigned long turboslot;
+   unsigned long server_off;
+   unsigned long line_off;
+   unsigned char csd;
+};
+
+#define CTB_NONE   0x00
+#define CTB_PRINTERPORT0x02
+#define CTB_GRAPHICS   0x03
+#define CTB_MULTIPURPOSE   0x04
+
+/*
+ * Format of the Console Terminal Block MULTIPURPOSE `turboslot' field:
+ *
+ *  63   40 39   32 31 24 23  16 15   8 70
+ *  |  reserved|  channel  |  hose   | bus type |  bus | slot |
+ */
+
+#define CTB_TURBOSLOT_TYPE_TC   0   /* TURBOchannel */
+#define CTB_TURBOSLOT_TYPE_ISA  1   /* ISA */
+#define CTB_TURBOSLOT_TYPE_EISA 2   /* EISA */
+#define CTB_TURBOSLOT_TYPE_PCI  3   /* PCI */
+
 struct memclust_struct {
unsigned long start_pfn;
unsigned long numpages;
diff --git a/init.c b/init.c
index 6edfdf2..a58734f 100644
--- a/init.c
+++ b/init.c
@@ -36,11 +36,20 @@
 
 #define HZ 1024
 
+/* Upon entry, register a2 contains configuration information from the VM:
+
+   bits 0-5 -- ncpus
+   bit  6   -- "nographics" option (used to initialize CTB)  */
+
+#define CONFIG_NCPUS(x)  ((x) & 63)
+#define CONFIG_NOGRAPHICS(x) ((x) & (1ull << 6))
+
 struct hwrpb_combine {
   struct hwrpb_struct hwrpb;
   struct percpu_struct processor[4];
   struct memdesc_struct md;
   struct memclust_struct mc[2];
+  struct ctb_struct ctb;
   struct crb_struct crb;
   struct procdesc_struct proc_dispatch;
   struct procdesc_struct proc_fixup;
@@ -59,6 +68,8 @@ struct hwrpb_combine hwrpb 
__attribute__((aligned(PAGE_SIZE)));
 
 void *last_alloc;
 bool have_vga;
+unsigned int pci_vga_bus;
+unsigned int pci_vga_dev;
 
 static void *
 alloc (unsigned long size, unsigned long align)
@@ -136,12 +147,13 @@ init_page_table(void)
 }
 
 static void
-init_hwrpb (unsigned long memsize, unsigned long cpus)
+init_hwrpb (unsigned long memsize, unsigned long config)
 {
   unsigned long pal_pages;
   unsigned long amask;
   unsigned long i;
   unsigned long proc_type = EV4_CPU;
+  unsigned long cpus = CONFIG_NCPUS(config);
   
   hwrpb.hwrpb.phys_addr = PA();
 
@@ -226,6 +238,23 @@ init_hwrpb (unsigned long memsize, unsigned long cpus)
   hwrpb.mc[1].start_pfn = pal_pages;
   hwrpb.mc[1].numpages = (memsize >> PAGE_SHIFT) - pal_pages;
 
+  hwrpb.hwrpb.ctbt_offset = offsetof(struct hwrpb_combine, ctb);
+  memset(, 0, sizeof(hwrpb.ctb));
+  hwrpb.hwrpb.ctb_size = sizeof(hwrpb.ctb);
+  hwrpb.ctb.len = sizeof(hwrpb.ctb) - offsetof(struct ctb_struct, ipl);
+  if (have_vga && !CONFIG_NOGRAPHICS(config))
+{
+  hwrpb.ctb.type = CTB_MULTIPURPOSE;
+  hwrpb.ctb.term_type = CTB_GRAPHICS;
+  hwrpb.ctb.turboslot = (CTB_TURBOSLOT_TYPE_PCI << 16) |
+   (pci_vga_bus << 8) | pci_vga_dev;
+}
+  else
+{
+  hwrpb.ctb.type = CTB_PRINTERPORT;
+  hwrpb.ctb.term_type = 

[PATCH v2 0/1] PALcode fixes for guest OS console initialization

2021-06-13 Thread Jason Thorpe
This is a follow-up on my previous set of patches for the Qemu PALcode,
which were merged except for the CTB patch.  The patch has incorporated
review feedback, but there is still some research going on about how
real DEC SRM initializes a particular field in various circumstances;
this is my best guess based on available documentation and observed
behavior of real machines, and is sufficient for the BSD operating systems.

Jason Thorpe (1):
  Provide a minimal Console Terminal Block in the HWRPB.

 hwrpb.h  | 54 ++
 init.c   | 36 +---
 protos.h |  2 ++
 vgaio.c  |  2 ++
 4 files changed, 91 insertions(+), 3 deletions(-)

-- 
2.30.2




[PATCH] target/i386/tcg/sysemu/bpt_helper.c: typo since cpu_breakpoint and cpu_watchpoint are in a union, the code should access only one of them

2021-06-13 Thread Dmitry Voronetskiy
Signed-off-by: Dmitry Voronetskiy 
---
 target/i386/tcg/sysemu/bpt_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/tcg/sysemu/bpt_helper.c 
b/target/i386/tcg/sysemu/bpt_helper.c
index 9bdf7e170b..de284dbdcb 100644
--- a/target/i386/tcg/sysemu/bpt_helper.c
+++ b/target/i386/tcg/sysemu/bpt_helper.c
@@ -109,9 +109,9 @@ static void hw_breakpoint_remove(CPUX86State *env, int 
index)
 
 case DR7_TYPE_DATA_WR:
 case DR7_TYPE_DATA_RW:
-if (env->cpu_breakpoint[index]) {
+if (env->cpu_watchpoint[index]) {
 cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
-env->cpu_breakpoint[index] = NULL;
+env->cpu_watchpoint[index] = NULL;
 }
 break;
 
-- 
2.25.1




Re: [PULL 00/34] tcg patch queue

2021-06-13 Thread Peter Maydell
On Sun, 13 Jun 2021 at 16:13, Peter Maydell  wrote:
>
> On Sat, 12 Jun 2021 at 00:47, Richard Henderson
>  wrote:
> >
> > This is mostly my code_gen_buffer cleanup, plus a few other random
> > changes thrown in.  Including a fix for a recent float32_exp2 bug.
> >
> >
> > r~
> >
> >
> > The following changes since commit 894fc4fd670aaf04a67dc7507739f914ff4bacf2:
> >
> >   Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' 
> > into staging (2021-06-11 09:21:48 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210611
> >
> > for you to fetch changes up to 60afaddc208d34f6dc86dd974f6e02724fba6eb6:
> >
> >   docs/devel: Explain in more detail the TB chaining mechanisms (2021-06-11 
> > 09:41:25 -0700)
> >
> > 
> > Clean up code_gen_buffer allocation.
> > Add tcg_remove_ops_after.
> > Fix tcg_constant_* documentation.
> > Improve TB chaining documentation.
> > Fix float32_exp2.
>
> Compile failure on OSX:
>
> ../../tcg/region.c:706:12: error: incompatible pointer to integer
> conversion assigning to 'mach_vm_address_t' (aka 'unsigned long long')
> from 'void *' [-Werror,-Wint-conversion]
> buf_rw = region.start_aligned;
>^ 
> 1 error generated.

Also on x86-64 host, this failure in check-tcg:

make[2]: Leaving directory
'/home/petmay01/linaro/qemu-for-merges/build/all-linux-static/tests/tcg/hppa-linux-user'
make[2]: Entering directory
'/home/petmay01/linaro/qemu-for-merges/build/all-linux-static/tests/tcg/hppa-linux-user'
timeout --foreground 60
/home/petmay01/linaro/qemu-for-merges/build/all-linux-static/qemu-hppa
 signals >  signals.out
timeout: the monitored command dumped core
Illegal instruction
../Makefile.target:156: recipe for target 'run-signals' failed
make[2]: *** [run-signals] Error 132
make[2]: Leaving directory
'/home/petmay01/linaro/qemu-for-merges/build/all-linux-static/tests/tcg/hppa-linux-user'
/home/petmay01/linaro/qemu-for-merges/tests/tcg/Makefile.qemu:102:
recipe for target 'run-guest-tests' failed

but I think this is a pre-existing intermittent.

-- PMM



[PATCH 3/5] dp8393x: fix PROM checksum and MAC address storage

2021-06-13 Thread Mark Cave-Ayland
The checksum used by MacOS to validate the PROM content is an exclusive-OR
rather than a sum over the corresponding bytes. In addition the MAC address
must be stored in bit-reversed format as indicated in comments in Linux's
macsonic.c.

With the PROM contents fixed MacOS starts to probe the device registers
when AppleTalk is enabled in the Control Panel.

Signed-off-by: Mark Cave-Ayland 
---
 hw/net/dp8393x.c | 43 ++-
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index ea5b22f680..7c745d7963 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -173,6 +173,42 @@ struct dp8393xState {
 AddressSpace as;
 };
 
+/* Table lookup for bitrev8 */
+static const uint8_t byte_rev_table[256] = {
+0x00, 0x80, 0x40, 0xc0, 0x20, 0xa0, 0x60, 0xe0,
+0x10, 0x90, 0x50, 0xd0, 0x30, 0xb0, 0x70, 0xf0,
+0x08, 0x88, 0x48, 0xc8, 0x28, 0xa8, 0x68, 0xe8,
+0x18, 0x98, 0x58, 0xd8, 0x38, 0xb8, 0x78, 0xf8,
+0x04, 0x84, 0x44, 0xc4, 0x24, 0xa4, 0x64, 0xe4,
+0x14, 0x94, 0x54, 0xd4, 0x34, 0xb4, 0x74, 0xf4,
+0x0c, 0x8c, 0x4c, 0xcc, 0x2c, 0xac, 0x6c, 0xec,
+0x1c, 0x9c, 0x5c, 0xdc, 0x3c, 0xbc, 0x7c, 0xfc,
+0x02, 0x82, 0x42, 0xc2, 0x22, 0xa2, 0x62, 0xe2,
+0x12, 0x92, 0x52, 0xd2, 0x32, 0xb2, 0x72, 0xf2,
+0x0a, 0x8a, 0x4a, 0xca, 0x2a, 0xaa, 0x6a, 0xea,
+0x1a, 0x9a, 0x5a, 0xda, 0x3a, 0xba, 0x7a, 0xfa,
+0x06, 0x86, 0x46, 0xc6, 0x26, 0xa6, 0x66, 0xe6,
+0x16, 0x96, 0x56, 0xd6, 0x36, 0xb6, 0x76, 0xf6,
+0x0e, 0x8e, 0x4e, 0xce, 0x2e, 0xae, 0x6e, 0xee,
+0x1e, 0x9e, 0x5e, 0xde, 0x3e, 0xbe, 0x7e, 0xfe,
+0x01, 0x81, 0x41, 0xc1, 0x21, 0xa1, 0x61, 0xe1,
+0x11, 0x91, 0x51, 0xd1, 0x31, 0xb1, 0x71, 0xf1,
+0x09, 0x89, 0x49, 0xc9, 0x29, 0xa9, 0x69, 0xe9,
+0x19, 0x99, 0x59, 0xd9, 0x39, 0xb9, 0x79, 0xf9,
+0x05, 0x85, 0x45, 0xc5, 0x25, 0xa5, 0x65, 0xe5,
+0x15, 0x95, 0x55, 0xd5, 0x35, 0xb5, 0x75, 0xf5,
+0x0d, 0x8d, 0x4d, 0xcd, 0x2d, 0xad, 0x6d, 0xed,
+0x1d, 0x9d, 0x5d, 0xdd, 0x3d, 0xbd, 0x7d, 0xfd,
+0x03, 0x83, 0x43, 0xc3, 0x23, 0xa3, 0x63, 0xe3,
+0x13, 0x93, 0x53, 0xd3, 0x33, 0xb3, 0x73, 0xf3,
+0x0b, 0x8b, 0x4b, 0xcb, 0x2b, 0xab, 0x6b, 0xeb,
+0x1b, 0x9b, 0x5b, 0xdb, 0x3b, 0xbb, 0x7b, 0xfb,
+0x07, 0x87, 0x47, 0xc7, 0x27, 0xa7, 0x67, 0xe7,
+0x17, 0x97, 0x57, 0xd7, 0x37, 0xb7, 0x77, 0xf7,
+0x0f, 0x8f, 0x4f, 0xcf, 0x2f, 0xaf, 0x6f, 0xef,
+0x1f, 0x9f, 0x5f, 0xdf, 0x3f, 0xbf, 0x7f, 0xff,
+};
+
 /*
  * Accessor functions for values which are formed by
  * concatenating two 16 bit device registers. By putting these
@@ -996,11 +1032,8 @@ static void dp8393x_realize(DeviceState *dev, Error 
**errp)
 prom = memory_region_get_ram_ptr(>prom);
 checksum = 0;
 for (i = 0; i < 6; i++) {
-prom[i] = s->conf.macaddr.a[i];
-checksum += prom[i];
-if (checksum > 0xff) {
-checksum = (checksum + 1) & 0xff;
-}
+prom[i] = byte_rev_table[s->conf.macaddr.a[i]];
+checksum ^= prom[i];
 }
 prom[7] = 0xff - checksum;
 }
-- 
2.20.1




[PATCH 1/5] dp8393x: checkpatch fixes

2021-06-13 Thread Mark Cave-Ayland
Also fix a simple comment typo of "constrainst" to "constraints".

Signed-off-by: Mark Cave-Ayland 
---
 hw/net/dp8393x.c | 231 +--
 1 file changed, 122 insertions(+), 109 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 533a8304d0..56af08f0fe 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -29,14 +29,14 @@
 #include 
 #include "qom/object.h"
 
-//#define DEBUG_SONIC
+/* #define DEBUG_SONIC */
 
 #define SONIC_PROM_SIZE 0x1000
 
 #ifdef DEBUG_SONIC
 #define DPRINTF(fmt, ...) \
 do { printf("sonic: " fmt , ##  __VA_ARGS__); } while (0)
-static const char* reg_names[] = {
+static const char *reg_names[] = {
 "CR", "DCR", "RCR", "TCR", "IMR", "ISR", "UTDA", "CTDA",
 "TPS", "TFC", "TSA0", "TSA1", "TFS", "URDA", "CRDA", "CRBA0",
 "CRBA1", "RBWC0", "RBWC1", "EOBC", "URRA", "RSA", "REA", "RRP",
@@ -185,7 +185,8 @@ struct dp8393xState {
 AddressSpace as;
 };
 
-/* Accessor functions for values which are formed by
+/*
+ * Accessor functions for values which are formed by
  * concatenating two 16 bit device registers. By putting these
  * in their own functions with a uint32_t return type we avoid the
  * pitfall of implicit sign extension where ((x << 16) | y) is a
@@ -350,8 +351,7 @@ static void dp8393x_do_read_rra(dp8393xState *s)
 }
 
 /* Warn the host if CRBA now has the last available resource */
-if (s->regs[SONIC_RRP] == s->regs[SONIC_RWP])
-{
+if (s->regs[SONIC_RRP] == s->regs[SONIC_RWP]) {
 s->regs[SONIC_ISR] |= SONIC_ISR_RBE;
 dp8393x_update_irq(s);
 }
@@ -364,7 +364,8 @@ static void dp8393x_do_software_reset(dp8393xState *s)
 {
 timer_del(s->watchdog);
 
-s->regs[SONIC_CR] &= ~(SONIC_CR_LCAM | SONIC_CR_RRRA | SONIC_CR_TXP | 
SONIC_CR_HTX);
+s->regs[SONIC_CR] &= ~(SONIC_CR_LCAM | SONIC_CR_RRRA | SONIC_CR_TXP |
+   SONIC_CR_HTX);
 s->regs[SONIC_CR] |= SONIC_CR_RST | SONIC_CR_RXDIS;
 }
 
@@ -490,8 +491,10 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
 
 /* Handle Ethernet checksum */
 if (!(s->regs[SONIC_TCR] & SONIC_TCR_CRCI)) {
-/* Don't append FCS there, to look like slirp packets
- * which don't have one */
+/*
+ * Don't append FCS there, to look like slirp packets
+ * which don't have one
+ */
 } else {
 /* Remove existing FCS */
 tx_len -= 4;
@@ -558,26 +561,34 @@ static void dp8393x_do_command(dp8393xState *s, uint16_t 
command)
 
 s->regs[SONIC_CR] |= (command & SONIC_CR_MASK);
 
-if (command & SONIC_CR_HTX)
+if (command & SONIC_CR_HTX) {
 dp8393x_do_halt_transmission(s);
-if (command & SONIC_CR_TXP)
+}
+if (command & SONIC_CR_TXP) {
 dp8393x_do_transmit_packets(s);
-if (command & SONIC_CR_RXDIS)
+}
+if (command & SONIC_CR_RXDIS) {
 dp8393x_do_receiver_disable(s);
-if (command & SONIC_CR_RXEN)
+}
+if (command & SONIC_CR_RXEN) {
 dp8393x_do_receiver_enable(s);
-if (command & SONIC_CR_STP)
+}
+if (command & SONIC_CR_STP) {
 dp8393x_do_stop_timer(s);
-if (command & SONIC_CR_ST)
+}
+if (command & SONIC_CR_ST) {
 dp8393x_do_start_timer(s);
-if (command & SONIC_CR_RST)
+}
+if (command & SONIC_CR_RST) {
 dp8393x_do_software_reset(s);
+}
 if (command & SONIC_CR_RRRA) {
 dp8393x_do_read_rra(s);
 s->regs[SONIC_CR] &= ~SONIC_CR_RRRA;
 }
-if (command & SONIC_CR_LCAM)
+if (command & SONIC_CR_LCAM) {
 dp8393x_do_load_cam(s);
+}
 }
 
 static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size)
@@ -587,24 +598,24 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr, 
unsigned int size)
 uint16_t val = 0;
 
 switch (reg) {
-/* Update data before reading it */
-case SONIC_WT0:
-case SONIC_WT1:
-dp8393x_update_wt_regs(s);
-val = s->regs[reg];
-break;
-/* Accept read to some registers only when in reset mode */
-case SONIC_CAP2:
-case SONIC_CAP1:
-case SONIC_CAP0:
-if (s->regs[SONIC_CR] & SONIC_CR_RST) {
-val = s->cam[s->regs[SONIC_CEP] & 0xf][2* (SONIC_CAP0 - reg) + 
1] << 8;
-val |= s->cam[s->regs[SONIC_CEP] & 0xf][2* (SONIC_CAP0 - reg)];
-}
-break;
-/* All other registers have no special contrainst */
-default:
-val = s->regs[reg];
+/* Update data before reading it */
+case SONIC_WT0:
+case SONIC_WT1:
+dp8393x_update_wt_regs(s);
+val = s->regs[reg];
+break;
+/* Accept read to some registers only when in reset mode */
+case SONIC_CAP2:
+case SONIC_CAP1:
+case SONIC_CAP0:
+if (s->regs[SONIC_CR] & SONIC_CR_RST) {
+val = s->cam[s->regs[SONIC_CEP] & 0xf][2 

[PATCH 2/5] dp8393x: convert to trace-events

2021-06-13 Thread Mark Cave-Ayland
Signed-off-by: Mark Cave-Ayland 
---
 hw/net/dp8393x.c| 55 +
 hw/net/trace-events | 17 ++
 2 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 56af08f0fe..ea5b22f680 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -28,14 +28,10 @@
 #include "qemu/timer.h"
 #include 
 #include "qom/object.h"
-
-/* #define DEBUG_SONIC */
+#include "trace.h"
 
 #define SONIC_PROM_SIZE 0x1000
 
-#ifdef DEBUG_SONIC
-#define DPRINTF(fmt, ...) \
-do { printf("sonic: " fmt , ##  __VA_ARGS__); } while (0)
 static const char *reg_names[] = {
 "CR", "DCR", "RCR", "TCR", "IMR", "ISR", "UTDA", "CTDA",
 "TPS", "TFC", "TSA0", "TSA1", "TFS", "URDA", "CRDA", "CRBA0",
@@ -45,12 +41,6 @@ static const char *reg_names[] = {
 "SR", "WT0", "WT1", "RSC", "CRCT", "FAET", "MPT", "MDT",
 "0x30", "0x31", "0x32", "0x33", "0x34", "0x35", "0x36", "0x37",
 "0x38", "0x39", "0x3a", "0x3b", "0x3c", "0x3d", "0x3e", "DCR2" };
-#else
-#define DPRINTF(fmt, ...) do {} while (0)
-#endif
-
-#define SONIC_ERROR(fmt, ...) \
-do { printf("sonic ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); } while (0)
 
 #define SONIC_CR 0x00
 #define SONIC_DCR0x01
@@ -161,9 +151,7 @@ struct dp8393xState {
 bool big_endian;
 bool last_rba_is_full;
 qemu_irq irq;
-#ifdef DEBUG_SONIC
 int irq_level;
-#endif
 QEMUTimer *watchdog;
 int64_t wt_last_update;
 NICConf conf;
@@ -270,16 +258,14 @@ static void dp8393x_update_irq(dp8393xState *s)
 {
 int level = (s->regs[SONIC_IMR] & s->regs[SONIC_ISR]) ? 1 : 0;
 
-#ifdef DEBUG_SONIC
 if (level != s->irq_level) {
 s->irq_level = level;
 if (level) {
-DPRINTF("raise irq, isr is 0x%04x\n", s->regs[SONIC_ISR]);
+trace_dp8393x_raise_irq(s->regs[SONIC_ISR]);
 } else {
-DPRINTF("lower irq\n");
+trace_dp8393x_lower_irq();
 }
 }
-#endif
 
 qemu_set_irq(s->irq, level);
 }
@@ -302,9 +288,9 @@ static void dp8393x_do_load_cam(dp8393xState *s)
 s->cam[index][3] = dp8393x_get(s, width, 2) >> 8;
 s->cam[index][4] = dp8393x_get(s, width, 3) & 0xff;
 s->cam[index][5] = dp8393x_get(s, width, 3) >> 8;
-DPRINTF("load cam[%d] with %02x%02x%02x%02x%02x%02x\n", index,
-s->cam[index][0], s->cam[index][1], s->cam[index][2],
-s->cam[index][3], s->cam[index][4], s->cam[index][5]);
+trace_dp8393x_load_cam(index, s->cam[index][0], s->cam[index][1],
+   s->cam[index][2], s->cam[index][3],
+   s->cam[index][4], s->cam[index][5]);
 /* Move to next entry */
 s->regs[SONIC_CDC]--;
 s->regs[SONIC_CDP] += size;
@@ -315,7 +301,7 @@ static void dp8393x_do_load_cam(dp8393xState *s)
 address_space_read(>as, dp8393x_cdp(s),
MEMTXATTRS_UNSPECIFIED, s->data, size);
 s->regs[SONIC_CE] = dp8393x_get(s, width, 0);
-DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]);
+trace_dp8393x_load_cam_done(s->regs[SONIC_CE]);
 
 /* Done */
 s->regs[SONIC_CR] &= ~SONIC_CR_LCAM;
@@ -338,9 +324,8 @@ static void dp8393x_do_read_rra(dp8393xState *s)
 s->regs[SONIC_CRBA1] = dp8393x_get(s, width, 1);
 s->regs[SONIC_RBWC0] = dp8393x_get(s, width, 2);
 s->regs[SONIC_RBWC1] = dp8393x_get(s, width, 3);
-DPRINTF("CRBA0/1: 0x%04x/0x%04x, RBWC0/1: 0x%04x/0x%04x\n",
-s->regs[SONIC_CRBA0], s->regs[SONIC_CRBA1],
-s->regs[SONIC_RBWC0], s->regs[SONIC_RBWC1]);
+trace_dp8393x_read_rra_regs(s->regs[SONIC_CRBA0], s->regs[SONIC_CRBA1],
+s->regs[SONIC_RBWC0], s->regs[SONIC_RBWC1]);
 
 /* Go to next entry */
 s->regs[SONIC_RRP] += size;
@@ -444,7 +429,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
 /* Read memory */
 size = sizeof(uint16_t) * 6 * width;
 s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA];
-DPRINTF("Transmit packet at %08x\n", dp8393x_ttda(s));
+trace_dp8393x_transmit_packet(dp8393x_ttda(s));
 address_space_read(>as, dp8393x_ttda(s) + sizeof(uint16_t) * width,
MEMTXATTRS_UNSPECIFIED, s->data, size);
 tx_len = 0;
@@ -499,7 +484,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
 /* Remove existing FCS */
 tx_len -= 4;
 if (tx_len < 0) {
-SONIC_ERROR("tx_len is %d\n", tx_len);
+trace_dp8393x_transmit_txlen_error(tx_len);
 break;
 }
 }
@@ -618,7 +603,7 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr, 
unsigned int size)
 val = s->regs[reg];
 }
 
-DPRINTF("read 0x%04x from reg %s\n", val, reg_names[reg]);
+trace_dp8393x_read(reg, reg_names[reg], val, size);
 
 return s->big_endian ? val << 16 : val;
 }
@@ -630,7 +615,7 @@ static 

[PATCH 5/5] dp8393x: fix CAM descriptor entry index

2021-06-13 Thread Mark Cave-Ayland
Currently when a LOAD CAM command is executed the entries are loaded into the
CAM from memory in order which is incorrect. According to the datasheet the
first entry in the CAM descriptor is the entry index which means that each
descriptor may update any single entry in the CAM rather than the Nth entry.

Decode the CAM entry index and use it store the descriptor in the appropriate
slot in the CAM. This fixes the issue where the MacOS toolbox loads a single
CAM descriptor into the final slot in order to perform a loopback test which
must succeed before the Ethernet port is enabled.

Signed-off-by: Mark Cave-Ayland 
---
 hw/net/dp8393x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index f9cb10a573..3ec9810b84 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -309,7 +309,7 @@ static void dp8393x_update_irq(dp8393xState *s)
 static void dp8393x_do_load_cam(dp8393xState *s)
 {
 int width, size;
-uint16_t index = 0;
+uint16_t index;
 
 width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
 size = sizeof(uint16_t) * 4 * width;
@@ -318,6 +318,7 @@ static void dp8393x_do_load_cam(dp8393xState *s)
 /* Fill current entry */
 address_space_read(>as, dp8393x_cdp(s),
MEMTXATTRS_UNSPECIFIED, s->data, size);
+index = dp8393x_get(s, width, 0) & 0xf;
 s->cam[index][0] = dp8393x_get(s, width, 1) & 0xff;
 s->cam[index][1] = dp8393x_get(s, width, 1) >> 8;
 s->cam[index][2] = dp8393x_get(s, width, 2) & 0xff;
@@ -330,7 +331,6 @@ static void dp8393x_do_load_cam(dp8393xState *s)
 /* Move to next entry */
 s->regs[SONIC_CDC]--;
 s->regs[SONIC_CDP] += size;
-index++;
 }
 
 /* Read CAM enable */
-- 
2.20.1




[PATCH 0/5] dp8393x: fixes for MacOS toolbox ROM

2021-06-13 Thread Mark Cave-Ayland
Here is the next set of patches from my attempts to boot MacOS under QEMU's
Q800 machine related to the Sonic network adapter.

Patches 1 and 2 sort out checkpatch and convert from DPRINTF macros to
trace-events.

Patch 3 fixes the PROM checksum and MAC address storage format as found by
stepping through the MacOS toolbox.

Patch 4 ensures that the CPU loads/stores are correctly converted to 16-bit
accesses for the network card and patch 5 fixes a bug when selecting the
index specified for CAM entries.

NOTE TO MIPS MAINTAINERS:

- The Sonic network adapter is used as part of the MIPS jazz machine, however
  I don't have a working kernel and system to test it with. Any pointers to
  test images would be appreciated.

- The changes to the PROM checksum in patch 3 were determined by stepping
  through the MacOS toolbox, and is different from the existing algorithm.
  Has the current PROM checksum algorithm been validated on a MIPS guest or
  was it just a guess? It might be that 2 different algorithms are needed for
  the Q800 vs. Jazz machine.

- My current guess is the jazzsonic driver is broken since the last set of
  dp8393x changes as the MIPS jazz machine does not set the "big_endian"
  property on the dp8393x device. I'd expect that the following diff would
  be needed, but I can't confirm this without a suitable test image.

diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index 1e1cf8154e..1df67035aa 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -280,6 +280,7 @@ static void mips_jazz_init(MachineState *machine,
 dev = qdev_new("dp8393x");
 qdev_set_nic_properties(dev, nd);
 qdev_prop_set_uint8(dev, "it_shift", 2);
+qdev_prop_set_bit(dev, "big_endian", true);
 object_property_set_link(OBJECT(dev), "dma_mr",
  OBJECT(rc4030_dma_mr), _abort);
 sysbus = SYS_BUS_DEVICE(dev);

Signed-off-by: Mark Cave-Ayland 

[q800-macos-upstream patchset series: 3]

Mark Cave-Ayland (5):
  dp8393x: checkpatch fixes
  dp8393x: convert to trace-events
  dp8393x: fix PROM checksum and MAC address storage
  dp8393x: don't force 32-bit register access
  dp8393x: fix CAM descriptor entry index

 hw/net/dp8393x.c| 332 
 hw/net/trace-events |  17 +++
 2 files changed, 198 insertions(+), 151 deletions(-)

-- 
2.20.1




[PATCH 4/5] dp8393x: don't force 32-bit register access

2021-06-13 Thread Mark Cave-Ayland
Commit 3fe9a838ec "dp8393x: Always use 32-bit accesses" assumed that all 
accesses
to the registers were 32-bit but this is actually not the case. The access size 
is
determined by the CPU instruction used and not the number of physical address 
lines.

The big_endian workaround applied to the register read/writes was actually 
caused
by forcing the access size to 32-bit when the guest OS was using a 16-bit 
access.
Since the registers are 16-bit then we can simply set .impl.min_access to 2 and
then the memory API will automatically do the right thing for both 16-bit 
accesses
used by Linux and 32-bit accesses used by the MacOS toolbox ROM.

Signed-off-by: Mark Cave-Ayland 
Fixes: 3fe9a838ec ("dp8393x: Always use 32-bit accesses")
---
 hw/net/dp8393x.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 7c745d7963..f9cb10a573 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -641,15 +641,14 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr, 
unsigned int size)
 
 trace_dp8393x_read(reg, reg_names[reg], val, size);
 
-return s->big_endian ? val << 16 : val;
+return val;
 }
 
-static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
+static void dp8393x_write(void *opaque, hwaddr addr, uint64_t val,
   unsigned int size)
 {
 dp8393xState *s = opaque;
 int reg = addr >> s->it_shift;
-uint32_t val = s->big_endian ? data >> 16 : data;
 
 trace_dp8393x_write(reg, reg_names[reg], val, size);
 
@@ -733,7 +732,7 @@ static void dp8393x_write(void *opaque, hwaddr addr, 
uint64_t data,
 static const MemoryRegionOps dp8393x_ops = {
 .read = dp8393x_read,
 .write = dp8393x_write,
-.impl.min_access_size = 4,
+.impl.min_access_size = 2,
 .impl.max_access_size = 4,
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
-- 
2.20.1




Re: [PATCH v4 6/8] hw/intc: GICv3 redistributor ITS processing

2021-06-13 Thread Eric Auger
Hi Shashi,

On 6/2/21 8:00 PM, Shashi Mallela wrote:
> Implemented lpi processing at redistributor to get lpi config info
s/Implemented/Implement here are elsewhere.
> from lpi configuration table,determine priority,set pending state in
> lpi pending table and forward the lpi to cpuif.Added logic to invoke
> redistributor lpi processing with translated LPI which set/clear LPI
> from ITS device as part of ITS INT,CLEAR,DISCARD command and
> GITS_TRANSLATER processing.
> 
> Signed-off-by: Shashi Mallela 
> ---
>  hw/intc/arm_gicv3.c|   9 ++
>  hw/intc/arm_gicv3_common.c |   1 +
>  hw/intc/arm_gicv3_cpuif.c  |   7 +-
>  hw/intc/arm_gicv3_its.c|  14 ++-
>  hw/intc/arm_gicv3_redist.c | 145 +
>  hw/intc/gicv3_internal.h   |  10 ++
>  include/hw/intc/arm_gicv3_common.h |  10 ++
>  7 files changed, 190 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
> index d63f8af604..4d19190b9c 100644
> --- a/hw/intc/arm_gicv3.c
> +++ b/hw/intc/arm_gicv3.c
> @@ -165,6 +165,15 @@ static void gicv3_redist_update_noirqset(GICv3CPUState 
> *cs)
>  cs->hppi.grp = gicv3_irq_group(cs->gic, cs, cs->hppi.irq);
>  }
>  
> +if (cs->gic->lpi_enable && cs->lpivalid) {
> +if (irqbetter(cs, cs->hpplpi.irq, cs->hpplpi.prio)) {
> +cs->hppi.irq = cs->hpplpi.irq;
> +cs->hppi.prio = cs->hpplpi.prio;
> +cs->hppi.grp = cs->hpplpi.grp;
> +seenbetter = true;
> +}
> +}
> +
>  /* If the best interrupt we just found would preempt whatever
>   * was the previous best interrupt before this update, then
>   * we know it's definitely the best one now.
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 53dea2a775..223db16fec 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -435,6 +435,7 @@ static void arm_gicv3_common_reset(DeviceState *dev)
>  memset(cs->gicr_ipriorityr, 0, sizeof(cs->gicr_ipriorityr));
>  
>  cs->hppi.prio = 0xff;
> +cs->hpplpi.prio = 0xff;
>  
>  /* State in the CPU interface must *not* be reset here, because it
>   * is part of the CPU's reset domain, not the GIC device's.
> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
> index 81f94c7f4a..5be3efaa3f 100644
> --- a/hw/intc/arm_gicv3_cpuif.c
> +++ b/hw/intc/arm_gicv3_cpuif.c
> @@ -898,10 +898,12 @@ static void icc_activate_irq(GICv3CPUState *cs, int irq)
>  cs->gicr_iactiver0 = deposit32(cs->gicr_iactiver0, irq, 1, 1);
>  cs->gicr_ipendr0 = deposit32(cs->gicr_ipendr0, irq, 1, 0);
>  gicv3_redist_update(cs);
> -} else {
> +} else if (irq < GICV3_LPI_INTID_START) {
>  gicv3_gicd_active_set(cs->gic, irq);
>  gicv3_gicd_pending_clear(cs->gic, irq);
>  gicv3_update(cs->gic, irq, 1);
> +} else {
> +gicv3_redist_lpi_pending(cs, irq, 0);
>  }
>  }
>  
> @@ -1317,7 +1319,8 @@ static void icc_eoir_write(CPUARMState *env, const 
> ARMCPRegInfo *ri,
>  trace_gicv3_icc_eoir_write(is_eoir0 ? 0 : 1,
> gicv3_redist_affid(cs), value);
>  
> -if (irq >= cs->gic->num_irq) {
> +if ((irq >= cs->gic->num_irq) &&  (!(cs->gic->lpi_enable &&
> +(irq >= GICV3_LPI_INTID_START {
>  /* This handles two cases:
>   * 1. If software writes the ID of a spurious interrupt [ie 
> 1020-1023]
>   * to the GICC_EOIR, the GIC ignores that write.
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> index 0a978cf55b..e0fbd4041f 100644
> --- a/hw/intc/arm_gicv3_its.c
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -211,6 +211,7 @@ static MemTxResult process_int(GICv3ITSState *s, uint64_t 
> value,
>  bool ite_valid = false;
>  uint64_t cte = 0;
>  bool cte_valid = false;
> +uint64_t rdbase;
>  uint64_t itel = 0;
>  uint32_t iteh = 0;
>  
> @@ -267,10 +268,15 @@ static MemTxResult process_int(GICv3ITSState *s, 
> uint64_t value,
>   * command in the queue
>   */
>  } else {
> -/*
> - * Current implementation only supports rdbase == procnum
> - * Hence rdbase physical address is ignored
> - */
> +rdbase = (cte >> 1U) & RDBASE_PROCNUM_MASK;
> +assert(rdbase <= s->gicv3->num_cpu);
> +
> +if ((cmd == CLEAR) || (cmd == DISCARD)) {
> +gicv3_redist_process_lpi(>gicv3->cpu[rdbase], pIntid, 0);
> +} else {
> +gicv3_redist_process_lpi(>gicv3->cpu[rdbase], pIntid, 1);
> +}
> +
>  if (cmd == DISCARD) {
>  /* remove mapping from interrupt translation table */
>  res = update_ite(s, eventid, dte, itel, iteh);
> diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
> index fb9a4ee3cc..bfc6e4e9b9 100644
> --- a/hw/intc/arm_gicv3_redist.c
> +++ 

Re: [PATCH v4 4/8] hw/intc: GICv3 ITS Command processing

2021-06-13 Thread Eric Auger
Hi Shashi,

On 6/2/21 8:00 PM, Shashi Mallela wrote:
> Added ITS command queue handling for MAPTI,MAPI commands,handled ITS
> translation which triggers an LPI via INT command as well as write
> to GITS_TRANSLATER register,defined enum to differentiate between ITS
> command interrupt trigger and GITS_TRANSLATER based interrupt trigger.
> Each of these commands make use of other functionalities implemented to
> get device table entry,collection table entry or interrupt translation
> table entry required for their processing.
> 
> Signed-off-by: Shashi Mallela 
> ---
>  hw/intc/arm_gicv3_its.c| 334 +
>  hw/intc/gicv3_internal.h   |  12 ++
>  include/hw/intc/arm_gicv3_common.h |   2 +
>  3 files changed, 348 insertions(+)
> 
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> index 6551c577b3..82bb5b84ef 100644
> --- a/hw/intc/arm_gicv3_its.c
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -28,6 +28,13 @@ struct GICv3ITSClass {
>  void (*parent_reset)(DeviceState *dev);
>  };
>  
> +typedef enum ItsCmdType {
> +NONE = 0, /* internal indication for GITS_TRANSLATER write */
> +CLEAR = 1,
> +DISCARD = 2,
> +INT = 3,
> +} ItsCmdType;
Add a comment to explain what this enum stand for. This sounds
misleading to me versus the command IDs. Why don't you use the cmd id
then and add NONE?
> +
>  static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz)
>  {
>  uint64_t result = 0;
> @@ -49,6 +56,315 @@ static uint64_t baser_base_addr(uint64_t value, uint32_t 
> page_sz)
>  return result;
>  }
>  
> +static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
> +MemTxResult *res)
> +{
> +AddressSpace *as = >gicv3->dma_as;
> +uint64_t l2t_addr;
> +uint64_t value;
> +bool valid_l2t;
> +uint32_t l2t_id;
> +uint32_t max_l2_entries;
> +bool status = false;
> +
> +if (s->ct.indirect) {
> +l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE);
> +
> +value = address_space_ldq_le(as,
> + s->ct.base_addr +
> + (l2t_id * L1TABLE_ENTRY_SIZE),
> + MEMTXATTRS_UNSPECIFIED, res);
> +
> +if (*res == MEMTX_OK) {
> +valid_l2t = (value >> VALID_SHIFT) & VALID_MASK;
> +
> +if (valid_l2t) {
> +max_l2_entries = s->ct.page_sz / s->ct.entry_sz;
> +
> +l2t_addr = value & ((1ULL << 51) - 1);
> +
> +*cte =  address_space_ldq_le(as, l2t_addr +
> +((icid % max_l2_entries) * 
> GITS_CTE_SIZE),
> +MEMTXATTRS_UNSPECIFIED, res);
> +   }
> +   }
> +} else {
> +/* Flat level table */
> +*cte =  address_space_ldq_le(as, s->ct.base_addr +
> + (icid * GITS_CTE_SIZE),
> +  MEMTXATTRS_UNSPECIFIED, res);
> +}
> +
> +if (*cte & VALID_MASK) {
> +status = true;
> +}
> +
> +return status;
> +}
> +
> +static MemTxResult update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t 
> dte,
> +  uint64_t itel, uint32_t iteh)
why not introducing an ite struct instead of the h/l args?
> +{
> +AddressSpace *as = >gicv3->dma_as;
> +uint64_t itt_addr;
> +MemTxResult res = MEMTX_OK;
> +
> +itt_addr = (dte >> 6ULL) & ITTADDR_MASK;
> +itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */
> +
> +address_space_stq_le(as, itt_addr + (eventid * sizeof(uint64_t)),
> + itel, MEMTXATTRS_UNSPECIFIED, );
> +
> +if (res == MEMTX_OK) {
> +address_space_stl_le(as, itt_addr + ((eventid + sizeof(uint64_t)) *
> + sizeof(uint32_t)), iteh, MEMTXATTRS_UNSPECIFIED,
> + );
> +}
> +   return res;
> +}
> +
> +static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
> +uint16_t *icid, uint32_t *pIntid, MemTxResult *res)
> +{
> +AddressSpace *as = >gicv3->dma_as;
> +uint64_t itt_addr;
> +bool status = false;
> +uint64_t itel = 0;
> +uint32_t iteh = 0;
> +
> +itt_addr = (dte >> 6ULL) & ITTADDR_MASK;
> +itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */
> +
> +itel = address_space_ldq_le(as, itt_addr + (eventid * sizeof(uint64_t)),
> +MEMTXATTRS_UNSPECIFIED, res);
> +
> +if (*res == MEMTX_OK) {
> +iteh = address_space_ldl_le(as, itt_addr + ((eventid +
> +sizeof(uint64_t)) * sizeof(uint32_t)),
> +MEMTXATTRS_UNSPECIFIED, res);
> +
> +if (*res == MEMTX_OK) {
> +if (itel & VALID_MASK) {
> +if ((itel >> ITE_ENTRY_INTTYPE_SHIFT) & GITS_TYPE_PHYSICAL) {
> +*pIntid = (itel >> ITE_ENTRY_INTID_SHIFT) 

Re: [PULL 00/34] tcg patch queue

2021-06-13 Thread Peter Maydell
On Sat, 12 Jun 2021 at 00:47, Richard Henderson
 wrote:
>
> This is mostly my code_gen_buffer cleanup, plus a few other random
> changes thrown in.  Including a fix for a recent float32_exp2 bug.
>
>
> r~
>
>
> The following changes since commit 894fc4fd670aaf04a67dc7507739f914ff4bacf2:
>
>   Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into 
> staging (2021-06-11 09:21:48 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210611
>
> for you to fetch changes up to 60afaddc208d34f6dc86dd974f6e02724fba6eb6:
>
>   docs/devel: Explain in more detail the TB chaining mechanisms (2021-06-11 
> 09:41:25 -0700)
>
> 
> Clean up code_gen_buffer allocation.
> Add tcg_remove_ops_after.
> Fix tcg_constant_* documentation.
> Improve TB chaining documentation.
> Fix float32_exp2.

Compile failure on OSX:

../../tcg/region.c:706:12: error: incompatible pointer to integer
conversion assigning to 'mach_vm_address_t' (aka 'unsigned long long')
from 'void *' [-Werror,-Wint-conversion]
buf_rw = region.start_aligned;
   ^ 
1 error generated.

thanks
-- PMM



Re: [PATCH] block: fix build waring

2021-06-13 Thread Peter Maydell
On Sun, 13 Jun 2021 at 15:20, Zhiwei Jiang  wrote:
>
> when i compile this file with some error message
> ../block.c: In function ‘bdrv_replace_node_common’:
> ../block.c:4903:9: error: ‘to_cow_parent’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>  bdrv_remove_filter_or_cow_child(to_cow_parent, tran);
>  ^~~~
> cc1: all warnings being treated as errors


Could you provide the compiler version when reporting
fails-to-compile issues, please? (This is useful for us
to get an idea of whether the problem is an old compiler
that's not smart enough to figure out that something's not
used uninitialized, or a new compiler that does more checking.)

thanks
-- PMM



Re: [PATCH v2] semihosting/arm-compat: remove heuristic softmmu SYS_HEAPINFO

2021-06-13 Thread Peter Maydell
On Fri, 11 Jun 2021 at 18:03, Alex Bennée  wrote:
>
>
> Peter Maydell  writes:
>
> > On Thu, 10 Jun 2021 at 15:16, Alex Bennée  wrote:
> >>
> >>
> >> Peter Maydell  writes:
> >> >  (2) find the largest contiguous extent of that RAM which
> >> >  is not covered by a ROM blob, by iterating through the
> >> >  ROM blob data. (This sounds like one of those slightly
> >> >  irritating but entirely tractable algorithms questions :-))
> >>
> >> Does that assume that any rom blob (so anything from -kernel, -pflash or
> >> -generic-loader?) will have also included space for guest data and bss?
> >
> > Yes; the elf loader code creates rom blobs whose rom->romsize
> > covers both initialized data from the ELF file and space to
> > be zeroed.
>
> Hmm I'm not seeing the RAM get bifurcated by the loader. The flatview
> only has one RAM block in my test case and it covers the whole of RAM.

I dunno what you're tracing here, but rom blobs do not affect
the MemoryRegion setup (which can be rom, ram,the romd "writes
go to callbacks, reads are host memory" hybrid, or whatever).
ROM blobs are just a list of "write this data into memory at
this address", which gets iterated through on reset to write
the data into the RAM/ROM/whatever.

-- PMM



Re: [PATCH v4 3/8] hw/intc: GICv3 ITS command queue framework

2021-06-13 Thread Eric Auger
Hi,

On 6/2/21 8:00 PM, Shashi Mallela wrote:
> Added functionality to trigger ITS command queue processing on
> write to CWRITE register and process each command queue entry to
> identify the command type and handle commands like MAPD,MAPC,SYNC.
> 
> Signed-off-by: Shashi Mallela 
> ---
>  hw/intc/arm_gicv3_its.c  | 295 +++
>  hw/intc/gicv3_internal.h |  37 +
>  2 files changed, 332 insertions(+)
> 
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> index af60f19c98..6551c577b3 100644
> --- a/hw/intc/arm_gicv3_its.c
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -49,6 +49,295 @@ static uint64_t baser_base_addr(uint64_t value, uint32_t 
> page_sz)
>  return result;
>  }
>  
> +static MemTxResult update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
> +  uint64_t rdbase)
> +{
> +AddressSpace *as = >gicv3->dma_as;
> +uint64_t value;
> +uint64_t l2t_addr;
> +bool valid_l2t;
> +uint32_t l2t_id;
> +uint32_t max_l2_entries;
> +uint64_t cte = 0;
> +MemTxResult res = MEMTX_OK;
> +
> +if (!s->ct.valid) {
> +return res;
> +}
> +
> +if (valid) {
> +/* add mapping entry to collection table */
> +cte = (valid & VALID_MASK) |
> +  ((rdbase & RDBASE_PROCNUM_MASK) << 1ULL);
> +}
> +
> +/*
> + * The specification defines the format of level 1 entries of a
> + * 2-level table, but the format of level 2 entries and the format
> + * of flat-mapped tables is IMPDEF.
> + */
> +if (s->ct.indirect) {
> +l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE);
> +
> +value = address_space_ldq_le(as,
> + s->ct.base_addr +
> + (l2t_id * L1TABLE_ENTRY_SIZE),
> + MEMTXATTRS_UNSPECIFIED, );
> +
> +if (res != MEMTX_OK) {
> +return res;
> +}
> +
> +valid_l2t = (value >> VALID_SHIFT) & VALID_MASK;
> +
> +if (valid_l2t) {
> +max_l2_entries = s->ct.page_sz / s->ct.entry_sz;
> +
> +l2t_addr = value & ((1ULL << 51) - 1);
> +
> +address_space_stq_le(as, l2t_addr +
> + ((icid % max_l2_entries) * GITS_CTE_SIZE),
> + cte, MEMTXATTRS_UNSPECIFIED, );
> +}
> +} else {
> +/* Flat level table */
> +address_space_stq_le(as, s->ct.base_addr + (icid * GITS_CTE_SIZE),
> + cte, MEMTXATTRS_UNSPECIFIED, );
> +}
> +return res;
> +}

Looking at your DevTableDesc and CollTableDesc types again, they are
basically the same except max_devids/max_collids. You may use a single
one and it may be possible to define helpers to access an entry in the
DT or CT.

update_cte/update_dte looks quite similar if you compute the cte and dte
externally and pass a pointer to the associated TableDesc?

Thanks

Eric


> +
> +static MemTxResult process_mapc(GICv3ITSState *s, uint32_t offset)
> +{
> +AddressSpace *as = >gicv3->dma_as;
> +uint16_t icid;
> +uint64_t rdbase;
> +bool valid;
> +MemTxResult res = MEMTX_OK;
> +uint64_t value;
> +
> +offset += NUM_BYTES_IN_DW;
> +offset += NUM_BYTES_IN_DW;
> +
> +value = address_space_ldq_le(as, s->cq.base_addr + offset,
> + MEMTXATTRS_UNSPECIFIED, );
> +
> +if (res != MEMTX_OK) {
> +return res;
> +}
> +
> +icid = value & ICID_MASK;
> +
> +rdbase = (value >> R_MAPC_RDBASE_SHIFT) & RDBASE_PROCNUM_MASK;
> +
> +valid = (value >> VALID_SHIFT) & VALID_MASK;
> +
> +if ((icid > s->ct.max_collids) || (rdbase > s->gicv3->num_cpu)) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "ITS MAPC: invalid collection table attributes "
> +  "icid %d rdbase %lu\n",  icid, rdbase);
> +/*
> + * in this implementation,in case of error
> + * we ignore this command and move onto the next
> + * command in the queue
> + */
> +} else {
> +res = update_cte(s, icid, valid, rdbase);
> +}
> +
> +return res;
> +}
> +
> +static MemTxResult update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
> +  uint8_t size, uint64_t itt_addr)
> +{
> +AddressSpace *as = >gicv3->dma_as;
> +uint64_t value;
> +uint64_t l2t_addr;
> +bool valid_l2t;
> +uint32_t l2t_id;
> +uint32_t max_l2_entries;
> +uint64_t dte = 0;
> +MemTxResult res = MEMTX_OK;
> +
> +if (s->dt.valid) {
> +if (valid) {
> +/* add mapping entry to device table */
> +dte = (valid & VALID_MASK) |
> +  ((size & SIZE_MASK) << 1U) |
> +  ((itt_addr & ITTADDR_MASK) << 6ULL);
> +}
> +} else {
> +return res;
> +}
> +
> +/*
> + * The specification defines the format of 

[PATCH v3 2/2] hw/char: sifive_uart

2021-06-13 Thread Lukas Jünger
QOMify sifive_uart model

Signed-off-by: Lukas Jünger 
---
 include/hw/char/sifive_uart.h |  11 ++--
 hw/char/sifive_uart.c | 114 +++---
 2 files changed, 109 insertions(+), 16 deletions(-)

diff --git a/include/hw/char/sifive_uart.h b/include/hw/char/sifive_uart.h
index 3e962be659..7f6c79f8bd 100644
--- a/include/hw/char/sifive_uart.h
+++ b/include/hw/char/sifive_uart.h
@@ -21,6 +21,7 @@
 #define HW_SIFIVE_UART_H
 
 #include "chardev/char-fe.h"
+#include "hw/qdev-properties.h"
 #include "hw/sysbus.h"
 #include "qom/object.h"
 
@@ -49,12 +50,10 @@ enum {
 
 #define SIFIVE_UART_GET_TXCNT(txctrl)   ((txctrl >> 16) & 0x7)
 #define SIFIVE_UART_GET_RXCNT(rxctrl)   ((rxctrl >> 16) & 0x7)
+#define SIFIVE_UART_RX_FIFO_SIZE 8
 
 #define TYPE_SIFIVE_UART "riscv.sifive.uart"
-
-typedef struct SiFiveUARTState SiFiveUARTState;
-DECLARE_INSTANCE_CHECKER(SiFiveUARTState, SIFIVE_UART,
- TYPE_SIFIVE_UART)
+OBJECT_DECLARE_SIMPLE_TYPE(SiFiveUARTState, SIFIVE_UART)
 
 struct SiFiveUARTState {
 /*< private >*/
@@ -64,8 +63,8 @@ struct SiFiveUARTState {
 qemu_irq irq;
 MemoryRegion mmio;
 CharBackend chr;
-uint8_t rx_fifo[8];
-unsigned int rx_fifo_len;
+uint8_t rx_fifo[SIFIVE_UART_RX_FIFO_SIZE];
+uint8_t rx_fifo_len;
 uint32_t ie;
 uint32_t ip;
 uint32_t txctrl;
diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
index 5df8212961..278e21c434 100644
--- a/hw/char/sifive_uart.c
+++ b/hw/char/sifive_uart.c
@@ -19,10 +19,12 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/log.h"
+#include "migration/vmstate.h"
 #include "chardev/char.h"
 #include "chardev/char-fe.h"
 #include "hw/irq.h"
 #include "hw/char/sifive_uart.h"
+#include "hw/qdev-properties-system.h"
 
 /*
  * Not yet implemented:
@@ -175,20 +177,112 @@ static int sifive_uart_be_change(void *opaque)
 return 0;
 }
 
+static Property sifive_uart_properties[] = {
+DEFINE_PROP_CHR("chardev", SiFiveUARTState, chr),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void sifive_uart_init(Object *obj)
+{
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+SiFiveUARTState *s = SIFIVE_UART(obj);
+
+memory_region_init_io(>mmio, OBJECT(s), _uart_ops, s,
+  TYPE_SIFIVE_UART, SIFIVE_UART_MAX);
+sysbus_init_mmio(sbd, >mmio);
+sysbus_init_irq(sbd, >irq);
+}
+
+static void sifive_uart_realize(DeviceState *dev, Error **errp)
+{
+SiFiveUARTState *s = SIFIVE_UART(dev);
+
+qemu_chr_fe_set_handlers(>chr, sifive_uart_can_rx, sifive_uart_rx,
+ sifive_uart_event, sifive_uart_be_change, s,
+ NULL, true);
+
+}
+
+static void sifive_uart_reset_enter(Object *obj, ResetType type)
+{
+SiFiveUARTState *s = SIFIVE_UART(obj);
+s->ie = 0;
+s->ip = 0;
+s->txctrl = 0;
+s->rxctrl = 0;
+s->div = 0;
+s->rx_fifo_len = 0;
+}
+
+static void sifive_uart_reset_hold(Object *obj)
+{
+SiFiveUARTState *s = SIFIVE_UART(obj);
+qemu_irq_lower(s->irq);
+}
+
+static const VMStateDescription vmstate_sifive_uart = {
+.name = TYPE_SIFIVE_UART,
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT8_ARRAY(rx_fifo, SiFiveUARTState,
+SIFIVE_UART_RX_FIFO_SIZE),
+VMSTATE_UINT8(rx_fifo_len, SiFiveUARTState),
+VMSTATE_UINT32(ie, SiFiveUARTState),
+VMSTATE_UINT32(ip, SiFiveUARTState),
+VMSTATE_UINT32(txctrl, SiFiveUARTState),
+VMSTATE_UINT32(rxctrl, SiFiveUARTState),
+VMSTATE_UINT32(div, SiFiveUARTState),
+VMSTATE_END_OF_LIST()
+},
+};
+
+
+static void sifive_uart_class_init(ObjectClass *oc, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(oc);
+ResettableClass *rc = RESETTABLE_CLASS(oc);
+
+dc->realize = sifive_uart_realize;
+dc->vmsd = _sifive_uart;
+rc->phases.enter = sifive_uart_reset_enter;
+rc->phases.hold  = sifive_uart_reset_hold;
+device_class_set_props(dc, sifive_uart_properties);
+}
+
+static const TypeInfo sifive_uart_info = {
+.name  = TYPE_SIFIVE_UART,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(SiFiveUARTState),
+.instance_init = sifive_uart_init,
+.class_init= sifive_uart_class_init,
+};
+
+static void sifive_uart_register_types(void)
+{
+type_register_static(_uart_info);
+}
+
+type_init(sifive_uart_register_types)
+
 /*
  * Create UART device.
  */
 SiFiveUARTState *sifive_uart_create(MemoryRegion *address_space, hwaddr base,
 Chardev *chr, qemu_irq irq)
 {
-SiFiveUARTState *s = g_malloc0(sizeof(SiFiveUARTState));
-s->irq = irq;
-qemu_chr_fe_init(>chr, chr, _abort);
-qemu_chr_fe_set_handlers(>chr, sifive_uart_can_rx, sifive_uart_rx,
- sifive_uart_event, sifive_uart_be_change, s,
- NULL, true);
-memory_region_init_io(>mmio, NULL, _uart_ops, s,
-  

[Qemu-devel] GSoC Introduction

2021-06-13 Thread Lara Lazier
Hi everyone!

My name is Lara, and I am one of this year's GSoC students. I am studying
computer science in Zürich, and I will start my master's in September. For
my GSoC project, I am working with Paolo Bonzini on fixing and extending
the SVM implementation in QEMU.

I am very excited to learn many new things this summer and to get to know
this community.

Best
Lara


[PATCH] block: fix build waring

2021-06-13 Thread Zhiwei Jiang
when i compile this file with some error message
../block.c: In function ‘bdrv_replace_node_common’:
../block.c:4903:9: error: ‘to_cow_parent’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
 bdrv_remove_filter_or_cow_child(to_cow_parent, tran);
 ^~~~
cc1: all warnings being treated as errors

Signed-off-by: Zhiwei Jiang 
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 3f456892d0..08f29e6b65 100644
--- a/block.c
+++ b/block.c
@@ -4866,7 +4866,7 @@ static int bdrv_replace_node_common(BlockDriverState 
*from,
 Transaction *tran = tran_new();
 g_autoptr(GHashTable) found = NULL;
 g_autoptr(GSList) refresh_list = NULL;
-BlockDriverState *to_cow_parent;
+BlockDriverState *to_cow_parent = NULL;
 int ret;
 
 if (detach_subchain) {
-- 
2.25.1




[PATCH v3 0/2] QOMify Sifive UART Model

2021-06-13 Thread Lukas Jünger
Hello,

I have cleaned up the indentation as requested by Philippe.
Also I added reset and migration capabilities as requested by Peter.

Thank you for your feedback.
I hope this is satisfactory now.

Best regards,
Lukas

Lukas Jünger (2):
  hw/char: sifive_uart
  hw/char: sifive_uart

 include/hw/char/sifive_uart.h |  11 ++-
 hw/char/sifive_uart.c | 152 +++---
 2 files changed, 129 insertions(+), 34 deletions(-)

-- 
2.31.1




Re: [PATCH v4 3/8] hw/intc: GICv3 ITS command queue framework

2021-06-13 Thread Eric Auger
Hi Sashi,

On 6/2/21 8:00 PM, Shashi Mallela wrote:
> Added functionality to trigger ITS command queue processing on
> write to CWRITE register and process each command queue entry to
> identify the command type and handle commands like MAPD,MAPC,SYNC.
> 
> Signed-off-by: Shashi Mallela 
> ---
>  hw/intc/arm_gicv3_its.c  | 295 +++
>  hw/intc/gicv3_internal.h |  37 +
>  2 files changed, 332 insertions(+)
> 
> diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> index af60f19c98..6551c577b3 100644
> --- a/hw/intc/arm_gicv3_its.c
> +++ b/hw/intc/arm_gicv3_its.c
> @@ -49,6 +49,295 @@ static uint64_t baser_base_addr(uint64_t value, uint32_t 
> page_sz)
>  return result;
>  }
>  
> +static MemTxResult update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
> +  uint64_t rdbase)
> +{
> +AddressSpace *as = >gicv3->dma_as;
> +uint64_t value;
> +uint64_t l2t_addr;
> +bool valid_l2t;
> +uint32_t l2t_id;
> +uint32_t max_l2_entries;
> +uint64_t cte = 0;
> +MemTxResult res = MEMTX_OK;
> +
> +if (!s->ct.valid) {
Isn't it a guest log error case. Also you return MEMTX_OK in that case.
Is that what you want?
> +return res;
> +}
> +
> +if (valid) {
> +/* add mapping entry to collection table */
> +cte = (valid & VALID_MASK) |
> +  ((rdbase & RDBASE_PROCNUM_MASK) << 1ULL);
Do you really need to sanitize rdbase again?
> +}
> +
> +/*
> + * The specification defines the format of level 1 entries of a
> + * 2-level table, but the format of level 2 entries and the format
> + * of flat-mapped tables is IMPDEF.
> + */
> +if (s->ct.indirect) {
> +l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE);
> +
> +value = address_space_ldq_le(as,
> + s->ct.base_addr +
> + (l2t_id * L1TABLE_ENTRY_SIZE),
> + MEMTXATTRS_UNSPECIFIED, );
> +
> +if (res != MEMTX_OK) {
> +return res;
> +}
> +
> +valid_l2t = (value >> VALID_SHIFT) & VALID_MASK;
> +
> +if (valid_l2t) {
> +max_l2_entries = s->ct.page_sz / s->ct.entry_sz;
> +
> +l2t_addr = value & ((1ULL << 51) - 1);
> +
> +address_space_stq_le(as, l2t_addr +
> + ((icid % max_l2_entries) * GITS_CTE_SIZE),
> + cte, MEMTXATTRS_UNSPECIFIED, );
> +}
> +} else {
> +/* Flat level table */
> +address_space_stq_le(as, s->ct.base_addr + (icid * GITS_CTE_SIZE),
> + cte, MEMTXATTRS_UNSPECIFIED, );
> +}
> +return res;
> +}
> +
> +static MemTxResult process_mapc(GICv3ITSState *s, uint32_t offset)
> +{
> +AddressSpace *as = >gicv3->dma_as;
> +uint16_t icid;
> +uint64_t rdbase;
> +bool valid;
> +MemTxResult res = MEMTX_OK;
> +uint64_t value;
> +
> +offset += NUM_BYTES_IN_DW;
> +offset += NUM_BYTES_IN_DW;
May be relevant to add some trace points for debuggability.
> +
> +value = address_space_ldq_le(as, s->cq.base_addr + offset,
> + MEMTXATTRS_UNSPECIFIED, );
> +
> +if (res != MEMTX_OK) {
> +return res;
> +}
> +
> +icid = value & ICID_MASK;
> +
> +rdbase = (value >> R_MAPC_RDBASE_SHIFT) & RDBASE_PROCNUM_MASK;
usually the mask is applied before the shift.
> +
> +valid = (value >> VALID_SHIFT) & VALID_MASK;
use FIELD, see below
> +
> +if ((icid > s->ct.max_collids) || (rdbase > s->gicv3->num_cpu)) {
you also need to check against ITS_CIDBITS limit?
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "ITS MAPC: invalid collection table attributes "
> +  "icid %d rdbase %lu\n",  icid, rdbase);
> +/*
> + * in this implementation,in case of error
> + * we ignore this command and move onto the next
> + * command in the queue
spec says a command error occurs in that case.
> + */
> +} else {
> +res = update_cte(s, icid, valid, rdbase);
> +}
> +
> +return res;
> +}
> +
> +static MemTxResult update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
> +  uint8_t size, uint64_t itt_addr)
> +{
> +AddressSpace *as = >gicv3->dma_as;
> +uint64_t value;
> +uint64_t l2t_addr;
> +bool valid_l2t;
> +uint32_t l2t_id;
> +uint32_t max_l2_entries;
> +uint64_t dte = 0;
> +MemTxResult res = MEMTX_OK;
> +
> +if (s->dt.valid) {
> +if (valid) {
> +/* add mapping entry to device table */
> +dte = (valid & VALID_MASK) |
> +  ((size & SIZE_MASK) << 1U) |
> +  ((itt_addr & ITTADDR_MASK) << 6ULL);
> +}
> +} else {
> +return res;
> +}
> +
> +/*
> + * The specification defines the format of 

[PATCH v3 1/2] hw/char: sifive_uart

2021-06-13 Thread Lukas Jünger
Make function names consistent

Signed-off-by: Lukas Jünger 
---
 hw/char/sifive_uart.c | 46 ++-
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
index fe12666789..5df8212961 100644
--- a/hw/char/sifive_uart.c
+++ b/hw/char/sifive_uart.c
@@ -31,7 +31,7 @@
  */
 
 /* Returns the state of the IP (interrupt pending) register */
-static uint64_t uart_ip(SiFiveUARTState *s)
+static uint64_t sifive_uart_ip(SiFiveUARTState *s)
 {
 uint64_t ret = 0;
 
@@ -48,7 +48,7 @@ static uint64_t uart_ip(SiFiveUARTState *s)
 return ret;
 }
 
-static void update_irq(SiFiveUARTState *s)
+static void sifive_uart_update_irq(SiFiveUARTState *s)
 {
 int cond = 0;
 if ((s->ie & SIFIVE_UART_IE_TXWM) ||
@@ -63,7 +63,7 @@ static void update_irq(SiFiveUARTState *s)
 }
 
 static uint64_t
-uart_read(void *opaque, hwaddr addr, unsigned int size)
+sifive_uart_read(void *opaque, hwaddr addr, unsigned int size)
 {
 SiFiveUARTState *s = opaque;
 unsigned char r;
@@ -74,7 +74,7 @@ uart_read(void *opaque, hwaddr addr, unsigned int size)
 memmove(s->rx_fifo, s->rx_fifo + 1, s->rx_fifo_len - 1);
 s->rx_fifo_len--;
 qemu_chr_fe_accept_input(>chr);
-update_irq(s);
+sifive_uart_update_irq(s);
 return r;
 }
 return 0x8000;
@@ -84,7 +84,7 @@ uart_read(void *opaque, hwaddr addr, unsigned int size)
 case SIFIVE_UART_IE:
 return s->ie;
 case SIFIVE_UART_IP:
-return uart_ip(s);
+return sifive_uart_ip(s);
 case SIFIVE_UART_TXCTRL:
 return s->txctrl;
 case SIFIVE_UART_RXCTRL:
@@ -99,8 +99,8 @@ uart_read(void *opaque, hwaddr addr, unsigned int size)
 }
 
 static void
-uart_write(void *opaque, hwaddr addr,
-   uint64_t val64, unsigned int size)
+sifive_uart_write(void *opaque, hwaddr addr,
+  uint64_t val64, unsigned int size)
 {
 SiFiveUARTState *s = opaque;
 uint32_t value = val64;
@@ -109,11 +109,11 @@ uart_write(void *opaque, hwaddr addr,
 switch (addr) {
 case SIFIVE_UART_TXFIFO:
 qemu_chr_fe_write(>chr, , 1);
-update_irq(s);
+sifive_uart_update_irq(s);
 return;
 case SIFIVE_UART_IE:
 s->ie = val64;
-update_irq(s);
+sifive_uart_update_irq(s);
 return;
 case SIFIVE_UART_TXCTRL:
 s->txctrl = val64;
@@ -129,9 +129,9 @@ uart_write(void *opaque, hwaddr addr,
   __func__, (int)addr, (int)value);
 }
 
-static const MemoryRegionOps uart_ops = {
-.read = uart_read,
-.write = uart_write,
+static const MemoryRegionOps sifive_uart_ops = {
+.read = sifive_uart_read,
+.write = sifive_uart_write,
 .endianness = DEVICE_NATIVE_ENDIAN,
 .valid = {
 .min_access_size = 4,
@@ -139,7 +139,7 @@ static const MemoryRegionOps uart_ops = {
 }
 };
 
-static void uart_rx(void *opaque, const uint8_t *buf, int size)
+static void sifive_uart_rx(void *opaque, const uint8_t *buf, int size)
 {
 SiFiveUARTState *s = opaque;
 
@@ -150,26 +150,27 @@ static void uart_rx(void *opaque, const uint8_t *buf, int 
size)
 }
 s->rx_fifo[s->rx_fifo_len++] = *buf;
 
-update_irq(s);
+sifive_uart_update_irq(s);
 }
 
-static int uart_can_rx(void *opaque)
+static int sifive_uart_can_rx(void *opaque)
 {
 SiFiveUARTState *s = opaque;
 
 return s->rx_fifo_len < sizeof(s->rx_fifo);
 }
 
-static void uart_event(void *opaque, QEMUChrEvent event)
+static void sifive_uart_event(void *opaque, QEMUChrEvent event)
 {
 }
 
-static int uart_be_change(void *opaque)
+static int sifive_uart_be_change(void *opaque)
 {
 SiFiveUARTState *s = opaque;
 
-qemu_chr_fe_set_handlers(>chr, uart_can_rx, uart_rx, uart_event,
-uart_be_change, s, NULL, true);
+qemu_chr_fe_set_handlers(>chr, sifive_uart_can_rx, sifive_uart_rx,
+ sifive_uart_event, sifive_uart_be_change, s,
+ NULL, true);
 
 return 0;
 }
@@ -183,9 +184,10 @@ SiFiveUARTState *sifive_uart_create(MemoryRegion 
*address_space, hwaddr base,
 SiFiveUARTState *s = g_malloc0(sizeof(SiFiveUARTState));
 s->irq = irq;
 qemu_chr_fe_init(>chr, chr, _abort);
-qemu_chr_fe_set_handlers(>chr, uart_can_rx, uart_rx, uart_event,
-uart_be_change, s, NULL, true);
-memory_region_init_io(>mmio, NULL, _ops, s,
+qemu_chr_fe_set_handlers(>chr, sifive_uart_can_rx, sifive_uart_rx,
+ sifive_uart_event, sifive_uart_be_change, s,
+ NULL, true);
+memory_region_init_io(>mmio, NULL, _uart_ops, s,
   TYPE_SIFIVE_UART, SIFIVE_UART_MAX);
 memory_region_add_subregion(address_space, base, >mmio);
 return s;
-- 
2.31.1




Re: tb_flush() calls causing long Windows XP boot times

2021-06-13 Thread Mark Cave-Ayland

On 11/06/2021 19:22, Alex Bennée wrote:

(added Gitlab on CC)


Paolo Bonzini  writes:


On 11/06/21 17:01, Programmingkid wrote:

Hello Alex,
The good news is the source code to Windows XP is available
online:https://github.com/cryptoAlgorithm/nt5src


It's leaked, so I doubt anybody who's paid to work on Linux or QEMU
would touch that with a ten-foot pole.


Indeed.

Anyway what the OP could do is run QEMU with gdb and -d nochain and
stick a breakpoint (sic) in breakpoint_invalidate. Then each time it
hits you can examine the backtrace to cpu_loop_exec_tb and collect the
data from tb->pc. Then you will have a bunch of addresses in Windows
that keep triggering the behaviour. You can then re-run with -dfilter
and -d in_asm,cpu to get some sort of idea of what Windows is up to.


I have been able to recreate this locally using my WinXP and it looks like during 
boot WinXP goes into a tight loop where it writes and clears a set of breakpoints via 
writes to DB7 which is what causes the very slow boot time.


Once boot proceeds further into the login screen, the same code seems to called 
periodically once every second or so which has less of a performance impact.


For testing I applied the following diff:

diff --git a/cpu.c b/cpu.c
index 164fefeaa3..3155d935f1 100644
--- a/cpu.c
+++ b/cpu.c
@@ -252,13 +252,7 @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, 
MemTxAttrs attrs)


 static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
 {
-/*
- * There may not be a virtual to physical translation for the pc
- * right now, but there may exist cached TB for this pc.
- * Flush the whole TB cache to force re-translation of such TBs.
- * This is heavyweight, but we're debugging anyway.
- */
-tb_flush(cpu);
+fprintf(stderr, "# bpi @ 0x%lx\n", pc);
 }
 #endif

diff --git a/target/i386/tcg/sysemu/bpt_helper.c 
b/target/i386/tcg/sysemu/bpt_helper.c
index 9bdf7e170b..37ee49fd56 100644
--- a/target/i386/tcg/sysemu/bpt_helper.c
+++ b/target/i386/tcg/sysemu/bpt_helper.c
@@ -61,6 +61,7 @@ static int hw_breakpoint_insert(CPUX86State *env, int index)
 switch (hw_breakpoint_type(dr7, index)) {
 case DR7_TYPE_BP_INST:
 if (hw_breakpoint_enabled(dr7, index)) {
+fprintf(stderr, "### dp7 add bp inst @ 0x%lx, index %d\n", 
env->eip, index);
 err = cpu_breakpoint_insert(cs, drN, BP_CPU,
 >cpu_breakpoint[index]);
 }
@@ -102,6 +103,7 @@ static void hw_breakpoint_remove(CPUX86State *env, int 
index)
 switch (hw_breakpoint_type(env->dr[7], index)) {
 case DR7_TYPE_BP_INST:
 if (env->cpu_breakpoint[index]) {
+fprintf(stderr, "### dp7 remove bp inst @ 0x%lx, index %d\n", env->eip, 
index);

 cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]);
 env->cpu_breakpoint[index] = NULL;
 }


This gives a repeated set of outputs like this:

# bpi @ 0x90
### dp7 add bp inst @ 0x8053cab8, index 1
# bpi @ 0xa4
### dp7 add bp inst @ 0x8053cab8, index 2
# bpi @ 0xff
### dp7 add bp inst @ 0x8053cab8, index 3
# bpi @ 0xf
### dp7 remove bp inst @ 0x8053f58a, index 0
# bpi @ 0x90
### dp7 remove bp inst @ 0x8053f58a, index 1
# bpi @ 0xa4
### dp7 remove bp inst @ 0x8053f58a, index 2
# bpi @ 0xff
### dp7 remove bp inst @ 0x8053f58a, index 3
...
...
### dp7 add bp inst @ 0x8053c960, index 0
# bpi @ 0x90
### dp7 add bp inst @ 0x8053c960, index 1
# bpi @ 0xa4
### dp7 add bp inst @ 0x8053c960, index 2
# bpi @ 0xff
### dp7 add bp inst @ 0x8053c960, index 3
# bpi @ 0xf
### dp7 remove bp inst @ 0x8053c730, index 0
# bpi @ 0x90
### dp7 remove bp inst @ 0x8053c730, index 1
# bpi @ 0xa4
### dp7 remove bp inst @ 0x8053c730, index 2
# bpi @ 0xff
### dp7 remove bp inst @ 0x8053c730, index 3
...
...

From a vanilla XP install the 2 main sections of code which alter the breakpoint 
registers are at 0x8053cab8 (enable) and 0x8053f58a (disable):



-d in_asm output when enabling breakpoints
==


IN:
0x8053ca92:  33 dbxorl %ebx, %ebx
0x8053ca94:  8b 75 18 movl 0x18(%ebp), %esi
0x8053ca97:  8b 7d 1c movl 0x1c(%ebp), %edi
0x8053ca9a:  0f 23 fb movl %ebx, %dr7


IN:
0x8053ca9d:  0f 23 c6 movl %esi, %dr0


IN:
0x8053caa0:  8b 5d 20 movl 0x20(%ebp), %ebx
0x8053caa3:  0f 23 cf movl %edi, %dr1


IN:
0x8053caa6:  0f 23 d3 movl %ebx, %dr2


IN:
0x8053caa9:  8b 75 24 movl 0x24(%ebp), %esi
0x8053caac:  8b 7d 28 movl 0x28(%ebp), %edi
0x8053caaf:  8b 5d 2c movl 0x2c(%ebp), %ebx
0x8053cab2:  0f 23 de movl %esi, %dr3


IN:
0x8053cab5:  0f 23 f7 

Re: [PATCH] Fix assertion failure in lsi53c810 emulator

2021-06-13 Thread Qiang Liu
Hi Phil,

> You didn't Cc'ed the maintainers of the SCSI subsystem (see
> https://wiki.qemu.org/Contribute/SubmitAPatch#CC_the_relevant_maintainer
> ) so I'm doing it for you:
Thank you!

> It seems you didn't send your patch with the proper tool, see
> https://wiki.qemu.org/Contribute/SubmitAPatch#Use_git_format-patch
>
> Please also mention the reporter:
>
>   Reported-by: Cheolwoo Myung 
>
> Also your git-config is missing your name. Fixable using:
>
>   $ git config user.name "Liu Cyrus"
>
> for your QEMU repository, or globally:
>
>   $ git config --global user.name "Liu Cyrus"
Thank you again. I'm sorry that I do miss several basic settings.
I will do better next time.

> Instead of duplicating multiple times the same complex code, you could
> add a helper once and call it.
This is really a good idea.

> However back to the bug you are trying to fix, I wonder if your check
> is correct regarding the hardware we are modeling. Have you looked
> at the specifications? See https://docs.broadcom.com/doc/12352475
> Chapter 5.3 Block Move Instruction (from SCSI SCRIPTS Instruction Set),
> "DMA Command" register.
To be honest, I didn't check the specification at that time. I formed this patch
following the idea that we could discard an invalid MMIO operation to
avoid crashing.
Does it seem that this idea is not enough to form a good patch? What are
the best practices to fix an assertion failure in QEMU?

> Why are we reaching these places with s->current == NULL in the first
> place? We are probably missing something earlier.
I checked the specification for several hours today, but it is too
difficult for me.
I need more time to understand it and form a better patch.

When reproducing the crash, I found that I didn't prepare any script to be
executed by lsi_execute_script. However, `insn = read_dword(s, s->dsp)` did get
an instruction at `s->dsp`. Maybe I should check this more.

Best,
Qiang



Re: [PATCH] virtio-gpu: move scanout_id sanity check

2021-06-13 Thread Li Qiang
Gerd Hoffmann  于2021年6月4日周五 下午3:50写道:
>
> Checking scanout_id in virtio_gpu_do_set_scanout() is too late, for the
> "resource_id == 0" case (aka disable scanout) the scanout_id is used
> unchecked.  Move the check into the callers to fix that.
>
> Fixes: e64d4b6a9bc3 ("virtio-gpu: Refactor virtio_gpu_set_scanout")
> Fixes: 32db3c63ae11 ("virtio-gpu: Add virtio_gpu_set_scanout_blob")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/383
> Reported-by: Alexander Bulekov 
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Li Qiang 

> ---
>  hw/display/virtio-gpu.c | 20 ++--
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 4d549377cbc1..e183f4ecdaa5 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -610,12 +610,6 @@ static void virtio_gpu_do_set_scanout(VirtIOGPU *g,
>  struct virtio_gpu_scanout *scanout;
>  uint8_t *data;
>
> -if (scanout_id >= g->parent_obj.conf.max_outputs) {
> -qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified %d",
> -  __func__, scanout_id);
> -*error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
> -return;
> -}
>  scanout = >parent_obj.scanout[scanout_id];
>
>  if (r->x > fb->width ||
> @@ -694,6 +688,13 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
>  trace_virtio_gpu_cmd_set_scanout(ss.scanout_id, ss.resource_id,
>   ss.r.width, ss.r.height, ss.r.x, 
> ss.r.y);
>
> +if (ss.scanout_id >= g->parent_obj.conf.max_outputs) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified %d",
> +  __func__, ss.scanout_id);
> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
> +return;
> +}
> +
>  if (ss.resource_id == 0) {
>  virtio_gpu_disable_scanout(g, ss.scanout_id);
>  return;
> @@ -730,6 +731,13 @@ static void virtio_gpu_set_scanout_blob(VirtIOGPU *g,
>ss.r.width, ss.r.height, ss.r.x,
>ss.r.y);
>
> +if (ss.scanout_id >= g->parent_obj.conf.max_outputs) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified %d",
> +  __func__, ss.scanout_id);
> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
> +return;
> +}
> +
>  if (ss.resource_id == 0) {
>  virtio_gpu_disable_scanout(g, ss.scanout_id);
>  return;
> --
> 2.31.1
>
>



Re: [PATCH v2] fuzz: Display hexadecimal value with '0x' prefix

2021-06-13 Thread Laurent Vivier
Le 12/06/2021 à 21:58, Philippe Mathieu-Daudé a écrit :
> Use memory_region_size() to get the MemoryRegion size,
> and display it with the '0x' prefix.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> v2:
> - Use PRIx64 (lvivier)
> - Use memory_region_size()
> ---
>  tests/qtest/fuzz/generic_fuzz.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index cea7d4058e8..6c675227174 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -841,9 +841,9 @@ static void generic_pre_fuzz(QTestState *s)
>  
>  g_hash_table_iter_init(, fuzzable_memoryregions);
>  while (g_hash_table_iter_next(, (gpointer), NULL)) {
> -printf("  * %s (size %lx)\n",
> +printf("  * %s (size 0x%" PRIx64 ")\n",
> object_get_canonical_path_component(&(mr->parent_obj)),
> -   (uint64_t)mr->size);
> +   memory_region_size(mr));
>  }
>  
>  if (!g_hash_table_size(fuzzable_memoryregions)) {
> 

Reviewed-by: Laurent Vivier 



Re: [PATCH] esp: store lun coming from the MESSAGE OUT phase

2021-06-13 Thread Mark Cave-Ayland

On 11/06/2021 12:57, Paolo Bonzini wrote:


The LUN is selected with an IDENTIFY message, and persists
until the next message out phase.  Instead of passing it to
do_busid_cmd, store it in ESPState.  Because do_cmd can simply
skip the message out phase if cmdfifo_cdb_offset is zero, it
can now be used for the S without ATN cases as well.

Signed-off-by: Paolo Bonzini 
---
  hw/scsi/esp.c | 39 +++
  hw/scsi/trace-events  |  3 ++-
  include/hw/scsi/esp.h |  1 +
  3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 3e6f4094fc..1f1c02de79 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -221,7 +221,7 @@ static int esp_select(ESPState *s)
  
  /*

   * Note that we deliberately don't raise the IRQ here: this will be done
- * either in do_busid_cmd() for DATA OUT transfers or by the deferred
+ * either in do_command_phase() for DATA OUT transfers or by the deferred
   * IRQ mechanism in esp_transfer_data() for DATA IN transfers
   */
  s->rregs[ESP_RINTR] |= INTR_FC;
@@ -272,24 +272,22 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
  return dmalen;
  }
  
-static void do_busid_cmd(ESPState *s, uint8_t busid)

+static void do_command_phase(ESPState *s)
  {
  uint32_t cmdlen;
  int32_t datalen;
-int lun;
  SCSIDevice *current_lun;
  uint8_t buf[ESP_CMDFIFO_SZ];
  
-trace_esp_do_busid_cmd(busid);

-lun = busid & 7;
+trace_esp_do_command_phase(s->lun);
  cmdlen = fifo8_num_used(>cmdfifo);
  if (!cmdlen || !s->current_dev) {
  return;
  }
  esp_fifo_pop_buf(>cmdfifo, buf, cmdlen);
  
-current_lun = scsi_device_find(>bus, 0, s->current_dev->id, lun);

-s->current_req = scsi_req_new(current_lun, 0, lun, buf, s);
+current_lun = scsi_device_find(>bus, 0, s->current_dev->id, s->lun);
+s->current_req = scsi_req_new(current_lun, 0, s->lun, buf, s);
  datalen = scsi_req_enqueue(s->current_req);
  s->ti_size = datalen;
  fifo8_reset(>cmdfifo);
@@ -316,21 +314,29 @@ static void do_busid_cmd(ESPState *s, uint8_t busid)
  }
  }
  
-static void do_cmd(ESPState *s)

+static void do_message_phase(ESPState *s)
  {
-uint8_t busid = esp_fifo_pop(>cmdfifo);
-int len;
+if (s->cmdfifo_cdb_offset) {
+uint8_t message = esp_fifo_pop(>cmdfifo);
  
-s->cmdfifo_cdb_offset--;

+trace_esp_do_identify(message);
+s->lun = message & 7;
+s->cmdfifo_cdb_offset--;
+}
  
  /* Ignore extended messages for now */

  if (s->cmdfifo_cdb_offset) {
-len = MIN(s->cmdfifo_cdb_offset, fifo8_num_used(>cmdfifo));
+int len = MIN(s->cmdfifo_cdb_offset, fifo8_num_used(>cmdfifo));
  esp_fifo_pop_buf(>cmdfifo, NULL, len);
  s->cmdfifo_cdb_offset = 0;
  }
+}
  
-do_busid_cmd(s, busid);

+static void do_cmd(ESPState *s)
+{
+do_message_phase(s);
+assert(s->cmdfifo_cdb_offset == 0);
+do_command_phase(s);
  }
  
  static void satn_pdma_cb(ESPState *s)

@@ -369,7 +375,7 @@ static void s_without_satn_pdma_cb(ESPState *s)
  if (!esp_get_tc(s) && !fifo8_is_empty(>cmdfifo)) {
  s->cmdfifo_cdb_offset = 0;
  s->do_cmd = 0;
-do_busid_cmd(s, 0);
+do_cmd(s);
  }
  }
  
@@ -386,7 +392,7 @@ static void handle_s_without_atn(ESPState *s)

  if (cmdlen > 0) {
  s->cmdfifo_cdb_offset = 0;
  s->do_cmd = 0;
-do_busid_cmd(s, 0);
+do_cmd(s);
  } else if (cmdlen == 0) {
  s->do_cmd = 1;
  /* Target present, but no cmd yet - switch to command phase */
@@ -1168,7 +1174,7 @@ static int esp_post_load(void *opaque, int version_id)
  
  const VMStateDescription vmstate_esp = {

  .name = "esp",
-.version_id = 5,
+.version_id = 6,
  .minimum_version_id = 3,
  .post_load = esp_post_load,
  .fields = (VMStateField[]) {
@@ -1197,6 +1203,7 @@ const VMStateDescription vmstate_esp = {
  VMSTATE_FIFO8_TEST(fifo, ESPState, esp_is_version_5),
  VMSTATE_FIFO8_TEST(cmdfifo, ESPState, esp_is_version_5),
  VMSTATE_UINT8_TEST(ti_cmd, ESPState, esp_is_version_5),
+VMSTATE_UINT8_V(lun, ESPState, 6),
  VMSTATE_END_OF_LIST()
  },
  };
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index 1a27e141ae..92d5b40f89 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -166,7 +166,8 @@ esp_dma_disable(void) "Lower enable"
  esp_pdma_read(int size) "pDMA read %u bytes"
  esp_pdma_write(int size) "pDMA write %u bytes"
  esp_get_cmd(uint32_t dmalen, int target) "len %d target %d"
-esp_do_busid_cmd(uint8_t busid) "busid 0x%x"
+esp_do_command_phase(uint8_t busid) "busid 0x%x"
+esp_do_identify(uint8_t byte) "0x%x"
  esp_handle_satn_stop(uint32_t cmdlen) "cmdlen %d"
  esp_write_response(uint32_t status) "Transfer status (status=%d)"
  esp_do_dma(uint32_t cmdlen, uint32_t len) "command len %d + %d"
diff --git 

[PATCH] esp: fix migration version check in esp_is_version_5()

2021-06-13 Thread Mark Cave-Ayland
Commit 4e78f3bf35 "esp: defer command completion interrupt on incoming data
transfers" added a version check for use with VMSTATE_*_TEST macros to allow
migration from older QEMU versions. Unfortunately the version check fails to
work in its current form since if the VMStateDescription version_id is
incremented, the test returns false and so the fields are not included in the
outgoing migration stream.

Change the version check to use >= rather == to ensure that migration works
correctly when the ESPState VMStateDescription has version_id > 5.

Signed-off-by: Mark Cave-Ayland 
Fixes: 4e78f3bf35 ("esp: defer command completion interrupt on incoming data 
transfers")
---
 hw/scsi/esp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index bfdb94292b..39756ddd99 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -1120,7 +1120,7 @@ static bool esp_is_version_5(void *opaque, int version_id)
 ESPState *s = ESP(opaque);
 
 version_id = MIN(version_id, s->mig_version_id);
-return version_id == 5;
+return version_id >= 5;
 }
 
 int esp_pre_save(void *opaque)
-- 
2.20.1




Re: [RFC PATCH 03/11] hw/intc: Add CLIC device

2021-06-13 Thread Frank Chang
LIU Zhiwei  於 2021年4月9日 週五 下午3:57寫道:

> The Core-Local Interrupt Controller (CLIC) provides low-latency,
> vectored, pre-emptive interrupts for RISC-V systems.
>
> The CLIC also supports a new Selective Hardware Vectoring feature
> that allow users to optimize each interrupt for either faster
> response or smaller code size.
>
> Signed-off-by: LIU Zhiwei 
> ---
>  default-configs/devices/riscv32-softmmu.mak |   1 +
>  default-configs/devices/riscv64-softmmu.mak |   1 +
>  hw/intc/Kconfig |   3 +
>  hw/intc/meson.build |   1 +
>  hw/intc/riscv_clic.c| 835 
>  include/hw/intc/riscv_clic.h| 103 +++
>  target/riscv/cpu.h  |   2 +
>  7 files changed, 946 insertions(+)
>  create mode 100644 hw/intc/riscv_clic.c
>  create mode 100644 include/hw/intc/riscv_clic.h
>
> diff --git a/default-configs/devices/riscv32-softmmu.mak
> b/default-configs/devices/riscv32-softmmu.mak
> index d847bd5692..1430c30588 100644
> --- a/default-configs/devices/riscv32-softmmu.mak
> +++ b/default-configs/devices/riscv32-softmmu.mak
> @@ -5,6 +5,7 @@
>  #CONFIG_PCI_DEVICES=n
>  CONFIG_SEMIHOSTING=y
>  CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> +CONFIG_RISCV_CLIC=y
>
>  # Boards:
>  #
> diff --git a/default-configs/devices/riscv64-softmmu.mak
> b/default-configs/devices/riscv64-softmmu.mak
> index d5eec75f05..396800bbbd 100644
> --- a/default-configs/devices/riscv64-softmmu.mak
> +++ b/default-configs/devices/riscv64-softmmu.mak
> @@ -5,6 +5,7 @@
>  #CONFIG_PCI_DEVICES=n
>  CONFIG_SEMIHOSTING=y
>  CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> +CONFIG_RISCV_CLIC=y
>
>  # Boards:
>  #
> diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
> index f4694088a4..5bf492b48f 100644
> --- a/hw/intc/Kconfig
> +++ b/hw/intc/Kconfig
> @@ -68,6 +68,9 @@ config SIFIVE_CLINT
>  config SIFIVE_PLIC
>  bool
>
> +config RISCV_CLIC
> +bool
> +
>  config GOLDFISH_PIC
>  bool
>
> diff --git a/hw/intc/meson.build b/hw/intc/meson.build
> index 1c299039f6..2aa71b6738 100644
> --- a/hw/intc/meson.build
> +++ b/hw/intc/meson.build
> @@ -50,6 +50,7 @@ specific_ss.add(when: 'CONFIG_S390_FLIC_KVM', if_true:
> files('s390_flic_kvm.c'))
>  specific_ss.add(when: 'CONFIG_SH_INTC', if_true: files('sh_intc.c'))
>  specific_ss.add(when: 'CONFIG_SIFIVE_CLINT', if_true:
> files('sifive_clint.c'))
>  specific_ss.add(when: 'CONFIG_SIFIVE_PLIC', if_true:
> files('sifive_plic.c'))
> +specific_ss.add(when: 'CONFIG_RISCV_CLIC', if_true: files('riscv_clic.c'))
>  specific_ss.add(when: 'CONFIG_XICS', if_true: files('xics.c'))
>  specific_ss.add(when: ['CONFIG_KVM', 'CONFIG_XICS'],
> if_true: files('xics_kvm.c'))
> diff --git a/hw/intc/riscv_clic.c b/hw/intc/riscv_clic.c
> new file mode 100644
> index 00..8ad534c506
> --- /dev/null
> +++ b/hw/intc/riscv_clic.c
> @@ -0,0 +1,835 @@
> +/*
> + * RISC-V CLIC(Core Local Interrupt Controller) for QEMU.
> + *
> + * Copyright (c) 2021 T-Head Semiconductor Co., Ltd. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> along with
> + * this program.  If not, see .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu/log.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/qtest.h"
> +#include "target/riscv/cpu.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/intc/riscv_clic.h"
> +
> +/*
> + * The 2-bit trig WARL field specifies the trigger type and polarity for
> each
> + * interrupt input. Bit 1, trig[0], is defined as "edge-triggered"
> + * (0: level-triggered, 1: edge-triggered); while bit 2, trig[1], is
> defined as
> + * "negative-edge" (0: positive-edge, 1: negative-edge). (Section 3.6)
> + */
> +
> +static inline TRIG_TYPE
> +riscv_clic_get_trigger_type(RISCVCLICState *clic, size_t irq_offset)
> +{
> +return (clic->clicintattr[irq_offset] >> 1) & 0x3;
> +}
> +
> +static inline bool
> +riscv_clic_is_edge_triggered(RISCVCLICState *clic, size_t irq_offset)
> +{
> +return (clic->clicintattr[irq_offset] >> 1) & 0x1;
> +}
> +
> +static inline bool
> +riscv_clic_is_shv_interrupt(RISCVCLICState *clic, size_t irq_offset)
> +{
> +return (clic->clicintattr[irq_offset] & 0x1) && clic->nvbits;
> +}
> +
> +static uint8_t
> +riscv_clic_get_interrupt_level(RISCVCLICState *clic, uint8_t intctl)
> +{
> +int nlbits = clic->nlbits;
> +
> +uint8_t mask_il = ((1 << nlbits) - 1) << (8 - nlbits);
> +

Re: [gitlab] Renamed issue labels for target architecture

2021-06-13 Thread Stefan Weil

Am 13.06.21 um 00:32 schrieb Richard Henderson:

I've renamed arch:* to target:* as there was some amount of confusion 
as to what "arch" really meant without context.  I've removed labels 
for lm32 and unicore32 which have been removed from qemu 6.1.  I've 
added a label for hexagon.


I have not yet added labels for host architecture, because I couldn't 
figure out how best to word the description, or even if all of the 
target:* labels need re-wording to emphasize target.


And then there's the special case of TCI.

Thoughts on these?



A pragmatic solution for TCI could use the label "accel: TCI" as a 
special case and instead of "accel: TCG".


We have an ambiguity for "os:" because it is unclear whether it relates 
to the host or to the target system. That could be handled by using four 
labels "host:", "target:" (architecture), "host-os:", "target-os:" 
(operating system). I'd prefer dropping the "os:" label and extending 
"target:" (and the new "host:") to allow either architecture, operating 
system or a combination of both (for example target: i386, target: 
i386-Windows, host: Windows).


Stefan







Re: [PATCH] meson.build: Support ncurses on MacOS

2021-06-13 Thread Stefan Weil

Am 13.06.21 um 03:40 schrieb Brad Smith:


This same problem also applies to OpenBSD as we have the same
version of ncurses with support for wide characters. I have a similar
patch in our QEMU port.



Then we should either extend the conditional statement to handle OpenBSD 
as well, or simply define both macros unconditionally:


    # Newer versions of curses use NCURSES_WIDECHAR.
    # Older versions (e. g. on MacOS, OpenBSD) still require 
_XOPEN_SOURCE_EXTENDED.
    curses_compile_args = ['-DNCURSES_WIDECHAR=1', 
'-D_XOPEN_SOURCE_EXTENDED=1']


Defining only _XOPEN_SOURCE_EXTENDED would also work with old and new 
versions, so that's another option.


Stefan





Re: [PATCH v2] fuzz: Display hexadecimal value with '0x' prefix

2021-06-13 Thread Thomas Huth

On 12/06/2021 21.58, Philippe Mathieu-Daudé wrote:

Use memory_region_size() to get the MemoryRegion size,
and display it with the '0x' prefix.

Signed-off-by: Philippe Mathieu-Daudé 
---
v2:
- Use PRIx64 (lvivier)
- Use memory_region_size()
---
  tests/qtest/fuzz/generic_fuzz.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index cea7d4058e8..6c675227174 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -841,9 +841,9 @@ static void generic_pre_fuzz(QTestState *s)
  
  g_hash_table_iter_init(, fuzzable_memoryregions);

  while (g_hash_table_iter_next(, (gpointer), NULL)) {
-printf("  * %s (size %lx)\n",
+printf("  * %s (size 0x%" PRIx64 ")\n",
 object_get_canonical_path_component(&(mr->parent_obj)),
-   (uint64_t)mr->size);
+   memory_region_size(mr));
  }
  
  if (!g_hash_table_size(fuzzable_memoryregions)) {




Reviewed-by: Thomas Huth