[Bug 1916394] Re: [git] Cannot build qemu: FAILED: target/hexagon/semantics_generated.pyinc

[Frederic Bezies]

I looked at it, and there seems to be no difference. Looks like my
installation is rotten.

Let's close this bug as invalid.

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

Title:
  [git] Cannot build qemu: FAILED:
  target/hexagon/semantics_generated.pyinc

Status in QEMU:
  New

Bug description:
  Hello.

  I'm using Archlinux and I maintain qemu-git AUR package.

  I tried to build Qemu at commit
  4115aec9af2a3de5fa89a0b1daa12debcd7741ff but it stops with this error
  message:

  Found ninja-1.10.2 at /usr/bin/ninja
  [632/9068] Generating semantics_generated.pyinc with a custom command
  FAILED: target/hexagon/semantics_generated.pyinc
  @INPUT@ target/hexagon/semantics_generated.pyinc
  /bin/sh: line 1: @INPUT@: command not found
  [637/9068] Compiling C object 
fsdev/vi...proxy-helper.p/virtfs-proxy-helper.c.o
  ninja: build stopped: subcommand failed.

  ninja version: 1.10.2
  meson version: 0.57.1

  Downgrading meson doesn't change anything.

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

Re: [PATCH v4 03/28] cpu: Introduce cpu_virtio_is_big_endian()

[Greg Kurz]

On Wed, 3 Mar 2021 17:08:32 -0500
"Michael S. Tsirkin"  wrote:

> On Wed, Mar 03, 2021 at 10:46:43PM +0100, Philippe Mathieu-Daudé wrote:
> > Introduce the cpu_virtio_is_big_endian() generic helper to avoid
> > calling CPUClass internal virtio_is_big_endian() one.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé 
> 
> Using virtio in the name here probably because virtio wants this?
> That doesn't sound like a good naming strategy, name should
> tell us what function does not how it's used.
> 

I tend to agree but there was a consensus to deliberately put
virtio in the name when this was first introduced, so that
nobody else ever try to use it, as recorded in the commit log.

commit bf7663c4bd8f8f619d6dbb5780025d92ace250a8
Author: Greg Kurz 
Date:   Tue Jun 24 19:33:21 2014 +0200

cpu: introduce CPUClass::virtio_is_big_endian()

If we want to support targets that can change endianness (modern PPC and
ARM for the moment), we need to add a per-CPU class method to be called
from the virtio code. The virtio_ prefix in the name is a hint for people
to avoid misusage (aka. anywhere but from the virtio code).

The default behaviour is to return the compile-time default target
endianness.

Suggested-by: Peter Maydell 
Signed-off-by: Greg Kurz 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 

Is there something new on this front ? I'm not convinced that anything
but legacy virtio en POWER (or any other target that can change endian
at runtime) needs this. The next step I see for this is_big_endian()
stuff is deprecation and removal. In the meantime, I think we should
keep the virtio wording to prevent additional users for this.

> > ---
> >  include/hw/core/cpu.h | 9 +
> >  hw/core/cpu.c | 8 ++--
> >  hw/virtio/virtio.c| 4 +---
> >  3 files changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 2d43f78819f..b12028c3c03 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -602,6 +602,15 @@ hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr 
> > addr);
> >   */
> >  int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs);
> >  
> > +/**
> > + * cpu_virtio_is_big_endian:
> > + * @cpu: CPU
> > +
> > + * Returns %true if a CPU which supports runtime configurable endianness
> > + * is currently big-endian.
> > + */
> > +bool cpu_virtio_is_big_endian(CPUState *cpu);
> > +
> >  #endif /* CONFIG_USER_ONLY */
> >  
> >  /**
> > diff --git a/hw/core/cpu.c b/hw/core/cpu.c
> > index 4dce35f832f..daaff56a79e 100644
> > --- a/hw/core/cpu.c
> > +++ b/hw/core/cpu.c
> > @@ -218,8 +218,13 @@ static int cpu_common_gdb_write_register(CPUState 
> > *cpu, uint8_t *buf, int reg)
> >  return 0;
> >  }
> >  
> > -static bool cpu_common_virtio_is_big_endian(CPUState *cpu)
> > +bool cpu_virtio_is_big_endian(CPUState *cpu)
> >  {
> > +CPUClass *cc = CPU_GET_CLASS(cpu);
> > +
> > +if (cc->virtio_is_big_endian) {
> > +return cc->virtio_is_big_endian(cpu);
> > +}
> >  return target_words_bigendian();
> >  }
> >  
> > @@ -438,7 +443,6 @@ static void cpu_class_init(ObjectClass *klass, void 
> > *data)
> >  k->write_elf64_note = cpu_common_write_elf64_note;
> >  k->gdb_read_register = cpu_common_gdb_read_register;
> >  k->gdb_write_register = cpu_common_gdb_write_register;
> > -k->virtio_is_big_endian = cpu_common_virtio_is_big_endian;
> >  set_bit(DEVICE_CATEGORY_CPU, dc->categories);
> >  dc->realize = cpu_common_realizefn;
> >  dc->unrealize = cpu_common_unrealizefn;
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 1fd1917ca0f..fe6a4be99e4 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -1973,9 +1973,7 @@ static enum virtio_device_endian 
> > virtio_default_endian(void)
> >  
> >  static enum virtio_device_endian virtio_current_cpu_endian(void)
> >  {
> > -CPUClass *cc = CPU_GET_CLASS(current_cpu);
> > -
> > -if (cc->virtio_is_big_endian(current_cpu)) {
> > +if (cpu_virtio_is_big_endian(current_cpu)) {
> >  return VIRTIO_DEVICE_ENDIAN_BIG;
> >  } else {
> >  return VIRTIO_DEVICE_ENDIAN_LITTLE;
> > -- 
> > 2.26.2
> 
> 

Re: [RFC PATCH 4/5] hw/acpi/aml-build: add processor hierarchy node structure

[Ying Fang]

On 3/1/2021 11:50 PM, Michael S. Tsirkin wrote:

On Mon, Mar 01, 2021 at 10:39:19AM +0100, Andrew Jones wrote:

On Fri, Feb 26, 2021 at 10:23:03AM +0800, Ying Fang wrote:



On 2/25/2021 7:47 PM, Andrew Jones wrote:

On Thu, Feb 25, 2021 at 04:56:26PM +0800, Ying Fang wrote:

Add the processor hierarchy node structures to build ACPI information
for CPU topology. Since the private resources may be used to describe
cache hierarchy and it is variable among different topology level,
three helpers are introduced to describe the hierarchy.

(1) build_socket_hierarchy for socket description
(2) build_processor_hierarchy for processor description
(3) build_smt_hierarchy for thread (logic processor) description

Signed-off-by: Ying Fang 
Signed-off-by: Henglong Fan 
---
   hw/acpi/aml-build.c | 40 +
   include/hw/acpi/acpi-defs.h | 13 
   include/hw/acpi/aml-build.h |  7 +++
   3 files changed, 60 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index a2cd7a5830..a0af3e9d73 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1888,6 +1888,46 @@ void build_slit(GArray *table_data, BIOSLinker *linker, 
MachineState *ms,
table_data->len - slit_start, 1, oem_id, oem_table_id);
   }
+/*
+ * ACPI 6.3: 5.2.29.1 Processor hierarchy node structure (Type 0)
+ */
+void build_socket_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, ACPI_PPTT_TYPE_PROCESSOR); /* Type 0 - processor */
+build_append_byte(tbl, 20); /* Length, no private resources */
+build_append_int_noprefix(tbl, 0, 2);  /* Reserved */
+build_append_int_noprefix(tbl, ACPI_PPTT_PHYSICAL_PACKAGE, 4);


Missing '/* Flags */'


Will fix.




+build_append_int_noprefix(tbl, parent, 4); /* Parent */
+build_append_int_noprefix(tbl, id, 4); /* ACPI processor ID */
+build_append_int_noprefix(tbl, 0, 4);  /* Number of private resources */
+}
+
+void build_processor_hierarchy(GArray *tbl, uint32_t flags,
+   uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, ACPI_PPTT_TYPE_PROCESSOR);  /* Type 0 - processor */
+build_append_byte(tbl, 20); /* Length, no private resources */
+build_append_int_noprefix(tbl, 0, 2);  /* Reserved */
+build_append_int_noprefix(tbl, flags, 4);  /* Flags */
+build_append_int_noprefix(tbl, parent, 4); /* Parent */
+build_append_int_noprefix(tbl, id, 4); /* ACPI processor ID */
+build_append_int_noprefix(tbl, 0, 4);  /* Number of private resources */
+}
+
+void build_thread_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, ACPI_PPTT_TYPE_PROCESSOR); /* Type 0 - processor */
+build_append_byte(tbl, 20);   /* Length, no private resources */
+build_append_int_noprefix(tbl, 0, 2); /* Reserved */
+build_append_int_noprefix(tbl,
+  ACPI_PPTT_ACPI_PROCESSOR_ID_VALID |
+  ACPI_PPTT_ACPI_PROCESSOR_IS_THREAD |
+  ACPI_PPTT_ACPI_LEAF_NODE, 4);  /* Flags */
+build_append_int_noprefix(tbl, parent , 4); /* parent */


'parent' not capitalized. We want these comments to exactly match the text
in the spec.


Will fix.




+build_append_int_noprefix(tbl, id, 4);  /* ACPI processor ID */
+build_append_int_noprefix(tbl, 0, 4);   /* Num of private resources */
+}
+
   /* build rev1/rev3/rev5.1 FADT */
   void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
   const char *oem_id, const char *oem_table_id)
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index cf9f44299c..45e10d886f 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -618,4 +618,17 @@ struct AcpiIortRC {
   } QEMU_PACKED;
   typedef struct AcpiIortRC AcpiIortRC;
+enum {
+ACPI_PPTT_TYPE_PROCESSOR = 0,
+ACPI_PPTT_TYPE_CACHE,
+ACPI_PPTT_TYPE_ID,
+ACPI_PPTT_TYPE_RESERVED
+};
+
+#define ACPI_PPTT_PHYSICAL_PACKAGE  (1)
+#define ACPI_PPTT_ACPI_PROCESSOR_ID_VALID   (1 << 1)
+#define ACPI_PPTT_ACPI_PROCESSOR_IS_THREAD  (1 << 2)  /* ACPI 6.3 */
+#define ACPI_PPTT_ACPI_LEAF_NODE(1 << 3)  /* ACPI 6.3 */
+#define ACPI_PPTT_ACPI_IDENTICAL(1 << 4)  /* ACPI 6.3 */


You need to quote specific place in spec where this appeared, not
just version. and what about previous ones?


Thanks, Will fix.





+
   #endif
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 380d3e3924..7f0ca1a198 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -462,6 +462,13 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, 
uint64_t base,
   void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms,
   const char *oem_id, const char *oem_table_id);
+void build_socket_hierarchy(GArray *tbl, uint32_t paren

Re: [PATCH 02/19] target/s390x/kvm: Simplify debug code

[Thomas Huth]

On 03/03/2021 19.22, Philippe Mathieu-Daudé wrote:

We already have the 'run' variable holding 'cs->kvm_run' value.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/s390x/kvm.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 7a892d663df..73f816a7222 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1785,8 +1785,7 @@ static int handle_intercept(S390CPU *cpu)
  int icpt_code = run->s390_sieic.icptcode;
  int r = 0;
  
-DPRINTF("intercept: 0x%x (at 0x%lx)\n", icpt_code,

-(long)cs->kvm_run->psw_addr);
+DPRINTF("intercept: 0x%x (at 0x%lx)\n", icpt_code, (long)run->psw_addr);
  switch (icpt_code) {
  case ICPT_INSTRUCTION:
  case ICPT_PV_INSTR:



Reviewed-by: Thomas Huth 

Re: [PATCH 4/7] target/s390x: Move s390_cpu_has_work to excp_helper.c

[Thomas Huth]

On 02/03/2021 11.27, Philippe Mathieu-Daudé wrote:

We will restrict the s390_cpu_has_work() function to TCG.
First declare it in "internal.h" and move it to excp_helper.c.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/s390x/internal.h|  1 +
  target/s390x/cpu.c | 17 -
  target/s390x/excp_helper.c | 18 ++
  3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 11515bb6173..7184e38631c 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -263,6 +263,7 @@ ObjectClass *s390_cpu_class_by_name(const char *name);
  
  
  /* excp_helper.c */

+bool s390_cpu_has_work(CPUState *cs);
  void s390x_cpu_debug_excp_handler(CPUState *cs);
  void s390_cpu_do_interrupt(CPUState *cpu);
  bool s390_cpu_exec_interrupt(CPUState *cpu, int int_req);
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index d35eb39a1bb..91142db1097 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -56,23 +56,6 @@ static void s390_cpu_set_pc(CPUState *cs, vaddr value)
  cpu->env.psw.addr = value;
  }
  
-static bool s390_cpu_has_work(CPUState *cs)

-{
-S390CPU *cpu = S390_CPU(cs);
-
-/* STOPPED cpus can never wake up */
-if (s390_cpu_get_state(cpu) != S390_CPU_STATE_LOAD &&
-s390_cpu_get_state(cpu) != S390_CPU_STATE_OPERATING) {
-return false;
-}
-
-if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
-return false;
-}
-
-return s390_cpu_has_int(cpu);
-}
-
  #if !defined(CONFIG_USER_ONLY)
  /* S390CPUClass::load_normal() */
  static void s390_cpu_load_normal(CPUState *s)
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index ce16af394b1..64923ffb83a 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -28,12 +28,30 @@
  #include "hw/s390x/ioinst.h"
  #include "exec/address-spaces.h"
  #include "tcg_s390x.h"
+#include "qapi/qapi-types-machine.h"
  #ifndef CONFIG_USER_ONLY
  #include "sysemu/sysemu.h"
  #include "hw/s390x/s390_flic.h"
  #include "hw/boards.h"
  #endif
  
+bool s390_cpu_has_work(CPUState *cs)

+{
+S390CPU *cpu = S390_CPU(cs);
+
+/* STOPPED cpus can never wake up */
+if (s390_cpu_get_state(cpu) != S390_CPU_STATE_LOAD &&
+s390_cpu_get_state(cpu) != S390_CPU_STATE_OPERATING) {
+return false;
+}
+
+if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
+return false;
+}
+
+return s390_cpu_has_int(cpu);
+}
+
  void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env,
uint32_t code, uintptr_t ra)
  {



Reviewed-by: Thomas Huth 

Re: [RFC PATCH 6/7] cpu: Move CPUClass::has_work() to TCGCPUOps

[Thomas Huth]

On 02/03/2021 11.27, Philippe Mathieu-Daudé wrote:

We can only check if a vCPU has work with TCG.
Restrict the has_work() handler to TCG by moving it to
the TCGCPUOps structure, and adapt all the targets.

Signed-off-by: Philippe Mathieu-Daudé 
---
RFC: PPC target incomplete
---

[...]

diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 00330ba07de..3110867c3a3 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -261,11 +261,6 @@ static void cpu_common_reset(DeviceState *dev)
  }
  }
  
-static bool cpu_common_has_work(CPUState *cs)

-{
-return false;
-}
-
  ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
  {
  CPUClass *cc = CPU_CLASS(object_class_by_name(typename));
@@ -397,7 +392,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
  
  k->parse_features = cpu_common_parse_features;

  k->get_arch_id = cpu_common_get_arch_id;
-k->has_work = cpu_common_has_work;
  k->get_paging_enabled = cpu_common_get_paging_enabled;
  k->get_memory_mapping = cpu_common_get_memory_mapping;
  k->write_elf32_qemunote = cpu_common_write_elf32_qemunote;


cpu_common_has_work() is gone without replacement? Can you be sure that the 
pointer in tcg_ops is always initialized? If so, could you please add a 
comment to the patch description?


 Thomas

[Bug 1916394] Re: [git] Cannot build qemu: FAILED: target/hexagon/semantics_generated.pyinc

[briancain]

docker_arch_qemu_build.log.xz shows the output from the docker build via
arch linux

** Attachment added: "docker_arch_qemu_build.log.xz"
   
https://bugs.launchpad.net/qemu/+bug/1916394/+attachment/5472326/+files/docker_arch_qemu_build.log.xz

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

Title:
  [git] Cannot build qemu: FAILED:
  target/hexagon/semantics_generated.pyinc

Status in QEMU:
  New

Bug description:
  Hello.

  I'm using Archlinux and I maintain qemu-git AUR package.

  I tried to build Qemu at commit
  4115aec9af2a3de5fa89a0b1daa12debcd7741ff but it stops with this error
  message:

  Found ninja-1.10.2 at /usr/bin/ninja
  [632/9068] Generating semantics_generated.pyinc with a custom command
  FAILED: target/hexagon/semantics_generated.pyinc
  @INPUT@ target/hexagon/semantics_generated.pyinc
  /bin/sh: line 1: @INPUT@: command not found
  [637/9068] Compiling C object 
fsdev/vi...proxy-helper.p/virtfs-proxy-helper.c.o
  ninja: build stopped: subcommand failed.

  ninja version: 1.10.2
  meson version: 0.57.1

  Downgrading meson doesn't change anything.

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

[Bug 1916394] Re: [git] Cannot build qemu: FAILED: target/hexagon/semantics_generated.pyinc

[briancain]

Still having trouble reproducing the issue.

I don't have an arch system so I used Docker.  Would you be willing to
check whether the Dockerfile represents a close enough match to your
build process?

Also, if you can think of anything particularly unique about your
configuration, maybe I can try to come closer to reproducing this with
some of those critical factors.

** Attachment added: "Dockerfile"
   
https://bugs.launchpad.net/qemu/+bug/1916394/+attachment/5472325/+files/Dockerfile

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

Title:
  [git] Cannot build qemu: FAILED:
  target/hexagon/semantics_generated.pyinc

Status in QEMU:
  New

Bug description:
  Hello.

  I'm using Archlinux and I maintain qemu-git AUR package.

  I tried to build Qemu at commit
  4115aec9af2a3de5fa89a0b1daa12debcd7741ff but it stops with this error
  message:

  Found ninja-1.10.2 at /usr/bin/ninja
  [632/9068] Generating semantics_generated.pyinc with a custom command
  FAILED: target/hexagon/semantics_generated.pyinc
  @INPUT@ target/hexagon/semantics_generated.pyinc
  /bin/sh: line 1: @INPUT@: command not found
  [637/9068] Compiling C object 
fsdev/vi...proxy-helper.p/virtfs-proxy-helper.c.o
  ninja: build stopped: subcommand failed.

  ninja version: 1.10.2
  meson version: 0.57.1

  Downgrading meson doesn't change anything.

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

Re: [PATCH v3 5/5] tests/fp/fp-test: Replace the word 'blacklist'

[Thomas Huth]

On 03/03/2021 19.46, Philippe Mathieu-Daudé wrote:

Follow the inclusive terminology from the "Conscious Language in your
Open Source Projects" guidelines [*] and replace the word "blacklist"
appropriately.

[*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md

Acked-by: Alex Bennée 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Philippe Mathieu-Daudé 
---
  tests/fp/fp-test.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c
index 06ffebd6db1..5a4cad8c8b2 100644
--- a/tests/fp/fp-test.c
+++ b/tests/fp/fp-test.c
@@ -123,7 +123,7 @@ static void not_implemented(void)
  fprintf(stderr, "Not implemented.\n");
  }
  
-static bool blacklisted(unsigned op, int rmode)

+static bool is_allowed(unsigned op, int rmode)
  {
  /* odd has not been implemented for any 80-bit ops */
  if (rmode == softfloat_round_odd) {
@@ -161,10 +161,10 @@ static bool blacklisted(unsigned op, int rmode)
  case F32_TO_EXTF80:
  case F64_TO_EXTF80:
  case F128_TO_EXTF80:
-return true;
+return false;
  }
  }
-return false;
+return true;
  }
  
  static void do_testfloat(int op, int rmode, bool exact)

@@ -194,7 +194,7 @@ static void do_testfloat(int op, int rmode, bool exact)
  verCases_writeFunctionName(stderr);
  fputs("\n", stderr);
  
-if (blacklisted(op, rmode)) {

+if (!is_allowed(op, rmode)) {
  not_implemented();
  return;
  }



Reviewed-by: Thomas Huth 

Re: [PATCH v3 4/5] qemu-options: Replace the word 'blacklist'

[Thomas Huth]

On 03/03/2021 19.46, Philippe Mathieu-Daudé wrote:

Follow the inclusive terminology from the "Conscious Language in your
Open Source Projects" guidelines [*] and replace the word "blacklist"
appropriately.

[*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md

Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Philippe Mathieu-Daudé 
---
  qemu-options.hx | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 252db9357ca..8462dc5f158 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4299,12 +4299,12 @@ DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
  "use 'obsolete' to allow obsolete system calls that are 
provided\n" \
  "by the kernel, but typically no longer used by 
modern\n" \
  "C library implementations.\n" \
-"use 'elevateprivileges' to allow or deny QEMU process to 
elevate\n" \
-"its privileges by blacklisting all set*uid|gid system 
calls.\n" \
+"use 'elevateprivileges' to allow or deny the QEMU process 
ability\n" \
+"to elevate privileges using set*uid|gid system 
calls.\n" \
  "The value 'children' will deny set*uid|gid system calls 
for\n" \
  "main QEMU process but will allow forks and execves to run 
unprivileged\n" \
  "use 'spawn' to avoid QEMU to spawn new threads or processes 
by\n" \
-" blacklisting *fork and execve\n" \
+" blocking *fork and execve\n" \
  "use 'resourcecontrol' to disable process affinity and 
schedular priority\n",
  QEMU_ARCH_ALL)
  SRST



Reviewed-by: Thomas Huth 

Re: [PATCH v3 3/5] seccomp: Replace the word 'blacklist'

[Thomas Huth]

On 03/03/2021 19.46, Philippe Mathieu-Daudé wrote:

Follow the inclusive terminology from the "Conscious Language in your
Open Source Projects" guidelines [*] and replace the word "blacklist"
appropriately.

[*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md

Reviewed-by: Daniel P. Berrangé 
Acked-by: Eduardo Otubo 
Reviewed-by: Alex Bennée 
Signed-off-by: Philippe Mathieu-Daudé 
---
v3: Reworded comment (thuth)
---
  softmmu/qemu-seccomp.c | 16 
  1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/softmmu/qemu-seccomp.c b/softmmu/qemu-seccomp.c
index 377ef6937ca..9c29d9cf007 100644
--- a/softmmu/qemu-seccomp.c
+++ b/softmmu/qemu-seccomp.c
@@ -45,8 +45,8 @@ const struct scmp_arg_cmp sched_setscheduler_arg[] = {
  { .arg = 1, .op = SCMP_CMP_NE, .datum_a = SCHED_IDLE }
  };
  
-static const struct QemuSeccompSyscall blacklist[] = {

-/* default set of syscalls to blacklist */
+static const struct QemuSeccompSyscall denylist[] = {
+/* default set of syscalls that should get blocked */
  { SCMP_SYS(reboot), QEMU_SECCOMP_SET_DEFAULT },
  { SCMP_SYS(swapon), QEMU_SECCOMP_SET_DEFAULT },
  { SCMP_SYS(swapoff),QEMU_SECCOMP_SET_DEFAULT },
@@ -175,18 +175,18 @@ static int seccomp_start(uint32_t seccomp_opts, Error 
**errp)
  goto seccomp_return;
  }
  
-for (i = 0; i < ARRAY_SIZE(blacklist); i++) {

+for (i = 0; i < ARRAY_SIZE(denylist); i++) {
  uint32_t action;
-if (!(seccomp_opts & blacklist[i].set)) {
+if (!(seccomp_opts & denylist[i].set)) {
  continue;
  }
  
-action = qemu_seccomp_get_action(blacklist[i].set);

-rc = seccomp_rule_add_array(ctx, action, blacklist[i].num,
-blacklist[i].narg, blacklist[i].arg_cmp);
+action = qemu_seccomp_get_action(denylist[i].set);
+rc = seccomp_rule_add_array(ctx, action, denylist[i].num,
+denylist[i].narg, denylist[i].arg_cmp);
  if (rc < 0) {
  error_setg_errno(errp, -rc,
- "failed to add seccomp blacklist rules");
+ "failed to add seccomp denylist rules");
  goto seccomp_return;
  }
  }



Reviewed-by: Thomas Huth 

USB port claiming / set configuration problems

[Ben Leslie]

I have encountered a number of devices (mostly mobile phones) which seem to
get very confused if a "SET CONFIGURATION" control transfer (for the same
interface) is performed twice.

Specifically, after receiving a 2nd SET CONFIGURATION (for the same
interface) the device times out on future bulk output transfers. (Sometimes
PING-NAK continues until the overall transfer times out, in other cases the
PING itself timeouts after ~200 PING-NAKs).

While I'm fairly confident in saying that the USB firmware on these devices
is broken they seem to work enough to be operable when running a native
operating system.

Unfortunately when running the same operating system virtualized under Qemu
we are able to trigger this bug.

This was originally found using an older (4.2) version of Qemu. It seems
like that the patch bfe44898848614cfcb3a269bc965afbe1f0f331c was able to
solve the issue for some of the devices we see. Specifically, this avoids
actually performing a SET CONFIGURATION control transfer if there is only a
single configuration. The commit message "Seems some devices become
confused when we call libusb_set_configuration()." seems to confirm some of
the behaviour we have been seeing.

Unfortunately, while this appears to have solved the issue for devices with
a single configuration we still appear to have problems when hitting
devices with multiple configurations (which is not surprising given that
the commit only changed behaviour for single-configuration devices).

To attempt a work-around and validate the theory I change the
`usb_host_set_config` function (in host-libusb.c) such that it first checks
if the current active configuration matches the request configuration, and
if so skips performing the actual SET CONFIGURATION control transfer.

Would a patch of this nature be the right approach?
Perhaps this check could replace the number of configurations check?

Taking a step back here, the larger problem is that Linux host performs
various control transfers prior to qemu (and therefore the guest operating
system) gaining control of the device.
This means the sequence of control transfers with the device is inherently
going to be different when the guest OS is virtualized as compared to
running natively. For well behaving devices this really shouldn't matter,
but not all devices are well behaving!

USBDEVFS has support for `USBDEVFS_CLAIM_PORT` (and
`USBDEVFS_RELEASE_PORT`) ioctls. From the definition this seem designed to
limit the interaction that Linux kernel might have with a device on a
claimed port, which seems perfect for this use case. This in fact used in
previous version of qemu if we go back to the host-linux.c days, but with
the change over to host-libusb.c this functionality was lost.

Was this intentional? Would adding support to host-libusb to use these
ioctl to claim the port be beneficial? Based on a simple test program and
hardware USB traces for a device connected to a 'claimed' port the kernel
does indeed leave the device in an unconfigured state. (Although it still
performs some basic control transfers to gather descriptor, and strangely
seems to in this case make an explicit SET CONFIGURATION transfer, but sets
configuration to zero, rather than an actual configuration, which, at least
for the devices I was able to test with, avoided the problems of calling
SET CONFIGURATION (1) twice). Integrating this support back into
host-libusb.c is a little more involved than the work around described
above, so I'd appreciate any feedback before going down that path.

Thanks,

Ben

[PATCH v3 2/2] gitlab-ci.yml: Add jobs to test CFI flags

[Daniele Buono]

QEMU has had options to enable control-flow integrity features
for a few months now. Add two sets of build/check/acceptance
jobs to ensure the binary produced is working fine.

The three sets allow testing of x86_64 binaries for x86_64, s390x,
ppc64 and aarch64 targets

Signed-off-by: Daniele Buono 
---
 .gitlab-ci.yml | 119 +
 1 file changed, 119 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 814f51873f..7b1f25c92e 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -483,6 +483,125 @@ clang-user:
   --extra-cflags=-fsanitize=undefined 
--extra-cflags=-fno-sanitize-recover=undefined
 MAKE_CHECK_ARGS: check-unit check-tcg
 
+# Set LD_JOBS=1 because this requires LTO and ld consumes a large amount of 
memory.
+# On gitlab runners, default value sometimes end up calling 2 lds concurrently 
and
+# triggers an Out-Of-Memory error
+#
+# Since slirp callbacks are used in QEMU Timers, slirp needs to be compiled 
together
+# with QEMU and linked as a static library to avoid false positives in CFI 
checks.
+# This can be accomplished by using -enable-slirp=git, which avoids the use of
+# a system-wide version of the library
+#
+# Split in three sets of build/check/acceptance to limit the execution time of 
each
+# job
+build-cfi-arm:
+  <<: *native_build_job_definition
+  needs:
+  - job: amd64-fedora-container
+  variables:
+LD_JOBS: 1
+AR: llvm-ar
+IMAGE: fedora
+CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
+  --enable-safe-stack --enable-slirp=git
+TARGETS: aarch64-softmmu
+MAKE_CHECK_ARGS: check-build
+  artifacts:
+expire_in: 2 days
+paths:
+  - build
+
+check-cfi-arm:
+  <<: *native_test_job_definition
+  needs:
+- job: build-cfi-arm
+  artifacts: true
+  variables:
+IMAGE: fedora
+MAKE_CHECK_ARGS: check
+
+acceptance-cfi-arm:
+  <<: *native_test_job_definition
+  needs:
+- job: build-cfi-arm
+  artifacts: true
+  variables:
+IMAGE: fedora
+MAKE_CHECK_ARGS: check-acceptance
+  <<: *acceptance_definition
+
+build-cfi-ibm:
+  <<: *native_build_job_definition
+  needs:
+  - job: amd64-fedora-container
+  variables:
+LD_JOBS: 1
+AR: llvm-ar
+IMAGE: fedora
+CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
+  --enable-safe-stack --enable-slirp=git
+TARGETS: ppc64-softmmu s390x-softmmu
+MAKE_CHECK_ARGS: check-build
+  artifacts:
+expire_in: 2 days
+paths:
+  - build
+
+check-cfi-ibm:
+  <<: *native_test_job_definition
+  needs:
+- job: build-cfi-ibm
+  artifacts: true
+  variables:
+IMAGE: fedora
+MAKE_CHECK_ARGS: check
+
+acceptance-cfi-ibm:
+  <<: *native_test_job_definition
+  needs:
+- job: build-cfi-ibm
+  artifacts: true
+  variables:
+IMAGE: fedora
+MAKE_CHECK_ARGS: check-acceptance
+  <<: *acceptance_definition
+
+build-cfi-intel:
+  <<: *native_build_job_definition
+  needs:
+  - job: amd64-fedora-container
+  variables:
+LD_JOBS: 1
+AR: llvm-ar
+IMAGE: fedora
+CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
+  --enable-safe-stack --enable-slirp=git
+TARGETS: x86_64-softmmu
+MAKE_CHECK_ARGS: check-build
+  artifacts:
+expire_in: 2 days
+paths:
+  - build
+
+check-cfi-intel:
+  <<: *native_test_job_definition
+  needs:
+- job: build-cfi-intel
+  artifacts: true
+  variables:
+IMAGE: fedora
+MAKE_CHECK_ARGS: check
+
+acceptance-cfi-intel:
+  <<: *native_test_job_definition
+  needs:
+- job: build-cfi-intel
+  artifacts: true
+  variables:
+IMAGE: fedora
+MAKE_CHECK_ARGS: check-acceptance
+  <<: *acceptance_definition
+
 tsan-build:
   <<: *native_build_job_definition
   variables:
-- 
2.30.0

[PATCH v3 1/2] gitlab-ci.yml: Allow custom # of parallel linkers

[Daniele Buono]

Define a new variable LD_JOBS, that can be used to select
the maximum number of linking jobs to be executed in parallel.
If the variable is not defined, maintain the default given by
make -j

Currently, make parallelism at build time is based on the number
of cpus available.

This doesn't work well with LTO at linking, because with LTO the
linker has to load in memory all the intermediate object files
for optimization.
The end result is that, if the gitlab runner happens to run two
linking processes at the same time, the job will fail with an
out-of-memory error,

This patch leverages the ability to maintain high parallelism at
compile time, but limit the number of linkers executed in parallel.

Signed-off-by: Daniele Buono 
---
 .gitlab-ci.yml | 4 
 1 file changed, 4 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 8b6d495288..814f51873f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -27,6 +27,10 @@ include:
   else
 ../configure --enable-werror $CONFIGURE_ARGS ;
   fi || { cat config.log meson-logs/meson-log.txt && exit 1; }
+- if test -n "$LD_JOBS";
+  then
+meson configure . -Dbackend_max_links="$LD_JOBS" ;
+  fi || exit 1;
 - make -j"$JOBS"
 - if test -n "$MAKE_CHECK_ARGS";
   then
-- 
2.30.0

[PATCH v3 0/2] gitlab-ci.yml: Add jobs to test CFI

[Daniele Buono]

For a few months now QEMU has had options to enable compiler-based
control-flow integrity if built with clang.

While this feature has a low maintenance, It's probably still better to
add tests to the CI environment to check that an update doesn't break it.

The patchset allow gitlab testing of:
* --enable-cfi: forward-edge cfi (function pointers)
* --enable-safe-stack: backward-edge cfi (return pointers)
As an added benefit, this also inherently tests LTO. 

The first patch allows a custom selection for linker parallelism.
Currently, make parallelism at build time is based on the number
of cpus available.
This doesn't work well with LTO at linking, because the linker has to
load in memory all the intermediate object files for optimization.
If the gitlab runner happens to run two linking processes at the same
time, the job will fail with an out-of-memory error,
The patch leverages the ability to maintain high parallelism at
compile time, but limit the number of linkers executed in parallel.

The second patch introduces the ci/cd jobs in the gitlab pipeline.
To maintain a limited number of short jobs, Daniel suggested to only
test targets where KVM is available. This restricted the jobs to
x86_64, ppc64, aarch64 and s390x. To keep the jobs under 1 hour, I
created three chains of build -> check -> acceptance jobs, divided by
architecture vendor (Intel, ARM, IBM).

For build, we have to select --enable-slirp=git, because CFI needs a
statically linked version of slirp, with CFI information. More info on
this can be found in a comment in .gitlab-ci.yml, or on a patch for
mason currently in ML:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg787636.html

Test runs of the full pipeline are here (cfi-ci-v3 branch):
https://gitlab.com/dbuono/qemu/-/pipelines/264484574

v3:
- Restricted the targets to x86_64, ppc64, aarch64 and s390x, under
  suggestion from Daniel.

v2:
- More details in the code about the issue of using system-wide slirp
- Use meson to only limit linker parallelism instead of forcing no
  parallelism on the whole compilation process.

Daniele Buono (2):
  gitlab-ci.yml: Allow custom # of parallel linkers
  gitlab-ci.yml: Add jobs to test CFI flags

 .gitlab-ci.yml | 123 +
 1 file changed, 123 insertions(+)

-- 
2.30.0

[PATCH] meson: Stop if cfi is enabled with system slirp

[Daniele Buono]

For CFI, we need to compile slirp as a static library together with qemu.
This is because we register slirp functions as callbacks for QEMU Timers.
When using a system-wide shared libslirp, the type information for the
callback is missing and the timer call produces a false positive with CFI.

With this patch, meson will stop if CFI is enabled with system-wide slirp

Signed-off-by: Daniele Buono 
---
 meson.build | 12 
 1 file changed, 12 insertions(+)

diff --git a/meson.build b/meson.build
index f3db83e974..e1ec5020ac 100644
--- a/meson.build
+++ b/meson.build
@@ -1569,6 +1569,18 @@ if have_system
   endif
 endif
 
+# For CFI, we need to compile slirp as a static library together with qemu.
+# This is because we register slirp functions as callbacks for QEMU Timers.
+# When using a system-wide shared libslirp, the type information for the
+# callback is missing and the timer call produces a false positive with CFI.
+#
+# Now that slirp_opt has been defined, check if the selected slirp is 
compatible
+# with control-flow integrity.
+if get_option('cfi') and slirp_opt == 'system'
+  error('Control-Flow Integrity is not compatible with system-wide slirp.' \
+ + ' Please configure with --enable-slirp=git')
+endif
+
 fdt = not_found
 fdt_opt = get_option('fdt')
 if have_system
-- 
2.30.0

Re: [PATCH 01/44] clock: Add ClockEvent parameter to callbacks

[Richard Henderson]

On 2/19/21 6:45 AM, Peter Maydell wrote:

The Clock framework allows users to specify a callback which is
called after the clock's period has been updated.  Some users need to
also have a callback which is called before the clock period is
updated.

As the first step in adding support for notifying Clock users on
pre-update events, add an argument to the ClockCallback to specify
what event is being notified, and add an argument to the various
functions for registering a callback to specify which events are
of interest to that callback.

Note that the documentation update renders correct the previously
incorrect claim in 'Adding a new clock' that callbacks "will be
explained in a following section".

Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Luc Michel
---


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 24/24] hw/arm/mps2: Update old infocenter.arm.com URLs

[Richard Henderson]

On 2/15/21 3:51 AM, Peter Maydell wrote:

Update old infocenter.arm.com URLs to the equivalent developer.arm.com
ones (the old URLs should redirect, but we might as well avoid the
redirection notice, and the new URLs are pleasantly shorter).

This commit covers the links to the MPS2 board TRM, the various
Application Notes, the IoTKit and SSE-200 documents.

Signed-off-by: Peter Maydell
---


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 23/24] docs/system/arm/mps2.rst: Document the new mps3-an524 board

[Richard Henderson]

On 2/15/21 3:51 AM, Peter Maydell wrote:

Add brief documentation of the new mps3-an524 board.

Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
---
  docs/system/arm/mps2.rst | 24 ++--
  1 file changed, 18 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 22/24] hw/arm/mps2-tz: Provide PL031 RTC on mps3-an524

[Richard Henderson]

On 2/15/21 3:51 AM, Peter Maydell wrote:

The AN524 has a PL031 RTC, which we have a model of; provide it
rather than an unimplemented-device stub.

Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
---
  hw/arm/mps2-tz.c | 22 --
  1 file changed, 20 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson 

r~

[PATCH v2] multi-process: Initialize variables declared with g_auto*

[Zenghui Yu]

Quote docs/devel/style.rst (section "Automatic memory deallocation"):

* Variables declared with g_auto* MUST always be initialized,
  otherwise the cleanup function will use uninitialized stack memory

Initialize @name properly to get rid of the compilation error:

../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
/usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
   g_free (*pp);
   ^~~~
../hw/remote/proxy.c:350:30: note: 'name' was declared here
 g_autofree char *name;
  ^~~~

Signed-off-by: Zenghui Yu 
Reviewed-by: Jagannathan Raman 
---
* From v1:
  - Move the suffix iteration out of the loop (Philippe)
  - Add Jagannathan's R-b

 hw/remote/memory.c | 5 ++---
 hw/remote/proxy.c  | 3 +--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/remote/memory.c b/hw/remote/memory.c
index 32085b1e05..d97947d4b8 100644
--- a/hw/remote/memory.c
+++ b/hw/remote/memory.c
@@ -42,10 +42,9 @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
 
 remote_sysmem_reset();
 
-for (region = 0; region < msg->num_fds; region++) {
-g_autofree char *name;
+for (region = 0; region < msg->num_fds; region++, suffix++) {
+g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix);
 subregion = g_new(MemoryRegion, 1);
-name = g_strdup_printf("remote-mem-%u", suffix++);
 memory_region_init_ram_from_fd(subregion, NULL,
name, sysmem_info->sizes[region],
true, msg->fds[region],
diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
index 4fa4be079d..6dda705fc2 100644
--- a/hw/remote/proxy.c
+++ b/hw/remote/proxy.c
@@ -347,13 +347,12 @@ static void probe_pci_info(PCIDevice *dev, Error **errp)
PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY;
 
 if (size) {
-g_autofree char *name;
+g_autofree char *name = g_strdup_printf("bar-region-%d", i);
 pdev->region[i].dev = pdev;
 pdev->region[i].present = true;
 if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) {
 pdev->region[i].memory = true;
 }
-name = g_strdup_printf("bar-region-%d", i);
 memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev),
   &proxy_mr_ops, &pdev->region[i],
   name, size);
-- 
2.19.1

Re: [PATCH v2 21/24] hw/arm/mps2-tz: Stub out USB controller for mps3-an524

[Richard Henderson]

On 2/15/21 3:51 AM, Peter Maydell wrote:

The AN524 has a USB controller (an ISP1763); we don't have a model of
it but we should provide a stub "unimplemented-device" for it.  This
is slightly complicated because the USB controller shares a PPC port
with the ethernet controller.

Implement a make_* function which provides creates a container
MemoryRegion with both the ethernet controller and an
unimplemented-device stub for the USB controller.

Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
---
  hw/arm/mps2-tz.c | 48 +++-
  1 file changed, 47 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH] multi-process: Initialize variables declared with g_auto*

[Zenghui Yu]

On 2021/3/3 18:17, Daniel P. Berrangé wrote:

This is a bit wierd.  There should only be risk of uninitialized
variable if there is a 'return' or 'goto' statement between the
variable declaration and and initialization, which is not the
case in either scenario here.

What OS distro and compiler + version are you seeing this with ?


This was noticed when compiling QEMU with gcc-7.3.0 on CentOS.


Thanks,
Zenghui

Re: Some more questions with regards to QEMU clock record and replay

[Arnabjyoti Kalita]

Thank you Pavel.

Your answers make the clock record-replay process much clearer to me now.

Best Regards,
Arnab

On Tue, Mar 2, 2021 at 12:49 PM Pavel Dovgalyuk 
wrote:

> On 01.03.2021 20:16, Arnabjyoti Kalita wrote:
> > Hello all,
> >
> > I am really thankful for the wonderful answers in my last post linked
> below-
> >
> > https://lists.nongnu.org/archive/html/qemu-discuss/2021-02/msg00131.html
> >
> > In continuation with the last post, I have a few more questions to ask -
> >
> > My experiment is still, mostly the same. I record clock values in KVM
> > mode, and then replay the clock values in TCG mode. However, now I am
> > recording and replaying all of the clock values (I was only
> > recording/replaying the host clock previously). However, I do not use
> > the -icount feature.
> >
> > - Why are clock values being replayed at checkpoints?
>
> Timers are replayed at checkpoints to be synchronized with vCPU.
> Other clock requests (e.g., caused by vCPU instruction) are replayed
> immediately.
>
> > - Can we ignore replaying at checkpoints and do a dumb replay as and
> > when the clock read actually happens?
>
> I think we can, if we need just clock synchronization.
>
> > - Based on the documentation available, I can see that checkpoints are
> > necessary for thread synchronization. Does this mean, if I do not replay
> > clock values at checkpoints, the guest kernel scheduler might behave
> > incorrectly during replay ?
>
> Checkpoints are related to QEMU threads, not guest threads.
> Timers are needed for virtual devices, that can generate interrupts, DMA
> requests and so on. Therefore we synchronize them with vCPU to make
> execution deterministic.
>
>
> Pavel Dovgalyuk
>

Re: [PATCH v2 20/24] hw/arm/mps2-tz: Add new mps3-an524 board

[Richard Henderson]

On 2/15/21 3:51 AM, Peter Maydell wrote:

Add support for the mps3-an524 board; this is an SSE-200 based FPGA
image, like the existing mps2-an521.  It has a usefully larger amount
of RAM, and a PL031 RTC, as well as some more minor differences.

In real hardware this image runs on a newer generation of the FPGA
board, the MPS3 rather than the older MPS2.  Architecturally the two
boards are similar, so we implement the MPS3 boards in the mps2-tz.c
file as variations of the existing MPS2 boards.

Signed-off-by: Peter Maydell
---
  hw/arm/mps2-tz.c | 139 +--
  1 file changed, 135 insertions(+), 4 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 08/24] hw/arm/mps2-tz: Condition IRQ splitting on number of CPUs, not board type

[Richard Henderson]

On 2/15/21 3:51 AM, Peter Maydell wrote:

In the mps2-tz board code, we handle devices whose interrupt lines
must be wired to all CPUs by creating IRQ splitter devices for the
AN521, because it has 2 CPUs, but wiring the device IRQ directly to
the SSE/IoTKit input for the AN505, which has only 1 CPU.

We can avoid making an explicit check on the board type constant by
instead creating and using the IRQ splitters for any board with more
than 1 CPU.  This avoids having to add extra cases to the
conditionals every time we add new boards.

Signed-off-by: Peter Maydell
---


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v4 00/28] cpu: Introduce SysemuCPUOps structure, remove watchpoints from usermode

[Richard Henderson]

On 3/3/21 1:46 PM, Philippe Mathieu-Daudé wrote:

Patches 1-6 are generic cleanups.
Patches 7-15 move from CPUClass to SysemuCPUOps
Patch   16 restricts SysemuCPUOps to sysemu
Patches 17-26 remove watchpoint code from user emulation
Patches 27-28 remove USER_ONLY #ifdef'ry from "cpu.h"


Patches 1-18:
Reviewed-by: Richard Henderson 

While mst has asked for a name change vs patch 4, I think that if we do that it 
should be separate, because it would involve a rename through hw/ as well.


The watchpoint patches that follow need some more careful thought.


r~

Re: [PATCH] file-posix: allow -EBUSY errors during write zeros on block

[ChangLimin]

>> After Linux 5.10, write zeros to a multipath device using
>> ioctl(fd, BLKZEROOUT, range) with cache none or directsync will return EBUSY.
>>
>> Similar to handle_aiocb_write_zeroes_unmap, handle_aiocb_write_zeroes_block
>> allow -EBUSY errors during ioctl(fd, BLKZEROOUT, range).
>>
>> Reference commit in Linux 5.10:
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=384d87ef2c954fc58e6c5fd8253e4a1984f5fe02
> 
>But this can happen only when the block device is used by a file system or
>maybe someone else. In qemu we assume that we are the only user of the
>block device, so EBUSY is a fatal error that should never happen, no?
> 
>Can you explain a real world use case when we get EBUSY?
> 
>Nir
> 

Please refer to 
https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/configuring_device_mapper_multipath/index
Where multipath is configured to a SAN LUN, ioctl(fd, BLKZEROOUT, range)
to the /dev/dm-x return EBUSY permanently since Linux 5.10.

ChangLimin

>> Signed-off-by: ChangLimin 
>> ---
>>  block/file-posix.c | 7 ++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/file-posix.c b/block/file-posix.c
>> index 05079b40ca..3e60c96214 100644
>> --- a/block/file-posix.c
>> +++ b/block/file-posix.c
>> @@ -1629,8 +1629,13 @@ static ssize_t 
>> handle_aiocb_write_zeroes_block(RawPosixAIOData *aiocb)
>>  } while (errno == EINTR);
>>
>>  ret = translate_err(-errno);
>> -if (ret == -ENOTSUP) {
>> +switch (ret) {
>> +case -ENOTSUP:
>> +case -EINVAL:
>> +case -EBUSY:
>>  s->has_write_zeroes = false;
>> +return -ENOTSUP;
>> +break;
>>  }
>>  }
>>  #endif
>> --
>> 2.27.0
>>
 
 

Re: [PATCH v4 26/28] cpu: Remove watchpoint stubs for user emulation

[Richard Henderson]

On 3/3/21 1:47 PM, Philippe Mathieu-Daudé wrote:

Since we remove all access to the watchpoint methods from user-mode
code, we can now remove them, as they are not used anymore.

Signed-off-by: Philippe Mathieu-Daudé
---
  include/hw/core/cpu.h | 34 +-
  1 file changed, 1 insertion(+), 33 deletions(-)


I do not think this is a good tradeoff, removing one central ifdef and 
substituting lots of others, scattered throughout target/.



r~

RE: [RFC PATCH 6/7] cpu: Move CPUClass::has_work() to TCGCPUOps

[Taylor Simpson]

> -Original Message-
> From: Philippe Mathieu-Daudé  On
> Behalf Of Philippe Mathieu-Daudé
> Sent: Tuesday, March 2, 2021 4:28 AM
> To: qemu-devel@nongnu.org
> Cc: Yoshinori Sato ; Chris Wulff
> ; qemu-...@nongnu.org; Marcel Apfelbaum
> ; Greg Kurz ; qemu-
> ri...@nongnu.org; Richard Henderson ;
> Peter Maydell ; Michael Walle
> ; Palmer Dabbelt ; Sarah Harris
> ; Anthony Green ;
> Eduardo Habkost ; Bastian Koppelmann
> ; Laurent Vivier ;
> Edgar E. Iglesias ; Claudio Fontana
> ; Artyom Tarasenko ; qemu-
> s3...@nongnu.org; Thomas Huth ; Paolo Bonzini
> ; Cornelia Huck ; Taylor
> Simpson ; Alistair Francis
> ; Michael Rolnik ; David
> Hildenbrand ; Aleksandar Rikalo
> ; Stafford Horne ;
> Jiaxun Yang ; Marek Vasut ;
> Mark Cave-Ayland ; Philippe Mathieu-
> Daudé ; qemu-...@nongnu.org; David Gibson
> ; Sagar Karandikar
> ; Guan Xuetao ; Max
> Filippov ; Aurelien Jarno 
> Subject: [RFC PATCH 6/7] cpu: Move CPUClass::has_work() to TCGCPUOps
>
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
> index b0b3040dd13..d597fe12cdf 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -268,6 +268,7 @@ static bool hexagon_tlb_fill(CPUState *cs, vaddr
> address, int size,
>
>  static struct TCGCPUOps hexagon_tcg_ops = {
>  .initialize = hexagon_translate_init,
> +.has_work = hexagon_cpu_has_work,
>  .synchronize_from_tb = hexagon_cpu_synchronize_from_tb,
>  .tlb_fill = hexagon_tlb_fill,
>  };
> @@ -284,7 +285,6 @@ static void hexagon_cpu_class_init(ObjectClass *c,
> void *data)
>  device_class_set_parent_reset(dc, hexagon_cpu_reset, &mcc-
> >parent_reset);
>
>  cc->class_by_name = hexagon_cpu_class_by_name;
> -cc->has_work = hexagon_cpu_has_work;
>  cc->dump_state = hexagon_dump_state;
>  cc->set_pc = hexagon_cpu_set_pc;
>  cc->gdb_read_register = hexagon_gdb_read_register;

Reviewed-by: Taylor Simpson 

Re: [PATCH v3 0/2] hw/riscv: Add fw_cfg support, allow ramfb

[Alistair Francis]

On Sun, Feb 28, 2021 at 6:18 AM Asherah Connor  wrote:
>
> This is version 3 of the series to bring fw_cfg support to riscv's virt
> machine, including ramfb support.  It is tested as working against a
> modified U-Boot with ramfb support.

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>
> Changes in v3:
> * Document why fw_cfg is done when it is.
> * Move VIRT_FW_CFG before VIRT_FLASH.
>
> Changes in v2:
> * Add DMA support (needed for writes).
> * Add ramfb as allowed on riscv virt machine class.
>
> Asherah Connor (2):
>   hw/riscv: Add fw_cfg support to virt
>   hw/riscv: allow ramfb on virt
>
>  hw/riscv/Kconfig|  1 +
>  hw/riscv/virt.c | 33 +
>  include/hw/riscv/virt.h |  2 ++
>  3 files changed, 36 insertions(+)
>
> --
> 2.20.1
>
>

Re: [PATCH v1 1/2] hw/misc: versal: Add a model of the XRAM controller

[Alistair Francis]

On Tue, Mar 2, 2021 at 6:12 AM Edgar E. Iglesias
 wrote:
>
> From: "Edgar E. Iglesias" 
>
> Add a model of the Xilinx Versal Accelerator RAM (XRAM).
> This is mainly a stub to make firmware happy. The size of
> the RAMs can be probed. The interrupt mask logic is
> modelled but none of the interrups will ever be raised
> unless injected.
>
> Signed-off-by: Edgar E. Iglesias 

Acked-by: Alistair Francis 

Alistair

> ---
>  include/hw/misc/xlnx-versal-xramc.h | 102 +++
>  hw/misc/xlnx-versal-xramc.c | 253 
>  hw/misc/meson.build |   1 +
>  3 files changed, 356 insertions(+)
>  create mode 100644 include/hw/misc/xlnx-versal-xramc.h
>  create mode 100644 hw/misc/xlnx-versal-xramc.c
>
> diff --git a/include/hw/misc/xlnx-versal-xramc.h 
> b/include/hw/misc/xlnx-versal-xramc.h
> new file mode 100644
> index 00..68163cf330
> --- /dev/null
> +++ b/include/hw/misc/xlnx-versal-xramc.h
> @@ -0,0 +1,102 @@
> +/*
> + * QEMU model of the Xilinx XRAM Controller.
> + *
> + * Copyright (c) 2021 Xilinx Inc.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Written by Edgar E. Iglesias 
> + */
> +
> +#ifndef XLNX_VERSAL_XRAMC_H
> +#define XLNX_VERSAL_XRAMC_H
> +
> +#include "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "hw/register.h"
> +#include "qemu/bitops.h"
> +#include "qemu/log.h"
> +#include "migration/vmstate.h"
> +#include "hw/irq.h"
> +
> +#define TYPE_XLNX_XRAM_CTRL "xlnx.versal-xramc"
> +
> +#define XLNX_XRAM_CTRL(obj) \
> + OBJECT_CHECK(XlnxXramCtrl, (obj), TYPE_XLNX_XRAM_CTRL)
> +
> +REG32(XRAM_ERR_CTRL, 0x0)
> +FIELD(XRAM_ERR_CTRL, UE_RES, 3, 1)
> +FIELD(XRAM_ERR_CTRL, PWR_ERR_RES, 2, 1)
> +FIELD(XRAM_ERR_CTRL, PZ_ERR_RES, 1, 1)
> +FIELD(XRAM_ERR_CTRL, APB_ERR_RES, 0, 1)
> +REG32(XRAM_ISR, 0x4)
> +FIELD(XRAM_ISR, INV_APB, 0, 1)
> +REG32(XRAM_IMR, 0x8)
> +FIELD(XRAM_IMR, INV_APB, 0, 1)
> +REG32(XRAM_IEN, 0xc)
> +FIELD(XRAM_IEN, INV_APB, 0, 1)
> +REG32(XRAM_IDS, 0x10)
> +FIELD(XRAM_IDS, INV_APB, 0, 1)
> +REG32(XRAM_ECC_CNTL, 0x14)
> +FIELD(XRAM_ECC_CNTL, FI_MODE, 2, 1)
> +FIELD(XRAM_ECC_CNTL, DET_ONLY, 1, 1)
> +FIELD(XRAM_ECC_CNTL, ECC_ON_OFF, 0, 1)
> +REG32(XRAM_CLR_EXE, 0x18)
> +FIELD(XRAM_CLR_EXE, MON_7, 7, 1)
> +FIELD(XRAM_CLR_EXE, MON_6, 6, 1)
> +FIELD(XRAM_CLR_EXE, MON_5, 5, 1)
> +FIELD(XRAM_CLR_EXE, MON_4, 4, 1)
> +FIELD(XRAM_CLR_EXE, MON_3, 3, 1)
> +FIELD(XRAM_CLR_EXE, MON_2, 2, 1)
> +FIELD(XRAM_CLR_EXE, MON_1, 1, 1)
> +FIELD(XRAM_CLR_EXE, MON_0, 0, 1)
> +REG32(XRAM_CE_FFA, 0x1c)
> +FIELD(XRAM_CE_FFA, ADDR, 0, 20)
> +REG32(XRAM_CE_FFD0, 0x20)
> +REG32(XRAM_CE_FFD1, 0x24)
> +REG32(XRAM_CE_FFD2, 0x28)
> +REG32(XRAM_CE_FFD3, 0x2c)
> +REG32(XRAM_CE_FFE, 0x30)
> +FIELD(XRAM_CE_FFE, SYNDROME, 0, 16)
> +REG32(XRAM_UE_FFA, 0x34)
> +FIELD(XRAM_UE_FFA, ADDR, 0, 20)
> +REG32(XRAM_UE_FFD0, 0x38)
> +REG32(XRAM_UE_FFD1, 0x3c)
> +REG32(XRAM_UE_FFD2, 0x40)
> +REG32(XRAM_UE_FFD3, 0x44)
> +REG32(XRAM_UE_FFE, 0x48)
> +FIELD(XRAM_UE_FFE, SYNDROME, 0, 16)
> +REG32(XRAM_FI_D0, 0x4c)
> +REG32(XRAM_FI_D1, 0x50)
> +REG32(XRAM_FI_D2, 0x54)
> +REG32(XRAM_FI_D3, 0x58)
> +REG32(XRAM_FI_SY, 0x5c)
> +FIELD(XRAM_FI_SY, DATA, 0, 16)
> +REG32(XRAM_RMW_UE_FFA, 0x70)
> +FIELD(XRAM_RMW_UE_FFA, ADDR, 0, 20)
> +REG32(XRAM_FI_CNTR, 0x74)
> +FIELD(XRAM_FI_CNTR, COUNT, 0, 24)
> +REG32(XRAM_IMP, 0x80)
> +FIELD(XRAM_IMP, SIZE, 0, 4)
> +REG32(XRAM_PRDY_DBG, 0x84)
> +FIELD(XRAM_PRDY_DBG, ISLAND3, 12, 4)
> +FIELD(XRAM_PRDY_DBG, ISLAND2, 8, 4)
> +FIELD(XRAM_PRDY_DBG, ISLAND1, 4, 4)
> +FIELD(XRAM_PRDY_DBG, ISLAND0, 0, 4)
> +REG32(XRAM_SAFETY_CHK, 0xff8)
> +
> +#define XRAM_CTRL_R_MAX (R_XRAM_SAFETY_CHK + 1)
> +
> +typedef struct XlnxXramCtrl {
> +SysBusDevice parent_obj;
> +MemoryRegion iomem;
> +MemoryRegion ram;
> +qemu_irq irq;
> +
> +struct {
> +uint64_t size;
> +unsigned int encoded_size;
> +} cfg;
> +
> +uint32_t regs[XRAM_CTRL_R_MAX];
> +RegisterInfo regs_info[XRAM_CTRL_R_MAX];
> +} XlnxXramCtrl;
> +#endif
> diff --git a/hw/misc/xlnx-versal-xramc.c b/hw/misc/xlnx-versal-xramc.c
> new file mode 100644
> index 00..c7f449fefa
> --- /dev/null
> +++ b/hw/misc/xlnx-versal-xramc.c
> @@ -0,0 +1,253 @@
> +/*
> + * QEMU model of the Xilinx XRAM Controller.
> + *
> + * Copyright (c) 2021 Xilinx Inc.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Written by Edgar E. Iglesias 
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/units.h"
> +#include "qemu/bitops.h"
> +#include "qapi/error.h"
> +#include "migration/vmstate.h"
> +#include "hw/sysbus.h"
> +#include "hw/register.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/irq.h"
> +#include "hw/misc/xlnx-versal-xramc.h"
> +
> +#ifndef XLNX_XRAM_CTRL_ERR_DEBUG
> +#define XLNX_XRAM_CTRL_ERR_DEBUG 0
> +#endif
> +
> +static void xram_update_irq(XlnxXramCtrl *s)
> +{
> +bool pending = s->regs[R_XRAM_ISR] & ~s->regs[R_XRAM_IMR];
> +q

Re: [PATCH v1 2/2] hw/arm: versal: Add support for the XRAMs

[Alistair Francis]

On Tue, Mar 2, 2021 at 6:10 AM Edgar E. Iglesias
 wrote:
>
> From: "Edgar E. Iglesias" 
>
> Connect the support for the Versal Accelerator RAMs (XRAMs).
>
> Signed-off-by: Edgar E. Iglesias 

Acked-by: Alistair Francis 

Alistair

> ---
>  docs/system/arm/xlnx-versal-virt.rst |  1 +
>  include/hw/arm/xlnx-versal.h | 13 ++
>  hw/arm/xlnx-versal.c | 36 
>  3 files changed, 50 insertions(+)
>
> diff --git a/docs/system/arm/xlnx-versal-virt.rst 
> b/docs/system/arm/xlnx-versal-virt.rst
> index 2602d0f995..27f73500d9 100644
> --- a/docs/system/arm/xlnx-versal-virt.rst
> +++ b/docs/system/arm/xlnx-versal-virt.rst
> @@ -30,6 +30,7 @@ Implemented devices:
>  - 8 ADMA (Xilinx zDMA) channels
>  - 2 SD Controllers
>  - OCM (256KB of On Chip Memory)
> +- XRAM (4MB of on chip Accelerator RAM)
>  - DDR memory
>
>  QEMU does not yet model any other devices, including the PL and the AI 
> Engine.
> diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
> index 2b76885afd..22a8fa5d11 100644
> --- a/include/hw/arm/xlnx-versal.h
> +++ b/include/hw/arm/xlnx-versal.h
> @@ -14,6 +14,7 @@
>
>  #include "hw/sysbus.h"
>  #include "hw/arm/boot.h"
> +#include "hw/or-irq.h"
>  #include "hw/sd/sdhci.h"
>  #include "hw/intc/arm_gicv3.h"
>  #include "hw/char/pl011.h"
> @@ -22,6 +23,7 @@
>  #include "hw/rtc/xlnx-zynqmp-rtc.h"
>  #include "qom/object.h"
>  #include "hw/usb/xlnx-usb-subsystem.h"
> +#include "hw/misc/xlnx-versal-xramc.h"
>
>  #define TYPE_XLNX_VERSAL "xlnx-versal"
>  OBJECT_DECLARE_SIMPLE_TYPE(Versal, XLNX_VERSAL)
> @@ -31,6 +33,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(Versal, XLNX_VERSAL)
>  #define XLNX_VERSAL_NR_GEMS2
>  #define XLNX_VERSAL_NR_ADMAS   8
>  #define XLNX_VERSAL_NR_SDS 2
> +#define XLNX_VERSAL_NR_XRAM4
>  #define XLNX_VERSAL_NR_IRQS192
>
>  struct Versal {
> @@ -62,6 +65,11 @@ struct Versal {
>  XlnxZDMA adma[XLNX_VERSAL_NR_ADMAS];
>  VersalUsb2 usb;
>  } iou;
> +
> +struct {
> +qemu_or_irq irq_orgate;
> +XlnxXramCtrl ctrl[XLNX_VERSAL_NR_XRAM];
> +} xram;
>  } lpd;
>
>  /* The Platform Management Controller subsystem.  */
> @@ -96,6 +104,7 @@ struct Versal {
>  #define VERSAL_GEM1_IRQ_0  58
>  #define VERSAL_GEM1_WAKE_IRQ_0 59
>  #define VERSAL_ADMA_IRQ_0  60
> +#define VERSAL_XRAM_IRQ_0  79
>  #define VERSAL_RTC_APB_ERR_IRQ 121
>  #define VERSAL_SD0_IRQ_0   126
>  #define VERSAL_RTC_ALARM_IRQ   142
> @@ -128,6 +137,10 @@ struct Versal {
>  #define MM_OCM  0xfffcU
>  #define MM_OCM_SIZE 0x4
>
> +#define MM_XRAM 0xfe80
> +#define MM_XRAMC0xff8e
> +#define MM_XRAMC_SIZE   0x1
> +
>  #define MM_USB2_CTRL_REGS   0xFF9D
>  #define MM_USB2_CTRL_REGS_SIZE  0x1
>
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index 628e77ef66..79609692e4 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -10,6 +10,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "qapi/error.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> @@ -278,6 +279,40 @@ static void versal_create_rtc(Versal *s, qemu_irq *pic)
>  sysbus_connect_irq(sbd, 1, pic[VERSAL_RTC_APB_ERR_IRQ]);
>  }
>
> +static void versal_create_xrams(Versal *s, qemu_irq *pic)
> +{
> +int nr_xrams = ARRAY_SIZE(s->lpd.xram.ctrl);
> +DeviceState *orgate;
> +int i;
> +
> +/* XRAM IRQs get ORed into a single line.  */
> +object_initialize_child(OBJECT(s), "xram-irq-orgate",
> +&s->lpd.xram.irq_orgate, TYPE_OR_IRQ);
> +orgate = DEVICE(&s->lpd.xram.irq_orgate);
> +object_property_set_int(OBJECT(orgate),
> +"num-lines", nr_xrams, &error_fatal);
> +qdev_realize(orgate, NULL, &error_fatal);
> +qdev_connect_gpio_out(orgate, 0, pic[VERSAL_XRAM_IRQ_0]);
> +
> +for (i = 0; i < ARRAY_SIZE(s->lpd.xram.ctrl); i++) {
> +SysBusDevice *sbd;
> +MemoryRegion *mr;
> +
> +object_initialize_child(OBJECT(s), "xram[*]", &s->lpd.xram.ctrl[i],
> +TYPE_XLNX_XRAM_CTRL);
> +sbd = SYS_BUS_DEVICE(&s->lpd.xram.ctrl[i]);
> +sysbus_realize(sbd, &error_fatal);
> +
> +mr = sysbus_mmio_get_region(sbd, 0);
> +memory_region_add_subregion(&s->mr_ps,
> +MM_XRAMC + i * MM_XRAMC_SIZE, mr);
> +mr = sysbus_mmio_get_region(sbd, 1);
> +memory_region_add_subregion(&s->mr_ps, MM_XRAM + i * MiB, mr);
> +
> +sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(orgate, i));
> +}
> +}
> +
>  /* This takes the board allocated linear DDR memory and creates aliases
>   * for each split DDR range/aperture on the Versal address map.
>   */
> @@ -363,6 +398,7 @@ static void versal_realize(DeviceState *dev

Re: [PATCH v4 22/28] target/arm: Restrict watchpoint code to system emulation

[Richard Henderson]

On 3/3/21 1:47 PM, Philippe Mathieu-Daudé wrote:

diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 844db08bd57..ed3f22d78a5 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4849,6 +4849,7 @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const 
target_ulong addr,
  /* Some page is MMIO, see below. */
  goto do_fault;
  }
+#ifndef CONFIG_USER_ONLY
  if (unlikely(flags & TLB_WATCHPOINT) &&
  (cpu_watchpoint_address_matches
   (env_cpu(env), addr + mem_off, 1 << msz)
@@ -4856,6 +4857,7 @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const 
target_ulong addr,
  /* Watchpoint hit, see below. */
  goto do_fault;
  }
+#endif
  


Nack.  These are eliminated via TLB_WATCHPOINT 0 in exec/cpu-all.h, and an 
empty inline function in hw/core/cpu.h.



r~

Re: [PATCH v3 1/2] hw/riscv: Add fw_cfg support to virt

[Alistair Francis]

On Sun, Feb 28, 2021 at 6:45 AM Bin Meng  wrote:
>
> On Sun, Feb 28, 2021 at 7:17 PM Asherah Connor  wrote:
> >
> > Provides fw_cfg for the virt machine on riscv.  This enables
> > using e.g.  ramfb later.
> >
> > Signed-off-by: Asherah Connor 

Reviewed-by: Alistair Francis 

Alistair

> > ---
> >
> > Changes in v3:
> > * Document why fw_cfg is done when it is.
> > * Move VIRT_FW_CFG before VIRT_FLASH.
> >
> > Changes in v2:
> > * Add DMA support (needed for writes).
> >
> >  hw/riscv/Kconfig|  1 +
> >  hw/riscv/virt.c | 30 ++
> >  include/hw/riscv/virt.h |  2 ++
> >  3 files changed, 33 insertions(+)
> >
>
> Reviewed-by: Bin Meng 
>

Re: [PATCH v3 2/2] hw/riscv: allow ramfb on virt

[Alistair Francis]

On Sun, Feb 28, 2021 at 6:20 AM Asherah Connor  wrote:
>
> Allow ramfb on virt.  This lets `-device ramfb' work.
>
> Signed-off-by: Asherah Connor 
> Reviewed-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> * Add ramfb as allowed on riscv virt machine class.
>
>  hw/riscv/virt.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 82eff42c37..700b481fa2 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -42,6 +42,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/pci/pci.h"
>  #include "hw/pci-host/gpex.h"
> +#include "hw/display/ramfb.h"
>
>  static const struct MemmapEntry {
>  hwaddr base;
> @@ -743,6 +744,8 @@ static void virt_machine_class_init(ObjectClass *oc, void 
> *data)
>  mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props;
>  mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id;
>  mc->numa_mem_supported = true;
> +
> +machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
>  }
>
>  static const TypeInfo virt_machine_typeinfo = {
> --
> 2.20.1
>
>

RE: [PATCH v4 16/28] cpu: Restrict "hw/core/sysemu-cpu-ops.h" to target/cpu.c

[Taylor Simpson]

> -Original Message-
> From: Philippe Mathieu-Daudé  On
> Behalf Of Philippe Mathieu-Daudé
> Sent: Wednesday, March 3, 2021 3:47 PM
> To: qemu-devel@nongnu.org
> Cc: Claudio Fontana ; qemu-...@nongnu.org; Peter
> Maydell ; Paolo Bonzini
> ; Richard Henderson
> ; qemu-s3...@nongnu.org; Thomas Huth
> ; qemu-...@nongnu.org; Laurent Vivier
> ; Cornelia Huck ; Alex Bennée
> ; Philippe Mathieu-Daudé ;
> Eduardo Habkost ; Marcel Apfelbaum
> ; Michael Rolnik ;
> Sarah Harris ; Edgar E. Iglesias
> ; Taylor Simpson ;
> Michael Walle ; Aurelien Jarno ;
> Jiaxun Yang ; Aleksandar Rikalo
> ; Anthony Green
> ; Chris Wulff ; Marek Vasut
> ; Stafford Horne ; David Gibson
> ; Greg Kurz ; Palmer
> Dabbelt ; Alistair Francis
> ; Sagar Karandikar ;
> Bastian Koppelmann ; Yoshinori Sato
> ; David Hildenbrand ;
> Mark Cave-Ayland ; Artyom Tarasenko
> ; Guan Xuetao ; Max
> Filippov ; open list:RISC-V TCG CPUs  ri...@nongnu.org>
> Subject: [PATCH v4 16/28] cpu: Restrict "hw/core/sysemu-cpu-ops.h" to
> target/cpu.c
>
> Somehow similar to commit 78271684719 ("cpu: tcg_ops: move to
> tcg-cpu-ops.h, keep a pointer in CPUClass"):
>
> We cannot in principle make the SysEmu Operations field definitions
> conditional on CONFIG_SOFTMMU in code that is included by both
> common_ss and specific_ss modules.
>
> Therefore, what we can do safely to restrict the SysEmu fields to
> system emulation builds, is to move all sysemu operations into a
> separate header file, which is only included by system-specific code.
>
> This leaves just a NULL pointer in the cpu.h for the user-mode builds.
>
> Inspired-by: Claudio Fontana 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> diff --git a/target/hexagon/cpu.h b/target/hexagon/cpu.h
> index e04eac591c8..2a878e77f08 100644
> --- a/target/hexagon/cpu.h
> +++ b/target/hexagon/cpu.h
> @@ -26,6 +26,9 @@ typedef struct CPUHexagonState CPUHexagonState;
>  #include "qemu-common.h"
>  #include "exec/cpu-defs.h"
>  #include "hex_regs.h"
> +#ifndef CONFIG_USER_ONLY
> +#include "hw/core/sysemu-cpu-ops.h"
> +#endif

Reviewed-by: Taylor Simpson 

Re: [PATCH v4 03/28] cpu: Introduce cpu_virtio_is_big_endian()

[Richard Henderson]

On 3/3/21 2:18 PM, Richard Henderson wrote:

On 3/3/21 2:15 PM, Michael S. Tsirkin wrote:

On Wed, Mar 03, 2021 at 05:08:36PM -0500, Michael S. Tsirkin wrote:

On Wed, Mar 03, 2021 at 10:46:43PM +0100, Philippe Mathieu-Daudé wrote:

Introduce the cpu_virtio_is_big_endian() generic helper to avoid
calling CPUClass internal virtio_is_big_endian() one.

Signed-off-by: Philippe Mathieu-Daudé 


Using virtio in the name here probably because virtio wants this?
That doesn't sound like a good naming strategy, name should
tell us what function does not how it's used.


On a more concrete proposal, how about using this change
to rename the virtio_is_big_endian field to guest_is_big_endian(),
and put the wrapper somewhere in a virtio header instead?


We already one for normal guest endianness.  This one is for when virtio 
specifically differs from that.


Hmph.  I remembered incorrectly.  I thought we had a bit of ppc weirdness in 
which endianness was reversed for virtio, but no.


I'm ok with the renaming.


r~

Re: [PATCH v4 03/28] cpu: Introduce cpu_virtio_is_big_endian()

[Richard Henderson]

On 3/3/21 2:15 PM, Michael S. Tsirkin wrote:

On Wed, Mar 03, 2021 at 05:08:36PM -0500, Michael S. Tsirkin wrote:

On Wed, Mar 03, 2021 at 10:46:43PM +0100, Philippe Mathieu-Daudé wrote:

Introduce the cpu_virtio_is_big_endian() generic helper to avoid
calling CPUClass internal virtio_is_big_endian() one.

Signed-off-by: Philippe Mathieu-Daudé 


Using virtio in the name here probably because virtio wants this?
That doesn't sound like a good naming strategy, name should
tell us what function does not how it's used.


On a more concrete proposal, how about using this change
to rename the virtio_is_big_endian field to guest_is_big_endian(),
and put the wrapper somewhere in a virtio header instead?


We already one for normal guest endianness.  This one is for when virtio 
specifically differs from that.



r~

Re: [PATCH v4 03/28] cpu: Introduce cpu_virtio_is_big_endian()

[Michael S. Tsirkin]

On Wed, Mar 03, 2021 at 05:08:36PM -0500, Michael S. Tsirkin wrote:
> On Wed, Mar 03, 2021 at 10:46:43PM +0100, Philippe Mathieu-Daudé wrote:
> > Introduce the cpu_virtio_is_big_endian() generic helper to avoid
> > calling CPUClass internal virtio_is_big_endian() one.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé 
> 
> Using virtio in the name here probably because virtio wants this?
> That doesn't sound like a good naming strategy, name should
> tell us what function does not how it's used.

On a more concrete proposal, how about using this change
to rename the virtio_is_big_endian field to guest_is_big_endian(),
and put the wrapper somewhere in a virtio header instead?

> > ---
> >  include/hw/core/cpu.h | 9 +
> >  hw/core/cpu.c | 8 ++--
> >  hw/virtio/virtio.c| 4 +---
> >  3 files changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 2d43f78819f..b12028c3c03 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -602,6 +602,15 @@ hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr 
> > addr);
> >   */
> >  int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs);
> >  
> > +/**
> > + * cpu_virtio_is_big_endian:
> > + * @cpu: CPU
> > +
> > + * Returns %true if a CPU which supports runtime configurable endianness
> > + * is currently big-endian.
> > + */
> > +bool cpu_virtio_is_big_endian(CPUState *cpu);
> > +
> >  #endif /* CONFIG_USER_ONLY */
> >  
> >  /**
> > diff --git a/hw/core/cpu.c b/hw/core/cpu.c
> > index 4dce35f832f..daaff56a79e 100644
> > --- a/hw/core/cpu.c
> > +++ b/hw/core/cpu.c
> > @@ -218,8 +218,13 @@ static int cpu_common_gdb_write_register(CPUState 
> > *cpu, uint8_t *buf, int reg)
> >  return 0;
> >  }
> >  
> > -static bool cpu_common_virtio_is_big_endian(CPUState *cpu)
> > +bool cpu_virtio_is_big_endian(CPUState *cpu)
> >  {
> > +CPUClass *cc = CPU_GET_CLASS(cpu);
> > +
> > +if (cc->virtio_is_big_endian) {
> > +return cc->virtio_is_big_endian(cpu);
> > +}
> >  return target_words_bigendian();
> >  }
> >  
> > @@ -438,7 +443,6 @@ static void cpu_class_init(ObjectClass *klass, void 
> > *data)
> >  k->write_elf64_note = cpu_common_write_elf64_note;
> >  k->gdb_read_register = cpu_common_gdb_read_register;
> >  k->gdb_write_register = cpu_common_gdb_write_register;
> > -k->virtio_is_big_endian = cpu_common_virtio_is_big_endian;
> >  set_bit(DEVICE_CATEGORY_CPU, dc->categories);
> >  dc->realize = cpu_common_realizefn;
> >  dc->unrealize = cpu_common_unrealizefn;
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 1fd1917ca0f..fe6a4be99e4 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -1973,9 +1973,7 @@ static enum virtio_device_endian 
> > virtio_default_endian(void)
> >  
> >  static enum virtio_device_endian virtio_current_cpu_endian(void)
> >  {
> > -CPUClass *cc = CPU_GET_CLASS(current_cpu);
> > -
> > -if (cc->virtio_is_big_endian(current_cpu)) {
> > +if (cpu_virtio_is_big_endian(current_cpu)) {
> >  return VIRTIO_DEVICE_ENDIAN_BIG;
> >  } else {
> >  return VIRTIO_DEVICE_ENDIAN_LITTLE;
> > -- 
> > 2.26.2

Re: [PATCH] nbd: server: Report holes for raw images

[Eric Blake]

On 3/3/21 3:45 PM, Nir Soffer wrote:


 I'll wait a few days for any other reviewer commentary before taking
 this through my NBD tree.


>>
>> And thanks for CCing me. Hmm, maybe, I'll suggest myself as co-maintainer
>> for NBD?

Vladimir, I'd be happy if you want to do that in a separate patch
(you're already a co-maintainer on block bitmaps, which are somewhat
related).

> 
> 
> Kevin, Max, are you ok with this change?

I guess that means I should send my NBD pull request sooner rather than
later, since it's been a few days with no other comments?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v4 03/28] cpu: Introduce cpu_virtio_is_big_endian()

[Michael S. Tsirkin]

On Wed, Mar 03, 2021 at 10:46:43PM +0100, Philippe Mathieu-Daudé wrote:
> Introduce the cpu_virtio_is_big_endian() generic helper to avoid
> calling CPUClass internal virtio_is_big_endian() one.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Using virtio in the name here probably because virtio wants this?
That doesn't sound like a good naming strategy, name should
tell us what function does not how it's used.

> ---
>  include/hw/core/cpu.h | 9 +
>  hw/core/cpu.c | 8 ++--
>  hw/virtio/virtio.c| 4 +---
>  3 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 2d43f78819f..b12028c3c03 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -602,6 +602,15 @@ hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr 
> addr);
>   */
>  int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs);
>  
> +/**
> + * cpu_virtio_is_big_endian:
> + * @cpu: CPU
> +
> + * Returns %true if a CPU which supports runtime configurable endianness
> + * is currently big-endian.
> + */
> +bool cpu_virtio_is_big_endian(CPUState *cpu);
> +
>  #endif /* CONFIG_USER_ONLY */
>  
>  /**
> diff --git a/hw/core/cpu.c b/hw/core/cpu.c
> index 4dce35f832f..daaff56a79e 100644
> --- a/hw/core/cpu.c
> +++ b/hw/core/cpu.c
> @@ -218,8 +218,13 @@ static int cpu_common_gdb_write_register(CPUState *cpu, 
> uint8_t *buf, int reg)
>  return 0;
>  }
>  
> -static bool cpu_common_virtio_is_big_endian(CPUState *cpu)
> +bool cpu_virtio_is_big_endian(CPUState *cpu)
>  {
> +CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> +if (cc->virtio_is_big_endian) {
> +return cc->virtio_is_big_endian(cpu);
> +}
>  return target_words_bigendian();
>  }
>  
> @@ -438,7 +443,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
>  k->write_elf64_note = cpu_common_write_elf64_note;
>  k->gdb_read_register = cpu_common_gdb_read_register;
>  k->gdb_write_register = cpu_common_gdb_write_register;
> -k->virtio_is_big_endian = cpu_common_virtio_is_big_endian;
>  set_bit(DEVICE_CATEGORY_CPU, dc->categories);
>  dc->realize = cpu_common_realizefn;
>  dc->unrealize = cpu_common_unrealizefn;
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 1fd1917ca0f..fe6a4be99e4 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1973,9 +1973,7 @@ static enum virtio_device_endian 
> virtio_default_endian(void)
>  
>  static enum virtio_device_endian virtio_current_cpu_endian(void)
>  {
> -CPUClass *cc = CPU_GET_CLASS(current_cpu);
> -
> -if (cc->virtio_is_big_endian(current_cpu)) {
> +if (cpu_virtio_is_big_endian(current_cpu)) {
>  return VIRTIO_DEVICE_ENDIAN_BIG;
>  } else {
>  return VIRTIO_DEVICE_ENDIAN_LITTLE;
> -- 
> 2.26.2

[PATCH v4 23/28] target/i386: Restrict watchpoint code to system emulation

[Philippe Mathieu-Daudé]

We can not use watchpoints in user-mode emulation because we
need the softmmu slow path to detect accesses to watchpointed
memory. Add #ifdef'ry around it.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/i386/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 994d58aa754..4b8f06f6193 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6182,7 +6182,6 @@ static void x86_cpu_reset(DeviceState *dev)
 env->dr[6] = DR6_FIXED_1;
 env->dr[7] = DR7_FIXED_1;
 cpu_breakpoint_remove_all(s, BP_CPU);
-cpu_watchpoint_remove_all(s, BP_CPU);
 
 cr4 = 0;
 xcr0 = XSTATE_FP_MASK;
@@ -6205,6 +6204,8 @@ static void x86_cpu_reset(DeviceState *dev)
 if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_FSGSBASE) {
 cr4 |= CR4_FSGSBASE_MASK;
 }
+#else
+cpu_watchpoint_remove_all(s, BP_CPU);
 #endif
 
 env->xcr0 = xcr0;
-- 
2.26.2

[PATCH v4 22/28] target/arm: Restrict watchpoint code to system emulation

[Philippe Mathieu-Daudé]

We can not use watchpoints in user-mode emulation because we
need the softmmu slow path to detect accesses to watchpointed
memory. Add #ifdef'ry around it.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/internals.h|  2 ++
 target/arm/cpu.c  |  4 ++--
 target/arm/debug_helper.c |  8 
 target/arm/helper.c   |  4 
 target/arm/sve_helper.c   | 12 
 5 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 3fb295431ae..8fa0a244d59 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -263,6 +263,7 @@ static inline bool extended_addresses_enabled(CPUARMState 
*env)
(arm_feature(env, ARM_FEATURE_LPAE) && (tcr->raw_tcr & TTBCR_EAE));
 }
 
+#ifndef CONFIG_USER_ONLY
 /*
  * Update a QEMU watchpoint based on the information the guest has set in the
  * DBGWCR_EL1 and DBGWVR_EL1 registers.
@@ -286,6 +287,7 @@ bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint 
*wp);
  * addresses.
  */
 vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len);
+#endif /* !CONFIG_USER_ONLY */
 
 void hw_breakpoint_update(ARMCPU *cpu, int n);
 /*
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 09566a535e5..efc338b24eb 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -432,10 +432,10 @@ static void arm_cpu_reset(DeviceState *dev)
 if (kvm_enabled()) {
 kvm_arm_reset_vcpu(cpu);
 }
-#endif
 
-hw_breakpoint_update_all(cpu);
 hw_watchpoint_update_all(cpu);
+#endif
+hw_breakpoint_update_all(cpu);
 arm_rebuild_hflags(env);
 }
 
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 980110e1328..b8b7d81762d 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -11,6 +11,8 @@
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
 
+#ifndef CONFIG_USER_ONLY
+
 /* Return true if the linked breakpoint entry lbn passes its checks */
 static bool linked_bp_matches(ARMCPU *cpu, int lbn)
 {
@@ -227,6 +229,8 @@ bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint 
*wp)
 return check_watchpoints(cpu);
 }
 
+#endif /* !CONFIG_USER_ONLY */
+
 static bool check_breakpoints(ARMCPU *cpu)
 {
 CPUARMState *env = &cpu->env;
@@ -240,11 +244,13 @@ static bool check_breakpoints(ARMCPU *cpu)
 return false;
 }
 
+#ifndef CONFIG_USER_ONLY
 for (int n = 0; n < ARRAY_SIZE(env->cpu_breakpoint); n++) {
 if (bp_wp_matches(cpu, n, false)) {
 return true;
 }
 }
+#endif /* !CONFIG_USER_ONLY */
 return false;
 }
 
@@ -267,6 +273,7 @@ void arm_debug_excp_handler(CPUState *cs)
 CPUARMState *env = &cpu->env;
 uint64_t pc;
 bool same_el;
+#ifndef CONFIG_USER_ONLY
 CPUWatchpoint *wp_hit = cs->watchpoint_hit;
 
 if (wp_hit && (wp_hit->flags & BP_CPU)) {
@@ -282,6 +289,7 @@ void arm_debug_excp_handler(CPUState *cs)
 arm_debug_target_el(env));
 return;
 }
+#endif /* !CONFIG_USER_ONLY */
 
 pc = is_a64(env) ? env->pc : env->regs[15];
 same_el = (arm_debug_target_el(env) == arm_current_el(env));
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 54648c7fbb6..2e7a6356ae3 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6345,6 +6345,7 @@ static const ARMCPRegInfo zcr_el3_reginfo = {
 .writefn = zcr_write, .raw_writefn = raw_write
 };
 
+#ifndef CONFIG_USER_ONLY
 void hw_watchpoint_update(ARMCPU *cpu, int n)
 {
 CPUARMState *env = &cpu->env;
@@ -6470,6 +6471,7 @@ static void dbgwcr_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 raw_write(env, ri, value);
 hw_watchpoint_update(cpu, i);
 }
+#endif /* !CONFIG_USER_ONLY */
 
 void hw_breakpoint_update(ARMCPU *cpu, int n)
 {
@@ -6643,6 +6645,7 @@ static void define_debug_regs(ARMCPU *cpu)
 define_arm_cp_regs(cpu, dbgregs);
 }
 
+#ifndef CONFIG_USER_ONLY
 for (i = 0; i < arm_num_wrps(cpu); i++) {
 ARMCPRegInfo dbgregs[] = {
 { .name = "DBGWVR", .state = ARM_CP_STATE_BOTH,
@@ -6661,6 +6664,7 @@ static void define_debug_regs(ARMCPU *cpu)
 };
 define_arm_cp_regs(cpu, dbgregs);
 }
+#endif /* !CONFIG_USER_ONLY */
 }
 
 static void define_pmu_regs(ARMCPU *cpu)
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 844db08bd57..ed3f22d78a5 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4849,6 +4849,7 @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const 
target_ulong addr,
 /* Some page is MMIO, see below. */
 goto do_fault;
 }
+#ifndef CONFIG_USER_ONLY
 if (unlikely(flags & TLB_WATCHPOINT) &&
 (cpu_watchpoint_address_matches
  (env_cpu(env), addr + mem_off, 1 << msz)
@@ -4856,6 +4857,7 @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const 
target_ulong addr,
 /* Watchpoint hit, see below. */
 goto do_fault;
 }
+#

Re: [PATCH v2] vhost-user.rst: add clarifying language about protocol negotiation

[Michael S. Tsirkin]

On Wed, Mar 03, 2021 at 02:50:11PM +, Alex Bennée wrote:
> Make the language about feature negotiation explicitly clear about the
> handling of the VHOST_USER_F_PROTOCOL_FEATURES feature bit. Try and
> avoid the sort of bug introduced in vhost.rs REPLY_ACK processing:
> 
>   https://github.com/rust-vmm/vhost/pull/24
> 
> Signed-off-by: Alex Bennée 
> Cc: Jiang Liu 
> Message-Id: <20210226111619.21178-1-alex.ben...@linaro.org>
> 
> ---
> v2
>   - use Stefan's suggested wording
>   - Be super explicit in the message descriptions
> ---
>  docs/interop/vhost-user.rst | 18 --
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index 2918d7c757..7c1fb8c209 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -307,6 +307,18 @@ bit was dedicated for this purpose::
>  
>#define VHOST_USER_F_PROTOCOL_FEATURES 30
>  
> +Note that VHOST_USER_F_PROTOCOL_FEATURES is the UNUSED (30) feature
> +bit defined in `VIRTIO 1.1 6.3 Legacy Interface: Reserved Feature Bits
> +`_.
> +VIRTIO devices do not advertise this feature bit and therefore VIRTIO
> +drivers cannot negotiate it.
> +
> +This reserved feature bit was reused by the vhost-user protocol to add
> +vhost-user protocol feature negotiation in a backwards compatible
> +fashion. Old vhost-user master and slave implementations continue to
> +work even though they are not aware of vhost-user protocol feature
> +negotiation.
> +
>  Ring states
>  ---
>  
> @@ -865,7 +877,8 @@ Front-end message types
>Get the protocol feature bitmask from the underlying vhost
>implementation.  Only legal if feature bit
>``VHOST_USER_F_PROTOCOL_FEATURES`` is present in
> -  ``VHOST_USER_GET_FEATURES``.
> +  ``VHOST_USER_GET_FEATURES``.  It does not need to be acknowledged by
> +  ``VHOST_USER_SET_FEATURES``.
>  
>  .. Note::
> Back-ends that report ``VHOST_USER_F_PROTOCOL_FEATURES`` must
> @@ -881,7 +894,8 @@ Front-end message types
>Enable protocol features in the underlying vhost implementation.
>  
>Only legal if feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` is present in
> -  ``VHOST_USER_GET_FEATURES``.
> +  ``VHOST_USER_GET_FEATURES``.  It does not need to be acknowledged by
> +  ``VHOST_USER_SET_FEATURES``.
>  
>  .. Note::
> Back-ends that report ``VHOST_USER_F_PROTOCOL_FEATURES`` must support


Not really clear what does "It" refer to here.
Also, are we sure it's ok to send the messages and then send
VHOST_USER_SET_FEATURES with VHOST_USER_F_PROTOCOL_FEATURES clear?
Looks more like a violation to me ...


How about: It -> this bit
does not need to be -> before ... has been

so:

Only legal if feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` is present in
 -  ``VHOST_USER_GET_FEATURES``, and even before this bit has been
acknowledged by VHOST_USER_SET_FEATURES.




> -- 
> 2.20.1

[PATCH v4 28/28] cpu: Move sysemu specific declarations to 'sysemu-cpu-ops.h'

[Philippe Mathieu-Daudé]

All these prototypes and declarations don't need to be exposed
on user-mode emulation. Move them to "sysemu-cpu-ops.h".

Suggested-by: Claudio Fontana 
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 164 ---
 include/hw/core/sysemu-cpu-ops.h | 155 +
 gdbstub.c|   1 +
 softmmu/physmem.c|   1 +
 4 files changed, 157 insertions(+), 164 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 79dcc9a4e42..53598c02187 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -185,15 +185,6 @@ typedef struct CPUBreakpoint {
 QTAILQ_ENTRY(CPUBreakpoint) entry;
 } CPUBreakpoint;
 
-struct CPUWatchpoint {
-vaddr vaddr;
-vaddr len;
-vaddr hitaddr;
-MemTxAttrs hitattrs;
-int flags; /* BP_* */
-QTAILQ_ENTRY(CPUWatchpoint) entry;
-};
-
 #ifdef CONFIG_PLUGIN
 /*
  * For plugins we sometime need to save the resolved iotlb data before
@@ -428,76 +419,6 @@ static inline void cpu_tb_jmp_cache_clear(CPUState *cpu)
 extern bool mttcg_enabled;
 #define qemu_tcg_mttcg_enabled() (mttcg_enabled)
 
-/**
- * cpu_paging_enabled:
- * @cpu: The CPU whose state is to be inspected.
- *
- * Returns: %true if paging is enabled, %false otherwise.
- */
-bool cpu_paging_enabled(const CPUState *cpu);
-
-/**
- * cpu_get_memory_mapping:
- * @cpu: The CPU whose memory mappings are to be obtained.
- * @list: Where to write the memory mappings to.
- * @errp: Pointer for reporting an #Error.
- */
-void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
-Error **errp);
-
-#if !defined(CONFIG_USER_ONLY)
-
-/**
- * cpu_write_elf64_note:
- * @f: pointer to a function that writes memory to a file
- * @cpu: The CPU whose memory is to be dumped
- * @cpuid: ID number of the CPU
- * @opaque: pointer to the CPUState struct
- */
-int cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cpu,
- int cpuid, void *opaque);
-
-/**
- * cpu_write_elf64_qemunote:
- * @f: pointer to a function that writes memory to a file
- * @cpu: The CPU whose memory is to be dumped
- * @cpuid: ID number of the CPU
- * @opaque: pointer to the CPUState struct
- */
-int cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
- void *opaque);
-
-/**
- * cpu_write_elf32_note:
- * @f: pointer to a function that writes memory to a file
- * @cpu: The CPU whose memory is to be dumped
- * @cpuid: ID number of the CPU
- * @opaque: pointer to the CPUState struct
- */
-int cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cpu,
- int cpuid, void *opaque);
-
-/**
- * cpu_write_elf32_qemunote:
- * @f: pointer to a function that writes memory to a file
- * @cpu: The CPU whose memory is to be dumped
- * @cpuid: ID number of the CPU
- * @opaque: pointer to the CPUState struct
- */
-int cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
- void *opaque);
-
-/**
- * cpu_get_crash_info:
- * @cpu: The CPU to get crash information for
- *
- * Gets the previously saved crash information.
- * Caller is responsible for freeing the data.
- */
-GuestPanicInformation *cpu_get_crash_info(CPUState *cpu);
-
-#endif /* !CONFIG_USER_ONLY */
-
 /**
  * CPUDumpFlags:
  * @CPU_DUMP_CODE:
@@ -529,56 +450,6 @@ void cpu_dump_state(CPUState *cpu, FILE *f, int flags);
  */
 void cpu_dump_statistics(CPUState *cpu, int flags);
 
-#ifndef CONFIG_USER_ONLY
-/**
- * cpu_get_phys_page_attrs_debug:
- * @cpu: The CPU to obtain the physical page address for.
- * @addr: The virtual address.
- * @attrs: Updated on return with the memory transaction attributes to use
- * for this access.
- *
- * Obtains the physical page corresponding to a virtual one, together
- * with the corresponding memory transaction attributes to use for the access.
- * Use it only for debugging because no protection checks are done.
- *
- * Returns: Corresponding physical page address or -1 if no page found.
- */
-hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
- MemTxAttrs *attrs);
-
-/**
- * cpu_get_phys_page_debug:
- * @cpu: The CPU to obtain the physical page address for.
- * @addr: The virtual address.
- *
- * Obtains the physical page corresponding to a virtual one.
- * Use it only for debugging because no protection checks are done.
- *
- * Returns: Corresponding physical page address or -1 if no page found.
- */
-hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
-
-/**
- * cpu_asidx_from_attrs:
- * @cpu: CPU
- * @attrs: memory transaction attributes
- *
- * Returns the address space index specifying the CPU AddressSpace
- * to use for a memory access with the given transaction attributes.
- */
-int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs);
-
-/**
- * cpu_virtio_is_big_endian:
- * @cpu: CPU
-
- * Returns %true if a CPU wh

[PATCH v4 16/28] cpu: Restrict "hw/core/sysemu-cpu-ops.h" to target/cpu.c

[Philippe Mathieu-Daudé]

Somehow similar to commit 78271684719 ("cpu: tcg_ops: move to
tcg-cpu-ops.h, keep a pointer in CPUClass"):

We cannot in principle make the SysEmu Operations field definitions
conditional on CONFIG_SOFTMMU in code that is included by both
common_ss and specific_ss modules.

Therefore, what we can do safely to restrict the SysEmu fields to
system emulation builds, is to move all sysemu operations into a
separate header file, which is only included by system-specific code.

This leaves just a NULL pointer in the cpu.h for the user-mode builds.

Inspired-by: Claudio Fontana 
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h   | 3 ++-
 target/alpha/cpu.h  | 3 +++
 target/arm/cpu.h| 3 +++
 target/avr/cpu.h| 1 +
 target/cris/cpu.h   | 3 +++
 target/hexagon/cpu.h| 3 +++
 target/hppa/cpu.h   | 3 +++
 target/i386/cpu.h   | 3 +++
 target/lm32/cpu.h   | 3 +++
 target/m68k/cpu.h   | 3 +++
 target/microblaze/cpu.h | 1 +
 target/mips/cpu.h   | 3 +++
 target/moxie/cpu.h  | 3 +++
 target/nios2/cpu.h  | 1 +
 target/openrisc/cpu.h   | 3 +++
 target/ppc/cpu.h| 3 +++
 target/riscv/cpu.h  | 3 +++
 target/rx/cpu.h | 1 +
 target/s390x/cpu.h  | 3 +++
 target/sh4/cpu.h| 3 +++
 target/sparc/cpu.h  | 3 +++
 target/tilegx/cpu.h | 3 +++
 target/tricore/cpu.h| 3 +++
 target/unicore32/cpu.h  | 3 +++
 target/xtensa/cpu.h | 3 +++
 cpu.c   | 1 +
 hw/core/cpu.c   | 1 +
 27 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 960846d2b64..fe4206b540f 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -80,7 +80,8 @@ struct TCGCPUOps;
 /* see accel-cpu.h */
 struct AccelCPUClass;
 
-#include "hw/core/sysemu-cpu-ops.h"
+/* see sysemu-cpu-ops.h */
+struct SysemuCPUOps;
 
 /**
  * CPUClass:
diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
index 82df108967b..f1218a27706 100644
--- a/target/alpha/cpu.h
+++ b/target/alpha/cpu.h
@@ -22,6 +22,9 @@
 
 #include "cpu-qom.h"
 #include "exec/cpu-defs.h"
+#ifndef CONFIG_USER_ONLY
+#include "hw/core/sysemu-cpu-ops.h"
+#endif
 
 /* Alpha processors have a weak memory model */
 #define TCG_GUEST_DEFAULT_MO  (0)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index efa1618c4d5..265d00d55dd 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -25,6 +25,9 @@
 #include "cpu-qom.h"
 #include "exec/cpu-defs.h"
 #include "qapi/qapi-types-common.h"
+#ifndef CONFIG_USER_ONLY
+#include "hw/core/sysemu-cpu-ops.h"
+#endif
 
 /* ARM processors have a weak memory model */
 #define TCG_GUEST_DEFAULT_MO  (0)
diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index d148e8c75a4..e0419649fa7 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -23,6 +23,7 @@
 
 #include "cpu-qom.h"
 #include "exec/cpu-defs.h"
+#include "hw/core/sysemu-cpu-ops.h"
 
 #ifdef CONFIG_USER_ONLY
 #error "AVR 8-bit does not support user mode"
diff --git a/target/cris/cpu.h b/target/cris/cpu.h
index d3b64929096..4450f2268ea 100644
--- a/target/cris/cpu.h
+++ b/target/cris/cpu.h
@@ -23,6 +23,9 @@
 
 #include "cpu-qom.h"
 #include "exec/cpu-defs.h"
+#ifndef CONFIG_USER_ONLY
+#include "hw/core/sysemu-cpu-ops.h"
+#endif
 
 #define EXCP_NMI1
 #define EXCP_GURU   2
diff --git a/target/hexagon/cpu.h b/target/hexagon/cpu.h
index e04eac591c8..2a878e77f08 100644
--- a/target/hexagon/cpu.h
+++ b/target/hexagon/cpu.h
@@ -26,6 +26,9 @@ typedef struct CPUHexagonState CPUHexagonState;
 #include "qemu-common.h"
 #include "exec/cpu-defs.h"
 #include "hex_regs.h"
+#ifndef CONFIG_USER_ONLY
+#include "hw/core/sysemu-cpu-ops.h"
+#endif
 
 #define NUM_PREGS 4
 #define TOTAL_PER_THREAD_REGS 64
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 61178fa6a2a..94d2d4701c4 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -23,6 +23,9 @@
 #include "cpu-qom.h"
 #include "exec/cpu-defs.h"
 #include "exec/memory.h"
+#ifndef CONFIG_USER_ONLY
+#include "hw/core/sysemu-cpu-ops.h"
+#endif
 
 /* PA-RISC 1.x processors have a strong memory model.  */
 /* ??? While we do not yet implement PA-RISC 2.0, those processors have
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index b4b136cd0d1..06965f11100 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -25,6 +25,9 @@
 #include "kvm/hyperv-proto.h"
 #include "exec/cpu-defs.h"
 #include "qapi/qapi-types-common.h"
+#ifndef CONFIG_USER_ONLY
+#include "hw/core/sysemu-cpu-ops.h"
+#endif
 
 /* The x86 has a strong memory model with some store-after-load re-ordering */
 #define TCG_GUEST_DEFAULT_MO  (TCG_MO_ALL & ~TCG_MO_ST_LD)
diff --git a/target/lm32/cpu.h b/target/lm32/cpu.h
index ea7c01ca8b0..034183dad30 100644
--- a/target/lm32/cpu.h
+++ b/target/lm32/cpu.h
@@ -22,6 +22,9 @@
 
 #include "cpu-qom.h"
 #include "exec/cpu-defs.h"
+#ifndef CONFIG_USER_ONLY
+#include "hw/core/sysemu-cpu-ops.h"
+#endif
 
 typedef struct CPULM32State CPULM32State;
 
diff --git a/target/m68k/cpu.h b/ta

[PATCH v4 25/28] accel/tcg/cpu-exec: Restrict watchpoint code to system emulation

[Philippe Mathieu-Daudé]

We can not use watchpoints in user-mode emulation because we
need the softmmu slow path to detect accesses to watchpointed
memory. Add #ifdef'ry around it.

Signed-off-by: Philippe Mathieu-Daudé 
---
 accel/tcg/cpu-exec.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 16e4fe3ccd8..3a6436013ea 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -471,13 +471,16 @@ static inline bool cpu_handle_halt(CPUState *cpu)
 static inline void cpu_handle_debug_exception(CPUState *cpu)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
-CPUWatchpoint *wp;
 
+#ifndef CONFIG_USER_ONLY
 if (!cpu->watchpoint_hit) {
+CPUWatchpoint *wp;
+
 QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
 wp->flags &= ~BP_WATCHPOINT_HIT;
 }
 }
+#endif
 
 if (cc->tcg_ops->debug_excp_handler) {
 cc->tcg_ops->debug_excp_handler(cpu);
-- 
2.26.2

[PATCH v4 26/28] cpu: Remove watchpoint stubs for user emulation

[Philippe Mathieu-Daudé]

Since we remove all access to the watchpoint methods from user-mode
code, we can now remove them, as they are not used anymore.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h | 34 +-
 1 file changed, 1 insertion(+), 33 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index fe4206b540f..b708f365a7a 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -916,39 +916,7 @@ static inline bool cpu_breakpoint_test(CPUState *cpu, 
vaddr pc, int mask)
 return false;
 }
 
-#ifdef CONFIG_USER_ONLY
-static inline int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
-int flags, CPUWatchpoint **watchpoint)
-{
-return -ENOSYS;
-}
-
-static inline int cpu_watchpoint_remove(CPUState *cpu, vaddr addr,
-vaddr len, int flags)
-{
-return -ENOSYS;
-}
-
-static inline void cpu_watchpoint_remove_by_ref(CPUState *cpu,
-CPUWatchpoint *wp)
-{
-}
-
-static inline void cpu_watchpoint_remove_all(CPUState *cpu, int mask)
-{
-}
-
-static inline void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
-MemTxAttrs atr, int fl, uintptr_t ra)
-{
-}
-
-static inline int cpu_watchpoint_address_matches(CPUState *cpu,
- vaddr addr, vaddr len)
-{
-return 0;
-}
-#else
+#ifndef CONFIG_USER_ONLY
 int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
   int flags, CPUWatchpoint **watchpoint);
 int cpu_watchpoint_remove(CPUState *cpu, vaddr addr,
-- 
2.26.2

[PATCH v4 14/28] cpu: Move CPUClass::get_memory_mapping to SysemuCPUOps

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 3 ---
 include/hw/core/sysemu-cpu-ops.h | 5 +
 hw/core/cpu.c| 4 ++--
 target/i386/cpu.c| 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 9a86c707cf7..8af78cdde23 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -94,7 +94,6 @@ struct AccelCPUClass;
  * @dump_statistics: Callback for dumping statistics.
  * @get_arch_id: Callback for getting architecture-dependent CPU ID.
  * @get_paging_enabled: Callback for inquiring whether paging is enabled.
- * @get_memory_mapping: Callback for obtaining the memory mappings.
  * @set_pc: Callback for setting the Program Counter register. This
  *   should have the semantics used by the target architecture when
  *   setting the PC from a source such as an ELF file entry point;
@@ -138,8 +137,6 @@ struct CPUClass {
 void (*dump_statistics)(CPUState *cpu, int flags);
 int64_t (*get_arch_id)(CPUState *cpu);
 bool (*get_paging_enabled)(const CPUState *cpu);
-void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
-   Error **errp);
 void (*set_pc)(CPUState *cpu, vaddr value);
 int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
 int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 0c8f616a565..460e7d63b0c 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,11 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @get_memory_mapping: Callback for obtaining the memory mappings.
+ */
+void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
+   Error **errp);
 /**
  * @get_phys_page_debug: Callback for obtaining a physical address.
  */
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 6932781425a..339bdfadd7a 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -83,8 +83,8 @@ void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList 
*list,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (cc->get_memory_mapping) {
-cc->get_memory_mapping(cpu, list, errp);
+if (cc->sysemu_ops->get_memory_mapping) {
+cc->sysemu_ops->get_memory_mapping(cpu, list, errp);
 return;
 }
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0de4ee18bee..b2d3debd1f2 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7388,6 +7388,7 @@ static Property x86_cpu_properties[] = {
 
 #ifndef CONFIG_USER_ONLY
 static const struct SysemuCPUOps i386_sysemu_ops = {
+.get_memory_mapping = x86_cpu_get_memory_mapping,
 .get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug,
 .asidx_from_attrs = x86_asidx_from_attrs,
 .get_crash_info = x86_cpu_get_crash_info,
@@ -7431,7 +7432,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->get_paging_enabled = x86_cpu_get_paging_enabled;
 
 #ifndef CONFIG_USER_ONLY
-cc->get_memory_mapping = x86_cpu_get_memory_mapping;
 cc->sysemu_ops = &i386_sysemu_ops;
 #endif /* !CONFIG_USER_ONLY */
 
-- 
2.26.2

[PATCH v4 27/28] cpu: Fix code style for checkpatch.pl

[Philippe Mathieu-Daudé]

We are going to move this code, fix its style first.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index b708f365a7a..79dcc9a4e42 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -558,7 +558,8 @@ hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr 
addr,
  */
 hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 
-/** cpu_asidx_from_attrs:
+/**
+ * cpu_asidx_from_attrs:
  * @cpu: CPU
  * @attrs: memory transaction attributes
  *
-- 
2.26.2

[PATCH v4 24/28] target/xtensa: Restrict watchpoint code to system emulation

[Philippe Mathieu-Daudé]

We can not use watchpoints in user-mode emulation because we
need the softmmu slow path to detect accesses to watchpointed
memory. Add #ifdef'ry around it.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/xtensa/helper.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
index eeffee297d1..85e466b37d2 100644
--- a/target/xtensa/helper.c
+++ b/target/xtensa/helper.c
@@ -201,6 +201,7 @@ void xtensa_register_core(XtensaConfigList *node)
 g_free((gpointer)type.name);
 }
 
+#ifndef CONFIG_USER_ONLY
 static uint32_t check_hw_breakpoints(CPUXtensaState *env)
 {
 unsigned i;
@@ -213,9 +214,11 @@ static uint32_t check_hw_breakpoints(CPUXtensaState *env)
 }
 return 0;
 }
+#endif
 
 void xtensa_breakpoint_handler(CPUState *cs)
 {
+#ifndef CONFIG_USER_ONLY
 XtensaCPU *cpu = XTENSA_CPU(cs);
 CPUXtensaState *env = &cpu->env;
 
@@ -231,6 +234,7 @@ void xtensa_breakpoint_handler(CPUState *cs)
 cpu_loop_exit_noexc(cs);
 }
 }
+#endif
 }
 
 void xtensa_cpu_list(void)
-- 
2.26.2

[PATCH v4 19/28] target/arm/internals: Fix code style for checkpatch.pl

[Philippe Mathieu-Daudé]

We are going to move this code, fix its style first.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/internals.h | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 05cebc8597c..d6ace004855 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -251,7 +251,8 @@ static inline unsigned int arm_pamax(ARMCPU *cpu)
 return pamax_map[parange];
 }
 
-/* Return true if extended addresses are enabled.
+/*
+ * Return true if extended addresses are enabled.
  * This is always the case if our translation regime is 64 bit,
  * but depends on TTBCR.EAE for 32 bit.
  */
@@ -262,20 +263,24 @@ static inline bool extended_addresses_enabled(CPUARMState 
*env)
(arm_feature(env, ARM_FEATURE_LPAE) && (tcr->raw_tcr & TTBCR_EAE));
 }
 
-/* Update a QEMU watchpoint based on the information the guest has set in the
+/*
+ * Update a QEMU watchpoint based on the information the guest has set in the
  * DBGWCR_EL1 and DBGWVR_EL1 registers.
  */
 void hw_watchpoint_update(ARMCPU *cpu, int n);
-/* Update the QEMU watchpoints for every guest watchpoint. This does a
+/*
+ * Update the QEMU watchpoints for every guest watchpoint. This does a
  * complete delete-and-reinstate of the QEMU watchpoint list and so is
  * suitable for use after migration or on reset.
  */
 void hw_watchpoint_update_all(ARMCPU *cpu);
-/* Update a QEMU breakpoint based on the information the guest has set in the
+/*
+ * Update a QEMU breakpoint based on the information the guest has set in the
  * DBGBCR_EL1 and DBGBVR_EL1 registers.
  */
 void hw_breakpoint_update(ARMCPU *cpu, int n);
-/* Update the QEMU breakpoints for every guest breakpoint. This does a
+/*
+ * Update the QEMU breakpoints for every guest breakpoint. This does a
  * complete delete-and-reinstate of the QEMU breakpoint list and so is
  * suitable for use after migration or on reset.
  */
@@ -284,7 +289,8 @@ void hw_breakpoint_update_all(ARMCPU *cpu);
 /* Callback function for checking if a watchpoint should trigger. */
 bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
 
-/* Adjust addresses (in BE32 mode) before testing against watchpoint
+/*
+ * Adjust addresses (in BE32 mode) before testing against watchpoint
  * addresses.
  */
 vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len);
-- 
2.26.2

[PATCH v4 12/28] cpu: Move CPUClass::asidx_from_attrs to SysemuCPUOps

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 3 ---
 include/hw/core/sysemu-cpu-ops.h | 5 +
 hw/core/cpu.c| 4 ++--
 target/arm/cpu.c | 2 +-
 target/i386/cpu.c| 2 +-
 5 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 0a2c29c3735..6713a615916 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -108,8 +108,6 @@ struct AccelCPUClass;
  *   associated memory transaction attributes to use for the access.
  *   CPUs which use memory transaction attributes should implement this
  *   instead of get_phys_page_debug.
- * @asidx_from_attrs: Callback to return the CPU AddressSpace to use for
- *   a memory access with the specified memory transaction attributes.
  * @gdb_read_register: Callback for letting GDB read a register.
  * @gdb_write_register: Callback for letting GDB write a register.
  * @gdb_num_core_regs: Number of core registers accessible to GDB.
@@ -151,7 +149,6 @@ struct CPUClass {
 hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
 hwaddr (*get_phys_page_attrs_debug)(CPUState *cpu, vaddr addr,
 MemTxAttrs *attrs);
-int (*asidx_from_attrs)(CPUState *cpu, MemTxAttrs attrs);
 int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
 int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
 
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 60c667801ef..3c3f211136d 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,11 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @asidx_from_attrs: Callback to return the CPU AddressSpace to use for
+ *   a memory access with the specified memory transaction attributes.
+ */
+int (*asidx_from_attrs)(CPUState *cpu, MemTxAttrs attrs);
 /**
  * @get_crash_info: Callback for reporting guest crash information in
  * GUEST_PANICKED events.
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index c74390aafbf..c44229205ff 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -116,8 +116,8 @@ int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs)
 CPUClass *cc = CPU_GET_CLASS(cpu);
 int ret = 0;
 
-if (cc->asidx_from_attrs) {
-ret = cc->asidx_from_attrs(cpu, attrs);
+if (cc->sysemu_ops->asidx_from_attrs) {
+ret = cc->sysemu_ops->asidx_from_attrs(cpu, attrs);
 assert(ret < cpu->num_ases && ret >= 0);
 }
 return ret;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 0c06b9b1758..7edb9f581bc 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2262,6 +2262,7 @@ static gchar *arm_gdb_arch_name(CPUState *cs)
 
 #ifndef CONFIG_USER_ONLY
 static const struct SysemuCPUOps arm_sysemu_ops = {
+.asidx_from_attrs = arm_asidx_from_attrs,
 .write_elf32_note = arm_cpu_write_elf32_note,
 .write_elf64_note = arm_cpu_write_elf64_note,
 .virtio_is_big_endian = arm_cpu_virtio_is_big_endian,
@@ -2307,7 +2308,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->gdb_write_register = arm_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_attrs_debug = arm_cpu_get_phys_page_attrs_debug;
-cc->asidx_from_attrs = arm_asidx_from_attrs;
 cc->sysemu_ops = &arm_sysemu_ops;
 #endif
 cc->gdb_num_core_regs = 26;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 403ed65fc61..4f64f9c2d68 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7388,6 +7388,7 @@ static Property x86_cpu_properties[] = {
 
 #ifndef CONFIG_USER_ONLY
 static const struct SysemuCPUOps i386_sysemu_ops = {
+.asidx_from_attrs = x86_asidx_from_attrs,
 .get_crash_info = x86_cpu_get_crash_info,
 .write_elf32_note = x86_cpu_write_elf32_note,
 .write_elf64_note = x86_cpu_write_elf64_note,
@@ -7429,7 +7430,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->get_paging_enabled = x86_cpu_get_paging_enabled;
 
 #ifndef CONFIG_USER_ONLY
-cc->asidx_from_attrs = x86_asidx_from_attrs;
 cc->get_memory_mapping = x86_cpu_get_memory_mapping;
 cc->get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug;
 cc->sysemu_ops = &i386_sysemu_ops;
-- 
2.26.2

[PATCH v4 20/28] target/arm: Move code blocks around

[Philippe Mathieu-Daudé]

To simplify later #ifdef'ry, move some code around.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/internals.h| 16 
 target/arm/debug_helper.c | 22 +++---
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index d6ace004855..3fb295431ae 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -278,14 +278,6 @@ void hw_watchpoint_update_all(ARMCPU *cpu);
  * Update a QEMU breakpoint based on the information the guest has set in the
  * DBGBCR_EL1 and DBGBVR_EL1 registers.
  */
-void hw_breakpoint_update(ARMCPU *cpu, int n);
-/*
- * Update the QEMU breakpoints for every guest breakpoint. This does a
- * complete delete-and-reinstate of the QEMU breakpoint list and so is
- * suitable for use after migration or on reset.
- */
-void hw_breakpoint_update_all(ARMCPU *cpu);
-
 /* Callback function for checking if a watchpoint should trigger. */
 bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
 
@@ -295,6 +287,14 @@ bool arm_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp);
  */
 vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len);
 
+void hw_breakpoint_update(ARMCPU *cpu, int n);
+/*
+ * Update the QEMU breakpoints for every guest breakpoint. This does a
+ * complete delete-and-reinstate of the QEMU breakpoint list and so is
+ * suitable for use after migration or on reset.
+ */
+void hw_breakpoint_update_all(ARMCPU *cpu);
+
 /* Callback function for when a watchpoint or breakpoint triggers. */
 void arm_debug_excp_handler(CPUState *cs);
 
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 2ff72d47d19..c01d8524443 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -216,6 +216,17 @@ static bool check_watchpoints(ARMCPU *cpu)
 return false;
 }
 
+bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
+{
+/*
+ * Called by core code when a CPU watchpoint fires; need to check if this
+ * is also an architectural watchpoint match.
+ */
+ARMCPU *cpu = ARM_CPU(cs);
+
+return check_watchpoints(cpu);
+}
+
 static bool check_breakpoints(ARMCPU *cpu)
 {
 CPUARMState *env = &cpu->env;
@@ -247,17 +258,6 @@ void HELPER(check_breakpoints)(CPUARMState *env)
 }
 }
 
-bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
-{
-/*
- * Called by core code when a CPU watchpoint fires; need to check if this
- * is also an architectural watchpoint match.
- */
-ARMCPU *cpu = ARM_CPU(cs);
-
-return check_watchpoints(cpu);
-}
-
 void arm_debug_excp_handler(CPUState *cs)
 {
 /*
-- 
2.26.2

[PATCH v4 21/28] target/arm: Refactor some function bodies

[Philippe Mathieu-Daudé]

Refactor few fonctions body to ease #ifdef'ry review
in the next commit. No logical change intented.

Signed-off-by: Philippe Mathieu-Daudé 
---
Patch easier to review using:
'git-diff --color-moved-ws=allow-indentation-change'
---
 target/arm/debug_helper.c | 72 +++
 target/arm/helper.c   |  5 ++-
 2 files changed, 38 insertions(+), 39 deletions(-)

diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index c01d8524443..980110e1328 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -230,7 +230,6 @@ bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint 
*wp)
 static bool check_breakpoints(ARMCPU *cpu)
 {
 CPUARMState *env = &cpu->env;
-int n;
 
 /*
  * If breakpoints are disabled globally or we can't take debug
@@ -241,7 +240,7 @@ static bool check_breakpoints(ARMCPU *cpu)
 return false;
 }
 
-for (n = 0; n < ARRAY_SIZE(env->cpu_breakpoint); n++) {
+for (int n = 0; n < ARRAY_SIZE(env->cpu_breakpoint); n++) {
 if (bp_wp_matches(cpu, n, false)) {
 return true;
 }
@@ -266,47 +265,48 @@ void arm_debug_excp_handler(CPUState *cs)
  */
 ARMCPU *cpu = ARM_CPU(cs);
 CPUARMState *env = &cpu->env;
+uint64_t pc;
+bool same_el;
 CPUWatchpoint *wp_hit = cs->watchpoint_hit;
 
-if (wp_hit) {
-if (wp_hit->flags & BP_CPU) {
-bool wnr = (wp_hit->flags & BP_WATCHPOINT_HIT_WRITE) != 0;
-bool same_el = arm_debug_target_el(env) == arm_current_el(env);
+if (wp_hit && (wp_hit->flags & BP_CPU)) {
+bool wnr = (wp_hit->flags & BP_WATCHPOINT_HIT_WRITE) != 0;
+bool same_el = arm_debug_target_el(env) == arm_current_el(env);
 
-cs->watchpoint_hit = NULL;
-
-env->exception.fsr = arm_debug_exception_fsr(env);
-env->exception.vaddress = wp_hit->hitaddr;
-raise_exception(env, EXCP_DATA_ABORT,
-syn_watchpoint(same_el, 0, wnr),
-arm_debug_target_el(env));
-}
-} else {
-uint64_t pc = is_a64(env) ? env->pc : env->regs[15];
-bool same_el = (arm_debug_target_el(env) == arm_current_el(env));
-
-/*
- * (1) GDB breakpoints should be handled first.
- * (2) Do not raise a CPU exception if no CPU breakpoint has fired,
- * since singlestep is also done by generating a debug internal
- * exception.
- */
-if (cpu_breakpoint_test(cs, pc, BP_GDB)
-|| !cpu_breakpoint_test(cs, pc, BP_CPU)) {
-return;
-}
+cs->watchpoint_hit = NULL;
 
 env->exception.fsr = arm_debug_exception_fsr(env);
-/*
- * FAR is UNKNOWN: clear vaddress to avoid potentially exposing
- * values to the guest that it shouldn't be able to see at its
- * exception/security level.
- */
-env->exception.vaddress = 0;
-raise_exception(env, EXCP_PREFETCH_ABORT,
-syn_breakpoint(same_el),
+env->exception.vaddress = wp_hit->hitaddr;
+raise_exception(env, EXCP_DATA_ABORT,
+syn_watchpoint(same_el, 0, wnr),
 arm_debug_target_el(env));
+return;
 }
+
+pc = is_a64(env) ? env->pc : env->regs[15];
+same_el = (arm_debug_target_el(env) == arm_current_el(env));
+
+/*
+ * (1) GDB breakpoints should be handled first.
+ * (2) Do not raise a CPU exception if no CPU breakpoint has fired,
+ * since singlestep is also done by generating a debug internal
+ * exception.
+ */
+if (cpu_breakpoint_test(cs, pc, BP_GDB)
+|| !cpu_breakpoint_test(cs, pc, BP_CPU)) {
+return;
+}
+
+env->exception.fsr = arm_debug_exception_fsr(env);
+/*
+ * FAR is UNKNOWN: clear vaddress to avoid potentially exposing
+ * values to the guest that it shouldn't be able to see at its
+ * exception/security level.
+ */
+env->exception.vaddress = 0;
+raise_exception(env, EXCP_PREFETCH_ABORT,
+syn_breakpoint(same_el),
+arm_debug_target_el(env));
 }
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0e1a3b94211..54648c7fbb6 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6595,7 +6595,7 @@ static void define_debug_regs(ARMCPU *cpu)
  * These are just dummy implementations for now.
  */
 int i;
-int wrps, brps, ctx_cmps;
+int brps, ctx_cmps;
 
 /*
  * The Arm ARM says DBGDIDR is optional and deprecated if EL1 cannot
@@ -6614,7 +6614,6 @@ static void define_debug_regs(ARMCPU *cpu)
 
 /* Note that all these register fields hold "number of Xs minus 1". */
 brps = arm_num_brps(cpu);
-wrps = arm_num_wrps(cpu);
 ctx_cmps = arm_num_ctx_cmps(cpu);
 
 assert(ctx_cmps <= brps);
@@ -6644,7 +6643,7 @@ static void define_debug_regs(ARMCPU *cpu)
  

[PATCH v4 17/28] linux-user: Remove dead code

[Philippe Mathieu-Daudé]

We can not use watchpoints in user-mode emulation because we
need the softmmu slow path to detect accesses to watchpointed
memory. This code is expanded as empty stub in "hw/core/cpu.h"
anyway, so we can drop it.

Reviewed-by: Laurent Vivier 
Signed-off-by: Philippe Mathieu-Daudé 
---
 linux-user/main.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 81f48ff54ed..d7af3ffbc22 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -200,7 +200,6 @@ CPUArchState *cpu_copy(CPUArchState *env)
 CPUState *new_cpu = cpu_create(cpu_type);
 CPUArchState *new_env = new_cpu->env_ptr;
 CPUBreakpoint *bp;
-CPUWatchpoint *wp;
 
 /* Reset non arch specific state */
 cpu_reset(new_cpu);
@@ -211,13 +210,9 @@ CPUArchState *cpu_copy(CPUArchState *env)
Note: Once we support ptrace with hw-debug register access, make sure
BP_CPU break/watchpoints are handled correctly on clone. */
 QTAILQ_INIT(&new_cpu->breakpoints);
-QTAILQ_INIT(&new_cpu->watchpoints);
 QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
 cpu_breakpoint_insert(new_cpu, bp->pc, bp->flags, NULL);
 }
-QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
-cpu_watchpoint_insert(new_cpu, wp->vaddr, wp->len, wp->flags, NULL);
-}
 
 return new_env;
 }
-- 
2.26.2

[PATCH v4 11/28] cpu: Move CPUClass::write_elf* to SysemuCPUOps

[Philippe Mathieu-Daudé]

The write_elf*() handlers are used to dump vmcore images.
This feature is only meaningful for system emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 17 -
 include/hw/core/sysemu-cpu-ops.h | 24 
 hw/core/cpu.c| 16 
 target/arm/cpu.c |  4 ++--
 target/i386/cpu.c|  8 
 target/s390x/cpu.c   |  2 +-
 target/ppc/translate_init.c.inc  |  6 ++
 7 files changed, 41 insertions(+), 36 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 781cd8fc42b..0a2c29c3735 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -112,14 +112,6 @@ struct AccelCPUClass;
  *   a memory access with the specified memory transaction attributes.
  * @gdb_read_register: Callback for letting GDB read a register.
  * @gdb_write_register: Callback for letting GDB write a register.
- * @write_elf64_note: Callback for writing a CPU-specific ELF note to a
- * 64-bit VM coredump.
- * @write_elf32_qemunote: Callback for writing a CPU- and QEMU-specific ELF
- * note to a 32-bit VM coredump.
- * @write_elf32_note: Callback for writing a CPU-specific ELF note to a
- * 32-bit VM coredump.
- * @write_elf32_qemunote: Callback for writing a CPU- and QEMU-specific ELF
- * note to a 32-bit VM coredump.
  * @gdb_num_core_regs: Number of core registers accessible to GDB.
  * @gdb_core_xml_file: File name for core registers GDB XML description.
  * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
@@ -163,15 +155,6 @@ struct CPUClass {
 int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
 int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
 
-int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
-int cpuid, void *opaque);
-int (*write_elf64_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
-void *opaque);
-int (*write_elf32_note)(WriteCoreDumpFunction f, CPUState *cpu,
-int cpuid, void *opaque);
-int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
-void *opaque);
-
 const char *gdb_core_xml_file;
 gchar * (*gdb_arch_name)(CPUState *cpu);
 const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index b9ffca07665..60c667801ef 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -21,6 +21,30 @@ typedef struct SysemuCPUOps {
  * GUEST_PANICKED events.
  */
 GuestPanicInformation* (*get_crash_info)(CPUState *cpu);
+/**
+ * @write_elf32_note: Callback for writing a CPU-specific ELF note to a
+ * 32-bit VM coredump.
+ */
+int (*write_elf32_note)(WriteCoreDumpFunction f, CPUState *cpu,
+int cpuid, void *opaque);
+/**
+ * @write_elf64_note: Callback for writing a CPU-specific ELF note to a
+ * 64-bit VM coredump.
+ */
+int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
+int cpuid, void *opaque);
+/**
+ * @write_elf32_qemunote: Callback for writing a CPU- and QEMU-specific ELF
+ * note to a 32-bit VM coredump.
+ */
+int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
+void *opaque);
+/**
+ * @write_elf64_qemunote: Callback for writing a CPU- and QEMU-specific ELF
+ * note to a 64-bit VM coredump.
+ */
+int (*write_elf64_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
+void *opaque);
 /**
  * @virtio_is_big_endian: Callback to return %true if a CPU which supports
  *   runtime configurable endianness is currently big-endian.
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 0aebc18c41f..c74390aafbf 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -151,10 +151,10 @@ int cpu_write_elf32_qemunote(WriteCoreDumpFunction f, 
CPUState *cpu,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (!cc->write_elf32_qemunote) {
+if (!cc->sysemu_ops->write_elf32_qemunote) {
 return 0;
 }
-return (*cc->write_elf32_qemunote)(f, cpu, opaque);
+return (*cc->sysemu_ops->write_elf32_qemunote)(f, cpu, opaque);
 }
 
 int cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cpu,
@@ -162,10 +162,10 @@ int cpu_write_elf32_note(WriteCoreDumpFunction f, 
CPUState *cpu,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (!cc->write_elf32_note) {
+if (!cc->sysemu_ops->write_elf32_note) {
 return -1;
 }
-return (*cc->write_elf32_note)(f, cpu, cpuid, opaque);
+return (*cc->sysemu_ops->write_elf32_note)(f, cpu, cpuid, opaque);
 }
 
 int cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
@@ -173,10 +173,10 @@

[PATCH v4 10/28] cpu: Move CPUClass::get_crash_info to SysemuCPUOps

[Philippe Mathieu-Daudé]

cpu_get_crash_info() is called on GUEST_PANICKED events,
which only occur in system emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 1 -
 include/hw/core/sysemu-cpu-ops.h | 5 +
 hw/core/cpu.c| 4 ++--
 target/i386/cpu.c| 2 +-
 target/s390x/cpu.c   | 2 +-
 5 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index dfb50b60128..781cd8fc42b 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -150,7 +150,6 @@ struct CPUClass {
 int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
uint8_t *buf, int len, bool is_write);
 void (*dump_state)(CPUState *cpu, FILE *, int flags);
-GuestPanicInformation* (*get_crash_info)(CPUState *cpu);
 void (*dump_statistics)(CPUState *cpu, int flags);
 int64_t (*get_arch_id)(CPUState *cpu);
 bool (*get_paging_enabled)(const CPUState *cpu);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 9c3ac4f2280..b9ffca07665 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,11 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @get_crash_info: Callback for reporting guest crash information in
+ * GUEST_PANICKED events.
+ */
+GuestPanicInformation* (*get_crash_info)(CPUState *cpu);
 /**
  * @virtio_is_big_endian: Callback to return %true if a CPU which supports
  *   runtime configurable endianness is currently big-endian.
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 09eaa3fa49f..0aebc18c41f 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -220,8 +220,8 @@ GuestPanicInformation *cpu_get_crash_info(CPUState *cpu)
 CPUClass *cc = CPU_GET_CLASS(cpu);
 GuestPanicInformation *res = NULL;
 
-if (cc->get_crash_info) {
-res = cc->get_crash_info(cpu);
+if (cc->sysemu_ops->get_crash_info) {
+res = cc->sysemu_ops->get_crash_info(cpu);
 }
 return res;
 }
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b8f056849a7..362ad56ab68 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7388,6 +7388,7 @@ static Property x86_cpu_properties[] = {
 
 #ifndef CONFIG_USER_ONLY
 static const struct SysemuCPUOps i386_sysemu_ops = {
+.get_crash_info = x86_cpu_get_crash_info,
 .vmsd = &vmstate_x86_cpu,
 };
 #endif
@@ -7427,7 +7428,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->asidx_from_attrs = x86_asidx_from_attrs;
 cc->get_memory_mapping = x86_cpu_get_memory_mapping;
 cc->get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug;
-cc->get_crash_info = x86_cpu_get_crash_info;
 cc->write_elf64_note = x86_cpu_write_elf64_note;
 cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
 cc->write_elf32_note = x86_cpu_write_elf32_note;
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index fe908d9bc40..fdc169bb0ac 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -479,6 +479,7 @@ static void s390_cpu_reset_full(DeviceState *dev)
 
 #ifndef CONFIG_USER_ONLY
 static const struct SysemuCPUOps s390_sysemu_ops = {
+.get_crash_info = s390_cpu_get_crash_info,
 .vmsd = &vmstate_s390_cpu,
 };
 #endif
@@ -523,7 +524,6 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
 cc->gdb_write_register = s390_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_debug = s390_cpu_get_phys_page_debug;
-cc->get_crash_info = s390_cpu_get_crash_info;
 cc->write_elf64_note = s390_cpu_write_elf64_note;
 cc->sysemu_ops = &s390_sysemu_ops;
 #endif
-- 
2.26.2

[PATCH v4 13/28] cpu: Move CPUClass::get_phys_page_debug to SysemuCPUOps

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h|  8 
 include/hw/core/sysemu-cpu-ops.h | 13 +
 hw/core/cpu.c|  6 +++---
 target/alpha/cpu.c   |  2 +-
 target/arm/cpu.c |  2 +-
 target/avr/cpu.c |  2 +-
 target/cris/cpu.c|  2 +-
 target/hppa/cpu.c|  2 +-
 target/i386/cpu.c|  2 +-
 target/lm32/cpu.c|  2 +-
 target/m68k/cpu.c|  2 +-
 target/microblaze/cpu.c  |  2 +-
 target/mips/cpu.c|  2 +-
 target/moxie/cpu.c   |  4 +---
 target/nios2/cpu.c   |  2 +-
 target/openrisc/cpu.c|  2 +-
 target/riscv/cpu.c   |  2 +-
 target/rx/cpu.c  |  2 +-
 target/s390x/cpu.c   |  2 +-
 target/sh4/cpu.c |  2 +-
 target/sparc/cpu.c   |  2 +-
 target/tricore/cpu.c |  2 +-
 target/unicore32/cpu.c   |  2 +-
 target/xtensa/cpu.c  |  2 +-
 target/ppc/translate_init.c.inc  |  2 +-
 25 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 6713a615916..9a86c707cf7 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -103,11 +103,6 @@ struct AccelCPUClass;
  *   If the target behaviour here is anything other than "set
  *   the PC register to the value passed in" then the target must
  *   also implement the synchronize_from_tb hook.
- * @get_phys_page_debug: Callback for obtaining a physical address.
- * @get_phys_page_attrs_debug: Callback for obtaining a physical address and 
the
- *   associated memory transaction attributes to use for the access.
- *   CPUs which use memory transaction attributes should implement this
- *   instead of get_phys_page_debug.
  * @gdb_read_register: Callback for letting GDB read a register.
  * @gdb_write_register: Callback for letting GDB write a register.
  * @gdb_num_core_regs: Number of core registers accessible to GDB.
@@ -146,9 +141,6 @@ struct CPUClass {
 void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
Error **errp);
 void (*set_pc)(CPUState *cpu, vaddr value);
-hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
-hwaddr (*get_phys_page_attrs_debug)(CPUState *cpu, vaddr addr,
-MemTxAttrs *attrs);
 int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
 int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
 
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 3c3f211136d..0c8f616a565 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,19 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @get_phys_page_debug: Callback for obtaining a physical address.
+ */
+hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
+/**
+ * @get_phys_page_attrs_debug: Callback for obtaining a physical address
+ *   and the associated memory transaction attributes to use for the
+ *   access.
+ * CPUs which use memory transaction attributes should implement this
+ * instead of get_phys_page_debug.
+ */
+hwaddr (*get_phys_page_attrs_debug)(CPUState *cpu, vaddr addr,
+MemTxAttrs *attrs);
 /**
  * @asidx_from_attrs: Callback to return the CPU AddressSpace to use for
  *   a memory access with the specified memory transaction attributes.
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index c44229205ff..6932781425a 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -96,12 +96,12 @@ hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr 
addr,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (cc->get_phys_page_attrs_debug) {
-return cc->get_phys_page_attrs_debug(cpu, addr, attrs);
+if (cc->sysemu_ops->get_phys_page_attrs_debug) {
+return cc->sysemu_ops->get_phys_page_attrs_debug(cpu, addr, attrs);
 }
 /* Fallback for CPUs which don't implement the _attrs_ hook */
 *attrs = MEMTXATTRS_UNSPECIFIED;
-return cc->get_phys_page_debug(cpu, addr);
+return cc->sysemu_ops->get_phys_page_debug(cpu, addr);
 }
 
 hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index de8f9c648fa..fbef4c0923f 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -208,6 +208,7 @@ static void alpha_cpu_initfn(Object *obj)
 
 #ifndef CONFIG_USER_ONLY
 static const struct SysemuCPUOps alpha_sysemu_ops = {
+.get_phys_page_debug = alpha_cpu_get_phys_page_debug,
 .vmsd = &vmstate_alpha_cpu,
 };
 #endif
@@ -242,7 +243,6 @@ static void alpha_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->gdb_read_regist

[PATCH v4 18/28] gdbstub: Remove watchpoint dead code in gdbserver_fork()

[Philippe Mathieu-Daudé]

gdbserver_fork() is only used in user emulation where we can not
use watchpoints because we need the softmmu slow path to detect
accesses to watchpointed memory. This code doesn't do anything as
declared as stubs in "hw/core/cpu.h". Drop it.

Signed-off-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gdbstub.c b/gdbstub.c
index 3ee40479b69..f3614ebcc7a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3349,7 +3349,6 @@ void gdbserver_fork(CPUState *cpu)
 close(gdbserver_state.fd);
 gdbserver_state.fd = -1;
 cpu_breakpoint_remove_all(cpu, BP_GDB);
-cpu_watchpoint_remove_all(cpu, BP_GDB);
 }
 #else
 static int gdb_chr_can_receive(void *opaque)
-- 
2.26.2

[PATCH v4 09/28] cpu: Move CPUClass::virtio_is_big_endian to SysemuCPUOps

[Philippe Mathieu-Daudé]

VirtIO devices are only meaningful with system emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 5 -
 include/hw/core/sysemu-cpu-ops.h | 8 
 hw/core/cpu.c| 4 ++--
 target/arm/cpu.c | 2 +-
 target/ppc/translate_init.c.inc  | 4 +---
 5 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 471c99d9f04..dfb50b60128 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -89,10 +89,6 @@ struct AccelCPUClass;
  * @parse_features: Callback to parse command line arguments.
  * @reset_dump_flags: #CPUDumpFlags to use for reset logging.
  * @has_work: Callback for checking if there is work to do.
- * @virtio_is_big_endian: Callback to return %true if a CPU which supports
- * runtime configurable endianness is currently big-endian. Non-configurable
- * CPUs can use the default implementation of this method. This method should
- * not be used by any callers other than the pre-1.0 virtio devices.
  * @memory_rw_debug: Callback for GDB memory access.
  * @dump_state: Callback for dumping state.
  * @dump_statistics: Callback for dumping statistics.
@@ -151,7 +147,6 @@ struct CPUClass {
 
 int reset_dump_flags;
 bool (*has_work)(CPUState *cpu);
-bool (*virtio_is_big_endian)(CPUState *cpu);
 int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
uint8_t *buf, int len, bool is_write);
 void (*dump_state)(CPUState *cpu, FILE *, int flags);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 05f19b22070..9c3ac4f2280 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,14 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @virtio_is_big_endian: Callback to return %true if a CPU which supports
+ *   runtime configurable endianness is currently big-endian.
+ * Non-configurable CPUs can use the default implementation of this method.
+ * This method should not be used by any callers other than the pre-1.0
+ * virtio devices.
+ */
+bool (*virtio_is_big_endian)(CPUState *cpu);
 /**
  * @vmsd: State description for migration.
  */
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 5abf8bed2e4..09eaa3fa49f 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -204,8 +204,8 @@ bool cpu_virtio_is_big_endian(CPUState *cpu)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (cc->virtio_is_big_endian) {
-return cc->virtio_is_big_endian(cpu);
+if (cc->sysemu_ops->virtio_is_big_endian) {
+return cc->sysemu_ops->virtio_is_big_endian(cpu);
 }
 return target_words_bigendian();
 }
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 403422c6e76..fa0d4bba590 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2262,6 +2262,7 @@ static gchar *arm_gdb_arch_name(CPUState *cs)
 
 #ifndef CONFIG_USER_ONLY
 static const struct SysemuCPUOps arm_sysemu_ops = {
+.virtio_is_big_endian = arm_cpu_virtio_is_big_endian,
 .vmsd = &vmstate_arm_cpu,
 };
 #endif
@@ -2305,7 +2306,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void 
*data)
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_attrs_debug = arm_cpu_get_phys_page_attrs_debug;
 cc->asidx_from_attrs = arm_asidx_from_attrs;
-cc->virtio_is_big_endian = arm_cpu_virtio_is_big_endian;
 cc->write_elf64_note = arm_cpu_write_elf64_note;
 cc->write_elf32_note = arm_cpu_write_elf32_note;
 cc->sysemu_ops = &arm_sysemu_ops;
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index 819b807097d..789124debe7 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -10845,6 +10845,7 @@ static Property ppc_cpu_properties[] = {
 
 #ifndef CONFIG_USER_ONLY
 static const struct SysemuCPUOps ppc_sysemu_ops = {
+.virtio_is_big_endian = ppc_cpu_is_big_endian,
 .vmsd = &vmstate_ppc_cpu,
 };
 #endif
@@ -10913,9 +10914,6 @@ static void ppc_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->gdb_core_xml_file = "power64-core.xml";
 #else
 cc->gdb_core_xml_file = "power-core.xml";
-#endif
-#ifndef CONFIG_USER_ONLY
-cc->virtio_is_big_endian = ppc_cpu_is_big_endian;
 #endif
 cc->disas_set_info = ppc_disas_set_info;
 
-- 
2.26.2

[PATCH v4 15/28] cpu: Move CPUClass::get_paging_enabled to SysemuCPUOps

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h| 2 --
 include/hw/core/sysemu-cpu-ops.h | 4 
 hw/core/cpu.c| 4 ++--
 target/i386/cpu.c| 4 +++-
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 8af78cdde23..960846d2b64 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -93,7 +93,6 @@ struct AccelCPUClass;
  * @dump_state: Callback for dumping state.
  * @dump_statistics: Callback for dumping statistics.
  * @get_arch_id: Callback for getting architecture-dependent CPU ID.
- * @get_paging_enabled: Callback for inquiring whether paging is enabled.
  * @set_pc: Callback for setting the Program Counter register. This
  *   should have the semantics used by the target architecture when
  *   setting the PC from a source such as an ELF file entry point;
@@ -136,7 +135,6 @@ struct CPUClass {
 void (*dump_state)(CPUState *cpu, FILE *, int flags);
 void (*dump_statistics)(CPUState *cpu, int flags);
 int64_t (*get_arch_id)(CPUState *cpu);
-bool (*get_paging_enabled)(const CPUState *cpu);
 void (*set_pc)(CPUState *cpu, vaddr value);
 int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
 int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index 460e7d63b0c..3f9a5199dd1 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -21,6 +21,10 @@ typedef struct SysemuCPUOps {
  */
 void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
Error **errp);
+/**
+ * @get_paging_enabled: Callback for inquiring whether paging is enabled.
+ */
+bool (*get_paging_enabled)(const CPUState *cpu);
 /**
  * @get_phys_page_debug: Callback for obtaining a physical address.
  */
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 339bdfadd7a..7a8487d468f 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -71,8 +71,8 @@ bool cpu_paging_enabled(const CPUState *cpu)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-if (cc->get_paging_enabled) {
-return cc->get_paging_enabled(cpu);
+if (cc->sysemu_ops->get_paging_enabled) {
+return cc->sysemu_ops->get_paging_enabled(cpu);
 }
 
 return false;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b2d3debd1f2..994d58aa754 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7157,12 +7157,14 @@ static int64_t x86_cpu_get_arch_id(CPUState *cs)
 return cpu->apic_id;
 }
 
+#if !defined(CONFIG_USER_ONLY)
 static bool x86_cpu_get_paging_enabled(const CPUState *cs)
 {
 X86CPU *cpu = X86_CPU(cs);
 
 return cpu->env.cr[0] & CR0_PG_MASK;
 }
+#endif /* !CONFIG_USER_ONLY */
 
 static void x86_cpu_set_pc(CPUState *cs, vaddr value)
 {
@@ -7389,6 +7391,7 @@ static Property x86_cpu_properties[] = {
 #ifndef CONFIG_USER_ONLY
 static const struct SysemuCPUOps i386_sysemu_ops = {
 .get_memory_mapping = x86_cpu_get_memory_mapping,
+.get_paging_enabled = x86_cpu_get_paging_enabled,
 .get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug,
 .asidx_from_attrs = x86_asidx_from_attrs,
 .get_crash_info = x86_cpu_get_crash_info,
@@ -7429,7 +7432,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->gdb_read_register = x86_cpu_gdb_read_register;
 cc->gdb_write_register = x86_cpu_gdb_write_register;
 cc->get_arch_id = x86_cpu_get_arch_id;
-cc->get_paging_enabled = x86_cpu_get_paging_enabled;
 
 #ifndef CONFIG_USER_ONLY
 cc->sysemu_ops = &i386_sysemu_ops;
-- 
2.26.2

[PATCH v4 05/28] cpu: Directly use get_paging_enabled() fallback handlers in place

[Philippe Mathieu-Daudé]

No code uses CPUClass::get_paging_enabled() outside of hw/core/cpu.c:

  $ git grep -F -- '->get_paging_enabled'
  hw/core/cpu.c:74:return cc->get_paging_enabled(cpu);
  hw/core/cpu.c:438:k->get_paging_enabled = cpu_common_get_paging_enabled;
  target/i386/cpu.c:7418:cc->get_paging_enabled = 
x86_cpu_get_paging_enabled;

Check the handler presence in place and remove the common fallback code.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/core/cpu.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index a9ee2c74ec5..1de00bbb474 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -71,11 +71,10 @@ bool cpu_paging_enabled(const CPUState *cpu)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-return cc->get_paging_enabled(cpu);
-}
+if (cc->get_paging_enabled) {
+return cc->get_paging_enabled(cpu);
+}
 
-static bool cpu_common_get_paging_enabled(const CPUState *cpu)
-{
 return false;
 }
 
@@ -420,7 +419,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 k->parse_features = cpu_common_parse_features;
 k->get_arch_id = cpu_common_get_arch_id;
 k->has_work = cpu_common_has_work;
-k->get_paging_enabled = cpu_common_get_paging_enabled;
 k->get_memory_mapping = cpu_common_get_memory_mapping;
 k->gdb_read_register = cpu_common_gdb_read_register;
 k->gdb_write_register = cpu_common_gdb_write_register;
-- 
2.26.2

[PATCH v4 04/28] cpu: Directly use cpu_write_elf*() fallback handlers in place

[Philippe Mathieu-Daudé]

No code directly accesses CPUClass::write_elf*() handlers out
of hw/core/cpu.c (the rest are assignation in target/ code):

  $ git grep -F -- '->write_elf'
  hw/core/cpu.c:157:return (*cc->write_elf32_qemunote)(f, cpu, opaque);
  hw/core/cpu.c:171:return (*cc->write_elf32_note)(f, cpu, cpuid, opaque);
  hw/core/cpu.c:186:return (*cc->write_elf64_qemunote)(f, cpu, opaque);
  hw/core/cpu.c:200:return (*cc->write_elf64_note)(f, cpu, cpuid, opaque);
  hw/core/cpu.c:440:k->write_elf32_qemunote = 
cpu_common_write_elf32_qemunote;
  hw/core/cpu.c:441:k->write_elf32_note = cpu_common_write_elf32_note;
  hw/core/cpu.c:442:k->write_elf64_qemunote = 
cpu_common_write_elf64_qemunote;
  hw/core/cpu.c:443:k->write_elf64_note = cpu_common_write_elf64_note;
  target/arm/cpu.c:2304:cc->write_elf64_note = arm_cpu_write_elf64_note;
  target/arm/cpu.c:2305:cc->write_elf32_note = arm_cpu_write_elf32_note;
  target/i386/cpu.c:7425:cc->write_elf64_note = x86_cpu_write_elf64_note;
  target/i386/cpu.c:7426:cc->write_elf64_qemunote = 
x86_cpu_write_elf64_qemunote;
  target/i386/cpu.c:7427:cc->write_elf32_note = x86_cpu_write_elf32_note;
  target/i386/cpu.c:7428:cc->write_elf32_qemunote = 
x86_cpu_write_elf32_qemunote;
  target/ppc/translate_init.c.inc:10891:cc->write_elf64_note = 
ppc64_cpu_write_elf64_note;
  target/ppc/translate_init.c.inc:10892:cc->write_elf32_note = 
ppc32_cpu_write_elf32_note;
  target/s390x/cpu.c:522:cc->write_elf64_note = s390_cpu_write_elf64_note;

Check the handler presence in place and remove the common fallback code.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/core/cpu.c | 43 ---
 1 file changed, 12 insertions(+), 31 deletions(-)

diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index daaff56a79e..a9ee2c74ec5 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -154,60 +154,45 @@ int cpu_write_elf32_qemunote(WriteCoreDumpFunction f, 
CPUState *cpu,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
+if (!cc->write_elf32_qemunote) {
+return 0;
+}
 return (*cc->write_elf32_qemunote)(f, cpu, opaque);
 }
 
-static int cpu_common_write_elf32_qemunote(WriteCoreDumpFunction f,
-   CPUState *cpu, void *opaque)
-{
-return 0;
-}
-
 int cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cpu,
  int cpuid, void *opaque)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
+if (!cc->write_elf32_note) {
+return -1;
+}
 return (*cc->write_elf32_note)(f, cpu, cpuid, opaque);
 }
 
-static int cpu_common_write_elf32_note(WriteCoreDumpFunction f,
-   CPUState *cpu, int cpuid,
-   void *opaque)
-{
-return -1;
-}
-
 int cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
  void *opaque)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
+if (!cc->write_elf64_qemunote) {
+return 0;
+}
 return (*cc->write_elf64_qemunote)(f, cpu, opaque);
 }
 
-static int cpu_common_write_elf64_qemunote(WriteCoreDumpFunction f,
-   CPUState *cpu, void *opaque)
-{
-return 0;
-}
-
 int cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cpu,
  int cpuid, void *opaque)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
+if (!cc->write_elf64_note) {
+return -1;
+}
 return (*cc->write_elf64_note)(f, cpu, cpuid, opaque);
 }
 
-static int cpu_common_write_elf64_note(WriteCoreDumpFunction f,
-   CPUState *cpu, int cpuid,
-   void *opaque)
-{
-return -1;
-}
-
-
 static int cpu_common_gdb_read_register(CPUState *cpu, GByteArray *buf, int 
reg)
 {
 return 0;
@@ -437,10 +422,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 k->has_work = cpu_common_has_work;
 k->get_paging_enabled = cpu_common_get_paging_enabled;
 k->get_memory_mapping = cpu_common_get_memory_mapping;
-k->write_elf32_qemunote = cpu_common_write_elf32_qemunote;
-k->write_elf32_note = cpu_common_write_elf32_note;
-k->write_elf64_qemunote = cpu_common_write_elf64_qemunote;
-k->write_elf64_note = cpu_common_write_elf64_note;
 k->gdb_read_register = cpu_common_gdb_read_register;
 k->gdb_write_register = cpu_common_gdb_write_register;
 set_bit(DEVICE_CATEGORY_CPU, dc->categories);
-- 
2.26.2

[PATCH v4 08/28] cpu: Move CPUClass::vmsd to SysemuCPUOps

[Philippe Mathieu-Daudé]

Migration is specific to system emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h|  2 --
 include/hw/core/sysemu-cpu-ops.h |  4 
 cpu.c| 18 --
 target/alpha/cpu.c   |  2 +-
 target/arm/cpu.c |  2 +-
 target/avr/cpu.c |  2 +-
 target/cris/cpu.c|  2 +-
 target/hppa/cpu.c|  2 +-
 target/i386/cpu.c|  2 +-
 target/lm32/cpu.c|  2 +-
 target/m68k/cpu.c|  2 +-
 target/microblaze/cpu.c  |  2 +-
 target/mips/cpu.c|  2 +-
 target/moxie/cpu.c   |  2 +-
 target/nios2/cpu.c   |  7 +++
 target/openrisc/cpu.c|  2 +-
 target/riscv/cpu.c   |  4 ++--
 target/rx/cpu.c  |  6 ++
 target/s390x/cpu.c   |  2 +-
 target/sh4/cpu.c |  4 ++--
 target/sparc/cpu.c   |  2 +-
 target/tricore/cpu.c |  7 +++
 target/unicore32/cpu.c   |  2 +-
 target/xtensa/cpu.c  |  4 ++--
 target/ppc/translate_init.c.inc  |  2 +-
 25 files changed, 54 insertions(+), 34 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 3c26471d0fa..471c99d9f04 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -124,7 +124,6 @@ struct AccelCPUClass;
  * 32-bit VM coredump.
  * @write_elf32_qemunote: Callback for writing a CPU- and QEMU-specific ELF
  * note to a 32-bit VM coredump.
- * @vmsd: State description for migration.
  * @gdb_num_core_regs: Number of core registers accessible to GDB.
  * @gdb_core_xml_file: File name for core registers GDB XML description.
  * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
@@ -179,7 +178,6 @@ struct CPUClass {
 int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
 void *opaque);
 
-const VMStateDescription *vmsd;
 const char *gdb_core_xml_file;
 gchar * (*gdb_arch_name)(CPUState *cpu);
 const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index e54a08ea25e..05f19b22070 100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -16,6 +16,10 @@
  * struct SysemuCPUOps: System operations specific to a CPU class
  */
 typedef struct SysemuCPUOps {
+/**
+ * @vmsd: State description for migration.
+ */
+const VMStateDescription *vmsd;
 } SysemuCPUOps;
 
 #endif /* SYSEMU_CPU_OPS_H */
diff --git a/cpu.c b/cpu.c
index bfbe5a66f95..64e17537e21 100644
--- a/cpu.c
+++ b/cpu.c
@@ -126,7 +126,9 @@ const VMStateDescription vmstate_cpu_common = {
 
 void cpu_exec_realizefn(CPUState *cpu, Error **errp)
 {
+#ifndef CONFIG_USER_ONLY
 CPUClass *cc = CPU_GET_CLASS(cpu);
+#endif
 
 cpu_list_add(cpu);
 
@@ -137,27 +139,23 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
 }
 #endif /* CONFIG_TCG */
 
-#ifdef CONFIG_USER_ONLY
-assert(cc->vmsd == NULL);
-#else
+#ifndef CONFIG_USER_ONLY
 if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
 vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu);
 }
-if (cc->vmsd != NULL) {
-vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
+if (cc->sysemu_ops->vmsd != NULL) {
+vmstate_register(NULL, cpu->cpu_index, cc->sysemu_ops->vmsd, cpu);
 }
 #endif /* CONFIG_USER_ONLY */
 }
 
 void cpu_exec_unrealizefn(CPUState *cpu)
 {
+#ifndef CONFIG_USER_ONLY
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-#ifdef CONFIG_USER_ONLY
-assert(cc->vmsd == NULL);
-#else
-if (cc->vmsd != NULL) {
-vmstate_unregister(NULL, cc->vmsd, cpu);
+if (cc->sysemu_ops->vmsd != NULL) {
+vmstate_unregister(NULL, cc->sysemu_ops->vmsd, cpu);
 }
 if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
 vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 663b1a4fc4e..de8f9c648fa 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -208,6 +208,7 @@ static void alpha_cpu_initfn(Object *obj)
 
 #ifndef CONFIG_USER_ONLY
 static const struct SysemuCPUOps alpha_sysemu_ops = {
+.vmsd = &vmstate_alpha_cpu,
 };
 #endif
 
@@ -242,7 +243,6 @@ static void alpha_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->gdb_write_register = alpha_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_debug = alpha_cpu_get_phys_page_debug;
-cc->vmsd = &vmstate_alpha_cpu;
 cc->sysemu_ops = &alpha_sysemu_ops;
 #endif
 cc->disas_set_info = alpha_cpu_disas_set_info;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 1fe3c4ab273..403422c6e76 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2262,6 +2262,7 @@ static gchar *arm_gdb_arch_name(CPUState *cs)
 
 #ifndef CONFIG_USER_ONLY
 static const struct SysemuCPUOps arm_sysemu

[PATCH v4 03/28] cpu: Introduce cpu_virtio_is_big_endian()

[Philippe Mathieu-Daudé]

Introduce the cpu_virtio_is_big_endian() generic helper to avoid
calling CPUClass internal virtio_is_big_endian() one.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h | 9 +
 hw/core/cpu.c | 8 ++--
 hw/virtio/virtio.c| 4 +---
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 2d43f78819f..b12028c3c03 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -602,6 +602,15 @@ hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
  */
 int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs);
 
+/**
+ * cpu_virtio_is_big_endian:
+ * @cpu: CPU
+
+ * Returns %true if a CPU which supports runtime configurable endianness
+ * is currently big-endian.
+ */
+bool cpu_virtio_is_big_endian(CPUState *cpu);
+
 #endif /* CONFIG_USER_ONLY */
 
 /**
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 4dce35f832f..daaff56a79e 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -218,8 +218,13 @@ static int cpu_common_gdb_write_register(CPUState *cpu, 
uint8_t *buf, int reg)
 return 0;
 }
 
-static bool cpu_common_virtio_is_big_endian(CPUState *cpu)
+bool cpu_virtio_is_big_endian(CPUState *cpu)
 {
+CPUClass *cc = CPU_GET_CLASS(cpu);
+
+if (cc->virtio_is_big_endian) {
+return cc->virtio_is_big_endian(cpu);
+}
 return target_words_bigendian();
 }
 
@@ -438,7 +443,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 k->write_elf64_note = cpu_common_write_elf64_note;
 k->gdb_read_register = cpu_common_gdb_read_register;
 k->gdb_write_register = cpu_common_gdb_write_register;
-k->virtio_is_big_endian = cpu_common_virtio_is_big_endian;
 set_bit(DEVICE_CATEGORY_CPU, dc->categories);
 dc->realize = cpu_common_realizefn;
 dc->unrealize = cpu_common_unrealizefn;
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 1fd1917ca0f..fe6a4be99e4 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1973,9 +1973,7 @@ static enum virtio_device_endian 
virtio_default_endian(void)
 
 static enum virtio_device_endian virtio_current_cpu_endian(void)
 {
-CPUClass *cc = CPU_GET_CLASS(current_cpu);
-
-if (cc->virtio_is_big_endian(current_cpu)) {
+if (cpu_virtio_is_big_endian(current_cpu)) {
 return VIRTIO_DEVICE_ENDIAN_BIG;
 } else {
 return VIRTIO_DEVICE_ENDIAN_LITTLE;
-- 
2.26.2

[PATCH v4 02/28] cpu: Un-inline cpu_get_phys_page_debug and cpu_asidx_from_attrs

[Philippe Mathieu-Daudé]

To be able to later extract the cpu_get_phys_page_debug() and
cpu_asidx_from_attrs() handlers from CPUClass, un-inline them
from "hw/core/cpu.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h | 33 -
 hw/core/cpu.c | 32 
 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index c005d3dc2d8..2d43f78819f 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -578,18 +578,8 @@ void cpu_dump_statistics(CPUState *cpu, int flags);
  *
  * Returns: Corresponding physical page address or -1 if no page found.
  */
-static inline hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
-   MemTxAttrs *attrs)
-{
-CPUClass *cc = CPU_GET_CLASS(cpu);
-
-if (cc->get_phys_page_attrs_debug) {
-return cc->get_phys_page_attrs_debug(cpu, addr, attrs);
-}
-/* Fallback for CPUs which don't implement the _attrs_ hook */
-*attrs = MEMTXATTRS_UNSPECIFIED;
-return cc->get_phys_page_debug(cpu, addr);
-}
+hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
+ MemTxAttrs *attrs);
 
 /**
  * cpu_get_phys_page_debug:
@@ -601,12 +591,7 @@ static inline hwaddr 
cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
  *
  * Returns: Corresponding physical page address or -1 if no page found.
  */
-static inline hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
-{
-MemTxAttrs attrs = {};
-
-return cpu_get_phys_page_attrs_debug(cpu, addr, &attrs);
-}
+hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 
 /** cpu_asidx_from_attrs:
  * @cpu: CPU
@@ -615,17 +600,7 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState 
*cpu, vaddr addr)
  * Returns the address space index specifying the CPU AddressSpace
  * to use for a memory access with the given transaction attributes.
  */
-static inline int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs)
-{
-CPUClass *cc = CPU_GET_CLASS(cpu);
-int ret = 0;
-
-if (cc->asidx_from_attrs) {
-ret = cc->asidx_from_attrs(cpu, attrs);
-assert(ret < cpu->num_ases && ret >= 0);
-}
-return ret;
-}
+int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs);
 
 #endif /* CONFIG_USER_ONLY */
 
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 00330ba07de..4dce35f832f 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -94,6 +94,38 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
 error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
 }
 
+hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
+   MemTxAttrs *attrs)
+{
+CPUClass *cc = CPU_GET_CLASS(cpu);
+
+if (cc->get_phys_page_attrs_debug) {
+return cc->get_phys_page_attrs_debug(cpu, addr, attrs);
+}
+/* Fallback for CPUs which don't implement the _attrs_ hook */
+*attrs = MEMTXATTRS_UNSPECIFIED;
+return cc->get_phys_page_debug(cpu, addr);
+}
+
+hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
+{
+MemTxAttrs attrs = {};
+
+return cpu_get_phys_page_attrs_debug(cpu, addr, &attrs);
+}
+
+int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs)
+{
+CPUClass *cc = CPU_GET_CLASS(cpu);
+int ret = 0;
+
+if (cc->asidx_from_attrs) {
+ret = cc->asidx_from_attrs(cpu, attrs);
+assert(ret < cpu->num_ases && ret >= 0);
+}
+return ret;
+}
+
 /* Resetting the IRQ comes from across the code base so we take the
  * BQL here if we need to.  cpu_interrupt assumes it is held.*/
 void cpu_reset_interrupt(CPUState *cpu, int mask)
-- 
2.26.2

[PATCH v4 07/28] cpu: Introduce SysemuCPUOps structure

[Philippe Mathieu-Daudé]

Introduce a structure to hold handler specific to sysemu.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h|  5 +
 include/hw/core/sysemu-cpu-ops.h | 21 +
 target/alpha/cpu.c   |  6 ++
 target/arm/cpu.c |  6 ++
 target/avr/cpu.c |  4 
 target/cris/cpu.c|  6 ++
 target/hppa/cpu.c|  6 ++
 target/i386/cpu.c|  6 ++
 target/lm32/cpu.c|  6 ++
 target/m68k/cpu.c|  6 ++
 target/microblaze/cpu.c  |  6 ++
 target/mips/cpu.c|  6 ++
 target/moxie/cpu.c   |  4 
 target/nios2/cpu.c   |  6 ++
 target/openrisc/cpu.c|  6 ++
 target/riscv/cpu.c   |  6 ++
 target/rx/cpu.c  |  8 
 target/s390x/cpu.c   |  6 ++
 target/sh4/cpu.c |  6 ++
 target/sparc/cpu.c   |  6 ++
 target/tricore/cpu.c |  4 
 target/unicore32/cpu.c   |  4 
 target/xtensa/cpu.c  |  6 ++
 target/ppc/translate_init.c.inc  |  6 ++
 24 files changed, 152 insertions(+)
 create mode 100644 include/hw/core/sysemu-cpu-ops.h

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index b12028c3c03..3c26471d0fa 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -80,6 +80,8 @@ struct TCGCPUOps;
 /* see accel-cpu.h */
 struct AccelCPUClass;
 
+#include "hw/core/sysemu-cpu-ops.h"
+
 /**
  * CPUClass:
  * @class_by_name: Callback to map -cpu command line model name to an
@@ -190,6 +192,9 @@ struct CPUClass {
 bool gdb_stop_before_watchpoint;
 struct AccelCPUClass *accel_cpu;
 
+/* when system emulation is not available, this pointer is NULL */
+const struct SysemuCPUOps *sysemu_ops;
+
 /* when TCG is not available, this pointer is NULL */
 struct TCGCPUOps *tcg_ops;
 };
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
new file mode 100644
index 000..e54a08ea25e
--- /dev/null
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -0,0 +1,21 @@
+/*
+ * CPU operations specific to system emulation
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef SYSEMU_CPU_OPS_H
+#define SYSEMU_CPU_OPS_H
+
+#include "hw/core/cpu.h"
+
+/*
+ * struct SysemuCPUOps: System operations specific to a CPU class
+ */
+typedef struct SysemuCPUOps {
+} SysemuCPUOps;
+
+#endif /* SYSEMU_CPU_OPS_H */
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index faabffe0796..663b1a4fc4e 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -206,6 +206,11 @@ static void alpha_cpu_initfn(Object *obj)
 #endif
 }
 
+#ifndef CONFIG_USER_ONLY
+static const struct SysemuCPUOps alpha_sysemu_ops = {
+};
+#endif
+
 #include "hw/core/tcg-cpu-ops.h"
 
 static struct TCGCPUOps alpha_tcg_ops = {
@@ -238,6 +243,7 @@ static void alpha_cpu_class_init(ObjectClass *oc, void 
*data)
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_debug = alpha_cpu_get_phys_page_debug;
 cc->vmsd = &vmstate_alpha_cpu;
+cc->sysemu_ops = &alpha_sysemu_ops;
 #endif
 cc->disas_set_info = alpha_cpu_disas_set_info;
 
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index b8bc89e71fc..1fe3c4ab273 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2260,6 +2260,11 @@ static gchar *arm_gdb_arch_name(CPUState *cs)
 return g_strdup("arm");
 }
 
+#ifndef CONFIG_USER_ONLY
+static const struct SysemuCPUOps arm_sysemu_ops = {
+};
+#endif
+
 #ifdef CONFIG_TCG
 static struct TCGCPUOps arm_tcg_ops = {
 .initialize = arm_translate_init,
@@ -2303,6 +2308,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->virtio_is_big_endian = arm_cpu_virtio_is_big_endian;
 cc->write_elf64_note = arm_cpu_write_elf64_note;
 cc->write_elf32_note = arm_cpu_write_elf32_note;
+cc->sysemu_ops = &arm_sysemu_ops;
 #endif
 cc->gdb_num_core_regs = 26;
 cc->gdb_core_xml_file = "arm-core.xml";
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 0f4596932ba..78ef4473c50 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -184,6 +184,9 @@ static void avr_cpu_dump_state(CPUState *cs, FILE *f, int 
flags)
 qemu_fprintf(f, "\n");
 }
 
+static const struct SysemuCPUOps avr_sysemu_ops = {
+};
+
 #include "hw/core/tcg-cpu-ops.h"
 
 static struct TCGCPUOps avr_tcg_ops = {
@@ -214,6 +217,7 @@ static void avr_cpu_class_init(ObjectClass *oc, void *data)
 cc->memory_rw_debug = avr_cpu_memory_rw_debug;
 cc->get_phys_page_debug = avr_cpu_get_phys_page_debug;
 cc->vmsd = &vms_avr_cpu;
+cc->sysemu_ops = &avr_sysemu_ops;
 cc->disas_set_info = avr_cpu_disas_set_info;
 cc->gdb_read_register = avr_cpu_gdb_read_register;
 cc->gdb_write_register = avr_cpu_gdb_write_r

[PATCH v4 06/28] cpu: Directly use get_memory_mapping() fallback handlers in place

[Philippe Mathieu-Daudé]

No code uses CPUClass::get_memory_mapping() outside of hw/core/cpu.c:

  $ git grep -F -- '->get_memory_mapping'
  hw/core/cpu.c:87:cc->get_memory_mapping(cpu, list, errp);
  hw/core/cpu.c:439:k->get_memory_mapping = cpu_common_get_memory_mapping;
  target/i386/cpu.c:7422:cc->get_memory_mapping = 
x86_cpu_get_memory_mapping;

Check the handler presence in place and remove the common fallback code.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/core/cpu.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 1de00bbb474..5abf8bed2e4 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -83,13 +83,11 @@ void cpu_get_memory_mapping(CPUState *cpu, 
MemoryMappingList *list,
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-cc->get_memory_mapping(cpu, list, errp);
-}
+if (cc->get_memory_mapping) {
+cc->get_memory_mapping(cpu, list, errp);
+return;
+}
 
-static void cpu_common_get_memory_mapping(CPUState *cpu,
-  MemoryMappingList *list,
-  Error **errp)
-{
 error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
 }
 
@@ -419,7 +417,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 k->parse_features = cpu_common_parse_features;
 k->get_arch_id = cpu_common_get_arch_id;
 k->has_work = cpu_common_has_work;
-k->get_memory_mapping = cpu_common_get_memory_mapping;
 k->gdb_read_register = cpu_common_gdb_read_register;
 k->gdb_write_register = cpu_common_gdb_write_register;
 set_bit(DEVICE_CATEGORY_CPU, dc->categories);
-- 
2.26.2

[PATCH v4 01/28] target: Set CPUClass::vmsd instead of DeviceClass::vmsd

[Philippe Mathieu-Daudé]

The cpu model is the single device available in user-mode.
Since we want to restrict some fields to user-mode emulation,
we prefer to set the vmsd field of CPUClass, rather than the
DeviceClass one.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/alpha/cpu.c  | 2 +-
 target/cris/cpu.c   | 2 +-
 target/hppa/cpu.c   | 2 +-
 target/m68k/cpu.c   | 2 +-
 target/microblaze/cpu.c | 2 +-
 target/openrisc/cpu.c   | 2 +-
 target/sh4/cpu.c| 2 +-
 target/unicore32/cpu.c  | 2 +-
 target/xtensa/cpu.c | 2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 27192b62e22..faabffe0796 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -237,7 +237,7 @@ static void alpha_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->gdb_write_register = alpha_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_debug = alpha_cpu_get_phys_page_debug;
-dc->vmsd = &vmstate_alpha_cpu;
+cc->vmsd = &vmstate_alpha_cpu;
 #endif
 cc->disas_set_info = alpha_cpu_disas_set_info;
 
diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index ed983380fca..29a865b75d2 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -293,7 +293,7 @@ static void cris_cpu_class_init(ObjectClass *oc, void *data)
 cc->gdb_write_register = cris_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_debug = cris_cpu_get_phys_page_debug;
-dc->vmsd = &vmstate_cris_cpu;
+cc->vmsd = &vmstate_cris_cpu;
 #endif
 
 cc->gdb_num_core_regs = 49;
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index d8fad52d1fe..4f142de6e45 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -162,7 +162,7 @@ static void hppa_cpu_class_init(ObjectClass *oc, void *data)
 cc->gdb_write_register = hppa_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_debug = hppa_cpu_get_phys_page_debug;
-dc->vmsd = &vmstate_hppa_cpu;
+cc->vmsd = &vmstate_hppa_cpu;
 #endif
 cc->disas_set_info = hppa_cpu_disas_set_info;
 cc->gdb_num_core_regs = 128;
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 37d2ed9dc79..c98fb1e33be 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -533,7 +533,7 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data)
 cc->gdb_write_register = m68k_cpu_gdb_write_register;
 #if defined(CONFIG_SOFTMMU)
 cc->get_phys_page_debug = m68k_cpu_get_phys_page_debug;
-dc->vmsd = &vmstate_m68k_cpu;
+cc->vmsd = &vmstate_m68k_cpu;
 #endif
 cc->disas_set_info = m68k_cpu_disas_set_info;
 
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 433ba202037..335dfdc734e 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -387,7 +387,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
 
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_attrs_debug = mb_cpu_get_phys_page_attrs_debug;
-dc->vmsd = &vmstate_mb_cpu;
+cc->vmsd = &vmstate_mb_cpu;
 #endif
 device_class_set_props(dc, mb_properties);
 cc->gdb_num_core_regs = 32 + 27;
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 2c64842f46b..79d246d1930 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -204,7 +204,7 @@ static void openrisc_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->gdb_write_register = openrisc_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
 cc->get_phys_page_debug = openrisc_cpu_get_phys_page_debug;
-dc->vmsd = &vmstate_openrisc_cpu;
+cc->vmsd = &vmstate_openrisc_cpu;
 #endif
 cc->gdb_num_core_regs = 32 + 3;
 cc->disas_set_info = openrisc_disas_set_info;
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index ac65c88f1f8..bd44de53729 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -262,7 +262,7 @@ static void superh_cpu_class_init(ObjectClass *oc, void 
*data)
 
 cc->gdb_num_core_regs = 59;
 
-dc->vmsd = &vmstate_sh_cpu;
+cc->vmsd = &vmstate_sh_cpu;
 cc->tcg_ops = &superh_tcg_ops;
 }
 
diff --git a/target/unicore32/cpu.c b/target/unicore32/cpu.c
index 0258884f845..12894ffac6a 100644
--- a/target/unicore32/cpu.c
+++ b/target/unicore32/cpu.c
@@ -146,7 +146,7 @@ static void uc32_cpu_class_init(ObjectClass *oc, void *data)
 cc->dump_state = uc32_cpu_dump_state;
 cc->set_pc = uc32_cpu_set_pc;
 cc->get_phys_page_debug = uc32_cpu_get_phys_page_debug;
-dc->vmsd = &vmstate_uc32_cpu;
+cc->vmsd = &vmstate_uc32_cpu;
 cc->tcg_ops = &uc32_tcg_ops;
 }
 
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index e2b2c7a71c1..6bedd5b97b8 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -218,7 +218,7 @@ static void xtensa_cpu_class_init(ObjectClass *oc, void 
*data)
 cc->get_phys_page_debug = xtensa_cpu_get_phys_page_debug;
 #endif
 cc->disas_set_info = xtensa_cpu_disas_set_info;
-dc->vmsd = &vmstate_xtensa_cpu;
+cc->vmsd = &vmstate_xtensa_cpu;
 cc->tcg_ops = &xtensa_tcg_ops;
 }
 
-- 
2.26.2

[PATCH v4 00/28] cpu: Introduce SysemuCPUOps structure, remove watchpoints from usermode

[Philippe Mathieu-Daudé]

Hi,

This series is inspired on Claudio TCG work.

Instead of separate TCG from other accelerators, here we
separate sysemu operations (system VS user).

Patches 1-6 are generic cleanups.
Patches 7-15 move from CPUClass to SysemuCPUOps
Patch   16 restricts SysemuCPUOps to sysemu
Patches 17-26 remove watchpoint code from user emulation
Patches 27-28 remove USER_ONLY #ifdef'ry from "cpu.h"

Since v3:
- SysemuCPUOps const (Richard)
- added missing xtensa #ifdef'ry
- added missing aa64/sve #ifdef'ry
- added Laurent R-b

Since v2:
- fixed lm32/unicore32
- remove USER_ONLY ifdef'ry from "cpu.h" (Claudio)

Since v1:
- Name 'sysemu' (Claudio)
- change each field progressively (Richard)

$ git backport-diff -u v3
Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respective=
ly

001/28:[] [--] 'target: Set CPUClass::vmsd instead of DeviceClass::vmsd'
002/28:[] [--] 'cpu: Un-inline cpu_get_phys_page_debug and cpu_asidx_from=
_attrs'
003/28:[] [--] 'cpu: Introduce cpu_virtio_is_big_endian()'
004/28:[] [--] 'cpu: Directly use cpu_write_elf*() fallback handlers in p=
lace'
005/28:[] [--] 'cpu: Directly use get_paging_enabled() fallback handlers =
in place'
006/28:[] [--] 'cpu: Directly use get_memory_mapping() fallback handlers =
in place'
007/28:[0044] [FC] 'cpu: Introduce SysemuCPUOps structure'
008/28:[] [-C] 'cpu: Move CPUClass::vmsd to SysemuCPUOps'
009/28:[] [-C] 'cpu: Move CPUClass::virtio_is_big_endian to SysemuCPUOps'
010/28:[] [-C] 'cpu: Move CPUClass::get_crash_info to SysemuCPUOps'
011/28:[] [-C] 'cpu: Move CPUClass::write_elf* to SysemuCPUOps'
012/28:[] [-C] 'cpu: Move CPUClass::asidx_from_attrs to SysemuCPUOps'
013/28:[] [-C] 'cpu: Move CPUClass::get_phys_page_debug to SysemuCPUOps'
014/28:[] [-C] 'cpu: Move CPUClass::get_memory_mapping to SysemuCPUOps'
015/28:[] [-C] 'cpu: Move CPUClass::get_paging_enabled to SysemuCPUOps'
016/28:[] [--] 'cpu: Restrict "hw/core/sysemu-cpu-ops.h" to target/cpu.c'
017/28:[] [--] 'linux-user: Remove dead code'
018/28:[] [--] 'gdbstub: Remove watchpoint dead code in gdbserver_fork()'
019/28:[] [--] 'target/arm/internals: Fix code style for checkpatch.pl'
020/28:[] [--] 'target/arm: Move code blocks around'
021/28:[] [--] 'target/arm: Refactor some function bodies'
022/28:[0012] [FC] 'target/arm: Restrict watchpoint code to system emulation'
023/28:[] [--] 'target/i386: Restrict watchpoint code to system emulation'
024/28:[down] 'target/xtensa: Restrict watchpoint code to system emulation'
025/28:[] [--] 'accel/tcg/cpu-exec: Restrict watchpoint code to system em=
ulation'
026/28:[] [--] 'cpu: Remove watchpoint stubs for user emulation'
027/28:[] [--] 'cpu: Fix code style for checkpatch.pl'
028/28:[] [--] 'cpu: Move sysemu specific declarations to 'sysemu-cpu-ops=
.h''

Regards,

Phil.

Philippe Mathieu-Daud=C3=A9 (28):
  target: Set CPUClass::vmsd instead of DeviceClass::vmsd
  cpu: Un-inline cpu_get_phys_page_debug and cpu_asidx_from_attrs
  cpu: Introduce cpu_virtio_is_big_endian()
  cpu: Directly use cpu_write_elf*() fallback handlers in place
  cpu: Directly use get_paging_enabled() fallback handlers in place
  cpu: Directly use get_memory_mapping() fallback handlers in place
  cpu: Introduce SysemuCPUOps structure
  cpu: Move CPUClass::vmsd to SysemuCPUOps
  cpu: Move CPUClass::virtio_is_big_endian to SysemuCPUOps
  cpu: Move CPUClass::get_crash_info to SysemuCPUOps
  cpu: Move CPUClass::write_elf* to SysemuCPUOps
  cpu: Move CPUClass::asidx_from_attrs to SysemuCPUOps
  cpu: Move CPUClass::get_phys_page_debug to SysemuCPUOps
  cpu: Move CPUClass::get_memory_mapping to SysemuCPUOps
  cpu: Move CPUClass::get_paging_enabled to SysemuCPUOps
  cpu: Restrict "hw/core/sysemu-cpu-ops.h" to target/cpu.c
  linux-user: Remove dead code
  gdbstub: Remove watchpoint dead code in gdbserver_fork()
  target/arm/internals: Fix code style for checkpatch.pl
  target/arm: Move code blocks around
  target/arm: Refactor some function bodies
  target/arm: Restrict watchpoint code to system emulation
  target/i386: Restrict watchpoint code to system emulation
  target/xtensa: Restrict watchpoint code to system emulation
  accel/tcg/cpu-exec: Restrict watchpoint code to system emulation
  cpu: Remove watchpoint stubs for user emulation
  cpu: Fix code style for checkpatch.pl
  cpu: Move sysemu specific declarations to 'sysemu-cpu-ops.h'

 include/hw/core/cpu.h| 258 +--
 include/hw/core/sysemu-cpu-ops.h | 244 +
 target/alpha/cpu.h   |   3 +
 target/arm/cpu.h |   3 +
 target/arm/internals.h   |  34 ++--
 target/avr/cpu.h |   1 +
 target/cris/cpu.h|   3 +
 target/hexagon/cpu.h |   3 +
 target/hpp

Re: [PATCH] nbd: server: Report holes for raw images

[Nir Soffer]

On Thu, Feb 25, 2021 at 8:51 PM Vladimir Sementsov-Ogievskiy <
vsement...@virtuozzo.com> wrote:

> 19.02.2021 19:58, Eric Blake wrote:
> > On 2/19/21 10:42 AM, Eric Blake wrote:
> >
> >>> To me, data=false looks compatible with NBD_STATE_HOLE. From user point
> >>> of view, getting same results from qemu-nbd and qemu-img is more
> >>> important than being more correct about allocation status.
> >>
> >> More to the point, here is our inconsistency:
> >>
> >> In nbd/server.c, we turn !BDRV_BLOCK_ALLOCATED into NBD_STATE_HOLE
> >>
> >> In block/nbd.c, we turn !NBD_STATE_HOLE into BDRV_BLOCK_DATA
> >>
> >> The fact that we are not doing a round-trip conversion means that one of
> >> the two places is wrong.  And your argument that the server side is
> >> wrong makes sense to me.
> >
> > In fact, when I went back and researched when this was introduced (see
> > commit e7b1948d51 in 2018), we may have been aware of the inconsistency
> > between client and server, but didn't make up our minds at the time:
> > https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg03465.html
> > "? Hm, don't remember, what we decided about DATA/HOLE flags mapping.."
> >
> >>
> >> I'll wait a few days for any other reviewer commentary before taking
> >> this through my NBD tree.
> >>
> >
>
>
> I can add the following.
>
> First, link to my research of block_status in Qemu:
> https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg05136.html
>
> And about HOLE and ZERO..
>
> As I've noted in the research above, SCSI may return HOLE & !ZERO:
>
> from SCSI:
> Logical Block Provisioning Read Zeros (LBPRZ) bit
> 1 If the logical block provisioning read zeros (LBPRZ) bit is set to
> one, then, for an unmapped LBA specified by a read operation, the
> deviceserver shall send user data with all bits set to zero to the data-in
> buffer.
> 0 If the TPRZ bit is set to zero, then, for an unmapped LBA specified
> by a read operation, the device server may send user data with all bitsset
> to any value to the data-in buffer.
>
> So we can have an unmapped area that can be read as any random data. Same
> thing can be said about null-co driver with read-zeroes=false
>
> Also, qcow2 support ALLOCATED ZERO clusters which reads as zero but data
> is allocated - they are reasonable to report as ZERO & !HOLE
>
> And of-course UNALLOCATED ZERO clusters in qcow2 and lseek-holes are
> reasonable to report as ZERO & HOLE,  because they reads as zero and
> "future writes to that area may cause fragmentation or encounter an
> NBD_ENOSPC"..
>
> So, all combination are reasonable, we just need to fix Qemu NBD server to
> report correct statuses in all these cases.
>
> It seems that ZERO/HOLE specification is a lot more reasonable than what
> we have with ZERO/DATA/ALLOCATED in Qemu, and may be true way is move
> internal block_status to use NBD terms.
>
>
> And thanks for CCing me. Hmm, maybe, I'll suggest myself as co-maintainer
> for NBD?


Kevin, Max, are you ok with this change?

Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit

[Richard Henderson]

On 3/3/21 1:22 PM, David Hildenbrand wrote:



Am 03.03.2021 um 22:19 schrieb Richard Henderson :

On 3/3/21 1:11 PM, David Hildenbrand wrote:

MMIO on s390x? :)


hw/s390x/s390-pci-bus.c, memory_region_init_io*().



... part of system address space where a CPU could stumble over it?


Impossible to tell within 3 layers of object wrappers.  :-(
I suppose I have no idea how "pci" was hacked onto s390x.

r~

Re: QEMU RBD is slow with QCOW2 images

[Peter Lieven]

Am 03.03.21 um 19:47 schrieb Jason Dillaman:
> On Wed, Mar 3, 2021 at 12:41 PM Stefano Garzarella  
> wrote:
>> Hi Jason,
>> as reported in this BZ [1], when qemu-img creates a QCOW2 image on RBD
>> writing data is very slow compared to a raw file.
>>
>> Comparing raw vs QCOW2 image creation with RBD I found that we use a
>> different object size, for the raw file I see '4 MiB objects', for QCOW2
>> I see '64 KiB objects' as reported on comment 14 [2].
>> This should be the main issue of slowness, indeed forcing in the code 4
>> MiB object size also for QCOW2 increased the speed a lot.
>>
>> Looking better I discovered that for raw files, we call rbd_create()
>> with obj_order = 0 (if 'cluster_size' options is not defined), so the
>> default object size is used.
>> Instead for QCOW2, we use obj_order = 16, since the default
>> 'cluster_size' defined for QCOW2, is 64 KiB.
>>
>> Using '-o cluster_size=2M' with qemu-img changed only the qcow2 cluster
>> size, since in qcow2_co_create_opts() we remove the 'cluster_size' from
>> QemuOpts calling qemu_opts_to_qdict_filtered().
>> For some reason that I have yet to understand, after this deletion,
>> however remains in QemuOpts the default value of 'cluster_size' for
>> qcow2 (64 KiB), that it's used in qemu_rbd_co_create_opts()
>>
>> At this point my doubts are:
>> Does it make sense to use the same cluster_size as qcow2 as object_size
>> in RBD?
> No, not really. But it also doesn't really make any sense to put a
> QCOW2 image within an RBD image. To clarify from the BZ, OpenStack
> does not put QCOW2 images on RBD, it converts QCOW2 images into raw
> images to store in RBD.


As discussed earlier the only reasonable format for rbd image is raw.

What is the idea behind putting a qcow2 on an rbd pool?

Jason and I even discussed shortly durign the review of the rbd driver rewrite 
I posted

earlier if it was ok to drop support for writing past the end of file.


Anyway the reason why it is so slow is that write requests serialize if the

qcow2 file grows. If there is a sane reason why we need qcow2 on rbd

we need to implement at least preallocation mode = full to overcome

the serialization.


Peter

Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit

[David Hildenbrand]

> Am 03.03.2021 um 22:19 schrieb Richard Henderson 
> :
> 
> On 3/3/21 1:11 PM, David Hildenbrand wrote:
>> MMIO on s390x? :)
> 
> hw/s390x/s390-pci-bus.c, memory_region_init_io*().
> 

... part of system address space where a CPU could stumble over it?

> r~
> 

Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit

[Richard Henderson]

On 3/3/21 1:11 PM, David Hildenbrand wrote:

MMIO on s390x? :)


hw/s390x/s390-pci-bus.c, memory_region_init_io*().


r~

Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit

[David Hildenbrand]

> Am 03.03.2021 um 22:05 schrieb Richard Henderson 
> :
> 
> On 3/3/21 11:39 AM, David Hildenbrand wrote:
>> Should we start wrapping that stuff into #ifdef CONFIG_TCG ?
>>> +uint64_t tlb_fill_tec;   /* translation exception code during tlb_fill 
>>> */
>>> +int tlb_fill_exc;/* exception number seen during tlb_fill */
> 
> Eh, probably not.  At least not until we elide the softmmu tlb, which is 
> fantastically larger.
> 
>>> +if (unlikely(flags & TLB_INVALID_MASK)) {
>>> +return false;
>> ^ I recall PAGE_WRITE_INV handling where we immediately set TLB_INVALID_MASK 
>> again on write access (to handle low-address protection cleanly). I suspect 
>> that TLB_INVALID_MASK will be set in that case (I could be wrong, though).
>> What certainly would work is checking for "haddr != NULL".
>> /* Don't rely on TLB_INVALID_MASK - see PAGE_WRITE_INV handling. */
>> if (unlikely(!haddr1)) {
>> return false;
>> }
> 
> Ah, right.  I consider TLB_INVALID_MASK being set in the return from 
> probe_access_flags for PAGE_WRITE_INV a bug.  I'm not sure how to fix that 
> right away.
> 
> Well, !haddr1 is also false for TLB_MMIO, so you'd need to check for that as 
> well.

MMIO on s390x? :)

Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit

[Richard Henderson]

On 3/3/21 11:39 AM, David Hildenbrand wrote:

Should we start wrapping that stuff into #ifdef CONFIG_TCG ?


+    uint64_t tlb_fill_tec;   /* translation exception code during tlb_fill */
+    int tlb_fill_exc;    /* exception number seen during tlb_fill */


Eh, probably not.  At least not until we elide the softmmu tlb, which is 
fantastically larger.



+    if (unlikely(flags & TLB_INVALID_MASK)) {
+    return false;


^ I recall PAGE_WRITE_INV handling where we immediately set TLB_INVALID_MASK 
again on write access (to handle low-address protection cleanly). I suspect 
that TLB_INVALID_MASK will be set in that case (I could be wrong, though).


What certainly would work is checking for "haddr != NULL".

/* Don't rely on TLB_INVALID_MASK - see PAGE_WRITE_INV handling. */
if (unlikely(!haddr1)) {
 return false;
}


Ah, right.  I consider TLB_INVALID_MASK being set in the return from 
probe_access_flags for PAGE_WRITE_INV a bug.  I'm not sure how to fix that 
right away.


Well, !haddr1 is also false for TLB_MMIO, so you'd need to check for that as 
well.


r~

[PATCH v3 2/2] tests: Add functional test for out-of-process device emulation

[Willian Rampazzo]

From: Jagannathan Raman 

Runs the Avocado acceptance test to check if a
remote lsi53c895a device gets identified by the guest.

Signed-off-by: Elena Ufimtseva 
Signed-off-by: John G Johnson 
Signed-off-by: Jagannathan Raman 
Reviewed-by: Wainer dos Santos Moschetta 
Reviewed-by: Marc-André Lureau 
[WR: Refactored code]
Signed-off-by: Willian Rampazzo 
Tested-by: Wainer dos Santos Moschetta 
---
 tests/acceptance/multiprocess.py | 95 
 1 file changed, 95 insertions(+)
 create mode 100644 tests/acceptance/multiprocess.py

diff --git a/tests/acceptance/multiprocess.py b/tests/acceptance/multiprocess.py
new file mode 100644
index 00..96627f022a
--- /dev/null
+++ b/tests/acceptance/multiprocess.py
@@ -0,0 +1,95 @@
+# Test for multiprocess qemu
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+
+import os
+import socket
+
+from avocado_qemu import Test
+from avocado_qemu import wait_for_console_pattern
+from avocado_qemu import exec_command
+from avocado_qemu import exec_command_and_wait_for_pattern
+
+class Multiprocess(Test):
+"""
+:avocado: tags=multiprocess
+"""
+KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
+
+def do_test(self, kernel_url, initrd_url, kernel_command_line,
+machine_type):
+"""Main test method"""
+self.require_accelerator('kvm')
+
+# Create socketpair to connect proxy and remote processes
+proxy_sock, remote_sock = socket.socketpair(socket.AF_UNIX,
+socket.SOCK_STREAM)
+os.set_inheritable(proxy_sock.fileno(), True)
+os.set_inheritable(remote_sock.fileno(), True)
+
+kernel_path = self.fetch_asset(kernel_url)
+initrd_path = self.fetch_asset(initrd_url)
+
+# Create remote process
+remote_vm = self.get_vm()
+remote_vm.add_args('-machine', 'x-remote')
+remote_vm.add_args('-nodefaults')
+remote_vm.add_args('-device', 'lsi53c895a,id=lsi1')
+remote_vm.add_args('-object', 'x-remote-object,id=robj1,'
+   'devid=lsi1,fd='+str(remote_sock.fileno()))
+remote_vm.launch()
+
+# Create proxy process
+self.vm.set_console()
+self.vm.add_args('-machine', machine_type)
+self.vm.add_args('-accel', 'kvm')
+self.vm.add_args('-cpu', 'host')
+self.vm.add_args('-object',
+ 'memory-backend-memfd,id=sysmem-file,size=2G')
+self.vm.add_args('--numa', 'node,memdev=sysmem-file')
+self.vm.add_args('-m', '2048')
+self.vm.add_args('-kernel', kernel_path,
+ '-initrd', initrd_path,
+ '-append', kernel_command_line)
+self.vm.add_args('-device',
+ 'x-pci-proxy-dev,'
+ 'id=lsi1,fd='+str(proxy_sock.fileno()))
+self.vm.launch()
+wait_for_console_pattern(self, 'as init process',
+ 'Kernel panic - not syncing')
+exec_command(self, 'mount -t sysfs sysfs /sys')
+exec_command_and_wait_for_pattern(self,
+  'cat /sys/bus/pci/devices/*/uevent',
+  'PCI_ID=1000:0012')
+
+def test_multiprocess_x86_64(self):
+"""
+:avocado: tags=arch:x86_64
+"""
+kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
+  '/linux/releases/31/Everything/x86_64/os/images'
+  '/pxeboot/vmlinuz')
+initrd_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
+  '/linux/releases/31/Everything/x86_64/os/images'
+  '/pxeboot/initrd.img')
+kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
+   'console=ttyS0 rdinit=/bin/bash')
+machine_type = 'pc'
+self.do_test(kernel_url, initrd_url, kernel_command_line, machine_type)
+
+def test_multiprocess_aarch64(self):
+"""
+:avocado: tags=arch:aarch64
+"""
+kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
+  '/linux/releases/31/Everything/aarch64/os/images'
+  '/pxeboot/vmlinuz')
+initrd_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
+  '/linux/releases/31/Everything/aarch64/os/images'
+  '/pxeboot/initrd.img')
+kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
+   'rdinit=/bin/bash console=ttyAMA0')
+machine_type = 'virt,gic-version=3'
+self.do_test(kernel_url, initrd_url, kernel_command_line, machine_type)
-- 
2.29.2

[PATCH v3 0/2] tests: Add functional test for out-of-process device emulation

[Willian Rampazzo]

This series is a respin to the "multi-process: Acceptance test for
multiprocess QEMU" patch sent in December which, runs an Avocado
functional test to check if a remote lsi53c895a device gets identified
by the guest:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg769188.html

First, we introduce the `exec_command` to the avocado_qemu package,
which sends a command to a console without the need to wait for a
pattern as a result. This is useful when a test needs to execute a
command that does not produce an output, like, for example, a `mount`
command.

Then, the original test is refactored to simplify the code using the
new `exec_command` function, remove the unnecessary change to the
`wait_for_console_pattern` method and normalize the use of single
quotes.

CI Pipeline: https://gitlab.com/willianrampazzo/qemu/-/pipelines/261946622

Supersedes: 
<785772783205140e219b8bfe7f793305ee768f03.1608705805.git.elena.ufimts...@oracle.com>

Change from v2:
  - Add command type to the `exec_command` docstring

Jagannathan Raman (1):
  tests: Add functional test for out-of-process device emulation

Willian Rampazzo (1):
  avocado_qemu: add exec_command function

 tests/acceptance/avocado_qemu/__init__.py | 14 +++-
 tests/acceptance/multiprocess.py  | 95 +++
 2 files changed, 108 insertions(+), 1 deletion(-)
 create mode 100644 tests/acceptance/multiprocess.py

-- 
2.29.2

[PATCH v3 1/2] avocado_qemu: add exec_command function

[Willian Rampazzo]

Sometimes a test needs to send a command to a console without waiting
for a pattern as a result, or the command issued do not produce any kind
of output, like, for example, a `mount` command.

This introduces the `exec_command` function to the avocado_qemu,
allowing the test to send a command to the console without the need to
match a pattern produced as a result.

Signed-off-by: Willian Rampazzo 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Wainer dos Santos Moschetta 
---
 tests/acceptance/avocado_qemu/__init__.py | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index df167b142c..0e6d286403 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -93,7 +93,7 @@ def _console_interaction(test, success_message, 
failure_message,
 if not msg:
 continue
 console_logger.debug(msg)
-if success_message in msg:
+if success_message is None or success_message in msg:
 break
 if failure_message and failure_message in msg:
 console.close()
@@ -139,6 +139,18 @@ def wait_for_console_pattern(test, success_message, 
failure_message=None,
 """
 _console_interaction(test, success_message, failure_message, None, vm=vm)
 
+def exec_command(test, command):
+"""
+Send a command to a console (appending CRLF characters), while logging
+the content.
+
+:param test: an Avocado test containing a VM.
+:type test: :class:`avocado_qemu.Test`
+:param command: the command to send
+:type command: str
+"""
+_console_interaction(test, None, None, command + '\r')
+
 def exec_command_and_wait_for_pattern(test, command,
   success_message, failure_message=None):
 """
-- 
2.29.2

Re: [PATCH] arm: bump amount of PMU counters to pass SBSA ACS

[Marcin Juszkiewicz]

W dniu 03.03.2021 o 19:06, Peter Maydell pisze:

On Wed, 3 Mar 2021 at 17:48, Leif Lindholm  wrote:

It would be good if we could get 6.0 closer to SBSA compliance.


How far away are we at the moment ?


Hard to tell me how many of those things are missing in QEMU and how 
many in EDK2 we use as firmware.


SBSA-ACS failures:

GIC ITS is missing (Shashi Mallela works on it):

 102 : If PCIe, then GIC implements ITS  : Result:  --FAIL-- 1
 104 : GIC Maintenance Interrupt : Result:  --FAIL-- 1


System timers are not present in GTDT so few more tests are not run:

 206 : SYS Timer if PE Timer not ON  : Result:  --FAIL-- 1
   PE Timers are not always-on.
 207 : CNTCTLBase & CNTBaseN access  : Result:  -SKIPPED- 1
   No System timers are defined
 505 : Wake from System Timer Interrupt  : Result:  -SKIPPED- 1
   No system timers implemented


There is no SMMU present so SMMU and IO virtualization tests are not 
run. This one is probably related:


 605 : Memory Access to Un-Populated addr: Result:  --FAIL-- 1
   Memory access check fails at address = 0x104C

Re: [PATCH v2 42/42] esp: add support for unaligned accesses

[Laurent Vivier]

Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> When the MacOS toolbox ROM transfers data from a target device to an unaligned
> memory address, the first/last byte of a 16-bit transfer needs to be handled
> separately. This means that the first byte is preloaded into the FIFO before
> the transfer, or the last byte remains in the FIFO after the transfer.
> 
> The result of this is that the PDMA routines must be updated so that the FIFO
> is loaded/unloaded if the last 16-bit word is used (rather than the last byte)
> and any remaining byte from a FIFO wraparound is handled correctly.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 48 +---
>  1 file changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index ae9e265a5d..d2d6366525 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -498,11 +498,22 @@ static void do_dma_pdma_cb(ESPState *s)
>  
>  if (to_device) {
>  /* Copy FIFO data to device */
> -len = MIN(fifo8_num_used(&s->fifo), ESP_FIFO_SZ);
> +len = MIN(s->async_len, ESP_FIFO_SZ);
> +len = MIN(len, fifo8_num_used(&s->fifo));
>  memcpy(s->async_buf, fifo8_pop_buf(&s->fifo, len, &n), len);
> -s->async_buf += len;
> -s->async_len -= len;
> -s->ti_size += len;
> +s->async_buf += n;
> +s->async_len -= n;
> +s->ti_size += n;
> +
> +if (n < len) {
> +/* Unaligned accesses can cause FIFO wraparound */
> +len = len - n;
> +memcpy(s->async_buf, fifo8_pop_buf(&s->fifo, len, &n), len);
> +s->async_buf += n;
> +s->async_len -= n;
> +s->ti_size += n;
> +}
> +
>  if (s->async_len == 0) {
>  scsi_req_continue(s->current_req);
>  return;
> @@ -532,12 +543,18 @@ static void do_dma_pdma_cb(ESPState *s)
>  
>  if (esp_get_tc(s) != 0) {
>  /* Copy device data to FIFO */
> -len = MIN(s->async_len, fifo8_num_free(&s->fifo));
> +len = MIN(s->async_len, esp_get_tc(s));
> +len = MIN(len, fifo8_num_free(&s->fifo));
>  fifo8_push_all(&s->fifo, s->async_buf, len);
>  s->async_buf += len;
>  s->async_len -= len;
>  s->ti_size -= len;
>  esp_set_tc(s, esp_get_tc(s) - len);
> +
> +if (esp_get_tc(s) == 0) {
> +/* Indicate transfer to FIFO is complete */
> + s->rregs[ESP_RSTAT] |= STAT_TC;
> +}
>  return;
>  }
>  
> @@ -612,12 +629,29 @@ static void esp_do_dma(ESPState *s)
>  if (s->dma_memory_write) {
>  s->dma_memory_write(s->dma_opaque, s->async_buf, len);
>  } else {
> +/* Adjust TC for any leftover data in the FIFO */
> +if (!fifo8_is_empty(&s->fifo)) {
> +esp_set_tc(s, esp_get_tc(s) - fifo8_num_used(&s->fifo));
> +}
> +
>  /* Copy device data to FIFO */
>  len = MIN(len, fifo8_num_free(&s->fifo));
>  fifo8_push_all(&s->fifo, s->async_buf, len);
>  s->async_buf += len;
>  s->async_len -= len;
>  s->ti_size -= len;
> +
> +/*
> + * MacOS toolbox uses a TI length of 16 bytes for all commands, 
> so
> + * commands shorter than this must be padded accordingly
> + */
> +if (len < esp_get_tc(s) && esp_get_tc(s) <= ESP_FIFO_SZ) {
> +while (fifo8_num_used(&s->fifo) < ESP_FIFO_SZ) {
> +esp_fifo_push(s, 0);
> +len++;
> +}
> +}
> +
>  esp_set_tc(s, esp_get_tc(s) - len);
>  s->pdma_cb = do_dma_pdma_cb;
>  esp_raise_drq(s);
> @@ -1168,7 +1202,7 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr 
> addr,
>  break;
>  }
>  dmalen = esp_get_tc(s);
> -if (dmalen == 0 || fifo8_is_full(&s->fifo)) {
> +if (dmalen == 0 || fifo8_num_free(&s->fifo) < 2) {
>  s->pdma_cb(s);
>  }
>  }
> @@ -1191,7 +1225,7 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, 
> hwaddr addr,
>  val = (val << 8) | esp_pdma_read(s);
>  break;
>  }
> -if (fifo8_is_empty(&s->fifo)) {
> +if (fifo8_num_used(&s->fifo) < 2) {
>  s->pdma_cb(s);
>  }
>  return val;
> 

Reviewed-by: Laurent Vivier 

Re: [PATCH v2 41/42] esp: implement non-DMA transfers in PDMA mode

[Laurent Vivier]

Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> The MacOS toolbox ROM uses non-DMA TI commands to handle the first/last byte
> of an unaligned 16-bit transfer to memory.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 133 ++
>  include/hw/scsi/esp.h |   1 +
>  2 files changed, 98 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 1f01f2314b..ae9e265a5d 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -296,6 +296,7 @@ static void do_busid_cmd(ESPState *s, uint8_t busid)
>  if (datalen != 0) {
>  s->rregs[ESP_RSTAT] = STAT_TC;
>  s->rregs[ESP_RSEQ] = SEQ_CD;
> +s->ti_cmd = 0;
>  esp_set_tc(s, 0);
>  if (datalen > 0) {
>  /*
> @@ -651,6 +652,71 @@ static void esp_do_dma(ESPState *s)
>  esp_lower_drq(s);
>  }
>  
> +static void esp_do_nodma(ESPState *s)
> +{
> +int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
> +uint32_t cmdlen, n;
> +int len;
> +
> +if (s->do_cmd) {
> +cmdlen = fifo8_num_used(&s->cmdfifo);
> +trace_esp_handle_ti_cmd(cmdlen);
> +s->ti_size = 0;
> +if ((s->rregs[ESP_RSTAT] & 7) == STAT_CD) {
> +/* No command received */
> +if (s->cmdfifo_cdb_offset == fifo8_num_used(&s->cmdfifo)) {
> +return;
> +}
> +
> +/* Command has been received */
> +s->do_cmd = 0;
> +do_cmd(s);
> +} else {
> +/*
> + * Extra message out bytes received: update cmdfifo_cdb_offset
> + * and then switch to commmand phase
> + */
> +s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
> +s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
> +s->rregs[ESP_RSEQ] = SEQ_CD;
> +s->rregs[ESP_RINTR] |= INTR_BS;
> +esp_raise_irq(s);
> +}
> +return;
> +}
> +
> +if (s->async_len == 0) {
> +/* Defer until data is available.  */
> +return;
> +}
> +
> +if (to_device) {
> +len = MIN(fifo8_num_used(&s->fifo), ESP_FIFO_SZ);
> +memcpy(s->async_buf, fifo8_pop_buf(&s->fifo, len, &n), len);
> +s->async_buf += len;
> +s->async_len -= len;
> +s->ti_size += len;
> +} else {
> +len = MIN(s->ti_size, s->async_len);
> +len = MIN(len, fifo8_num_free(&s->fifo));
> +fifo8_push_all(&s->fifo, s->async_buf, len);
> +s->async_buf += len;
> +s->async_len -= len;
> +s->ti_size -= len;
> +}
> +
> +if (s->async_len == 0) {
> +scsi_req_continue(s->current_req);
> +
> +if (to_device || s->ti_size == 0) {
> +return;
> +}
> +}
> +
> +s->rregs[ESP_RINTR] |= INTR_BS;
> +esp_raise_irq(s);
> +}
> +
>  void esp_command_complete(SCSIRequest *req, uint32_t status,
>size_t resid)
>  {
> @@ -708,56 +774,51 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
>  return;
>  }
>  
> -if (dmalen) {
> -esp_do_dma(s);
> -} else if (s->ti_size <= 0) {
> +if (s->ti_cmd == 0) {
>  /*
> - * If this was the last part of a DMA transfer then the
> - * completion interrupt is deferred to here.
> + * Always perform the initial transfer upon reception of the next TI
> + * command to ensure the DMA/non-DMA status of the command is 
> correct.
> + * It is not possible to use s->dma directly in the section below as
> + * some OSs send non-DMA NOP commands after a DMA transfer. Hence if 
> the
> + * async data transfer is delayed then s->dma is set incorrectly.
>   */
> -esp_dma_done(s);
> -esp_lower_drq(s);
> +return;
> +}
> +
> +if (s->ti_cmd & CMD_DMA) {
> +if (dmalen) {
> +esp_do_dma(s);
> +} else if (s->ti_size <= 0) {
> +/*
> + * If this was the last part of a DMA transfer then the
> + * completion interrupt is deferred to here.
> + */
> +esp_dma_done(s);
> +esp_lower_drq(s);
> +}
> +} else {
> +esp_do_nodma(s);
>  }
>  }
>  
>  static void handle_ti(ESPState *s)
>  {
> -uint32_t dmalen, cmdlen;
> +uint32_t dmalen;
>  
>  if (s->dma && !s->dma_enabled) {
>  s->dma_cb = handle_ti;
>  return;
>  }
>  
> -dmalen = esp_get_tc(s);
> +s->ti_cmd = s->rregs[ESP_CMD];
>  if (s->dma) {
> +dmalen = esp_get_tc(s);
>  trace_esp_handle_ti(dmalen);
>  s->rregs[ESP_RSTAT] &= ~STAT_TC;
>  esp_do_dma(s);
> -} else if (s->do_cmd) {
> -cmdlen = fifo8_num_used(&s->cmdfifo);
> -trace_esp_handle_ti_cmd(cmdlen);
> -s->ti_size = 0;
> -if ((s->rregs[ESP_RSTAT] & 7) == STAT_CD) {
> -/* No command received *

Re: [PATCH v2 40/42] esp: add trivial implementation of the ESP_RFLAGS register

[Laurent Vivier]

Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> The bottom 5 bits contain the number of bytes remaining in the FIFO which is
> trivial to implement with Fifo8 (the remaining bits are unimplemented and left
> as 0 for now).
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 9dd9947307..1f01f2314b 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -825,6 +825,10 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
>  val = s->rregs[saddr];
>  }
>  break;
> + case ESP_RFLAGS:
> +/* Bottom 5 bits indicate number of bytes in FIFO */
> +val = fifo8_num_used(&s->fifo);
> +break;
>  default:
>  val = s->rregs[saddr];
>  break;
> 

Reviewed-by: Laurent Vivier 

Re: [PATCH v2 39/42] esp: convert cmdbuf from array to Fifo8

[Laurent Vivier]

Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> Rename ESP_CMDBUF_SZ to ESP_CMDFIFO_SZ and cmdbuf_cdb_offset to 
> cmdfifo_cdb_offset
> to indicate that the command buffer type has changed from an array to a Fifo8.
> 
> This also enables us to remove the ESPState field cmdlen since the command 
> length
> is now simply the number of elements used in cmdfifo.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 151 +++---
>  include/hw/scsi/esp.h |   9 +--
>  2 files changed, 101 insertions(+), 59 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 98df357276..9dd9947307 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -117,6 +117,25 @@ static uint8_t esp_fifo_pop(ESPState *s)
>  return fifo8_pop(&s->fifo);
>  }
>  
> +static void esp_cmdfifo_push(ESPState *s, uint8_t val)
> +{
> +if (fifo8_num_used(&s->cmdfifo) == ESP_CMDFIFO_SZ) {
> +trace_esp_error_fifo_overrun();
> +return;
> +}
> +
> +fifo8_push(&s->cmdfifo, val);
> +}
> +
> +static uint8_t esp_cmdfifo_pop(ESPState *s)
> +{
> +if (fifo8_is_empty(&s->cmdfifo)) {
> +return 0;
> +}
> +
> +return fifo8_pop(&s->cmdfifo);
> +}
> +
>  static uint32_t esp_get_tc(ESPState *s)
>  {
>  uint32_t dmalen;
> @@ -151,7 +170,7 @@ static uint8_t esp_pdma_read(ESPState *s)
>  uint8_t val;
>  
>  if (s->do_cmd) {
> -val = s->cmdbuf[s->cmdlen++];
> +val = esp_cmdfifo_pop(s);
>  } else {
>  val = esp_fifo_pop(s);
>  }
> @@ -168,7 +187,7 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
>  }
>  
>  if (s->do_cmd) {
> -s->cmdbuf[s->cmdlen++] = val;
> +esp_cmdfifo_push(s, val);
>  } else {
>  esp_fifo_push(s, val);
>  }
> @@ -214,7 +233,7 @@ static int esp_select(ESPState *s)
>  
>  static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
>  {
> -uint8_t *buf = s->cmdbuf;
> +uint8_t buf[ESP_CMDFIFO_SZ];
>  uint32_t dmalen, n;
>  int target;
>  
> @@ -226,15 +245,18 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
>  }
>  if (s->dma_memory_read) {
>  s->dma_memory_read(s->dma_opaque, buf, dmalen);
> +fifo8_push_all(&s->cmdfifo, buf, dmalen);
>  } else {
>  if (esp_select(s) < 0) {
> +fifo8_reset(&s->cmdfifo);
>  return -1;
>  }
>  esp_raise_drq(s);
> +fifo8_reset(&s->cmdfifo);
>  return 0;
>  }
>  } else {
> -dmalen = MIN(s->ti_size, maxlen);
> +dmalen = MIN(fifo8_num_used(&s->fifo), maxlen);
>  if (dmalen == 0) {
>  return 0;
>  }
> @@ -242,27 +264,35 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
>  if (dmalen >= 3) {
>  buf[0] = buf[2] >> 5;
>  }
> +fifo8_push_all(&s->cmdfifo, buf, dmalen);
>  }
>  trace_esp_get_cmd(dmalen, target);
>  
>  if (esp_select(s) < 0) {
> +fifo8_reset(&s->cmdfifo);
>  return -1;
>  }
>  return dmalen;
>  }
>  
> -static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
> +static void do_busid_cmd(ESPState *s, uint8_t busid)
>  {
> +uint32_t n, cmdlen;
>  int32_t datalen;
>  int lun;
>  SCSIDevice *current_lun;
> +uint8_t *buf;
>  
>  trace_esp_do_busid_cmd(busid);
>  lun = busid & 7;
> +cmdlen = fifo8_num_used(&s->cmdfifo);
> +buf = (uint8_t *)fifo8_pop_buf(&s->cmdfifo, cmdlen, &n);
> +
>  current_lun = scsi_device_find(&s->bus, 0, s->current_dev->id, lun);
>  s->current_req = scsi_req_new(current_lun, 0, lun, buf, s);
>  datalen = scsi_req_enqueue(s->current_req);
>  s->ti_size = datalen;
> +fifo8_reset(&s->cmdfifo);
>  if (datalen != 0) {
>  s->rregs[ESP_RSTAT] = STAT_TC;
>  s->rregs[ESP_RSEQ] = SEQ_CD;
> @@ -287,18 +317,25 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, 
> uint8_t busid)
>  
>  static void do_cmd(ESPState *s)
>  {
> -uint8_t *buf = s->cmdbuf;
> -uint8_t busid = buf[0];
> +uint8_t busid = fifo8_pop(&s->cmdfifo);
> +uint32_t n;
> +
> +s->cmdfifo_cdb_offset--;
>  
>  /* Ignore extended messages for now */
> -do_busid_cmd(s, &buf[s->cmdbuf_cdb_offset], busid);
> +if (s->cmdfifo_cdb_offset) {
> +fifo8_pop_buf(&s->cmdfifo, s->cmdfifo_cdb_offset, &n);
> +s->cmdfifo_cdb_offset = 0;
> +}
> +
> +do_busid_cmd(s, busid);
>  }
>  
>  static void satn_pdma_cb(ESPState *s)
>  {
>  s->do_cmd = 0;
> -if (s->cmdlen) {
> -s->cmdbuf_cdb_offset = 1;
> +if (!fifo8_is_empty(&s->cmdfifo)) {
> +s->cmdfifo_cdb_offset = 1;
>  do_cmd(s);
>  }
>  }
> @@ -312,13 +349,11 @@ static void handle_satn(ESPState *s)
>  return;
>  }
>  s->pdma_cb = satn_pdma_cb;
> -cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
> +cmdlen = get_cmd(s, ESP_CMDFIFO_SZ);
>  

Re: [PATCH v2 38/42] esp: convert ti_buf from array to Fifo8

[Laurent Vivier]

Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> Rename TI_BUFSZ to ESP_FIFO_SZ since this constant is really describing the 
> size
> of the FIFO and is not directly related to the TI size.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 117 ++
>  include/hw/scsi/esp.h |   8 +--
>  2 files changed, 79 insertions(+), 46 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 5a66b7d710..98df357276 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -98,6 +98,25 @@ void esp_request_cancelled(SCSIRequest *req)
>  }
>  }
>  
> +static void esp_fifo_push(ESPState *s, uint8_t val)
> +{
> +if (fifo8_num_used(&s->fifo) == ESP_FIFO_SZ) {
> +trace_esp_error_fifo_overrun();
> +return;
> +}
> +
> +fifo8_push(&s->fifo, val);
> +}
> +
> +static uint8_t esp_fifo_pop(ESPState *s)
> +{
> +if (fifo8_is_empty(&s->fifo)) {
> +return 0;
> +}
> +
> +return fifo8_pop(&s->fifo);
> +}
> +
>  static uint32_t esp_get_tc(ESPState *s)
>  {
>  uint32_t dmalen;
> @@ -134,7 +153,7 @@ static uint8_t esp_pdma_read(ESPState *s)
>  if (s->do_cmd) {
>  val = s->cmdbuf[s->cmdlen++];
>  } else {
> -val = s->ti_buf[s->ti_rptr++];
> +val = esp_fifo_pop(s);
>  }
>  
>  return val;
> @@ -151,7 +170,7 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
>  if (s->do_cmd) {
>  s->cmdbuf[s->cmdlen++] = val;
>  } else {
> -s->ti_buf[s->ti_wptr++] = val;
> +esp_fifo_push(s, val);
>  }
>  
>  dmalen--;
> @@ -165,8 +184,7 @@ static int esp_select(ESPState *s)
>  target = s->wregs[ESP_WBUSID] & BUSID_DID;
>  
>  s->ti_size = 0;
> -s->ti_rptr = 0;
> -s->ti_wptr = 0;
> +fifo8_reset(&s->fifo);
>  
>  if (s->current_req) {
>  /* Started a new command before the old one finished.  Cancel it.  */
> @@ -197,7 +215,7 @@ static int esp_select(ESPState *s)
>  static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
>  {
>  uint8_t *buf = s->cmdbuf;
> -uint32_t dmalen;
> +uint32_t dmalen, n;
>  int target;
>  
>  target = s->wregs[ESP_WBUSID] & BUSID_DID;
> @@ -220,7 +238,7 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
>  if (dmalen == 0) {
>  return 0;
>  }
> -memcpy(buf, s->ti_buf, dmalen);
> +memcpy(buf, fifo8_pop_buf(&s->fifo, dmalen, &n), dmalen);
>  if (dmalen >= 3) {
>  buf[0] = buf[2] >> 5;
>  }
> @@ -392,12 +410,18 @@ static void write_response_pdma_cb(ESPState *s)
>  
>  static void write_response(ESPState *s)
>  {
> +uint32_t n;
> +
>  trace_esp_write_response(s->status);
> -s->ti_buf[0] = s->status;
> -s->ti_buf[1] = 0;
> +
> +fifo8_reset(&s->fifo);
> +esp_fifo_push(s, s->status);
> +esp_fifo_push(s, 0);
> +
>  if (s->dma) {
>  if (s->dma_memory_write) {
> -s->dma_memory_write(s->dma_opaque, s->ti_buf, 2);
> +s->dma_memory_write(s->dma_opaque,
> +(uint8_t *)fifo8_pop_buf(&s->fifo, 2, &n), 
> 2);
>  s->rregs[ESP_RSTAT] = STAT_TC | STAT_ST;
>  s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
>  s->rregs[ESP_RSEQ] = SEQ_CD;
> @@ -408,8 +432,6 @@ static void write_response(ESPState *s)
>  }
>  } else {
>  s->ti_size = 2;
> -s->ti_rptr = 0;
> -s->ti_wptr = 2;
>  s->rregs[ESP_RFLAGS] = 2;
>  }
>  esp_raise_irq(s);
> @@ -429,6 +451,7 @@ static void do_dma_pdma_cb(ESPState *s)
>  {
>  int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
>  int len;
> +uint32_t n;
>  
>  if (s->do_cmd) {
>  s->ti_size = 0;
> @@ -441,10 +464,8 @@ static void do_dma_pdma_cb(ESPState *s)
>  
>  if (to_device) {
>  /* Copy FIFO data to device */
> -len = MIN(s->ti_wptr, TI_BUFSZ);
> -memcpy(s->async_buf, s->ti_buf, len);
> -s->ti_wptr = 0;
> -s->ti_rptr = 0;
> +len = MIN(fifo8_num_used(&s->fifo), ESP_FIFO_SZ);
> +memcpy(s->async_buf, fifo8_pop_buf(&s->fifo, len, &n), len);
>  s->async_buf += len;
>  s->async_len -= len;
>  s->ti_size += len;
> @@ -477,11 +498,8 @@ static void do_dma_pdma_cb(ESPState *s)
>  
>  if (esp_get_tc(s) != 0) {
>  /* Copy device data to FIFO */
> -s->ti_wptr = 0;
> -s->ti_rptr = 0;
> -len = MIN(s->async_len, TI_BUFSZ);
> -memcpy(s->ti_buf, s->async_buf, len);
> -s->ti_wptr += len;
> +len = MIN(s->async_len, fifo8_num_free(&s->fifo));
> +fifo8_push_all(&s->fifo, s->async_buf, len);
>  s->async_buf += len;
>  s->async_len -= len;
>  s->ti_size -= len;
> @@ -561,9 +579,8 @@ static void esp_do_dma(ESPState *s)
>  s->dma_memory_write(s->dma_opaque, s->async_buf, len);
>  } else

Re: [PATCH v2 37/42] esp: transition to message out phase after SATN and stop command

[Laurent Vivier]

Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> The SCSI bus should remain in the message out phase after the SATN and stop
> command rather than transitioning to the command phase. A new ESPState 
> variable
> cmdbuf_cdb_offset is added which stores the offset of the CDB from the start
> of cmdbuf when accumulating extended message out phase data.
> 
> Currently any extended message out data is discarded in do_cmd() before the 
> CDB
> is processed in do_busid_cmd().
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 72 ++-
>  include/hw/scsi/esp.h |  2 ++
>  2 files changed, 60 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 058b482fda..5a66b7d710 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -272,13 +272,15 @@ static void do_cmd(ESPState *s)
>  uint8_t *buf = s->cmdbuf;
>  uint8_t busid = buf[0];
>  
> -do_busid_cmd(s, &buf[1], busid);
> +/* Ignore extended messages for now */
> +do_busid_cmd(s, &buf[s->cmdbuf_cdb_offset], busid);
>  }
>  
>  static void satn_pdma_cb(ESPState *s)
>  {
>  s->do_cmd = 0;
>  if (s->cmdlen) {
> +s->cmdbuf_cdb_offset = 1;
>  do_cmd(s);
>  }
>  }
> @@ -295,6 +297,7 @@ static void handle_satn(ESPState *s)
>  cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
>  if (cmdlen > 0) {
>  s->cmdlen = cmdlen;
> +s->cmdbuf_cdb_offset = 1;
>  do_cmd(s);
>  } else if (cmdlen == 0) {
>  s->cmdlen = 0;
> @@ -309,6 +312,7 @@ static void s_without_satn_pdma_cb(ESPState *s)
>  {
>  s->do_cmd = 0;
>  if (s->cmdlen) {
> +s->cmdbuf_cdb_offset = 0;
>  do_busid_cmd(s, s->cmdbuf, 0);
>  }
>  }
> @@ -325,6 +329,7 @@ static void handle_s_without_atn(ESPState *s)
>  cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
>  if (cmdlen > 0) {
>  s->cmdlen = cmdlen;
> +s->cmdbuf_cdb_offset = 0;
>  do_busid_cmd(s, s->cmdbuf, 0);
>  } else if (cmdlen == 0) {
>  s->cmdlen = 0;
> @@ -341,6 +346,7 @@ static void satn_stop_pdma_cb(ESPState *s)
>  if (s->cmdlen) {
>  trace_esp_handle_satn_stop(s->cmdlen);
>  s->do_cmd = 1;
> +s->cmdbuf_cdb_offset = 1;
>  s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
>  s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
>  s->rregs[ESP_RSEQ] = SEQ_CD;
> @@ -357,21 +363,22 @@ static void handle_satn_stop(ESPState *s)
>  return;
>  }
>  s->pdma_cb = satn_stop_pdma_cb;
> -cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
> +cmdlen = get_cmd(s, 1);
>  if (cmdlen > 0) {
> -trace_esp_handle_satn_stop(s->cmdlen);
> +trace_esp_handle_satn_stop(cmdlen);
>  s->cmdlen = cmdlen;
>  s->do_cmd = 1;
> -s->rregs[ESP_RSTAT] = STAT_CD;
> +s->cmdbuf_cdb_offset = 1;
> +s->rregs[ESP_RSTAT] = STAT_MO;
>  s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
> -s->rregs[ESP_RSEQ] = SEQ_CD;
> +s->rregs[ESP_RSEQ] = SEQ_MO;
>  esp_raise_irq(s);
>  } else if (cmdlen == 0) {
>  s->cmdlen = 0;
>  s->do_cmd = 1;
> -/* Target present, but no cmd yet - switch to command phase */
> -s->rregs[ESP_RSEQ] = SEQ_CD;
> -s->rregs[ESP_RSTAT] = STAT_CD;
> +/* Target present, switch to message out phase */
> +s->rregs[ESP_RSEQ] = SEQ_MO;
> +s->rregs[ESP_RSTAT] = STAT_MO;
>  }
>  }
>  
> @@ -511,9 +518,27 @@ static void esp_do_dma(ESPState *s)
>  }
>  trace_esp_handle_ti_cmd(s->cmdlen);
>  s->ti_size = 0;
> -s->cmdlen = 0;
> -s->do_cmd = 0;
> -do_cmd(s);
> +if ((s->rregs[ESP_RSTAT] & 7) == STAT_CD) {
> +/* No command received */
> +if (s->cmdbuf_cdb_offset == s->cmdlen) {
> +return;
> +}
> +
> +/* Command has been received */
> +s->cmdlen = 0;
> +s->do_cmd = 0;
> +do_cmd(s);
> +} else {
> +/*
> + * Extra message out bytes received: update cmdbuf_cdb_offset
> + * and then switch to commmand phase
> + */
> +s->cmdbuf_cdb_offset = s->cmdlen;
> +s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
> +s->rregs[ESP_RSEQ] = SEQ_CD;
> +s->rregs[ESP_RINTR] |= INTR_BS;
> +esp_raise_irq(s);
> +}
>  return;
>  }
>  if (s->async_len == 0) {
> @@ -662,9 +687,27 @@ static void handle_ti(ESPState *s)
>  } else if (s->do_cmd) {
>  trace_esp_handle_ti_cmd(s->cmdlen);
>  s->ti_size = 0;
> -s->cmdlen = 0;
> -s->do_cmd = 0;
> -do_cmd(s);
> +if ((s->rregs[ESP_RSTAT] & 7) == STAT_CD) {
> +/* No command received */
> +if (s->cmdbuf_cdb_offset == s->cmdlen) {
> +return;
> +}
> +
> +/* Command has been received */
> +   

Re: [PATCH v2 36/42] esp: add maxlen parameter to get_cmd()

[Laurent Vivier]

Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> Some guests use a mixture of DMA and non-DMA transfers in combination with the
> SATN and stop command to transfer message out phase and command phase bytes to
> the target. Prepare for the next commit by adding a maxlen parameter to
> get_cmd() to allow partial transfers.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 20 +++-
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 617fdcb3ed..058b482fda 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -194,7 +194,7 @@ static int esp_select(ESPState *s)
>  return 0;
>  }
>  
> -static uint32_t get_cmd(ESPState *s)
> +static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
>  {
>  uint8_t *buf = s->cmdbuf;
>  uint32_t dmalen;
> @@ -202,8 +202,8 @@ static uint32_t get_cmd(ESPState *s)
>  
>  target = s->wregs[ESP_WBUSID] & BUSID_DID;
>  if (s->dma) {
> -dmalen = esp_get_tc(s);
> -if (dmalen > ESP_CMDBUF_SZ) {
> +dmalen = MIN(esp_get_tc(s), maxlen);
> +if (dmalen == 0) {
>  return 0;
>  }
>  if (s->dma_memory_read) {
> @@ -216,12 +216,14 @@ static uint32_t get_cmd(ESPState *s)
>  return 0;
>  }
>  } else {
> -dmalen = s->ti_size;
> -if (dmalen > TI_BUFSZ) {
> +dmalen = MIN(s->ti_size, maxlen);
> +if (dmalen == 0) {
>  return 0;
>  }
>  memcpy(buf, s->ti_buf, dmalen);
> -buf[0] = buf[2] >> 5;
> +if (dmalen >= 3) {
> +buf[0] = buf[2] >> 5;
> +}
>  }
>  trace_esp_get_cmd(dmalen, target);
>  
> @@ -290,7 +292,7 @@ static void handle_satn(ESPState *s)
>  return;
>  }
>  s->pdma_cb = satn_pdma_cb;
> -cmdlen = get_cmd(s);
> +cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
>  if (cmdlen > 0) {
>  s->cmdlen = cmdlen;
>  do_cmd(s);
> @@ -320,7 +322,7 @@ static void handle_s_without_atn(ESPState *s)
>  return;
>  }
>  s->pdma_cb = s_without_satn_pdma_cb;
> -cmdlen = get_cmd(s);
> +cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
>  if (cmdlen > 0) {
>  s->cmdlen = cmdlen;
>  do_busid_cmd(s, s->cmdbuf, 0);
> @@ -355,7 +357,7 @@ static void handle_satn_stop(ESPState *s)
>  return;
>  }
>  s->pdma_cb = satn_stop_pdma_cb;
> -cmdlen = get_cmd(s);
> +cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
>  if (cmdlen > 0) {
>  trace_esp_handle_satn_stop(s->cmdlen);
>  s->cmdlen = cmdlen;
> 

Reviewed-by: Laurent Vivier 

Re: [PATCH v2 35/42] esp: raise interrupt after every non-DMA byte transferred to the FIFO

[Laurent Vivier]

Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> This matches the description in the datasheet and is required as support for
> non-DMA transfers is added.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 8b856155d1..617fdcb3ed 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -767,6 +767,12 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
> val)
>  s->ti_size++;
>  s->ti_buf[s->ti_wptr++] = val & 0xff;
>  }
> +
> +/* Non-DMA transfers raise an interrupt after every byte */
> +if (s->rregs[ESP_CMD] == CMD_TI) {
> +s->rregs[ESP_RINTR] |= INTR_FC | INTR_BS;
> +esp_raise_irq(s);
> +}
>  break;
>  case ESP_CMD:
>  s->rregs[saddr] = val;
> 

Reviewed-by: Laurent Vivier 

Re: [PATCH v2 34/42] esp: remove old deferred command completion mechanism

[Laurent Vivier]

Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> Commit ea84a44250 "scsi: esp: Defer command completion until previous 
> interrupts
> have been handled" provided a mechanism to delay the command completion 
> interrupt
> until ESP_RINTR is read after the command has completed.
> 
> With the previous fixes for latching the ESP_RINTR bits and deferring the 
> setting
> of the command completion interrupt for incoming data to the SCSI callback, 
> this
> workaround is no longer required and can be removed.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 33 -
>  include/hw/scsi/esp.h |  4 ++--
>  2 files changed, 10 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index ce6a7a1ed0..8b856155d1 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -574,8 +574,11 @@ static void esp_do_dma(ESPState *s)
>  esp_lower_drq(s);
>  }
>  
> -static void esp_report_command_complete(ESPState *s, uint32_t status)
> +void esp_command_complete(SCSIRequest *req, uint32_t status,
> +  size_t resid)
>  {
> +ESPState *s = req->hba_private;
> +
>  trace_esp_command_complete();
>  if (s->ti_size != 0) {
>  trace_esp_command_complete_unexpected();
> @@ -596,24 +599,6 @@ static void esp_report_command_complete(ESPState *s, 
> uint32_t status)
>  }
>  }
>  
> -void esp_command_complete(SCSIRequest *req, uint32_t status,
> -  size_t resid)
> -{
> -ESPState *s = req->hba_private;
> -
> -if (s->rregs[ESP_RSTAT] & STAT_INT) {
> -/*
> - * Defer handling command complete until the previous
> - * interrupt has been handled.
> - */
> -trace_esp_command_complete_deferred();
> -s->deferred_status = status;
> -s->deferred_complete = true;
> -return;
> -}
> -esp_report_command_complete(s, status);
> -}
> -
>  void esp_transfer_data(SCSIRequest *req, uint32_t len)
>  {
>  ESPState *s = req->hba_private;
> @@ -740,10 +725,6 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
>  s->rregs[ESP_RSTAT] &= ~STAT_TC;
>  s->rregs[ESP_RSEQ] = SEQ_0;
>  esp_lower_irq(s);
> -if (s->deferred_complete) {
> -esp_report_command_complete(s, s->deferred_status);
> -s->deferred_complete = false;
> -}
>  break;
>  case ESP_TCHI:
>  /* Return the unique id if the value has never been written */
> @@ -951,8 +932,10 @@ const VMStateDescription vmstate_esp = {
>  VMSTATE_UINT32(ti_wptr, ESPState),
>  VMSTATE_BUFFER(ti_buf, ESPState),
>  VMSTATE_UINT32(status, ESPState),
> -VMSTATE_UINT32(deferred_status, ESPState),
> -VMSTATE_BOOL(deferred_complete, ESPState),
> +VMSTATE_UINT32_TEST(mig_deferred_status, ESPState,
> +esp_is_before_version_5),
> +VMSTATE_BOOL_TEST(mig_deferred_complete, ESPState,
> +  esp_is_before_version_5),
>  VMSTATE_UINT32(dma, ESPState),
>  VMSTATE_PARTIAL_BUFFER(cmdbuf, ESPState, 16),
>  VMSTATE_BUFFER_START_MIDDLE_V(cmdbuf, ESPState, 16, 4),
> diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
> index 3b69aedebe..5e68908fcb 100644
> --- a/include/hw/scsi/esp.h
> +++ b/include/hw/scsi/esp.h
> @@ -30,8 +30,6 @@ struct ESPState {
>  int32_t ti_size;
>  uint32_t ti_rptr, ti_wptr;
>  uint32_t status;
> -uint32_t deferred_status;
> -bool deferred_complete;
>  uint32_t dma;
>  uint8_t ti_buf[TI_BUFSZ];
>  SCSIBus bus;
> @@ -57,6 +55,8 @@ struct ESPState {
>  
>  /* Legacy fields for vmstate_esp version < 5 */
>  uint32_t mig_dma_left;
> +uint32_t mig_deferred_status;
> +bool mig_deferred_complete;
>  };
>  
>  #define TYPE_SYSBUS_ESP "sysbus-esp"
> 

Reviewed-by: Laurent Vivier 

[PATCH] virtiofsd: Add qemu version and copyright info

[Vivek Goyal]

Option "-V" currently displays the fuse protocol version virtiofsd is
using. For example, I see this.

$ ./virtiofsd -V
"using FUSE kernel interface version 7.33"

People also want to know software version of virtiofsd so that they can
figure out if a certain fix is part of currently running virtiofsd or
not. Eric Ernst ran into this issue.

David Gilbert thinks that it probably is best that we simply carry the
qemu version and display that information given we are part of qemu
tree. 

So this patch enhances version information and also adds qemu version
and copyright info. Not sure if copyright information is supposed
to be displayed along with version info. Given qemu-storage-daemon
and other utilities are doing it, so I continued with same pattern.
This is how now output looks like.

$ ./virtiofsd -V
virtiofsd version 5.2.50 (v5.2.0-2357-gcbcf09872a-dirty)
Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers
using FUSE kernel interface version 7.33

Reported-by: Eric Ernst 
Signed-off-by: Vivek Goyal 
---
 tools/virtiofsd/passthrough_ll.c |8 
 1 file changed, 8 insertions(+)

Index: rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c
===
--- rhvgoyal-qemu.orig/tools/virtiofsd/passthrough_ll.c 2021-03-03 
14:28:04.893672524 -0500
+++ rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c  2021-03-03 
14:37:29.381712706 -0500
@@ -37,6 +37,8 @@
 
 #include "qemu/osdep.h"
 #include "qemu/timer.h"
+#include "qemu-version.h"
+#include "qemu-common.h"
 #include "fuse_virtio.h"
 #include "fuse_log.h"
 #include "fuse_lowlevel.h"
@@ -3591,6 +3593,11 @@ static void fuse_lo_data_cleanup(struct
 free(lo->source);
 }
 
+static void qemu_version(void)
+{
+printf("virtiofsd version " QEMU_FULL_VERSION "\n" QEMU_COPYRIGHT "\n");
+}
+
 int main(int argc, char *argv[])
 {
 struct fuse_args args = FUSE_ARGS_INIT(argc, argv);
@@ -3662,6 +3669,7 @@ int main(int argc, char *argv[])
 ret = 0;
 goto err_out1;
 } else if (opts.show_version) {
+qemu_version();
 fuse_lowlevel_version();
 ret = 0;
 goto err_out1;

Re: [PATCH v2 33/42] esp: defer command completion interrupt on incoming data transfers

[Laurent Vivier]

Le 18/02/2021 à 18:25, Mark Cave-Ayland a écrit :
> On 09/02/2021 19:30, Mark Cave-Ayland wrote:
> 
>> The MacOS toolbox ROM issues a command to the ESP controller as part of its
>> "FAST" SCSI routines and then proceeds to read the incoming data soon after
>> receiving the command completion interrupt.
>>
>> Unfortunately due to SCSI block transfers being asynchronous the incoming 
>> data
>> may not yet be present causing an underflow error. Resolve this by waiting 
>> for
>> the SCSI subsystem transfer_data callback before raising the command 
>> completion
>> interrupt.
>>
>> Signed-off-by: Mark Cave-Ayland 
>> ---
>>   hw/scsi/esp.c | 54 +++
>>   include/hw/scsi/esp.h |  1 +
>>   2 files changed, 51 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
>> index 728d4ddf99..ce6a7a1ed0 100644
>> --- a/hw/scsi/esp.c
>> +++ b/hw/scsi/esp.c
>> @@ -183,6 +183,14 @@ static int esp_select(ESPState *s)
>>   esp_raise_irq(s);
>>   return -1;
>>   }
>> +
>> +    /*
>> + * 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
>> + * IRQ mechanism in esp_transfer_data() for DATA IN transfers
>> + */
>> +    s->rregs[ESP_RINTR] |= INTR_FC;
>> +    s->rregs[ESP_RSEQ] = SEQ_CD;
>>   return 0;
>>   }
>>   @@ -237,18 +245,24 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, 
>> uint8_t busid)
>>   s->ti_size = datalen;
>>   if (datalen != 0) {
>>   s->rregs[ESP_RSTAT] = STAT_TC;
>> +    s->rregs[ESP_RSEQ] = SEQ_CD;
>>   esp_set_tc(s, 0);
>>   if (datalen > 0) {
>> +    /*
>> + * Switch to DATA IN phase but wait until initial data xfer is
>> + * complete before raising the command completion interrupt
>> + */
>> +    s->data_in_ready = false;
>>   s->rregs[ESP_RSTAT] |= STAT_DI;
>>   } else {
>>   s->rregs[ESP_RSTAT] |= STAT_DO;
>> +    s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
>> +    esp_raise_irq(s);
>> +    esp_lower_drq(s);
>>   }
>>   scsi_req_continue(s->current_req);
>> +    return;
>>   }
>> -    s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
>> -    s->rregs[ESP_RSEQ] = SEQ_CD;
>> -    esp_raise_irq(s);
>> -    esp_lower_drq(s);
>>   }
>>     static void do_cmd(ESPState *s)
>> @@ -603,12 +617,35 @@ void esp_command_complete(SCSIRequest *req, uint32_t 
>> status,
>>   void esp_transfer_data(SCSIRequest *req, uint32_t len)
>>   {
>>   ESPState *s = req->hba_private;
>> +    int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
>>   uint32_t dmalen = esp_get_tc(s);
>>     assert(!s->do_cmd);
>>   trace_esp_transfer_data(dmalen, s->ti_size);
>>   s->async_len = len;
>>   s->async_buf = scsi_req_get_buf(req);
>> +
>> +    if (!to_device && !s->data_in_ready) {
>> +    /*
>> + * Initial incoming data xfer is complete so raise command
>> + * completion interrupt
>> + */
>> +    s->data_in_ready = true;
>> +    s->rregs[ESP_RSTAT] |= STAT_TC;
>> +    s->rregs[ESP_RINTR] |= INTR_BS;
>> +    esp_raise_irq(s);
>> +
>> +    /*
>> + * If data is ready to transfer and the TI command has already
>> + * been executed, start DMA immediately. Otherwise DMA will start
>> + * when host sends the TI command
>> + */
>> +    if (s->ti_size && (s->rregs[ESP_CMD] == (CMD_TI | CMD_DMA))) {
>> +    esp_do_dma(s);
>> +    }
>> +    return;
>> +    }
>> +
>>   if (dmalen) {
>>   esp_do_dma(s);
>>   } else if (s->ti_size <= 0) {
>> @@ -870,6 +907,14 @@ static bool esp_is_before_version_5(void *opaque, int 
>> version_id)
>>   return version_id < 5;
>>   }
>>   +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;
>> +}
>> +
>>   static int esp_pre_save(void *opaque)
>>   {
>>   ESPState *s = ESP(opaque);
>> @@ -914,6 +959,7 @@ const VMStateDescription vmstate_esp = {
>>   VMSTATE_UINT32(cmdlen, ESPState),
>>   VMSTATE_UINT32(do_cmd, ESPState),
>>   VMSTATE_UINT32_TEST(mig_dma_left, ESPState, 
>> esp_is_before_version_5),
>> +    VMSTATE_BOOL_TEST(data_in_ready, ESPState, esp_is_version_5),
>>   VMSTATE_END_OF_LIST()
>>   },
>>   };
>> diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
>> index 6618f4e091..3b69aedebe 100644
>> --- a/include/hw/scsi/esp.h
>> +++ b/include/hw/scsi/esp.h
>> @@ -41,6 +41,7 @@ struct ESPState {
>>   uint32_t cmdlen;
>>   uint32_t do_cmd;
>>   +    bool data_in_ready;
>>   int dma_enabled;
>>     uint32_t async_len;
> 
> Whilst doing some testing earlier, I discovered that the same change is 
> requir

Re: [PATCH v2 32/42] esp: latch individual bits in ESP_RINTR register

[Laurent Vivier]

Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> Currently the ESP_RINTR register is set to a specific value as required within
> the ESP state machine. In order to implement the upcoming deferred interrupt
> functionality it is necessary to set individual bits within ESP_RINTR so that
> a deferred interrupt will not overwrite the value of any other interrupt bits.
> 
> This also requires fixing up a few locations where the ESP_RINTR and ESP_RSEQ
> registers are set/reset unexpectedly.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 29 +
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 0994673ff8..728d4ddf99 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -178,7 +178,7 @@ static int esp_select(ESPState *s)
>  if (!s->current_dev) {
>  /* No such drive */
>  s->rregs[ESP_RSTAT] = 0;
> -s->rregs[ESP_RINTR] = INTR_DC;
> +s->rregs[ESP_RINTR] |= INTR_DC;
>  s->rregs[ESP_RSEQ] = SEQ_0;
>  esp_raise_irq(s);
>  return -1;
> @@ -245,7 +245,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, 
> uint8_t busid)
>  }
>  scsi_req_continue(s->current_req);
>  }
> -s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
> +s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
>  s->rregs[ESP_RSEQ] = SEQ_CD;
>  esp_raise_irq(s);
>  esp_lower_drq(s);
> @@ -326,7 +326,7 @@ static void satn_stop_pdma_cb(ESPState *s)
>  trace_esp_handle_satn_stop(s->cmdlen);
>  s->do_cmd = 1;
>  s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
> -s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
> +s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
>  s->rregs[ESP_RSEQ] = SEQ_CD;
>  esp_raise_irq(s);
>  }
> @@ -346,8 +346,8 @@ static void handle_satn_stop(ESPState *s)
>  trace_esp_handle_satn_stop(s->cmdlen);
>  s->cmdlen = cmdlen;
>  s->do_cmd = 1;
> -s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
> -s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
> +s->rregs[ESP_RSTAT] = STAT_CD;
> +s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
>  s->rregs[ESP_RSEQ] = SEQ_CD;
>  esp_raise_irq(s);
>  } else if (cmdlen == 0) {
> @@ -362,7 +362,7 @@ static void handle_satn_stop(ESPState *s)
>  static void write_response_pdma_cb(ESPState *s)
>  {
>  s->rregs[ESP_RSTAT] = STAT_TC | STAT_ST;
> -s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
> +s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
>  s->rregs[ESP_RSEQ] = SEQ_CD;
>  esp_raise_irq(s);
>  }
> @@ -376,7 +376,7 @@ static void write_response(ESPState *s)
>  if (s->dma_memory_write) {
>  s->dma_memory_write(s->dma_opaque, s->ti_buf, 2);
>  s->rregs[ESP_RSTAT] = STAT_TC | STAT_ST;
> -s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
> +s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
>  s->rregs[ESP_RSEQ] = SEQ_CD;
>  } else {
>  s->pdma_cb = write_response_pdma_cb;
> @@ -395,7 +395,7 @@ static void write_response(ESPState *s)
>  static void esp_dma_done(ESPState *s)
>  {
>  s->rregs[ESP_RSTAT] |= STAT_TC;
> -s->rregs[ESP_RINTR] = INTR_BS;
> +s->rregs[ESP_RINTR] |= INTR_BS;
>  s->rregs[ESP_RSEQ] = 0;
>  s->rregs[ESP_RFLAGS] = 0;
>  esp_set_tc(s, 0);
> @@ -701,7 +701,7 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
>  val = s->rregs[ESP_RINTR];
>  s->rregs[ESP_RINTR] = 0;
>  s->rregs[ESP_RSTAT] &= ~STAT_TC;
> -s->rregs[ESP_RSEQ] = SEQ_CD;
> +s->rregs[ESP_RSEQ] = SEQ_0;
>  esp_lower_irq(s);
>  if (s->deferred_complete) {
>  esp_report_command_complete(s, s->deferred_status);
> @@ -772,9 +772,6 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
> val)
>  /*s->ti_size = 0;*/
>  s->ti_wptr = 0;
>  s->ti_rptr = 0;
> -s->rregs[ESP_RINTR] = INTR_FC;
> -s->rregs[ESP_RSEQ] = 0;
> -s->rregs[ESP_RFLAGS] = 0;
>  break;
>  case CMD_RESET:
>  trace_esp_mem_writeb_cmd_reset(val);
> @@ -782,8 +779,8 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
> val)
>  break;
>  case CMD_BUSRESET:
>  trace_esp_mem_writeb_cmd_bus_reset(val);
> -s->rregs[ESP_RINTR] = INTR_RST;
>  if (!(s->wregs[ESP_CFG1] & CFG1_RESREPT)) {
> +s->rregs[ESP_RINTR] |= INTR_RST;
>  esp_raise_irq(s);
>  }
>  break;
> @@ -794,12 +791,12 @@ void esp_reg_write(ESPState *s, uint32_t saddr, 
> uint64_t val)
>  case CMD_ICCS:
>  trace_esp_mem_writeb_cmd_iccs(val);
>  write_response(s);
> -s->rregs[ESP_RINTR] = INTR_FC;
> +s->rregs[ESP_RINTR] |= INTR_FC;
>  s->rregs[ESP_RSTAT] |= STAT_MI;
>  break;
>

Re: [PATCH v4] target/s390x: Implement the MVPG condition-code-option bit

[David Hildenbrand]

On 03.03.21 14:28, Thomas Huth wrote:

From: Richard Henderson 

If the CCO bit is set, MVPG should not generate an exception but
report page translation faults via a CC code.

Create a new helper, access_prepare_nf, which can use probe_access_flags
in non-faulting mode, and then handle watchpoints.

Signed-off-by: Richard Henderson 
[thuth: Added logic to still inject protection exceptions]
Signed-off-by: Thomas Huth 
---
  v4: Add logic to inject protection exceptions if necessary

  target/s390x/cpu.h |  3 ++
  target/s390x/excp_helper.c |  3 ++
  target/s390x/mem_helper.c  | 93 --
  3 files changed, 76 insertions(+), 23 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 60d434d5ed..825503c6c0 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -114,6 +114,9 @@ struct CPUS390XState {
  
  uint64_t diag318_info;
  


Should we start wrapping that stuff into #ifdef CONFIG_TCG ?


+uint64_t tlb_fill_tec;   /* translation exception code during tlb_fill */
+int tlb_fill_exc;/* exception number seen during tlb_fill */
+
  /* Fields up to this point are cleared by a CPU reset */
  struct {} end_reset_fields;
  
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c

index ce16af394b..c48cd6b46f 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -164,6 +164,9 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
  tec = 0; /* unused */
  }
  
+env->tlb_fill_exc = excp;

+env->tlb_fill_tec = tec;
+


Just what I had in mind.


  if (!excp) {
  qemu_log_mask(CPU_LOG_MMU,
"%s: set tlb %" PRIx64 " -> %" PRIx64 " (%x)\n",
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 25cfede806..cf741541d3 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -130,28 +130,62 @@ typedef struct S390Access {
  int mmu_idx;
  } S390Access;
  
+static bool access_prepare_nf(S390Access *access, CPUS390XState *env,

+  bool nofault, vaddr vaddr1, int size,
+  MMUAccessType access_type,
+  int mmu_idx, uintptr_t ra)
+{
+void *haddr1, *haddr2 = NULL;
+int size1, size2;
+vaddr vaddr2 = 0;
+int flags;
+
+assert(size > 0 && size <= 4096);
+
+size1 = MIN(size, -(vaddr1 | TARGET_PAGE_MASK)),
+size2 = size - size1;
+
+flags = probe_access_flags(env, vaddr1, access_type, mmu_idx,
+   nofault, &haddr1, ra);
+if (unlikely(size2)) {
+/* The access crosses page boundaries. */
+vaddr2 = wrap_address(env, vaddr1 + size1);
+flags |= probe_access_flags(env, vaddr2, access_type, mmu_idx,
+nofault, &haddr2, ra);
+}
+
+if (unlikely(flags & TLB_INVALID_MASK)) {
+return false;


^ I recall PAGE_WRITE_INV handling where we immediately set 
TLB_INVALID_MASK again on write access (to handle low-address protection 
cleanly). I suspect that TLB_INVALID_MASK will be set in that case (I 
could be wrong, though).


What certainly would work is checking for "haddr != NULL".

/* Don't rely on TLB_INVALID_MASK - see PAGE_WRITE_INV handling. */
if (unlikely(!haddr1)) {
return false;
}


+}
+if (unlikely(flags & TLB_WATCHPOINT)) {
+/* S390 does not presently use transaction attributes. */
+cpu_check_watchpoint(env_cpu(env), vaddr1, size,
+ MEMTXATTRS_UNSPECIFIED,
+ (access_type == MMU_DATA_STORE
+  ? BP_MEM_WRITE : BP_MEM_READ), ra);
+}
+


[...]


  /* Helper to handle memset on a single page. */
@@ -845,8 +879,10 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, 
uint64_t r1, uint64_t r2)
  const int mmu_idx = cpu_mmu_index(env, false);
  const bool f = extract64(r0, 11, 1);
  const bool s = extract64(r0, 10, 1);
+const bool cco = extract64(r0, 8, 1);
  uintptr_t ra = GETPC();
  S390Access srca, desta;
+bool ok;
  
  if ((f && s) || extract64(r0, 12, 4)) {

  tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
@@ -858,13 +894,24 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, 
uint64_t r1, uint64_t r2)
  /*
   * TODO:
   * - Access key handling
- * - CC-option with surpression of page-translation exceptions
   * - Store r1/r2 register identifiers at real location 162
   */
-srca = access_prepare(env, r2, TARGET_PAGE_SIZE, MMU_DATA_LOAD, mmu_idx,
-  ra);
-desta = access_prepare(env, r1, TARGET_PAGE_SIZE, MMU_DATA_STORE, mmu_idx,
-   ra);
+ok = access_prepare_nf(&srca, env, cco, r2, TARGET_PAGE_SIZE,
+   MMU_DATA_LOAD, mmu_idx, ra);
+if (!ok) {
+return 2;
+}
+ok = access_prepare_nf(&desta,

Re: [PATCH v2 31/42] esp: implement FIFO flush command

[Laurent Vivier]

Le 09/02/2021 à 20:30, Mark Cave-Ayland a écrit :
> At this point it is now possible to properly implement the FIFO flush command
> without causing guest errors.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/scsi/esp.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 1d56c99527..0994673ff8 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -770,6 +770,8 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
> val)
>  case CMD_FLUSH:
>  trace_esp_mem_writeb_cmd_flush(val);
>  /*s->ti_size = 0;*/
> +s->ti_wptr = 0;
> +s->ti_rptr = 0;
>  s->rregs[ESP_RINTR] = INTR_FC;
>  s->rregs[ESP_RSEQ] = 0;
>  s->rregs[ESP_RFLAGS] = 0;
> 

Why don't  you set aso ti_size to 0?

Anyway:

Reviwed-by: Laurent Vivier 

Thanks,
Laurent

[Bug 1917661] Re: qemu gdb wrong registers group for riscv64

[BogDan]

I forgot to specify the version, I built qemu sha
c40ae5a3ee387b13116948cbfe7824f03311db7e

$ qemu-system-riscv64 --version
QEMU emulator version 5.2.50 (v5.2.0-2392-gc40ae5a3ee-dirty)

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

Title:
  qemu gdb wrong registers group for riscv64

Status in QEMU:
  New

Bug description:
  Step to reproduce:
  1. run qemu-system-riscv64 in gdb mode
  2. attach gdb
  3. set a breakpoint and run
  4. print register-groups using "maintenance print register-groups" command

  ...
   sbadaddr   4162 4162   1628   8 longall,general
   msounteren 4163 4163   1636   8 longall,general
   mbadaddr   4164 4164   1644   8 longall,general
   htimedeltah 4165 4165   1652   8 longall,general

  These registers don't belong to general group, instead they belong to
  all, system and csr groups.

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

[Bug 1917661] [NEW] qemu gdb wrong registers group for riscv64

[BogDan]

Public bug reported:

Step to reproduce:
1. run qemu-system-riscv64 in gdb mode
2. attach gdb
3. set a breakpoint and run
4. print register-groups using "maintenance print register-groups" command

...
 sbadaddr   4162 4162   1628   8 longall,general
 msounteren 4163 4163   1636   8 longall,general
 mbadaddr   4164 4164   1644   8 longall,general
 htimedeltah 4165 4165   1652   8 longall,general

These registers don't belong to general group, instead they belong to
all, system and csr groups.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  qemu gdb wrong registers group for riscv64

Status in QEMU:
  New

Bug description:
  Step to reproduce:
  1. run qemu-system-riscv64 in gdb mode
  2. attach gdb
  3. set a breakpoint and run
  4. print register-groups using "maintenance print register-groups" command

  ...
   sbadaddr   4162 4162   1628   8 longall,general
   msounteren 4163 4163   1636   8 longall,general
   mbadaddr   4164 4164   1644   8 longall,general
   htimedeltah 4165 4165   1652   8 longall,general

  These registers don't belong to general group, instead they belong to
  all, system and csr groups.

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

[RFC PATCH v3 09/10] hw/net: sungem: Remove the logic of padding short frames in the receive path

[Philippe Mathieu-Daudé]

From: Bin Meng 

Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.

Signed-off-by: Bin Meng 
Message-Id: <1614763306-18026-9-git-send-email-bmeng...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/net/sungem.c | 14 --
 1 file changed, 14 deletions(-)

diff --git a/hw/net/sungem.c b/hw/net/sungem.c
index 33c3722df6f..3fa83168db0 100644
--- a/hw/net/sungem.c
+++ b/hw/net/sungem.c
@@ -550,7 +550,6 @@ static ssize_t sungem_receive(NetClientState *nc, const 
uint8_t *buf,
 PCIDevice *d = PCI_DEVICE(s);
 uint32_t mac_crc, done, kick, max_fsize;
 uint32_t fcs_size, ints, rxdma_cfg, rxmac_cfg, csum, coff;
-uint8_t smallbuf[60];
 struct gem_rxd desc;
 uint64_t dbase, baddr;
 unsigned int rx_cond;
@@ -584,19 +583,6 @@ static ssize_t sungem_receive(NetClientState *nc, const 
uint8_t *buf,
 return size;
 }
 
-/* We don't drop too small frames since we get them in qemu, we pad
- * them instead. We should probably use the min frame size register
- * but I don't want to use a variable size staging buffer and I
- * know both MacOS and Linux use the default 64 anyway. We use 60
- * here to account for the non-existent FCS.
- */
-if (size < 60) {
-memcpy(smallbuf, buf, size);
-memset(&smallbuf[size], 0, 60 - size);
-buf = smallbuf;
-size = 60;
-}
-
 /* Get MAC crc */
 mac_crc = net_crc32_le(buf, ETH_ALEN);
 
-- 
2.26.2