Re: [PATCH v5 20/31] target/tricore: Use generic cpu_list()

[Philippe Mathieu-Daudé]

On 15/11/23 00:56, Gavin Shan wrote:

No changes in the output from the following command.

[gshan@gshan q]$ ./build/qemu-system-tricore -cpu ?
Available CPUs:
   tc1796
   tc1797
   tc27x
   tc37x

Signed-off-by: Gavin Shan 
---
  target/tricore/cpu.h|  4 
  target/tricore/helper.c | 22 --
  2 files changed, 26 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v5 19/31] target/sh4: Use generic cpu_list()

[Philippe Mathieu-Daudé]

On 15/11/23 00:56, Gavin Shan wrote:

Before it's applied:

[gshan@gshan q]$ ./build/qemu-system-sh4 -cpu ?
sh7750r
sh7751r
sh7785

After it's applied:

[gshan@gshan q]$ ./build/qemu-system-sh4 -cpu ?
Available CPUs:
   sh7750r
   sh7751r
   sh7785

Signed-off-by: Gavin Shan 
---
  target/sh4/cpu.c | 17 -
  target/sh4/cpu.h |  3 ---
  2 files changed, 20 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v5 18/31] target/rx: Use generic cpu_list()

[Philippe Mathieu-Daudé]

On 15/11/23 00:56, Gavin Shan wrote:

Before it's applied:

[gshan@gshan q]$ ./build/qemu-system-rx -cpu ?
Available CPUs:
   rx62n-rx-cpu

After it's applied:

[gshan@gshan q]$ ./build/qemu-system-rx -cpu ?
Available CPUs:
   rx62n

Signed-off-by: Gavin Shan 
---
  target/rx/cpu.c | 16 
  target/rx/cpu.h |  3 ---
  2 files changed, 19 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v5 15/31] target/mips: Use generic cpu_list()

[Philippe Mathieu-Daudé]

On 15/11/23 00:56, Gavin Shan wrote:

Before it's applied:

[gshan@gshan q]$ ./build/qemu-system-mips64 -cpu ?
MIPS '4Kc'
MIPS '4Km'
MIPS '4KEcR1'
MIPS 'XBurstR1'
MIPS '4KEmR1'
MIPS '4KEc'
MIPS '4KEm'
MIPS '24Kc'
MIPS '24KEc'
MIPS '24Kf'
MIPS '34Kf'
MIPS '74Kf'
MIPS 'XBurstR2'
MIPS 'M14K'
MIPS 'M14Kc'
MIPS 'P5600'
MIPS 'mips32r6-generic'
MIPS 'I7200'
MIPS 'R4000'
MIPS 'VR5432'
MIPS '5Kc'
MIPS '5Kf'
MIPS '20Kc'
MIPS 'MIPS64R2-generic'
MIPS '5KEc'
MIPS '5KEf'
MIPS 'I6400'
MIPS 'I6500'
MIPS 'Loongson-2E'
MIPS 'Loongson-2F'
MIPS 'Loongson-3A1000'
MIPS 'Loongson-3A4000'
MIPS 'mips64dspr2'
MIPS 'Octeon68XX'

After it's applied:

[gshan@gshan q]$ ./build/qemu-system-mips64 -cpu ?
Available CPUs:
   20Kc
   24Kc
   24KEc
   24Kf
   34Kf
   4Kc
   4KEc
   4KEcR1
   4KEm
   4KEmR1
   4Km
   5Kc
   5KEc
   5KEf
   5Kf
   74Kf
   I6400
   I6500
   I7200
   Loongson-2E
   Loongson-2F
   Loongson-3A1000
   Loongson-3A4000
   M14K
   M14Kc
   mips32r6-generic
   mips64dspr2
   MIPS64R2-generic
   Octeon68XX
   P5600
   R4000
   VR5432
   XBurstR1
   XBurstR2

Signed-off-by: Gavin Shan 
---
  target/mips/cpu-defs.c.inc | 9 -
  target/mips/cpu.h  | 4 
  2 files changed, 13 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v5 12/31] target/hppa: Use generic cpu_list()

[Philippe Mathieu-Daudé]

On 15/11/23 00:56, Gavin Shan wrote:

No changes in the output from the following command.

[gshan@gshan q]$ ./build/qemu-system-hppa -cpu ?
Available CPUs:
   hppa
   hppa64

Signed-off-by: Gavin Shan 
---
  target/hppa/cpu.c | 24 
  target/hppa/cpu.h |  3 ---
  2 files changed, 27 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v5 11/31] target/hexagon: Use generic cpu_list()

[Philippe Mathieu-Daudé]

On 15/11/23 00:56, Gavin Shan wrote:

No changes in the output from the following command.

[gshan@gshan q]$ ./build/qemu-hexagon -cpu ?
Available CPUs:
   v67
   v68
   v69
   v71
   v73

Signed-off-by: Gavin Shan 
---
  target/hexagon/cpu.c | 20 
  target/hexagon/cpu.h |  3 ---
  2 files changed, 23 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v5 10/31] target/cris: Use generic cpu_list()

[Philippe Mathieu-Daudé]

On 15/11/23 00:56, Gavin Shan wrote:

Before it's applied:

[gshan@gshan q]$ ./build/qemu-system-cris -cpu ?
Available CPUs:
   crisv8
   crisv9
   crisv10
   crisv11
   crisv17
   crisv32

After it's applied:

[gshan@gshan q]$ ./build/qemu-system-cris -cpu ?
Available CPUs:
   crisv10
   crisv11
   crisv17
   crisv32
   crisv8
   crisv9

Signed-off-by: Gavin Shan 
---
  target/cris/cpu.c | 38 --
  target/cris/cpu.h |  3 ---
  2 files changed, 41 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v5 09/31] target/avr: Use generic cpu_list()

[Philippe Mathieu-Daudé]

On 15/11/23 00:56, Gavin Shan wrote:

Before it's applied:

[gshan@gshan q]$ ./build/qemu-system-avr -cpu ?
avr5-avr-cpu
avr51-avr-cpu
avr6-avr-cpu

After it's applied:

[gshan@gshan q]$ ./build/qemu-system-avr -cpu ?
Available CPUs:
   avr5
   avr51
   avr6

Signed-off-by: Gavin Shan 
---
  target/avr/cpu.c | 15 ---
  target/avr/cpu.h |  2 --
  2 files changed, 17 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v5 08/31] target/arm: Use generic cpu_list()

[Philippe Mathieu-Daudé]

On 15/11/23 00:56, Gavin Shan wrote:

No changes of the output from the following command before and
after it's applied.

[gshan@gshan q]$ ./build/qemu-system-aarch64 -cpu ?
Available CPUs:
   a64fx
   arm1026
   arm1136
   arm1136-r2
   arm1176
   arm11mpcore
   arm926
   arm946
   cortex-a15
   cortex-a35
   cortex-a53
   cortex-a55
   cortex-a57
   cortex-a7
   cortex-a710
   cortex-a72
   cortex-a76
   cortex-a8
   cortex-a9
   cortex-m0
   cortex-m3
   cortex-m33
   cortex-m4
   cortex-m55
   cortex-m7
   cortex-r5
   cortex-r52
   cortex-r5f
   max
   neoverse-n1
   neoverse-n2
   neoverse-v1
   pxa250
   pxa255
   pxa260
   pxa261
   pxa262
   pxa270-a0
   pxa270-a1
   pxa270
   pxa270-b0
   pxa270-b1
   pxa270-c0
   pxa270-c5
   sa1100
   sa1110
   ti925t

Signed-off-by: Gavin Shan 
---
  target/arm/cpu.h|  3 ---
  target/arm/helper.c | 46 -
  2 files changed, 49 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

[PULL 09/27] hw/cxl: spelling fixes: limitaions, potentialy, intialized

[Michael Tokarev]

Fixes: 388d6b574e28 "hw/cxl: Use switch statements for read and write of 
cachemem registers"
Fixes: 3314efd276ad "hw/cxl/mbox: Add Physical Switch Identify command."
Fixes: 004e3a93b814 "hw/cxl: Add tunneled command support to mailbox for switch 
cci."
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 hw/cxl/cxl-component-utils.c | 4 ++--
 hw/cxl/cxl-mailbox-utils.c   | 2 +-
 include/hw/cxl/cxl_device.h  | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index d0245cc55d..29d477492b 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -81,7 +81,7 @@ static uint64_t cxl_cache_mem_read_reg(void *opaque, hwaddr 
offset,
 return 0;
 default:
 /*
- * In line with specifiction limitaions on access sizes, this
+ * In line with specification limitaions on access sizes, this
  * routine is not called with other sizes.
  */
 g_assert_not_reached();
@@ -152,7 +152,7 @@ static void cxl_cache_mem_write_reg(void *opaque, hwaddr 
offset, uint64_t value,
 return;
 default:
 /*
- * In line with specifiction limitaions on access sizes, this
+ * In line with specification limitaions on access sizes, this
  * routine is not called with other sizes.
  */
 g_assert_not_reached();
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index b365575097..6eff56fb1b 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -431,7 +431,7 @@ static CXLRetCode cmd_identify_switch_device(const struct 
cxl_cmd *cmd,
 out = (struct cxl_fmapi_ident_switch_dev_resp_pl *)payload_out;
 *out = (struct cxl_fmapi_ident_switch_dev_resp_pl) {
 .num_physical_ports = num_phys_ports + 1, /* 1 USP */
-.num_vcss = 1, /* Not yet support multiple VCS - potentialy tricky */
+.num_vcss = 1, /* Not yet support multiple VCS - potentially tricky */
 .active_vcs_bitmask[0] = 0x1,
 .total_vppbs = num_phys_ports + 1,
 .bound_vppbs = num_phys_ports + 1,
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 61b7f897f7..befb5f884b 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -403,7 +403,7 @@ struct CXLType3Dev {
 CXLComponentState cxl_cstate;
 CXLDeviceState cxl_dstate;
 CXLCCI cci; /* Primary PCI mailbox CCI */
-/* Always intialized as no way to know if a VDM might show up */
+/* Always initialized as no way to know if a VDM might show up */
 CXLCCI vdm_fm_owned_ld_mctp_cci;
 CXLCCI ld0_cci;
 
-- 
2.39.2

Re: [PATCH v5 06/31] cpu: Add generic cpu_list()

[Philippe Mathieu-Daudé]

On 16/11/23 08:39, Philippe Mathieu-Daudé wrote:

Hi Gavin,

On 15/11/23 00:56, Gavin Shan wrote:

Add generic cpu_list() to replace the individual target's implementation
in the subsequent commits. Currently, there are 3 targets with no 
cpu_list()
implementation: microblaze and nios2. With this applied, those two 
targets

switch to the generic cpu_list().

[gshan@gshan q]$ ./build/qemu-system-microblaze -cpu ?
Available CPUs:
   microblaze-cpu

[gshan@gshan q]$ ./build/qemu-system-nios2 -cpu ?
Available CPUs:
   nios2-cpu

Suggested-by: Richard Henderson 
Signed-off-by: Gavin Shan 
---
  bsd-user/main.c |  5 +
  cpu-target.c    | 29 ++---
  2 files changed, 27 insertions(+), 7 deletions(-)




diff --git a/cpu-target.c b/cpu-target.c
index c078c0e91b..acfc654b95 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -24,6 +24,7 @@
  #include "hw/qdev-core.h"
  #include "hw/qdev-properties.h"
  #include "qemu/error-report.h"
+#include "qemu/qemu-print.h"
  #include "migration/vmstate.h"
  #ifdef CONFIG_USER_ONLY
  #include "qemu.h"
@@ -283,12 +284,34 @@ const char *parse_cpu_option(const char 
*cpu_option)

  return cpu_type;
  }
+#ifndef cpu_list
+static void cpu_list_entry(gpointer data, gpointer user_data)
+{
+    CPUClass *cc = CPU_CLASS(OBJECT_CLASS(data));
+    const char *typename = object_class_get_name(OBJECT_CLASS(data));
+    g_autofree char *model = cpu_model_from_type(typename);
+
+    if (cc->deprecation_note) {
+    qemu_printf("  %s (deprecated)\n", model);
+    } else {
+    qemu_printf("  %s\n", model);
+    }
+}
+
+static void cpu_list(void)
+{
+    GSList *list;
+
+    list = object_class_get_list_sorted(TYPE_CPU, false);
+    qemu_printf("Available CPUs:\n");


Since this output will likely be displayed a lot, IMHO it is worth
doing a first pass to get the number of available CPUs. If it is 1,
print using singular but even better smth like:

    "This machine can only be used with the following CPU:"


Hmm I missed this code is common to user/system emulation.

System helper could be clever by using the intersection of cpu_list()
and MachineClass::valid_cpu_types[] sets.


That said, this can be done later on top, so:
Reviewed-by: Philippe Mathieu-Daudé 


+    g_slist_foreach(list, cpu_list_entry, NULL);
+    g_slist_free(list);
+}
+#endif

[PULL 26/27] util/filemonitor-inotify.c: spelling fix: kenel

[Michael Tokarev]

Fixes: 2e12dd405c66 "util/filemonitor-inotify: qemu_file_monitor_watch(): 
assert no overflow"
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Michael Tokarev 
---
 util/filemonitor-inotify.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/filemonitor-inotify.c b/util/filemonitor-inotify.c
index 212f38..7352b9fe53 100644
--- a/util/filemonitor-inotify.c
+++ b/util/filemonitor-inotify.c
@@ -89,7 +89,7 @@ static void qemu_file_monitor_watch(void *arg)
 struct inotify_event *ev = (struct inotify_event *)(buf + used);
 
 /*
- * We trust the kenel to provide valid buffer with complete event
+ * We trust the kernel to provide valid buffer with complete event
  * records.
  */
 assert(len - used >= sizeof(struct inotify_event));
-- 
2.39.2

[PULL 13/27] docs/system/arm/emulation.rst: spelling fix: Enhacements

[Michael Tokarev]

Fixes: c7c807f6dd6d "target/arm: Implement FEAT_Pauth2"
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 docs/system/arm/emulation.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst
index 47fd648035..0b604f9005 100644
--- a/docs/system/arm/emulation.rst
+++ b/docs/system/arm/emulation.rst
@@ -70,7 +70,7 @@ the following architecture extensions:
 - FEAT_PAN2 (AT S1E1R and AT S1E1W instruction variants affected by PSTATE.PAN)
 - FEAT_PAN3 (Support for SCTLR_ELx.EPAN)
 - FEAT_PAuth (Pointer authentication)
-- FEAT_PAuth2 (Enhacements to pointer authentication)
+- FEAT_PAuth2 (Enhancements to pointer authentication)
 - FEAT_PMULL (PMULL, PMULL2 instructions)
 - FEAT_PMUv3p1 (PMU Extensions v3.1)
 - FEAT_PMUv3p4 (PMU Extensions v3.4)
-- 
2.39.2

[PULL 00/27] Trivial patches for 2023-11-16

[Michael Tokarev]

The following changes since commit 9c673a41eefc50f1cb2fe3c083e7de842c7d276a:

  Update version for v8.2.0-rc0 release (2023-11-14 12:35:47 -0500)

are available in the Git repository at:

  https://gitlab.com/mjt0k/qemu.git tags/pull-trivial-patches

for you to fetch changes up to f779357882b4758551ff30074b4b915d7d249d3d:

  util/range.c: spelling fix: inbetween (2023-11-15 12:06:05 +0300)


trivial patches for 2023-11-16

Mostly spelling fixes in various parts in code added since 8.1,
plus a few other tiny things (comment rewording, removal of unused
file, forgotten MAINTAINERS change).


Michael Tokarev (23):
  hw/audio/virtio-snd.c: spelling: initalize
  qapi/migration.json: spelling: transfering
  bsd-user: spelling fixes: necesary, agrument, undocummented
  linux-user: spelling fixes: othe, necesary
  hw/cxl: spelling fixes: limitaions, potentialy, intialized
  gdbstub: spelling fix: respectivelly
  docs/about/deprecated.rst: spelling fix: becase
  docs/devel/migration.rst: spelling fixes: doen't, diferent, 
responsability, recomend
  docs/system/arm/emulation.rst: spelling fix: Enhacements
  target/arm/tcg: spelling fixes: alse, addreses
  target/hppa: spelling fixes: Indicies, Truely
  migration/rdma.c: spelling fix: asume
  contrib/vhost-user-gpu/virgl.c: spelling fix: mesage
  hw/mem/memory-device.c: spelling fix: ontaining
  hw/net/cadence_gem.c: spelling fixes: Octects
  include/block/ufs.h: spelling fix: setted
  include/hw/hyperv/dynmem-proto.h: spelling fix: nunber, atleast
  include/hw/virtio/vhost.h: spelling fix: sate
  target/riscv/cpu.h: spelling fix: separatly
  tests/qtest/migration-test.c: spelling fix: bandwith
  tests/qtest/ufs-test.c: spelling fix: tranfer
  util/filemonitor-inotify.c: spelling fix: kenel
  util/range.c: spelling fix: inbetween

Philippe Mathieu-Daudé (1):
  hw/watchdog/wdt_aspeed: Remove unused 'hw/misc/aspeed_scu.h' header

Thomas Huth (3):
  MAINTAINERS: Add tests/decode/ to the "Overall TCG CPUs" section
  tests/data/qobject/qdict.txt: Avoid non-inclusive words
  qapi/pragma.json: Improve the comment about the lists of QAPI rule 
exceptions

 MAINTAINERS  |  1 +
 bsd-user/bsd-mem.h   |  2 +-
 bsd-user/freebsd/os-proc.c   |  2 +-
 bsd-user/freebsd/os-stat.h   |  6 +++---
 contrib/vhost-user-gpu/virgl.c   |  2 +-
 docs/about/deprecated.rst|  2 +-
 docs/devel/migration.rst | 10 +-
 docs/system/arm/emulation.rst|  2 +-
 gdbstub/gdbstub.c|  2 +-
 hw/audio/virtio-snd.c|  2 +-
 hw/cxl/cxl-component-utils.c |  4 ++--
 hw/cxl/cxl-mailbox-utils.c   |  2 +-
 hw/mem/memory-device.c   |  2 +-
 hw/net/cadence_gem.c |  8 
 hw/watchdog/wdt_aspeed.c |  1 -
 include/block/ufs.h  |  2 +-
 include/hw/cxl/cxl_device.h  |  2 +-
 include/hw/hyperv/dynmem-proto.h |  6 +++---
 include/hw/virtio/vhost.h|  2 +-
 linux-user/ppc/vdso.S|  2 +-
 linux-user/syscall.c |  2 +-
 migration/rdma.c |  2 +-
 qapi/migration.json  |  2 +-
 qapi/pragma.json |  4 ++--
 target/arm/tcg/helper-a64.c  |  2 +-
 target/arm/tcg/hflags.c  |  2 +-
 target/hppa/cpu.h|  2 +-
 target/hppa/machine.c|  2 +-
 target/riscv/cpu.h   |  4 ++--
 tests/data/qobject/qdict.txt |  4 
 tests/qtest/migration-test.c |  2 +-
 tests/qtest/ufs-test.c   |  2 +-
 util/filemonitor-inotify.c   |  2 +-
 util/range.c |  2 +-
 34 files changed, 46 insertions(+), 50 deletions(-)

Re: [PATCH v5 07/31] target/alpha: Use generic cpu_list()

[Philippe Mathieu-Daudé]

On 15/11/23 00:56, Gavin Shan wrote:

Before it's applied:

[gshan@gshan q]$ ./build/qemu-system-alpha -cpu ?
Available CPUs:
   ev4-alpha-cpu
   ev5-alpha-cpu
   ev56-alpha-cpu
   ev6-alpha-cpu
   ev67-alpha-cpu
   ev68-alpha-cpu
   pca56-alpha-cpu

After it's applied:

[gshan@gshan q]$ ./build/qemu-system-alpha -cpu ?
Available CPUs:
   ev4
   ev5
   ev56
   ev6
   ev67
   ev68
   pca56

Signed-off-by: Gavin Shan 
---
  target/alpha/cpu.c | 17 -
  target/alpha/cpu.h |  3 ---
  2 files changed, 20 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

[PULL 25/27] tests/qtest/ufs-test.c: spelling fix: tranfer

[Michael Tokarev]

Fixes: 631c872614ac "tests/qtest: Introduce tests for UFS"
Reviewed-by: Jeuk Kim 
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 tests/qtest/ufs-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c
index 5daf8c9c49..95e82f9472 100644
--- a/tests/qtest/ufs-test.c
+++ b/tests/qtest/ufs-test.c
@@ -319,7 +319,7 @@ static void ufs_init(QUfs *ufs, QGuestAllocator *alloc)
 ufs_wreg(ufs, A_IE, ie);
 ufs_wreg(ufs, A_UTRIACR, 0);
 
-/* Enable tranfer request and task management request */
+/* Enable transfer request and task management request */
 cap = ufs_rreg(ufs, A_CAP);
 nutrs = FIELD_EX32(cap, CAP, NUTRS) + 1;
 nutmrs = FIELD_EX32(cap, CAP, NUTMRS) + 1;
-- 
2.39.2

[PULL 23/27] target/riscv/cpu.h: spelling fix: separatly

[Michael Tokarev]

Fixes: 40336d5b1d4c "target/riscv: Add HS-mode virtual interrupt and IRQ 
filtering support."
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 target/riscv/cpu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index bf58b0f0b5..d74b361be6 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -214,13 +214,13 @@ struct CPUArchState {
 
 /*
  * When mideleg[i]=0 and mvien[i]=1, sie[i] is no more
- * alias of mie[i] and needs to be maintained separatly.
+ * alias of mie[i] and needs to be maintained separately.
  */
 uint64_t sie;
 
 /*
  * When hideleg[i]=0 and hvien[i]=1, vsie[i] is no more
- * alias of sie[i] (mie[i]) and needs to be maintained separatly.
+ * alias of sie[i] (mie[i]) and needs to be maintained separately.
  */
 uint64_t vsie;
 
-- 
2.39.2

[PULL 16/27] migration/rdma.c: spelling fix: asume

[Michael Tokarev]

Fixes: 67c31c9c1af1 "migration: Don't abuse qemu_file transferred for RDMA"
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 migration/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 6a29e53daf..04debab5d9 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2204,7 +2204,7 @@ retry:
 stat64_add(_stats.normal_pages, sge.length / qemu_target_page_size());
 /*
  * We are adding to transferred the amount of data written, but no
- * overhead at all.  I will asume that RDMA is magicaly and don't
+ * overhead at all.  I will assume that RDMA is magicaly and don't
  * need to transfer (at least) the addresses where it wants to
  * write the pages.  Here it looks like it should be something
  * like:
-- 
2.39.2

[PULL 15/27] target/hppa: spelling fixes: Indicies, Truely

[Michael Tokarev]

Fixes: bb67ec32a0bb "target/hppa: Include PSW_P in tb flags and mmu index"
Fixes: d7553f3591bb "target/hppa: Populate an interval tree with valid tlb 
entries"
Reviewed-by: Richard Henderson 
Signed-off-by: Michael Tokarev 
---
 target/hppa/cpu.h | 2 +-
 target/hppa/machine.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index bcfed04f7c..8be45c69c9 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -56,7 +56,7 @@
  1 << MMU_PL2_IDX| 1 << MMU_PL2_P_IDX|  \
  1 << MMU_USER_IDX   | 1 << MMU_USER_P_IDX)
 
-/* Indicies to flush for access_id changes. */
+/* Indices to flush for access_id changes. */
 #define HPPA_MMU_FLUSH_P_MASK \
 (1 << MMU_KERNEL_P_IDX | 1 << MMU_PL1_P_IDX  |  \
  1 << MMU_PL2_P_IDX| 1 << MMU_USER_P_IDX)
diff --git a/target/hppa/machine.c b/target/hppa/machine.c
index 2f8e8cc5a1..15cbc5e6d0 100644
--- a/target/hppa/machine.c
+++ b/target/hppa/machine.c
@@ -129,7 +129,7 @@ static int tlb_post_load(void *opaque, int version_id)
 
 /*
  * Re-create the interval tree from the valid entries.
- * Truely invalid entries should have start == end == 0.
+ * Truly invalid entries should have start == end == 0.
  * Otherwise it should be the in-flight tlb_partial entry.
  */
 for (uint32_t i = 0; i < ARRAY_SIZE(env->tlb); ++i) {
-- 
2.39.2

[PULL 21/27] include/hw/hyperv/dynmem-proto.h: spelling fix: nunber, atleast

[Michael Tokarev]

Fixes: 4f80cd2f033e "Add Hyper-V Dynamic Memory Protocol definitions"
Acked-by: Maciej S. Szmigiero 
Signed-off-by: Michael Tokarev 
---
 include/hw/hyperv/dynmem-proto.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/hw/hyperv/dynmem-proto.h b/include/hw/hyperv/dynmem-proto.h
index d0f9090ac4..a657786a94 100644
--- a/include/hw/hyperv/dynmem-proto.h
+++ b/include/hw/hyperv/dynmem-proto.h
@@ -232,7 +232,7 @@ struct dm_capabilities_resp_msg {
  * num_committed: Committed memory in pages.
  * page_file_size: The accumulated size of all page files
  * in the system in pages.
- * zero_free: The nunber of zero and free pages.
+ * zero_free: The number of zero and free pages.
  * page_file_writes: The writes to the page file in pages.
  * io_diff: An indicator of file cache efficiency or page file activity,
  *  calculated as File Cache Page Fault Count - Page Read Count.
@@ -275,7 +275,7 @@ struct dm_balloon {
  *
  * reservedz: Reserved; must be set to zero.
  * more_pages: If FALSE, this is the last message of the transaction.
- * if TRUE there will atleast one more message from the guest.
+ * if TRUE there will be at least one more message from the guest.
  *
  * range_count: The number of ranges in the range array.
  *
@@ -296,7 +296,7 @@ struct dm_balloon_response {
  * to the guest to give guest more memory.
  *
  * more_pages: If FALSE, this is the last message of the transaction.
- * if TRUE there will atleast one more message from the guest.
+ * if TRUE there will be at least one more message from the guest.
  *
  * reservedz: Reserved; must be set to zero.
  *
-- 
2.39.2

RE: [PATCH 1/4] vfio/pci: Move VFIODevice initializations in vfio_instance_init

[Duan, Zhenzhong]

>-Original Message-
>From: Cédric Le Goater 
>Sent: Thursday, November 16, 2023 3:29 PM
>To: Duan, Zhenzhong ; qemu-devel@nongnu.org
>Cc: alex.william...@redhat.com; j...@nvidia.com; nicol...@nvidia.com;
>joao.m.mart...@oracle.com; eric.au...@redhat.com; pet...@redhat.com;
>jasow...@redhat.com; Tian, Kevin ; Liu, Yi L
>; Sun, Yi Y ; Peng, Chao P
>
>Subject: Re: [PATCH 1/4] vfio/pci: Move VFIODevice initializations in
>vfio_instance_init
>
>On 11/16/23 03:16, Duan, Zhenzhong wrote:
>>
>>
>>> -Original Message-
>>> From: Cédric Le Goater 
>>> Sent: Wednesday, November 15, 2023 9:12 PM
>>> Subject: Re: [PATCH 1/4] vfio/pci: Move VFIODevice initializations in
>>> vfio_instance_init
>>>
>>> On 11/15/23 09:32, Zhenzhong Duan wrote:
 Some of the VFIODevice initializations is in vfio_realize,
 move all of them in vfio_instance_init.

 No functional change intended.

 Suggested-by: Cédric Le Goater 
 Signed-off-by: Zhenzhong Duan 
 ---
hw/vfio/pci.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)

 diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
 index b23b492cce..5a2b7a2d6b 100644
 --- a/hw/vfio/pci.c
 +++ b/hw/vfio/pci.c
 @@ -2969,9 +2969,6 @@ static void vfio_realize(PCIDevice *pdev, Error
>**errp)
if (vfio_device_get_name(vbasedev, errp)) {
return;
}
 -vbasedev->ops = _pci_ops;
 -vbasedev->type = VFIO_DEVICE_TYPE_PCI;
 -vbasedev->dev = DEVICE(vdev);

/*
 * Mediated devices *might* operate compatibly with discarding of 
 RAM,
>>> but
 @@ -3320,6 +3317,7 @@ static void vfio_instance_init(Object *obj)
{
PCIDevice *pci_dev = PCI_DEVICE(obj);
VFIOPCIDevice *vdev = VFIO_PCI(obj);
 +VFIODevice *vbasedev = >vbasedev;

device_add_bootindex_property(obj, >bootindex,
  "bootindex", NULL,
 @@ -3328,7 +3326,11 @@ static void vfio_instance_init(Object *obj)
vdev->host.bus = ~0U;
vdev->host.slot = ~0U;
vdev->host.function = ~0U;
 -vdev->vbasedev.fd = -1;
 +
 +vbasedev->type = VFIO_DEVICE_TYPE_PCI;
 +vbasedev->ops = _pci_ops;
 +vbasedev->dev = DEVICE(vdev);
 +vbasedev->fd = -1;
>>>
>>> VFIODevice is similar to a base QOM parent. Could we introduce an helper
>>> routine like we did with vfio_device_set_fd() ?
>>
>> Sure, will do.
>
>Since this series is reviewed, could you please consolidate with an extra
>patch on top of this v1 ?

Got it.

Thanks
Zhenzhong

[PULL 11/27] docs/about/deprecated.rst: spelling fix: becase

[Michael Tokarev]

Fixes: 864128df465a "migration: Deprecate old compression method"
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 docs/about/deprecated.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 78550c07bf..6c84db90b5 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -514,5 +514,5 @@ old compression method (since 8.2)
 
 Compression method fails too much.  Too many races.  We are going to
 remove it if nobody fixes it.  For starters, migration-test
-compression tests are disabled becase they fail randomly.  If you need
+compression tests are disabled because they fail randomly.  If you need
 compression, use multifd compression methods.
-- 
2.39.2

[PULL 24/27] tests/qtest/migration-test.c: spelling fix: bandwith

[Michael Tokarev]

Fixes: 17257b90be4f "tests: Add migration dirty-limit capability test"
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 tests/qtest/migration-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 5752412b64..0fbaa6a90f 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -3138,7 +3138,7 @@ static void test_migrate_dirty_limit(void)
 uint64_t throttle_us_per_full;
 /*
  * We want the test to be stable and as fast as possible.
- * E.g., with 1Gb/s bandwith migration may pass without dirty limit,
+ * E.g., with 1Gb/s bandwidth migration may pass without dirty limit,
  * so we need to decrease a bandwidth.
  */
 const int64_t dirtylimit_period = 1000, dirtylimit_value = 50;
-- 
2.39.2

[PULL 20/27] include/block/ufs.h: spelling fix: setted

[Michael Tokarev]

Fixes: bc4e68d362ec "hw/ufs: Initial commit for emulated 
Universal-Flash-Storage"
Reviewed-by: Jeuk Kim 
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 include/block/ufs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/block/ufs.h b/include/block/ufs.h
index 0b6ec0814d..d61598b8f3 100644
--- a/include/block/ufs.h
+++ b/include/block/ufs.h
@@ -657,7 +657,7 @@ typedef struct QEMU_PACKED UtpTaskReqDesc {
 #define UFS_UPIU_MAX_WB_LUN_ID 8
 
 /*
- * WriteBooster buffer lifetime has a limit setted by vendor.
+ * WriteBooster buffer lifetime has a limit set by vendor.
  * If it is over the limit, WriteBooster feature will be disabled.
  */
 #define UFS_WB_EXCEED_LIFETIME 0x0B
-- 
2.39.2

[PULL 12/27] docs/devel/migration.rst: spelling fixes: doen't, diferent, responsability, recomend

[Michael Tokarev]

Fixes: 593c28c02c81 "migration/doc: How to migrate when hosts have different 
features"
Fixes: 1aefe2ca1423 "migration/doc: Add documentation for backwards 
compatiblity"
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 docs/devel/migration.rst | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
index 5adf4f12f7..ec55089b25 100644
--- a/docs/devel/migration.rst
+++ b/docs/devel/migration.rst
@@ -1061,7 +1061,7 @@ QEMU version, in this case pc-5.1.
 
 4 - qemu-5.1 -M pc-5.2  -> migrates to -> qemu-5.1 -M pc-5.2
 
-  This combination is not possible as the qemu-5.1 doen't understand
+  This combination is not possible as the qemu-5.1 doesn't understand
   pc-5.2 machine type.  So nothing to worry here.
 
 Now it comes the interesting ones, when both QEMU processes are
@@ -1214,8 +1214,8 @@ machine types to have the right value::
  ...
  };
 
-A device with diferent features on both sides
--
+A device with different features on both sides
+--
 
 Let's assume that we are using the same QEMU binary on both sides,
 just to make the things easier.  But we have a device that has
@@ -1294,12 +1294,12 @@ Host B:
 
 $ qemu-system-x86_64 -cpu host,taa-no=off
 
-And you would be able to migrate between them.  It is responsability
+And you would be able to migrate between them.  It is responsibility
 of the management application or of the user to make sure that the
 configuration is correct.  QEMU doesn't know how to look at this kind
 of features in general.
 
-Notice that we don't recomend to use -cpu host for migration.  It is
+Notice that we don't recommend to use -cpu host for migration.  It is
 used in this example because it makes the example simpler.
 
 Other devices have worse control about individual features.  If they
-- 
2.39.2

[PULL 27/27] util/range.c: spelling fix: inbetween

[Michael Tokarev]

Fixes: b439595a08d7 "range: Introduce range_inverse_array()"
Reviewed-by: Eric Auger 
Reviewed-by: Cédric Le Goater 
Signed-off-by: Michael Tokarev 
---
 util/range.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/range.c b/util/range.c
index 9605ccfcbe..f3f40098d5 100644
--- a/util/range.c
+++ b/util/range.c
@@ -98,7 +98,7 @@ void range_inverse_array(GList *in, GList **rev,
 out = append_new_range(out, low, MIN(range_lob(r) - 1, high));
 }
 
-/* insert a range inbetween each original range until we reach high */
+/* insert a range in between each original range until we reach high */
 for (; l->next; l = l->next) {
 r = (Range *)l->data;
 rn = (Range *)l->next->data;
-- 
2.39.2

[PULL 07/27] bsd-user: spelling fixes: necesary, agrument, undocummented

[Michael Tokarev]

Fixes: a99d74034754 "bsd-user: Implement do_obreak function"
Fixes: 8632729060bf "bsd-user: Implement freebsd_exec_common, used in 
implementing execve/fexecve."
Fixes: bf14f13d8be8 "bsd-user: Implement stat related syscalls"
Reviewed-by: Warner Losh 
Signed-off-by: Michael Tokarev 
---
 bsd-user/bsd-mem.h | 2 +-
 bsd-user/freebsd/os-proc.c | 2 +-
 bsd-user/freebsd/os-stat.h | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index c3e72e3b86..21d9bab889 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -235,7 +235,7 @@ static inline abi_long do_obreak(abi_ulong brk_val)
 return target_brk;
 }
 
-/* Release heap if necesary */
+/* Release heap if necessary */
 if (new_brk < old_brk) {
 target_munmap(new_brk, old_brk - new_brk);
 
diff --git a/bsd-user/freebsd/os-proc.c b/bsd-user/freebsd/os-proc.c
index 4e67ae4d56..e0203e259b 100644
--- a/bsd-user/freebsd/os-proc.c
+++ b/bsd-user/freebsd/os-proc.c
@@ -115,7 +115,7 @@ abi_long freebsd_exec_common(abi_ulong path_or_fd, 
abi_ulong guest_argp,
 }
 
 qarg0 = argp = g_new0(char *, argc + 9);
-/* save the first agrument for the emulator */
+/* save the first argument for the emulator */
 *argp++ = (char *)getprogname();
 qargp = argp;
 *argp++ = (char *)getprogname();
diff --git a/bsd-user/freebsd/os-stat.h b/bsd-user/freebsd/os-stat.h
index b20e270774..3bdc66aa98 100644
--- a/bsd-user/freebsd/os-stat.h
+++ b/bsd-user/freebsd/os-stat.h
@@ -146,7 +146,7 @@ static inline abi_long do_freebsd_fstatat(abi_long arg1, 
abi_long arg2,
 return ret;
 }
 
-/* undocummented nstat(char *path, struct nstat *ub) syscall */
+/* undocumented nstat(char *path, struct nstat *ub) syscall */
 static abi_long do_freebsd11_nstat(abi_long arg1, abi_long arg2)
 {
 abi_long ret;
@@ -162,7 +162,7 @@ static abi_long do_freebsd11_nstat(abi_long arg1, abi_long 
arg2)
 return ret;
 }
 
-/* undocummented nfstat(int fd, struct nstat *sb) syscall */
+/* undocumented nfstat(int fd, struct nstat *sb) syscall */
 static abi_long do_freebsd11_nfstat(abi_long arg1, abi_long arg2)
 {
 abi_long ret;
@@ -175,7 +175,7 @@ static abi_long do_freebsd11_nfstat(abi_long arg1, abi_long 
arg2)
 return ret;
 }
 
-/* undocummented nlstat(char *path, struct nstat *ub) syscall */
+/* undocumented nlstat(char *path, struct nstat *ub) syscall */
 static abi_long do_freebsd11_nlstat(abi_long arg1, abi_long arg2)
 {
 abi_long ret;
-- 
2.39.2

Re: [PATCH v5 05/31] cpu: Add helper cpu_model_from_type()

[Philippe Mathieu-Daudé]

On 15/11/23 00:56, Gavin Shan wrote:

Add helper cpu_model_from_type() to extract the CPU model name from
the CPU type name in two circumstances: (1) The CPU type name is the
combination of the CPU model name and suffix. (2) The CPU type name
is same to the CPU model name.

The helper will be used in the subsequent commits to conver the
CPU type name to the CPU model name.

Suggested-by: Igor Mammedov 
Signed-off-by: Gavin Shan 
---
  cpu-target.c  | 15 +++
  include/hw/core/cpu.h | 12 
  2 files changed, 27 insertions(+)




diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index c0c8320413..57ceb46bc1 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -779,6 +779,18 @@ void cpu_reset(CPUState *cpu);
   */
  ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
  
+/**

+ * cpu_model_from_type:
+ * @typename: The CPU type name
+ *
+ * Extract the CPU model name from the CPU type name. The
+ * CPU type name is either the combination of the CPU model
+ * name and suffix, or same to the CPU model name.
+ *
+ * Returns: CPU model name or NULL if the CPU class doesn't exist


Worth adding:

*  The user should g_free() the string once no longer
*  needed.


+ */



Reviewed-by: Philippe Mathieu-Daudé 

[PULL 18/27] hw/mem/memory-device.c: spelling fix: ontaining

[Michael Tokarev]

Fixes: 6c1b28e9e405 "memory-device: Support empty memory devices"
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 hw/mem/memory-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
index e0704b8dc3..a1b1af26bc 100644
--- a/hw/mem/memory-device.c
+++ b/hw/mem/memory-device.c
@@ -29,7 +29,7 @@ static bool memory_device_is_empty(const MemoryDeviceState 
*md)
 /* dropping const here is fine as we don't touch the memory region */
 mr = mdc->get_memory_region((MemoryDeviceState *)md, _err);
 if (local_err) {
-/* Not empty, we'll report errors later when ontaining the MR again. */
+/* Not empty, we'll report errors later when containing the MR again. 
*/
 error_free(local_err);
 return false;
 }
-- 
2.39.2

[PULL 22/27] include/hw/virtio/vhost.h: spelling fix: sate

[Michael Tokarev]

Fixes: 4a00d5d7f4b6 "vhost: Add high-level state save/load functions"
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 include/hw/virtio/vhost.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 05d7204a08..02477788df 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -456,7 +456,7 @@ int vhost_save_backend_state(struct vhost_dev *dev, 
QEMUFile *f, Error **errp);
  * Must only be called while the device and all its vrings are stopped
  * (`VHOST_TRANSFER_STATE_PHASE_STOPPED`).
  *
- * @dev: The vhost device to which to send the sate
+ * @dev: The vhost device to which to send the state
  * @f: Migration stream from which to load the state
  * @errp: Potential error message
  *
-- 
2.39.2

[PULL 10/27] gdbstub: spelling fix: respectivelly

[Michael Tokarev]

Fixes: 761e3c10881b "gdbstub: fixes cases where wrong threads were reported to 
GDB on SIGINT"
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 gdbstub/gdbstub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index ebb912da1b..46d752bbc2 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -692,7 +692,7 @@ static int gdb_handle_vcont(const char *p)
 /*
  * target_count and last_target keep track of how many CPUs we are going to
  * step or resume, and a pointer to the state structure of one of them,
- * respectivelly
+ * respectively
  */
 int target_count = 0;
 CPUState *last_target = NULL;
-- 
2.39.2

[PULL 14/27] target/arm/tcg: spelling fixes: alse, addreses

[Michael Tokarev]

Fixes: 179e9a3baccc "target/arm: Define new TB flag for ATA0"
Fixes: 5d7b37b5f675 "target/arm: Implement the CPY* instructions"
Reviewed-by: Richard Henderson 
Signed-off-by: Michael Tokarev 
---
 target/arm/tcg/helper-a64.c | 2 +-
 target/arm/tcg/hflags.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index ce4800b8d1..8ad84623d3 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -1414,7 +1414,7 @@ void HELPER(setge)(CPUARMState *env, uint32_t syndrome, 
uint32_t mtedesc)
 /*
  * Perform part of a memory copy from the guest memory at fromaddr
  * and extending for copysize bytes, to the guest memory at
- * toaddr. Both addreses are dirty.
+ * toaddr. Both addresses are dirty.
  *
  * Returns the number of bytes actually set, which might be less than
  * copysize; the caller should loop until the whole copy has been done.
diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
index 3d7fdce5c3..a6ebd7571a 100644
--- a/target/arm/tcg/hflags.c
+++ b/target/arm/tcg/hflags.c
@@ -327,7 +327,7 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, 
int el, int fp_el,
 DP_TBFLAG_A64(flags, MTE0_ACTIVE, 1);
 }
 /*
- * For unpriv tag-setting accesses we alse need ATA0. Again, in
+ * For unpriv tag-setting accesses we also need ATA0. Again, in
  * contexts where unpriv and normal insns are the same we
  * duplicate the ATA bit to save effort for translate-a64.c.
  */
-- 
2.39.2

[PULL 03/27] tests/data/qobject/qdict.txt: Avoid non-inclusive words

[Michael Tokarev]

From: Thomas Huth 

qdict.txt only consists of more or less random test data. We
can simply drop the lines with the problematic words here.

Signed-off-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 tests/data/qobject/qdict.txt | 4 
 1 file changed, 4 deletions(-)

diff --git a/tests/data/qobject/qdict.txt b/tests/data/qobject/qdict.txt
index 122fda4524..e2edc88161 100644
--- a/tests/data/qobject/qdict.txt
+++ b/tests/data/qobject/qdict.txt
@@ -1866,10 +1866,6 @@ blackfin: 4096
 blackfin.c: 7552
 blackfin.h: 1089
 blackfin_sram.h: 1207
-blacklist.c: 8658
-blacklist.h: 108
-blackstamp.c: 9838
-BlackStamp_defconfig: 27434
 blinken.h: 617
 blizzard.c: 41338
 blizzard.h: 249
-- 
2.39.2

[PULL 19/27] hw/net/cadence_gem.c: spelling fixes: Octects

[Michael Tokarev]

Fixes: c755c943aa2e "hw/net/cadence_gem: use REG32 macro for register 
definitions"
Reviewed-by: Thomas Huth 
Reviewed-by: Luc Michel 
Signed-off-by: Michael Tokarev 
---
 hw/net/cadence_gem.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 5b989f5b52..19adbc0e19 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -225,8 +225,8 @@ REG32(WOLAN, 0xb8) /* Wake on LAN reg */
 REG32(IPGSTRETCH, 0xbc) /* IPG Stretch reg */
 REG32(SVLAN, 0xc0) /* Stacked VLAN reg */
 REG32(MODID, 0xfc) /* Module ID reg */
-REG32(OCTTXLO, 0x100) /* Octects transmitted Low reg */
-REG32(OCTTXHI, 0x104) /* Octects transmitted High reg */
+REG32(OCTTXLO, 0x100) /* Octets transmitted Low reg */
+REG32(OCTTXHI, 0x104) /* Octets transmitted High reg */
 REG32(TXCNT, 0x108) /* Error-free Frames transmitted */
 REG32(TXBCNT, 0x10c) /* Error-free Broadcast Frames */
 REG32(TXMCNT, 0x110) /* Error-free Multicast Frame */
@@ -245,8 +245,8 @@ REG32(EXCESSCOLLCNT, 0x140) /* Excessive Collision Frames */
 REG32(LATECOLLCNT, 0x144) /* Late Collision Frames */
 REG32(DEFERTXCNT, 0x148) /* Deferred Transmission Frames */
 REG32(CSENSECNT, 0x14c) /* Carrier Sense Error Counter */
-REG32(OCTRXLO, 0x150) /* Octects Received register Low */
-REG32(OCTRXHI, 0x154) /* Octects Received register High */
+REG32(OCTRXLO, 0x150) /* Octets Received register Low */
+REG32(OCTRXHI, 0x154) /* Octets Received register High */
 REG32(RXCNT, 0x158) /* Error-free Frames Received */
 REG32(RXBROADCNT, 0x15c) /* Error-free Broadcast Frames RX */
 REG32(RXMULTICNT, 0x160) /* Error-free Multicast Frames RX */
-- 
2.39.2

[PULL 08/27] linux-user: spelling fixes: othe, necesary

[Michael Tokarev]

Fixes: e34136d93059 "linux-user/ppc: Add vdso"
Fixes: 86f04735ac20 "linux-user: Fix brk() to release pages"
Reviewed-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 linux-user/ppc/vdso.S | 2 +-
 linux-user/syscall.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/ppc/vdso.S b/linux-user/ppc/vdso.S
index 689010db13..2e79ea9808 100644
--- a/linux-user/ppc/vdso.S
+++ b/linux-user/ppc/vdso.S
@@ -227,7 +227,7 @@ endf__kernel_sigtramp_rt
 #ifndef _ARCH_PPC64
/*
 * The non-rt sigreturn has the same layout at a different offset.
-* Move the CFA and leave all othe other descriptions the same.
+* Move the CFA and leave all the other descriptions the same.
 */
.cfi_def_cfa1, SIGNAL_FRAMESIZE + offsetof_sigframe_mcontext
nop
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 65ac3ac796..16ca5ea7b6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -831,7 +831,7 @@ abi_long do_brk(abi_ulong brk_val)
 return target_brk;
 }
 
-/* Release heap if necesary */
+/* Release heap if necessary */
 if (new_brk < old_brk) {
 target_munmap(new_brk, old_brk - new_brk);
 
-- 
2.39.2

[PULL 17/27] contrib/vhost-user-gpu/virgl.c: spelling fix: mesage

[Michael Tokarev]

Fixes: e3c82fe04f31 "contrib/vhost-user-gpu: add support for sending dmabuf 
modifiers"
Reviewed-by: Marc-André Lureau 
Signed-off-by: Michael Tokarev 
---
 contrib/vhost-user-gpu/virgl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/vhost-user-gpu/virgl.c b/contrib/vhost-user-gpu/virgl.c
index 1da6cc1588..d1ccdf7d06 100644
--- a/contrib/vhost-user-gpu/virgl.c
+++ b/contrib/vhost-user-gpu/virgl.c
@@ -401,7 +401,7 @@ virgl_cmd_set_scanout(VuGpu *g,
 
 if (g->use_modifiers) {
 /*
- * The mesage uses all the fields set in dmabuf_scanout plus
+ * The message uses all the fields set in dmabuf_scanout plus
  * modifiers which is appended after VhostUserGpuDMABUFScanout.
  */
 msg.request = VHOST_USER_GPU_DMABUF_SCANOUT2;
-- 
2.39.2

[PULL 05/27] hw/audio/virtio-snd.c: spelling: initalize

[Michael Tokarev]

Fixes: eb9ad377bb94 "virtio-sound: handle control messages and streams"
Signed-off-by: Michael Tokarev 
Reviewed-by: Stefan Weil 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/audio/virtio-snd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index a18a9949a7..2fe966e311 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -1126,7 +1126,7 @@ static void virtio_snd_realize(DeviceState *dev, Error 
**errp)
 status = virtio_snd_set_pcm_params(vsnd, i, _params);
 if (status != cpu_to_le32(VIRTIO_SND_S_OK)) {
 error_setg(errp,
-   "Can't initalize stream params, device responded with 
%s.",
+   "Can't initialize stream params, device responded with 
%s.",
print_code(status));
 return;
 }
-- 
2.39.2

[PULL 01/27] hw/watchdog/wdt_aspeed: Remove unused 'hw/misc/aspeed_scu.h' header

[Michael Tokarev]

From: Philippe Mathieu-Daudé 

Aspeed watchdog doesn't use anything from the System Control Unit.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Cédric Le Goater 
Signed-off-by: Michael Tokarev 
---
 hw/watchdog/wdt_aspeed.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index d267aa185c..273a49d360 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -14,7 +14,6 @@
 #include "qemu/module.h"
 #include "qemu/timer.h"
 #include "sysemu/watchdog.h"
-#include "hw/misc/aspeed_scu.h"
 #include "hw/qdev-properties.h"
 #include "hw/sysbus.h"
 #include "hw/watchdog/wdt_aspeed.h"
-- 
2.39.2

[PULL 02/27] MAINTAINERS: Add tests/decode/ to the "Overall TCG CPUs" section

[Michael Tokarev]

From: Thomas Huth 

The tests/decode/ folder belongs to scripts/decodetree.py, so
it should be listed in the same section as the script.

Signed-off-by: Thomas Huth 
Signed-off-by: Michael Tokarev 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ff1238bb98..695e0bd34f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -174,6 +174,7 @@ F: include/hw/core/tcg-cpu-ops.h
 F: host/include/*/host/cpuinfo.h
 F: util/cpuinfo-*.c
 F: include/tcg/
+F: tests/decode/
 
 FPU emulation
 M: Aurelien Jarno 
-- 
2.39.2

[PULL 04/27] qapi/pragma.json: Improve the comment about the lists of QAPI rule exceptions

[Michael Tokarev]

From: Thomas Huth 

Let's use more inclusive language here.

Signed-off-by: Thomas Huth 
Reviewed-by: Eric Blake 
Signed-off-by: Michael Tokarev 
---
 qapi/pragma.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qapi/pragma.json b/qapi/pragma.json
index 7f810b0e97..0aa4eeddd3 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -3,8 +3,8 @@
 
 { 'pragma': { 'doc-required': true } }
 
-# Whitelists to permit QAPI rule violations; think twice before you
-# add to them!
+# Entries in these lists are allowed to violate the QAPI rules (for
+# historical reasons); think twice before you add to them!
 { 'pragma': {
 # Command names containing '_'
 'command-name-exceptions': [
-- 
2.39.2

[PULL 06/27] qapi/migration.json: spelling: transfering

[Michael Tokarev]

Fixes: 074dbce5fcce "migration: New migrate and migrate-incoming argument 
'channels'"
Signed-off-by: Michael Tokarev 
---
 qapi/migration.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index 975761eebd..eb2f883513 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1658,7 +1658,7 @@
 #
 # Migration stream channel parameters.
 #
-# @channel-type: Channel type for transfering packet information.
+# @channel-type: Channel type for transferring packet information.
 #
 # @addr: Migration endpoint configuration on destination interface.
 #
-- 
2.39.2

RE: [PATCH v6 11/21] vfio/pci: Make vfio cdev pre-openable by passing a file handle

[Duan, Zhenzhong]

>-Original Message-
>From: Cédric Le Goater 
>Sent: Thursday, November 16, 2023 3:25 PM
>Subject: Re: [PATCH v6 11/21] vfio/pci: Make vfio cdev pre-openable by passing 
>a
>file handle
>
 Please add bare documentation:

     /* Returns 0 on success, or a negative errno. */

> +int vfio_device_get_name(VFIODevice *vbasedev, Error **errp);
>>
>> Will do, I'd like to wait a few days to collect more suggested changes and 
>> RB,
>> Then send all these updates to Cédric in once before he pushes this series to
>vfio-next.
>
>Yep. Could you respin a v7 with all the comments on v6 ? I will
>then apply directly on vfio-next.

Sure.

>
>Please wait for Eric to finish looking at the platform part.

Sure.

Thanks
Zhenzhong

Re: virtio-sound segmentation fault

[Manos Pitsidianakis]

On Wed, 15 Nov 2023 22:27, Volker Rümelin  wrote:

Cc: qemu-devel

Hi Manos,

it's easy to trigger a segmentation fault with the virtio-sound device.
The basic problem is that in function virtio_snd_realize() there is no
code in the errror paths to undo the previous steps.


Thank you for the report Volker! Patch posted:
https://lore.kernel.org/qemu-devel/20231116072046.4002957-1-manos.pitsidiana...@linaro.org/

Manos

Re: [PATCH v5 06/31] cpu: Add generic cpu_list()

[Philippe Mathieu-Daudé]

Hi Gavin,

On 15/11/23 00:56, Gavin Shan wrote:

Add generic cpu_list() to replace the individual target's implementation
in the subsequent commits. Currently, there are 3 targets with no cpu_list()
implementation: microblaze and nios2. With this applied, those two targets
switch to the generic cpu_list().

[gshan@gshan q]$ ./build/qemu-system-microblaze -cpu ?
Available CPUs:
   microblaze-cpu

[gshan@gshan q]$ ./build/qemu-system-nios2 -cpu ?
Available CPUs:
   nios2-cpu

Suggested-by: Richard Henderson 
Signed-off-by: Gavin Shan 
---
  bsd-user/main.c |  5 +
  cpu-target.c| 29 ++---
  2 files changed, 27 insertions(+), 7 deletions(-)




diff --git a/cpu-target.c b/cpu-target.c
index c078c0e91b..acfc654b95 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -24,6 +24,7 @@
  #include "hw/qdev-core.h"
  #include "hw/qdev-properties.h"
  #include "qemu/error-report.h"
+#include "qemu/qemu-print.h"
  #include "migration/vmstate.h"
  #ifdef CONFIG_USER_ONLY
  #include "qemu.h"
@@ -283,12 +284,34 @@ const char *parse_cpu_option(const char *cpu_option)
  return cpu_type;
  }
  
+#ifndef cpu_list

+static void cpu_list_entry(gpointer data, gpointer user_data)
+{
+CPUClass *cc = CPU_CLASS(OBJECT_CLASS(data));
+const char *typename = object_class_get_name(OBJECT_CLASS(data));
+g_autofree char *model = cpu_model_from_type(typename);
+
+if (cc->deprecation_note) {
+qemu_printf("  %s (deprecated)\n", model);
+} else {
+qemu_printf("  %s\n", model);
+}
+}
+
+static void cpu_list(void)
+{
+GSList *list;
+
+list = object_class_get_list_sorted(TYPE_CPU, false);
+qemu_printf("Available CPUs:\n");


Since this output will likely be displayed a lot, IMHO it is worth
doing a first pass to get the number of available CPUs. If it is 1,
print using singular but even better smth like:

   "This machine can only be used with the following CPU:"

That said, this can be done later on top, so:
Reviewed-by: Philippe Mathieu-Daudé 


+g_slist_foreach(list, cpu_list_entry, NULL);
+g_slist_free(list);
+}
+#endif

Re: [PATCH-for-8.2] virtio-sound: add realize() error cleanup path

[Manos Pitsidianakis]

On Thu, 16 Nov 2023 09:32, Philippe Mathieu-Daudé  wrote:

---

Notes:
 Requires patch <20231109162034.2108018-1-manos.pitsidiana...@linaro.org>


This is the 'Based-on: ' tag I guess.


There is

 prerequisite-patch-id: 484ec9f7f6109c10d4be0484fe8e3c2550c415f4

At the end of the patch, added by git-format-patch. Is it best practice 
to include it in the commit message too?


Thanks :)

Re: [PATCH-for-8.2] virtio-sound: add realize() error cleanup path

[Philippe Mathieu-Daudé]

Hi Manos,

On 16/11/23 08:20, Manos Pitsidianakis wrote:

QEMU crashes on exit when a virtio-sound device has failed to
realise. Its vmstate field was not cleaned up properly with
qemu_del_vm_change_state_handler().

This patch changes the realize() order as

1. Validate the given configuration values (no resources allocated
by us either on success or failure)
2. Try AUD_register_card() and return on failure (no resources allocated
by us on failure)
3. Initialize vmstate, virtio device, heap allocations and stream
parameters at once.
If error occurs, goto error_cleanup label which calls
virtio_snd_unrealize(). This cleans up all resources made in steps
1-3.

Reported-by: Volker Rümelin 
Fixes: 2880e676c000 ("Add virtio-sound device stub")
Signed-off-by: Manos Pitsidianakis 
---

Notes:
 Requires patch <20231109162034.2108018-1-manos.pitsidiana...@linaro.org>


This is the 'Based-on: ' tag I guess.

Reviewed-by: Philippe Mathieu-Daudé 


  hw/audio/virtio-snd.c | 39 ++-
  1 file changed, 22 insertions(+), 17 deletions(-)

Re: [PATCH 1/4] vfio/pci: Move VFIODevice initializations in vfio_instance_init

[Cédric Le Goater]

On 11/16/23 03:16, Duan, Zhenzhong wrote:




-Original Message-
From: Cédric Le Goater 
Sent: Wednesday, November 15, 2023 9:12 PM
Subject: Re: [PATCH 1/4] vfio/pci: Move VFIODevice initializations in
vfio_instance_init

On 11/15/23 09:32, Zhenzhong Duan wrote:

Some of the VFIODevice initializations is in vfio_realize,
move all of them in vfio_instance_init.

No functional change intended.

Suggested-by: Cédric Le Goater 
Signed-off-by: Zhenzhong Duan 
---
   hw/vfio/pci.c | 10 ++
   1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index b23b492cce..5a2b7a2d6b 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2969,9 +2969,6 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
   if (vfio_device_get_name(vbasedev, errp)) {
   return;
   }
-vbasedev->ops = _pci_ops;
-vbasedev->type = VFIO_DEVICE_TYPE_PCI;
-vbasedev->dev = DEVICE(vdev);

   /*
* Mediated devices *might* operate compatibly with discarding of RAM,

but

@@ -3320,6 +3317,7 @@ static void vfio_instance_init(Object *obj)
   {
   PCIDevice *pci_dev = PCI_DEVICE(obj);
   VFIOPCIDevice *vdev = VFIO_PCI(obj);
+VFIODevice *vbasedev = >vbasedev;

   device_add_bootindex_property(obj, >bootindex,
 "bootindex", NULL,
@@ -3328,7 +3326,11 @@ static void vfio_instance_init(Object *obj)
   vdev->host.bus = ~0U;
   vdev->host.slot = ~0U;
   vdev->host.function = ~0U;
-vdev->vbasedev.fd = -1;
+
+vbasedev->type = VFIO_DEVICE_TYPE_PCI;
+vbasedev->ops = _pci_ops;
+vbasedev->dev = DEVICE(vdev);
+vbasedev->fd = -1;


VFIODevice is similar to a base QOM parent. Could we introduce an helper
routine like we did with vfio_device_set_fd() ?


Sure, will do.


Since this series is reviewed, could you please consolidate with an extra
patch on top of this v1 ?

Thanks,

C.

Re: [PATCH] tests/avocado/mem-addr-space-check: Replace assertEquals() for Python 3.12

[Ani Sinha]

> On 16-Nov-2023, at 11:49 AM, Thomas Huth  wrote:
> 
> assertEquals() has been removed in Python 3.12 and should be replaced by
> assertEqual(). See: https://docs.python.org/3.12/whatsnew/3.12.html#id3
> 
> Signed-off-by: Thomas Huth 

Acked-by: Ani Sinha mailto:anisi...@redhat.com>>

> ---
> Note: The test has just been merged, that's why I missed this file in
> my earlier patch that replaces the assertEquals()
> 
> tests/avocado/mem-addr-space-check.py | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/avocado/mem-addr-space-check.py 
> b/tests/avocado/mem-addr-space-check.py
> index be949222a4..363c3f12a6 100644
> --- a/tests/avocado/mem-addr-space-check.py
> +++ b/tests/avocado/mem-addr-space-check.py
> @@ -49,7 +49,7 @@ def test_phybits_low_pse36(self):
> self.vm.set_qmp_monitor(enabled=False)
> self.vm.launch()
> self.vm.wait()
> -self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 
> 1")
> +self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
> self.assertRegex(self.vm.get_log(), r'phys-bits too low')
> 
> def test_phybits_low_pae(self):
> @@ -69,7 +69,7 @@ def test_phybits_low_pae(self):
> self.vm.set_qmp_monitor(enabled=False)
> self.vm.launch()
> self.vm.wait()
> -self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 
> 1")
> +self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
> self.assertRegex(self.vm.get_log(), r'phys-bits too low')
> 
> def test_phybits_ok_pentium_pse36(self):
> @@ -149,7 +149,7 @@ def test_phybits_low_nonpse36(self):
> self.vm.set_qmp_monitor(enabled=False)
> self.vm.launch()
> self.vm.wait()
> -self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 
> 1")
> +self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
> self.assertRegex(self.vm.get_log(), r'phys-bits too low')
> 
> # now lets test some 64-bit CPU cases.
> @@ -179,7 +179,7 @@ def test_phybits_low_tcg_q35_70_amd(self):
> self.vm.set_qmp_monitor(enabled=False)
> self.vm.launch()
> self.vm.wait()
> -self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 
> 1")
> +self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
> self.assertRegex(self.vm.get_log(), r'phys-bits too low')
> 
> def test_phybits_low_tcg_q35_71_amd(self):
> @@ -202,7 +202,7 @@ def test_phybits_low_tcg_q35_71_amd(self):
> self.vm.set_qmp_monitor(enabled=False)
> self.vm.launch()
> self.vm.wait()
> -self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 
> 1")
> +self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
> self.assertRegex(self.vm.get_log(), r'phys-bits too low')
> 
> def test_phybits_ok_tcg_q35_70_amd(self):
> @@ -288,7 +288,7 @@ def test_phybits_low_tcg_q35_71_amd_41bits(self):
> self.vm.set_qmp_monitor(enabled=False)
> self.vm.launch()
> self.vm.wait()
> -self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 
> 1")
> +self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
> self.assertRegex(self.vm.get_log(), r'phys-bits too low')
> 
> def test_phybits_ok_tcg_q35_71_amd_41bits(self):
> @@ -332,7 +332,7 @@ def test_phybits_low_tcg_q35_intel_cxl(self):
> self.vm.set_qmp_monitor(enabled=False)
> self.vm.launch()
> self.vm.wait()
> -self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 
> 1")
> +self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
> self.assertRegex(self.vm.get_log(), r'phys-bits too low')
> 
> def test_phybits_ok_tcg_q35_intel_cxl(self):
> -- 
> 2.41.0
> 

Re: [PATCH-for-8.2? 3/6] hw/arm/stm32f100: Report error when incorrect CPU is used

[Philippe Mathieu-Daudé]

On 16/11/23 00:21, Philippe Mathieu-Daudé wrote:

The 'stm32vldiscovery' machine ignores the CPU type requested by
the command line. This might confuse users, since the following
will create a machine with a Cortex-M3 CPU:

   $ qemu-system-aarch64 -M stm32vldiscovery -cpu neoverse-n1

Set the MachineClass::valid_cpu_types field (introduced in commit
c9cf636d48 "machine: Add a valid_cpu_types property").
Remove the now unused MachineClass::default_cpu_type field.

We now get:

   $ qemu-system-aarch64 -M stm32vldiscovery -cpu neoverse-n1
   qemu-system-aarch64: Invalid CPU type: neoverse-n1-arm-cpu
   The valid types are: cortex-m3-arm-cpu


With [2] from cover applied, this becomes:

 $ qemu-system-aarch64 -M stm32vldiscovery -cpu neoverse-n1 -S
 qemu-system-aarch64: Invalid CPU type: neoverse-n1
 The valid types are: cortex-m3


Since the SoC family can only use Cortex-M3 CPUs, hard-code the
CPU type name at the SoC level, removing the QOM property
entirely.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/arm/stm32f100_soc.h | 4 
  hw/arm/stm32f100_soc.c | 9 ++---
  hw/arm/stm32vldiscovery.c  | 7 ++-
  3 files changed, 8 insertions(+), 12 deletions(-)


[2] 
https://lore.kernel.org/qemu-devel/20231114235628.534334-1-gs...@redhat.com/

Re: [PATCH v6 11/21] vfio/pci: Make vfio cdev pre-openable by passing a file handle

[Cédric Le Goater]

Please add bare documentation:

    /* Returns 0 on success, or a negative errno. */


+int vfio_device_get_name(VFIODevice *vbasedev, Error **errp);


Will do, I'd like to wait a few days to collect more suggested changes and RB,
Then send all these updates to Cédric in once before he pushes this series to 
vfio-next.


Yep. Could you respin a v7 with all the comments on v6 ? I will
then apply directly on vfio-next.

Please wait for Eric to finish looking at the platform part.

Thanks,

C.

Re: [PATCH-for-8.2? 1/6] hw/arm/stm32f405: Report error when incorrect CPU is used

[Philippe Mathieu-Daudé]

On 16/11/23 00:21, Philippe Mathieu-Daudé wrote:

Both 'netduinoplus2' and 'olimex-stm32-h405' machines ignore the
CPU type requested by the command line. This might confuse users,
since the following will create a machine with a Cortex-M4 CPU:

   $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f

Set the MachineClass::valid_cpu_types field (introduced in commit
c9cf636d48 "machine: Add a valid_cpu_types property").
Remove the now unused MachineClass::default_cpu_type field.

We now get:

   $ qemu-system-aarch64 -M netduinoplus2 -cpu cortex-r5f
   qemu-system-aarch64: Invalid CPU type: cortex-r5f-arm-cpu
   The valid types are: cortex-m4-arm-cpu

Since the SoC family can only use Cortex-M4 CPUs, hard-code the
CPU type name at the SoC level, removing the QOM property
entirely.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/arm/stm32f405_soc.h | 4 
  hw/arm/netduinoplus2.c | 7 ++-
  hw/arm/olimex-stm32-h405.c | 8 ++--
  hw/arm/stm32f405_soc.c | 8 +---
  4 files changed, 13 insertions(+), 14 deletions(-)




diff --git a/hw/arm/olimex-stm32-h405.c b/hw/arm/olimex-stm32-h405.c
index 3aa61c91b7..694b1dd6ed 100644
--- a/hw/arm/olimex-stm32-h405.c
+++ b/hw/arm/olimex-stm32-h405.c
@@ -47,7 +47,6 @@ static void olimex_stm32_h405_init(MachineState *machine)
  clock_set_hz(sysclk, SYSCLK_FRQ);
  
  dev = qdev_new(TYPE_STM32F405_SOC);

-qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
  qdev_connect_clock_in(dev, "sysclk", sysclk);
  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
  
@@ -58,9 +57,14 @@ static void olimex_stm32_h405_init(MachineState *machine)
  
  static void olimex_stm32_h405_machine_init(MachineClass *mc)

  {
+static const char *machine_valid_cpu_types[] = {


const char * const

(in all this series).


+ARM_CPU_TYPE_NAME("cortex-m4"),
+NULL
+};
+
  mc->desc = "Olimex STM32-H405 (Cortex-M4)";
  mc->init = olimex_stm32_h405_init;
-mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
+mc->valid_cpu_types = machine_valid_cpu_types;
  
  /* SRAM pre-allocated as part of the SoC instantiation */

  mc->default_ram_size = 0;

[PATCH-for-8.2] virtio-sound: add realize() error cleanup path

[Manos Pitsidianakis]

QEMU crashes on exit when a virtio-sound device has failed to
realise. Its vmstate field was not cleaned up properly with
qemu_del_vm_change_state_handler().

This patch changes the realize() order as

1. Validate the given configuration values (no resources allocated
   by us either on success or failure)
2. Try AUD_register_card() and return on failure (no resources allocated
   by us on failure)
3. Initialize vmstate, virtio device, heap allocations and stream
   parameters at once.
   If error occurs, goto error_cleanup label which calls
   virtio_snd_unrealize(). This cleans up all resources made in steps
   1-3.

Reported-by: Volker Rümelin 
Fixes: 2880e676c000 ("Add virtio-sound device stub")
Signed-off-by: Manos Pitsidianakis 
---

Notes:
Requires patch <20231109162034.2108018-1-manos.pitsidiana...@linaro.org>

 hw/audio/virtio-snd.c | 39 ++-
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index ccf5fcf99e..c17eb435dc 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -36,6 +36,7 @@ static void virtio_snd_pcm_out_cb(void *data, int available);
 static void virtio_snd_process_cmdq(VirtIOSound *s);
 static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream);
 static void virtio_snd_pcm_in_cb(void *data, int available);
+static void virtio_snd_unrealize(DeviceState *dev);
 
 static uint32_t supported_formats = BIT(VIRTIO_SND_PCM_FMT_S8)
   | BIT(VIRTIO_SND_PCM_FMT_U8)
@@ -1065,23 +1066,9 @@ static void virtio_snd_realize(DeviceState *dev, Error 
**errp)
 virtio_snd_pcm_set_params default_params = { 0 };
 uint32_t status;
 
-vsnd->pcm = NULL;
-vsnd->vmstate =
-qemu_add_vm_change_state_handler(virtio_snd_vm_state_change, vsnd);
-
 trace_virtio_snd_realize(vsnd);
 
-vsnd->pcm = g_new0(VirtIOSoundPCM, 1);
-vsnd->pcm->snd = vsnd;
-vsnd->pcm->streams =
-g_new0(VirtIOSoundPCMStream *, vsnd->snd_conf.streams);
-vsnd->pcm->pcm_params =
-g_new0(virtio_snd_pcm_set_params, vsnd->snd_conf.streams);
-
-virtio_init(vdev, VIRTIO_ID_SOUND, sizeof(virtio_snd_config));
-virtio_add_feature(>features, VIRTIO_F_VERSION_1);
-
-/* set number of jacks and streams */
+/* check number of jacks and streams */
 if (vsnd->snd_conf.jacks > 8) {
 error_setg(errp,
"Invalid number of jacks: %"PRIu32,
@@ -1106,6 +1093,19 @@ static void virtio_snd_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+vsnd->vmstate =
+qemu_add_vm_change_state_handler(virtio_snd_vm_state_change, vsnd);
+
+vsnd->pcm = g_new0(VirtIOSoundPCM, 1);
+vsnd->pcm->snd = vsnd;
+vsnd->pcm->streams =
+g_new0(VirtIOSoundPCMStream *, vsnd->snd_conf.streams);
+vsnd->pcm->pcm_params =
+g_new0(virtio_snd_pcm_set_params, vsnd->snd_conf.streams);
+
+virtio_init(vdev, VIRTIO_ID_SOUND, sizeof(virtio_snd_config));
+virtio_add_feature(>features, VIRTIO_F_VERSION_1);
+
 /* set default params for all streams */
 default_params.features = 0;
 default_params.buffer_bytes = cpu_to_le32(8192);
@@ -1130,16 +1130,21 @@ static void virtio_snd_realize(DeviceState *dev, Error 
**errp)
 error_setg(errp,
"Can't initalize stream params, device responded with 
%s.",
print_code(status));
-return;
+goto error_cleanup;
 }
 status = virtio_snd_pcm_prepare(vsnd, i);
 if (status != cpu_to_le32(VIRTIO_SND_S_OK)) {
 error_setg(errp,
"Can't prepare streams, device responded with %s.",
print_code(status));
-return;
+goto error_cleanup;
 }
 }
+
+return;
+
+error_cleanup:
+virtio_snd_unrealize(dev);
 }
 
 static inline void return_tx_buffer(VirtIOSoundPCMStream *stream,

base-commit: 34a5cb6d8434303c170230644b2a7c1d5781d197
prerequisite-patch-id: 484ec9f7f6109c10d4be0484fe8e3c2550c415f4
-- 
2.39.2

Re: [PATCH] tests/avocado/reverse_debugging: Disable the ppc64 tests by default

[Nicholas Piggin]

On Thu Nov 16, 2023 at 1:55 PM AEST, Ani Sinha wrote:
>
>
> > On 16-Nov-2023, at 6:45 AM, Nicholas Piggin  wrote:
> > 
> > On Thu Nov 16, 2023 at 3:22 AM AEST, Daniel P. Berrangé wrote:
> >> On Wed, Nov 15, 2023 at 01:14:53PM +, Daniel P. Berrangé wrote:
> >>> On Wed, Nov 15, 2023 at 07:23:01AM +0100, Thomas Huth wrote:
>  On 15/11/2023 02.15, Nicholas Piggin wrote:
> > On Wed Nov 15, 2023 at 4:29 AM AEST, Thomas Huth wrote:
> >> On 14/11/2023 17.37, Philippe Mathieu-Daudé wrote:
> >>> On 14/11/23 17:31, Thomas Huth wrote:
>  The tests seem currently to be broken. Disable them by default
>  until someone fixes them.
>  
>  Signed-off-by: Thomas Huth 
>  ---
>    tests/avocado/reverse_debugging.py | 7 ---
>    1 file changed, 4 insertions(+), 3 deletions(-)
> >>> 
> >>> Similarly, I suspect 
> >>> https://gitlab.com/qemu-project/qemu/-/issues/1961
> >>> which has a fix ready:
> >>> https://lore.kernel.org/qemu-devel/20231110170831.185001-1-richard.hender...@linaro.org/
> >>> 
> >>> Maybe wait the fix gets in first?
> >> 
> >> No, I applied Richard's patch, but the problem persists. Does this test
> >> still work for you?
> > 
> > I bisected it to 1d4796cd008373 ("python/machine: use socketpair() for
> > console connections"),
>  
>  Maybe John (who wrote that commit) can help?
> >>> 
> >>> I find it hard to believe this commit is a direct root cause of the
> >>> problem since all it does is change the QEMU startup sequence so that
> >>> instead of QEMU listening for a monitor connection, it is given a
> >>> pre-opened monitor connection.
> >>> 
> >>> At the very most that should affect the startup timing a little.
> >>> 
> >>> I notice all the reverse debugging tests have a skip on gitlab
> >>> with a comment:
> >>> 
> >>># unidentified gitlab timeout problem
> >>> 
> >>> this makes be suspicious that John's patch has merely made this
> >>> (henceforth undiagnosed) timeout more likely to ocurr.
> >> 
> >> After an absolutely horrendous hours long debugging session I think
> >> I figured out the problem. The QEMU process is blocking in
> >> 
> >>qemu_chr_write_buffer
> >> 
> >> spinning in the loop on EAGAIN.
> > 
> > Great work.
> > 
> > Why does this make the gdb socket give an empty response? Something
> > just times out?
> > 
> >> 
> >> The Python  Machine() class has passed one of a pre-created socketpair
> >> FDs for the serial port chardev. The guest is trying to write to this
> >> and blocking.  Nothing in the Machine() class is reading from the
> >> other end of the serial port console.
> >> 
> >> 
> >> Before John's change, the serial port uses a chardev in server mode
> >> and crucially  'wait=off', and the Machine() class never opened the
> >> console socket unless the test case wanted to read from it.
> >> 
> >> IOW, QEMU had a background job setting there waiting for a connection
> >> that would never come.
> >> 
> >> As a result when QEMU started executing the guest, all the serial port
> >> writes get sent into to the void.
> >> 
> >> 
> >> So John's patch has had a semantic change in behaviour, because the
> >> console socket is permanently open, and thus socket buffers are liable
> >> to fill up.
> >> 
> >> As a demo I increased the socket buffers to 1MB and everything then
> >> succeeded.
> >> 
> >> @@ -357,6 +360,10 @@ def _pre_launch(self) -> None:
> >> 
> >> if self._console_set:
> >> self._cons_sock_pair = socket.socketpair()
> >> +self._cons_sock_pair[0].setsockopt(socket.SOL_SOCKET, 
> >> socket.SO_SNDBUF, 1024*1024);
> >> +self._cons_sock_pair[0].setsockopt(socket.SOL_SOCKET, 
> >> socket.SO_RCVBUF, 1024*1024);
> >> +self._cons_sock_pair[1].setsockopt(socket.SOL_SOCKET, 
> >> socket.SO_SNDBUF, 1024*1024);
> >> +self._cons_sock_pair[1].setsockopt(socket.SOL_SOCKET, 
> >> socket.SO_RCVBUF, 1024*1024);
> >> os.set_inheritable(self._cons_sock_pair[0].fileno(), True)
> >> 
> >> # NOTE: Make sure any opened resources are *definitely* freed in
> > 
> > So perhaps ppc64 fails just because it prints more to the console in early
> > boot than other targets?
> > 
> >> The Machine class doesn't know if anything will ever use the console,
> >> so as is the change is unsafe.
> >> 
> >> The original goal of John's change was to guarantee we capture early
> >> boot messages as some test need that.  
> >> 
> >> I think we need to be able to have a flag to say whether the caller needs
> >> an "early console" facility, and only use the pre-opened FD passing for
> >> that case. Tests we need early console will have to ask for that guarantee
> >> explicitly.
> > 
> > The below patch makes this test work. Maybe as a quick fix it is
> > better than disabling the test.
> > 
> > I guess we still have a problem if a test invokes vm.launch()
> > directly without subsequently 

Re: [PATCH] tests/avocado/reverse_debugging: Disable the ppc64 tests by default

[Thomas Huth]

On 16/11/2023 02.15, Nicholas Piggin wrote:

On Thu Nov 16, 2023 at 3:22 AM AEST, Daniel P. Berrangé wrote:

On Wed, Nov 15, 2023 at 01:14:53PM +, Daniel P. Berrangé wrote:

...

The Machine class doesn't know if anything will ever use the console,
so as is the change is unsafe.

The original goal of John's change was to guarantee we capture early
boot messages as some test need that.

I think we need to be able to have a flag to say whether the caller needs
an "early console" facility, and only use the pre-opened FD passing for
that case. Tests we need early console will have to ask for that guarantee
explicitly.


The below patch makes this test work. Maybe as a quick fix it is
better than disabling the test.

I guess we still have a problem if a test invokes vm.launch()
directly without subsequently waiting for a console pattern or
doing something with the console as you say. Your suggesstion is
add something like vm.launch(console=True) ?

Thanks,
Nick
---

diff --git a/tests/avocado/reverse_debugging.py 
b/tests/avocado/reverse_debugging.py
index fc47874eda..128d85bc0e 100644
--- a/tests/avocado/reverse_debugging.py
+++ b/tests/avocado/reverse_debugging.py
@@ -12,6 +12,7 @@
  
  from avocado import skipIf

  from avocado_qemu import BUILD_DIR
+from avocado.utils import datadrainer
  from avocado.utils import gdb
  from avocado.utils import process
  from avocado.utils.network.ports import find_free_port
@@ -52,6 +53,10 @@ def run_vm(self, record, shift, args, replay_path, 
image_path, port):
  if args:
  vm.add_args(*args)
  vm.launch()
+console_drainer = datadrainer.LineLogger(vm.console_socket.fileno(),
+logger=self.log.getChild('console'),
+stop_check=(lambda : not vm.is_running()))
+console_drainer.start()
  return vm
  
  @staticmethod


Tested-by: Thomas Huth 

Could you please send this as a proper patch, with a S-o-b line, and a short 
comment in front of the newly added code explaining it?


 Thanks,
  Thomas

Re: [PATCH v5 02/31] target/hppa: Remove object_class_is_abstract()

[Philippe Mathieu-Daudé]

On 15/11/23 00:55, Gavin Shan wrote:

No need to check if @oc is abstract because it has been covered
by cpu_class_by_name().


Since commit 3a9d0d7b64 ("hw/cpu: Call object_class_is_abstract() once 
in cpu_class_by_name()") ...




Signed-off-by: Gavin Shan 
---
  target/hppa/cpu.c | 1 -
  1 file changed, 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 09/19] qapi/schema: assert info is present when necessary

[Philippe Mathieu-Daudé]

On 16/11/23 02:43, John Snow wrote:

QAPISchemaInfo is sometimes defined as an Optional field because
built-in definitions don't *have* a source definition. As a consequence,
there are a few places where we need to assert that it's present because
the root entity definition only suggests it's "Optional".

Signed-off-by: John Snow 
---
  scripts/qapi/schema.py | 2 ++
  1 file changed, 2 insertions(+)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 06/19] qapi/schema: adjust type narrowing for mypy's benefit

[Philippe Mathieu-Daudé]

On 16/11/23 02:43, John Snow wrote:

We already take care to perform some type narrowing for arg_type and
ret_type, but not in a way where mypy can utilize the result. A simple
change to use a temporary variable helps the medicine go down.

Signed-off-by: John Snow 
---
  scripts/qapi/schema.py | 17 +
  1 file changed, 9 insertions(+), 8 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 05/19] qapi/schema: make c_type() and json_type() abstract methods

[Philippe Mathieu-Daudé]

On 16/11/23 02:43, John Snow wrote:

These methods should always return a str, it's only the default abstract
implementation that doesn't. They can be marked "abstract" by raising
NotImplementedError(), which requires subclasses to override the method
with the proper return type.

Signed-off-by: John Snow 
---
  scripts/qapi/schema.py | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 01/19] qapi/schema: fix QAPISchemaEntity.__repr__()

[Philippe Mathieu-Daudé]

On 16/11/23 02:43, John Snow wrote:

This needs parentheses to work how you want it to:


"%s-%s-%s" % 'a', 'b', 'c'

Traceback (most recent call last):
   File "", line 1, in 
TypeError: not enough arguments for format string


"%s-%s-%s" % ('a', 'b', 'c')

'a-b-c'

Signed-off-by: John Snow 
---
  scripts/qapi/schema.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index d739e558e9e..c79747b2a15 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -76,7 +76,7 @@ def __init__(self, name: str, info, doc, ifcond=None, 
features=None):
  def __repr__(self):
  if self.name is None:
  return "<%s at 0x%x>" % (type(self).__name__, id(self))
-return "<%s:%s at 0x%x>" % type(self).__name__, self.name, id(self)
+return "<%s:%s at 0x%x>" % (type(self).__name__, self.name, id(self))


This is commit 6d133eef98 ("qapi: Fix QAPISchemaEntity.__repr__()").

Re: [PATCH] tests/avocado/mem-addr-space-check: Replace assertEquals() for Python 3.12

[Philippe Mathieu-Daudé]

On 16/11/23 07:19, Thomas Huth wrote:

assertEquals() has been removed in Python 3.12 and should be replaced by
assertEqual(). See: https://docs.python.org/3.12/whatsnew/3.12.html#id3

Signed-off-by: Thomas Huth 
---
  Note: The test has just been merged, that's why I missed this file in
  my earlier patch that replaces the assertEquals()


Ah my bad! I was also wondering...

Reviewed-by: Philippe Mathieu-Daudé 



  tests/avocado/mem-addr-space-check.py | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

Re: [PATCH v5 01/31] target/alpha: Remove 'ev67' CPU class

[Philippe Mathieu-Daudé]

On 15/11/23 01:22, Richard Henderson wrote:

On 11/14/23 15:55, Gavin Shan wrote:

'ev67' CPU class will be returned to match everything, which makes
no sense as mentioned in the comments. Remove the logic to fall
back to 'ev67' CPU class to match everything.

Signed-off-by: Gavin Shan 
---
  target/alpha/cpu.c | 7 ++-
  1 file changed, 2 insertions(+), 5 deletions(-)


The subject is wrong -- ev67 cpu class is still present.
Better as

   target/alpha: Remove fallback to ev67 cpu class

with that,
Reviewed-by: Richard Henderson 


Also:
Reviewed-by: Philippe Mathieu-Daudé 

[PATCH v2] migration: free 'saddr' since be no longer used

[Zongmin Zhou]

Since socket_parse() will allocate memory for 'saddr',and its value
will pass to 'addr' that allocated by migrate_uri_parse(),
then 'saddr' will no longer used,need to free.
But due to 'saddr->u' is shallow copying the contents of the union,
the members of this union containing allocated strings,and will be used after 
that.
So just free 'saddr' itself without doing a deep free on the contents of the 
SocketAddress.

Fixes: 72a8192e225c ("migration: convert migration 'uri' into 'MigrateAddress'")
Signed-off-by: Zongmin Zhou
---
 migration/migration.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/migration.c b/migration/migration.c
index 28a34c9068..9bdbcdaf49 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -493,6 +493,7 @@ bool migrate_uri_parse(const char *uri, MigrationChannel 
**channel,
 }
 addr->u.socket.type = saddr->type;
 addr->u.socket.u = saddr->u;
+g_free(saddr);
 } else if (strstart(uri, "file:", NULL)) {
 addr->transport = MIGRATION_ADDRESS_TYPE_FILE;
 addr->u.file.filename = g_strdup(uri + strlen("file:"));
-- 
2.34.1

Re: [PATCH v3 14/70] target/i386: Implement mc->kvm_type() to get VM type

[Xiaoyao Li]

On 11/15/2023 6:49 PM, Daniel P. Berrangé wrote:

On Wed, Nov 15, 2023 at 02:14:23AM -0500, Xiaoyao Li wrote:

Implement mc->kvm_type() for i386 machines. It provides a way for user
to create SW_PROTECTE_VM.


Small typo there missing final 'D' in 'PROTECTED'


Thanks for catching it.

I find the "PROTECTED_VM" part is the leftover of previous series. Since 
this version drop the "protected-vm" part, it should be fall back the 
earlier version like 
https://lore.kernel.org/qemu-devel/20220802074750.2581308-4-xiaoyao...@intel.com/


I will merge next patch into this one, in next version.



Also store the vm_type in machinestate to other code to query what the
VM type is.

Signed-off-by: Xiaoyao Li 
---
  hw/i386/x86.c  | 12 
  include/hw/i386/x86.h  |  1 +
  target/i386/kvm/kvm.c  | 25 +
  target/i386/kvm/kvm_i386.h |  1 +
  4 files changed, 39 insertions(+)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index b3d054889bba..55678279bf3b 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1377,6 +1377,17 @@ static void machine_set_sgx_epc(Object *obj, Visitor *v, 
const char *name,
  qapi_free_SgxEPCList(list);
  }
  
+static int x86_kvm_type(MachineState *ms, const char *vm_type)

+{
+X86MachineState *x86ms = X86_MACHINE(ms);
+int kvm_type;
+
+kvm_type = kvm_get_vm_type(ms, vm_type);
+x86ms->vm_type = kvm_type;
+
+return kvm_type;
+}
+
  static void x86_machine_initfn(Object *obj)
  {
  X86MachineState *x86ms = X86_MACHINE(obj);
@@ -1401,6 +1412,7 @@ static void x86_machine_class_init(ObjectClass *oc, void 
*data)
  mc->cpu_index_to_instance_props = x86_cpu_index_to_props;
  mc->get_default_cpu_node_id = x86_get_default_cpu_node_id;
  mc->possible_cpu_arch_ids = x86_possible_cpu_arch_ids;
+mc->kvm_type = x86_kvm_type;
  x86mc->save_tsc_khz = true;
  x86mc->fwcfg_dma_enabled = true;
  nc->nmi_monitor_handler = x86_nmi;
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index da19ae15463a..ab1d38569019 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -41,6 +41,7 @@ struct X86MachineState {
  MachineState parent;
  
  /*< public >*/

+unsigned int vm_type;
  
  /* Pointers to devices and objects: */

  ISADevice *rtc;
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index b4b9ce89842f..2e47fda25f95 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -161,6 +161,31 @@ static KVMMSRHandlers 
msr_handlers[KVM_MSR_FILTER_MAX_RANGES];
  static RateLimit bus_lock_ratelimit_ctrl;
  static int kvm_get_one_msr(X86CPU *cpu, int index, uint64_t *value);
  
+static const char* vm_type_name[] = {


nitpick   'char *vm_type_name[]', is normal style


will fix it. Thanks!


+[KVM_X86_DEFAULT_VM] = "default",
+[KVM_X86_SW_PROTECTED_VM] = "sw-protected-vm",
+};
+
+int kvm_get_vm_type(MachineState *ms, const char *vm_type)
+{
+int kvm_type = KVM_X86_DEFAULT_VM;
+
+/*
+ * old KVM doesn't support KVM_CAP_VM_TYPES and KVM_X86_DEFAULT_VM
+ * is always supported
+ */
+if (kvm_type == KVM_X86_DEFAULT_VM) {
+return kvm_type;
+}
+
+if (!(kvm_check_extension(KVM_STATE(ms->accelerator), KVM_CAP_VM_TYPES) & 
BIT(kvm_type))) {
+error_report("vm-type %s not supported by KVM", 
vm_type_name[kvm_type]);
+exit(1);
+}
+
+return kvm_type;
+}
+
  bool kvm_has_smm(void)
  {
  return kvm_vm_check_extension(kvm_state, KVM_CAP_X86_SMM);
diff --git a/target/i386/kvm/kvm_i386.h b/target/i386/kvm/kvm_i386.h
index 30fedcffea3e..55fb25fa8e2e 100644
--- a/target/i386/kvm/kvm_i386.h
+++ b/target/i386/kvm/kvm_i386.h
@@ -37,6 +37,7 @@ bool kvm_hv_vpindex_settable(void);
  bool kvm_enable_sgx_provisioning(KVMState *s);
  bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp);
  
+int kvm_get_vm_type(MachineState *ms, const char *vm_type);

  void kvm_arch_reset_vcpu(X86CPU *cs);
  void kvm_arch_after_reset_vcpu(X86CPU *cpu);
  void kvm_arch_do_init_vcpu(X86CPU *cs);
--
2.34.1



With regards,
Daniel

[PATCH] tests/avocado/mem-addr-space-check: Replace assertEquals() for Python 3.12

[Thomas Huth]

assertEquals() has been removed in Python 3.12 and should be replaced by
assertEqual(). See: https://docs.python.org/3.12/whatsnew/3.12.html#id3

Signed-off-by: Thomas Huth 
---
 Note: The test has just been merged, that's why I missed this file in
 my earlier patch that replaces the assertEquals()

 tests/avocado/mem-addr-space-check.py | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/avocado/mem-addr-space-check.py 
b/tests/avocado/mem-addr-space-check.py
index be949222a4..363c3f12a6 100644
--- a/tests/avocado/mem-addr-space-check.py
+++ b/tests/avocado/mem-addr-space-check.py
@@ -49,7 +49,7 @@ def test_phybits_low_pse36(self):
 self.vm.set_qmp_monitor(enabled=False)
 self.vm.launch()
 self.vm.wait()
-self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1")
+self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
 self.assertRegex(self.vm.get_log(), r'phys-bits too low')
 
 def test_phybits_low_pae(self):
@@ -69,7 +69,7 @@ def test_phybits_low_pae(self):
 self.vm.set_qmp_monitor(enabled=False)
 self.vm.launch()
 self.vm.wait()
-self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1")
+self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
 self.assertRegex(self.vm.get_log(), r'phys-bits too low')
 
 def test_phybits_ok_pentium_pse36(self):
@@ -149,7 +149,7 @@ def test_phybits_low_nonpse36(self):
 self.vm.set_qmp_monitor(enabled=False)
 self.vm.launch()
 self.vm.wait()
-self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1")
+self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
 self.assertRegex(self.vm.get_log(), r'phys-bits too low')
 
 # now lets test some 64-bit CPU cases.
@@ -179,7 +179,7 @@ def test_phybits_low_tcg_q35_70_amd(self):
 self.vm.set_qmp_monitor(enabled=False)
 self.vm.launch()
 self.vm.wait()
-self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1")
+self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
 self.assertRegex(self.vm.get_log(), r'phys-bits too low')
 
 def test_phybits_low_tcg_q35_71_amd(self):
@@ -202,7 +202,7 @@ def test_phybits_low_tcg_q35_71_amd(self):
 self.vm.set_qmp_monitor(enabled=False)
 self.vm.launch()
 self.vm.wait()
-self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1")
+self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
 self.assertRegex(self.vm.get_log(), r'phys-bits too low')
 
 def test_phybits_ok_tcg_q35_70_amd(self):
@@ -288,7 +288,7 @@ def test_phybits_low_tcg_q35_71_amd_41bits(self):
 self.vm.set_qmp_monitor(enabled=False)
 self.vm.launch()
 self.vm.wait()
-self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1")
+self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
 self.assertRegex(self.vm.get_log(), r'phys-bits too low')
 
 def test_phybits_ok_tcg_q35_71_amd_41bits(self):
@@ -332,7 +332,7 @@ def test_phybits_low_tcg_q35_intel_cxl(self):
 self.vm.set_qmp_monitor(enabled=False)
 self.vm.launch()
 self.vm.wait()
-self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1")
+self.assertEqual(self.vm.exitcode(), 1, "QEMU exit code should be 1")
 self.assertRegex(self.vm.get_log(), r'phys-bits too low')
 
 def test_phybits_ok_tcg_q35_intel_cxl(self):
-- 
2.41.0

Re: [PATCH-for-8.2? v2] tests/avocado: Make fetch_asset() unconditionally require a crypto hash

[Thomas Huth]

On 15/11/2023 21.51, Philippe Mathieu-Daudé wrote:

In a perfect world we'd have reproducible tests,
but then we'd be sure we run the same binaries.
If a binary artifact isn't hashed, we have no idea
what we are running. Therefore enforce hashing for
all our artifacts.

With this change, unhashed artifacts produce:

   $ avocado run tests/avocado/multiprocess.py
(1/2) tests/avocado/multiprocess.py:Multiprocess.test_multiprocess_x86_64:
ERROR: QemuBaseTest.fetch_asset() missing 1 required positional argument: 
'asset_hash' (0.19 s)

Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
Reviewed-by: Alex Bennée 
---
Based-on: <20231115145852.494052-1-th...@redhat.com>
   "tests/avocado/multiprocess: Add asset hashes to silence warnings"
Based-on: <20231114143531.291820-1-th...@redhat.com>
 "tests/avocado/intel_iommu: Add asset hashes to avoid warnings"
Supersedes: <20231115153247.89486-1-phi...@linaro.org>

v2: Fixed type in subject (Alex)
---
  tests/avocado/avocado_qemu/__init__.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/avocado/avocado_qemu/__init__.py 
b/tests/avocado/avocado_qemu/__init__.py
index d71e989db6..304c428168 100644
--- a/tests/avocado/avocado_qemu/__init__.py
+++ b/tests/avocado/avocado_qemu/__init__.py
@@ -254,7 +254,7 @@ def setUp(self, bin_prefix):
  self.cancel("No QEMU binary defined or found in the build tree")
  
  def fetch_asset(self, name,

-asset_hash=None, algorithm=None,
+asset_hash, algorithm=None,
  locations=None, expire=None,
  find_only=False, cancel_on_missing=True):
  return super().fetch_asset(name,


Thanks, queued it now!

 Thomas

Re: [PATCH v2] tests/acpi/bios-tables-test: do not write new blobs unless there are changes

[Ani Sinha]

> On 07-Nov-2023, at 10:19 AM, Ani Sinha  wrote:
> 
> When dumping table blobs using rebuild-expected-aml.sh, table blobs from all
> test variants are dumped regardless of whether there are any actual changes to
> the tables or not. This creates lot of new files for various test variants 
> that
> are not part of the git repository. This is because we do not check in all 
> table
> blobs for all test variants into the repository. Only those blobs for those
> variants that are different from the generic test-variant agnostic blob are
> checked in.
> 
> This change makes the test smarter by checking if at all there are any changes
> in the tables from the checked-in gold master blobs and take actions
> accordingly.
> 
> When there are no changes:
> - No new table blobs would be written.
> - Existing table blobs will be refreshed (git diff will show no changes).
> When there are changes:
> - New table blob files will be dumped.
> - Existing table blobs will be refreshed (git diff will show that the files
>   changed, asl diff will show the actual changes).
> When new tables are introduced:
> - Zero byte empty file blobs for new tables as instructed in the header of
>   bios-tables-test.c will be regenerated to actual table blobs.
> 
> This would make analyzing changes to tables less confusing and there would
> be no need to clean useless untracked files when there are no table changes.
> 
> CC: peter.mayd...@linaro.org
> Signed-off-by: Ani Sinha 
> ---
> tests/qtest/bios-tables-test.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
> 
> changelog:
> v2: commit description updated to make things a little clearer.
>No actual changes.

Ping ...

> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 9f4bc15aab..743b509e93 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -109,6 +109,7 @@ static const char *iasl;
> #endif
> 
> static int verbosity_level;
> +static GArray *load_expected_aml(test_data *data);
> 
> static bool compare_signature(const AcpiSdtTable *sdt, const char *signature)
> {
> @@ -241,21 +242,32 @@ static void test_acpi_fadt_table(test_data *data)
> 
> static void dump_aml_files(test_data *data, bool rebuild)
> {
> -AcpiSdtTable *sdt;
> +AcpiSdtTable *sdt, *exp_sdt;
> GError *error = NULL;
> gchar *aml_file = NULL;
> +test_data exp_data = {};
> gint fd;
> ssize_t ret;
> int i;
> 
> +exp_data.tables = load_expected_aml(data);
> for (i = 0; i < data->tables->len; ++i) {
> const char *ext = data->variant ? data->variant : "";
> sdt = _array_index(data->tables, AcpiSdtTable, i);
> +exp_sdt = _array_index(exp_data.tables, AcpiSdtTable, i);
> g_assert(sdt->aml);
> +g_assert(exp_sdt->aml);
> 
> if (rebuild) {
> aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, 
> data->machine,
>sdt->aml, ext);
> +if (!g_file_test(aml_file, G_FILE_TEST_EXISTS) &&
> +sdt->aml_len == exp_sdt->aml_len &&
> +!memcmp(sdt->aml, exp_sdt->aml, sdt->aml_len)) {
> +/* identical tables, no need to write new files */
> +g_free(aml_file);
> +continue;
> +}
> fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
> S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
> if (fd < 0) {
> -- 
> 2.42.0
> 

Re: [PATCH v3 10/70] kvm: handle KVM_EXIT_MEMORY_FAULT

[Xiaoyao Li]

On 11/15/2023 6:42 PM, Daniel P. Berrangé wrote:

On Wed, Nov 15, 2023 at 02:14:19AM -0500, Xiaoyao Li wrote:

From: Chao Peng 

Currently only KVM_MEMORY_EXIT_FLAG_PRIVATE in flags is valid when
KVM_EXIT_MEMORY_FAULT happens. It indicates userspace needs to do
the memory conversion on the RAMBlock to turn the memory into desired
attribute, i.e., private/shared.

Note, KVM_EXIT_MEMORY_FAULT makes sense only when the RAMBlock has
guest_memfd memory backend.

Note, KVM_EXIT_MEMORY_FAULT returns with -EFAULT, so special handling is
added.

Signed-off-by: Chao Peng 
Co-developed-by: Xiaoyao Li 
Signed-off-by: Xiaoyao Li 
---
  accel/kvm/kvm-all.c | 76 +++--
  1 file changed, 66 insertions(+), 10 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 76e2404d54d2..58abbcb6926e 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2902,6 +2902,50 @@ static void kvm_eat_signals(CPUState *cpu)
  } while (sigismember(, SIG_IPI));
  }
  
+static int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)

+{
+MemoryRegionSection section;
+ram_addr_t offset;
+RAMBlock *rb;
+void *addr;
+int ret = -1;
+
+section = memory_region_find(get_system_memory(), start, size);
+if (!section.mr) {
+return ret;
+}
+
+if (memory_region_has_guest_memfd(section.mr)) {
+if (to_private) {
+ret = kvm_set_memory_attributes_private(start, size);
+} else {
+ret = kvm_set_memory_attributes_shared(start, size);
+}
+
+if (ret) {
+memory_region_unref(section.mr);
+return ret;
+}
+
+addr = memory_region_get_ram_ptr(section.mr) +
+   section.offset_within_region;
+rb = qemu_ram_block_from_host(addr, false, );
+/*
+ * With KVM_SET_MEMORY_ATTRIBUTES by kvm_set_memory_attributes(),
+ * operation on underlying file descriptor is only for releasing
+ * unnecessary pages.
+ */
+ram_block_convert_range(rb, offset, size, to_private);
+} else {
+warn_report("Convert non guest_memfd backed memory region "
+"(0x%"HWADDR_PRIx" ,+ 0x%"HWADDR_PRIx") to %s",
+start, size, to_private ? "private" : "shared");


Again, if you're returning '-1' to indicate error, then
using warn_report is wrong, it should be error_report.

warn_report is for when you return success, indicating
the problem was non-fatal.


Learned.

Thanks!


+}
+
+memory_region_unref(section.mr);
+return ret;
+}
+
  int kvm_cpu_exec(CPUState *cpu)
  {
  struct kvm_run *run = cpu->kvm_run;
@@ -2969,18 +3013,20 @@ int kvm_cpu_exec(CPUState *cpu)
  ret = EXCP_INTERRUPT;
  break;
  }
-fprintf(stderr, "error: kvm run failed %s\n",
-strerror(-run_ret));
+if (!(run_ret == -EFAULT && run->exit_reason == 
KVM_EXIT_MEMORY_FAULT)) {
+fprintf(stderr, "error: kvm run failed %s\n",
+strerror(-run_ret));
  #ifdef TARGET_PPC
-if (run_ret == -EBUSY) {
-fprintf(stderr,
-"This is probably because your SMT is enabled.\n"
-"VCPU can only run on primary threads with all "
-"secondary threads offline.\n");
-}
+if (run_ret == -EBUSY) {
+fprintf(stderr,
+"This is probably because your SMT is enabled.\n"
+"VCPU can only run on primary threads with all "
+"secondary threads offline.\n");
+}
  #endif
-ret = -1;
-break;
+ret = -1;
+break;
+}
  }
  
  trace_kvm_run_exit(cpu->cpu_index, run->exit_reason);

@@ -3067,6 +3113,16 @@ int kvm_cpu_exec(CPUState *cpu)
  break;
  }
  break;
+case KVM_EXIT_MEMORY_FAULT:
+if (run->memory_fault.flags & ~KVM_MEMORY_EXIT_FLAG_PRIVATE) {
+error_report("KVM_EXIT_MEMORY_FAULT: Unknown flag 0x%" PRIx64,
+ (uint64_t)run->memory_fault.flags);
+ret = -1;
+break;
+}
+ret = kvm_convert_memory(run->memory_fault.gpa, 
run->memory_fault.size,
+ run->memory_fault.flags & 
KVM_MEMORY_EXIT_FLAG_PRIVATE);
+break;
  default:
  DPRINTF("kvm_arch_handle_exit\n");
  ret = kvm_arch_handle_exit(cpu, run);
--
2.34.1



With regards,
Daniel

Re: [PATCH v6 11/21] virtio-net: Return an error when vhost cannot enable RSS

[Jason Wang]

On Wed, Nov 15, 2023 at 2:09 PM Akihiko Odaki  wrote:
>
>
>
> On 2023/11/15 7:09, Yuri Benditovich wrote:
> >
> >
> > On Tue, Nov 14, 2023 at 9:03 AM Akihiko Odaki  > > wrote:
> >
> > On 2023/11/14 2:26, Yuri Benditovich wrote:
> >  >
> >  >
> >  > On Mon, Nov 13, 2023 at 2:44 PM Akihiko Odaki
> > mailto:akihiko.od...@daynix.com>
> >  >  > >> wrote:
> >  >
> >  > On 2023/11/13 20:44, Yuri Benditovich wrote:
> >  >  >
> >  >  >
> >  >  > On Sat, Nov 11, 2023 at 5:28 PM Akihiko Odaki
> >  > mailto:akihiko.od...@daynix.com>
> > >
> >  >  >  > 
> >  >  >  >  >  >
> >  >  > On 2023/11/03 22:14, Yuri Benditovich wrote:
> >  >  >  >
> >  >  >  >
> >  >  >  > On Fri, Nov 3, 2023 at 11:55 AM Akihiko Odaki
> >  >  >  >   > >
> >  >  >   > >>
> >  >  >  >  > 
> >  >  > >
> >  >  >  > 
> >  >  >  wrote:
> >  >  >  >
> >  >  >  > On 2023/11/03 18:35, Yuri Benditovich wrote:
> >  >  >  >  >
> >  >  >  >  >
> >  >  >  >  > On Thu, Nov 2, 2023 at 4:56 PM Akihiko Odaki
> >  >  >  >  > 
> >  >  > >  > 
> >  >  > >>
> >  >  >  > 
> >  >  > >  > 
> >  >  >  >  >  >  >  >  > 
> >  >  > >
> >  >  >  > 
> >  >  > >>
> >  >  >  >  > 
> >  >  > >
> >  >  >  > 
> >  >  > > wrote:
> >  >  >  >  >
> >  >  >  >  > On 2023/11/02 19:20, Yuri Benditovich wrote:
> >  >  >  >  >  >
> >  >  >  >  >  >
> >  >  >  >  >  > On Thu, Nov 2, 2023 at 11:33 AM
> > Michael S.
> >  > Tsirkin
> >  >  >  >  > mailto:m...@redhat.com>
> > >
> >  > 
> > >>
> >  >  > 
> > >
> >  > 
> >  >  >  >  > 
> > >
> >  > 
> > >>
> >  >  > 
> > >
> >  > 
> > 
> >  >  >  >  >  >  > 
> >  > 

RE: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object

[Duan, Zhenzhong]

Hi Eric,

>-Original Message-
>From: Eric Auger 
>Sent: Wednesday, November 15, 2023 8:53 PM
>Subject: Re: [PATCH v6 01/21] backends/iommufd: Introduce the iommufd object
>
>Hi Zhenzhong,
>
>On 11/14/23 11:09, Zhenzhong Duan wrote:
>> From: Eric Auger 
>>
>> Introduce an iommufd object which allows the interaction
>> with the host /dev/iommu device.
>>
>> The /dev/iommu can have been already pre-opened outside of qemu,
>> in which case the fd can be passed directly along with the
>> iommufd object:
>>
>> This allows the iommufd object to be shared accross several
>> subsystems (VFIO, VDPA, ...). For example, libvirt would open
>> the /dev/iommu once.
>>
>> If no fd is passed along with the iommufd object, the /dev/iommu
>> is opened by the qemu code.
>>
>> Suggested-by: Alex Williamson 
>> Signed-off-by: Eric Auger 
>> Signed-off-by: Yi Liu 
>> Signed-off-by: Zhenzhong Duan 
>> ---
>> v6: remove redundant call, alloc_hwpt, get/put_ioas
>>
>>  MAINTAINERS  |   7 ++
>>  qapi/qom.json|  19 
>>  include/sysemu/iommufd.h |  44 
>>  backends/iommufd.c   | 228 +++
>>  backends/Kconfig |   4 +
>>  backends/meson.build |   1 +
>>  backends/trace-events|  10 ++
>>  qemu-options.hx  |  12 +++
>>  8 files changed, 325 insertions(+)
>>  create mode 100644 include/sysemu/iommufd.h
>>  create mode 100644 backends/iommufd.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index ff1238bb98..a4891f7bda 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2166,6 +2166,13 @@ F: hw/vfio/ap.c
>>  F: docs/system/s390x/vfio-ap.rst
>>  L: qemu-s3...@nongnu.org
>>
>> +iommufd
>> +M: Yi Liu 
>> +M: Eric Auger 
>Zhenzhong, don't you want to be added here?
>> +S: Supported
>> +F: backends/iommufd.c
>> +F: include/sysemu/iommufd.h
>> +
>>  vhost
>>  M: Michael S. Tsirkin 
>>  S: Supported
>> diff --git a/qapi/qom.json b/qapi/qom.json
>> index c53ef978ff..1fd8555a75 100644
>> --- a/qapi/qom.json
>> +++ b/qapi/qom.json
>> @@ -794,6 +794,23 @@
>>  { 'struct': 'VfioUserServerProperties',
>>'data': { 'socket': 'SocketAddress', 'device': 'str' } }
>>
>> +##
>> +# @IOMMUFDProperties:
>> +#
>> +# Properties for iommufd objects.
>> +#
>> +# @fd: file descriptor name previously passed via 'getfd' command,
>
>"previously passed via 'getfd' command", I wonder if this applies here or 
>whether
>it is copy/paste of
>RemoteObjectProperties.fd doc?

Maybe copied I thinks this applies here because I use monitor_fd_param to get 
fd.
Or I miss anything?

>
>> +# which represents a pre-opened /dev/iommu.  This allows the
>> +# iommufd object to be shared accross several subsystems
>> +# (VFIO, VDPA, ...), and the file descriptor to be shared
>> +# with other process, e.g. DPDK.  (default: QEMU opens
>> +# /dev/iommu by itself)
>> +#
>> +# Since: 8.2
>> +##
>> +{ 'struct': 'IOMMUFDProperties',
>> +  'data': { '*fd': 'str' } }
>> +
>>  ##
>>  # @RngProperties:
>>  #
>> @@ -934,6 +951,7 @@
>>  'input-barrier',
>>  { 'name': 'input-linux',
>>'if': 'CONFIG_LINUX' },
>> +'iommufd',
>>  'iothread',
>>  'main-loop',
>>  { 'name': 'memory-backend-epc',
>> @@ -1003,6 +1021,7 @@
>>'input-barrier':  'InputBarrierProperties',
>>'input-linux':{ 'type': 'InputLinuxProperties',
>>'if': 'CONFIG_LINUX' },
>> +  'iommufd':'IOMMUFDProperties',
>>'iothread':   'IothreadProperties',
>>'main-loop':  'MainLoopProperties',
>>'memory-backend-epc': { 'type': 'MemoryBackendEpcProperties',
>> diff --git a/include/sysemu/iommufd.h b/include/sysemu/iommufd.h
>> new file mode 100644
>> index 00..9b3a86f57d
>> --- /dev/null
>> +++ b/include/sysemu/iommufd.h
>> @@ -0,0 +1,44 @@
>> +#ifndef SYSEMU_IOMMUFD_H
>> +#define SYSEMU_IOMMUFD_H
>> +
>> +#include "qom/object.h"
>> +#include "qemu/thread.h"
>> +#include "exec/hwaddr.h"
>> +#include "exec/cpu-common.h"
>> +
>> +#define TYPE_IOMMUFD_BACKEND "iommufd"
>> +OBJECT_DECLARE_TYPE(IOMMUFDBackend, IOMMUFDBackendClass,
>> +IOMMUFD_BACKEND)
>> +#define IOMMUFD_BACKEND(obj) \
>> +OBJECT_CHECK(IOMMUFDBackend, (obj), TYPE_IOMMUFD_BACKEND)
>> +#define IOMMUFD_BACKEND_GET_CLASS(obj) \
>> +OBJECT_GET_CLASS(IOMMUFDBackendClass, (obj),
>TYPE_IOMMUFD_BACKEND)
>> +#define IOMMUFD_BACKEND_CLASS(klass) \
>> +OBJECT_CLASS_CHECK(IOMMUFDBackendClass, (klass),
>TYPE_IOMMUFD_BACKEND)
>> +struct IOMMUFDBackendClass {
>> +ObjectClass parent_class;
>> +};
>> +
>> +struct IOMMUFDBackend {
>> +Object parent;
>> +
>> +/*< protected >*/
>> +int fd;/* /dev/iommu file descriptor */
>> +bool owned;/* is the /dev/iommu opened internally */
>> +QemuMutex lock;
>> +uint32_t users;
>> +
>> +/*< public >*/
>> +};
>> +
>> +int 

Re: [PATCH] tests/avocado/reverse_debugging: Disable the ppc64 tests by default

[Ani Sinha]

> On 16-Nov-2023, at 6:45 AM, Nicholas Piggin  wrote:
> 
> On Thu Nov 16, 2023 at 3:22 AM AEST, Daniel P. Berrangé wrote:
>> On Wed, Nov 15, 2023 at 01:14:53PM +, Daniel P. Berrangé wrote:
>>> On Wed, Nov 15, 2023 at 07:23:01AM +0100, Thomas Huth wrote:
 On 15/11/2023 02.15, Nicholas Piggin wrote:
> On Wed Nov 15, 2023 at 4:29 AM AEST, Thomas Huth wrote:
>> On 14/11/2023 17.37, Philippe Mathieu-Daudé wrote:
>>> On 14/11/23 17:31, Thomas Huth wrote:
 The tests seem currently to be broken. Disable them by default
 until someone fixes them.
 
 Signed-off-by: Thomas Huth 
 ---
   tests/avocado/reverse_debugging.py | 7 ---
   1 file changed, 4 insertions(+), 3 deletions(-)
>>> 
>>> Similarly, I suspect https://gitlab.com/qemu-project/qemu/-/issues/1961
>>> which has a fix ready:
>>> https://lore.kernel.org/qemu-devel/20231110170831.185001-1-richard.hender...@linaro.org/
>>> 
>>> Maybe wait the fix gets in first?
>> 
>> No, I applied Richard's patch, but the problem persists. Does this test
>> still work for you?
> 
> I bisected it to 1d4796cd008373 ("python/machine: use socketpair() for
> console connections"),
 
 Maybe John (who wrote that commit) can help?
>>> 
>>> I find it hard to believe this commit is a direct root cause of the
>>> problem since all it does is change the QEMU startup sequence so that
>>> instead of QEMU listening for a monitor connection, it is given a
>>> pre-opened monitor connection.
>>> 
>>> At the very most that should affect the startup timing a little.
>>> 
>>> I notice all the reverse debugging tests have a skip on gitlab
>>> with a comment:
>>> 
>>># unidentified gitlab timeout problem
>>> 
>>> this makes be suspicious that John's patch has merely made this
>>> (henceforth undiagnosed) timeout more likely to ocurr.
>> 
>> After an absolutely horrendous hours long debugging session I think
>> I figured out the problem. The QEMU process is blocking in
>> 
>>qemu_chr_write_buffer
>> 
>> spinning in the loop on EAGAIN.
> 
> Great work.
> 
> Why does this make the gdb socket give an empty response? Something
> just times out?
> 
>> 
>> The Python  Machine() class has passed one of a pre-created socketpair
>> FDs for the serial port chardev. The guest is trying to write to this
>> and blocking.  Nothing in the Machine() class is reading from the
>> other end of the serial port console.
>> 
>> 
>> Before John's change, the serial port uses a chardev in server mode
>> and crucially  'wait=off', and the Machine() class never opened the
>> console socket unless the test case wanted to read from it.
>> 
>> IOW, QEMU had a background job setting there waiting for a connection
>> that would never come.
>> 
>> As a result when QEMU started executing the guest, all the serial port
>> writes get sent into to the void.
>> 
>> 
>> So John's patch has had a semantic change in behaviour, because the
>> console socket is permanently open, and thus socket buffers are liable
>> to fill up.
>> 
>> As a demo I increased the socket buffers to 1MB and everything then
>> succeeded.
>> 
>> @@ -357,6 +360,10 @@ def _pre_launch(self) -> None:
>> 
>> if self._console_set:
>> self._cons_sock_pair = socket.socketpair()
>> +self._cons_sock_pair[0].setsockopt(socket.SOL_SOCKET, 
>> socket.SO_SNDBUF, 1024*1024);
>> +self._cons_sock_pair[0].setsockopt(socket.SOL_SOCKET, 
>> socket.SO_RCVBUF, 1024*1024);
>> +self._cons_sock_pair[1].setsockopt(socket.SOL_SOCKET, 
>> socket.SO_SNDBUF, 1024*1024);
>> +self._cons_sock_pair[1].setsockopt(socket.SOL_SOCKET, 
>> socket.SO_RCVBUF, 1024*1024);
>> os.set_inheritable(self._cons_sock_pair[0].fileno(), True)
>> 
>> # NOTE: Make sure any opened resources are *definitely* freed in
> 
> So perhaps ppc64 fails just because it prints more to the console in early
> boot than other targets?
> 
>> The Machine class doesn't know if anything will ever use the console,
>> so as is the change is unsafe.
>> 
>> The original goal of John's change was to guarantee we capture early
>> boot messages as some test need that.  
>> 
>> I think we need to be able to have a flag to say whether the caller needs
>> an "early console" facility, and only use the pre-opened FD passing for
>> that case. Tests we need early console will have to ask for that guarantee
>> explicitly.
> 
> The below patch makes this test work. Maybe as a quick fix it is
> better than disabling the test.
> 
> I guess we still have a problem if a test invokes vm.launch()
> directly without subsequently waiting for a console pattern or
> doing something with the console as you say. Your suggesstion is
> add something like vm.launch(console=True) ? 

I think what he is saying is to add a new property for QEMUMachine() with which 
the test can explicitly tell the machine init code that it is going to 

Re: [PATCH] tests/avocado/reverse_debugging: Disable the ppc64 tests by default

[Ani Sinha]

> On 15-Nov-2023, at 10:52 PM, Daniel P. Berrangé  wrote:
> 
> On Wed, Nov 15, 2023 at 01:14:53PM +, Daniel P. Berrangé wrote:
>> On Wed, Nov 15, 2023 at 07:23:01AM +0100, Thomas Huth wrote:
>>> On 15/11/2023 02.15, Nicholas Piggin wrote:
 On Wed Nov 15, 2023 at 4:29 AM AEST, Thomas Huth wrote:
> On 14/11/2023 17.37, Philippe Mathieu-Daudé wrote:
>> On 14/11/23 17:31, Thomas Huth wrote:
>>> The tests seem currently to be broken. Disable them by default
>>> until someone fixes them.
>>> 
>>> Signed-off-by: Thomas Huth 
>>> ---
>>>   tests/avocado/reverse_debugging.py | 7 ---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>> 
>> Similarly, I suspect https://gitlab.com/qemu-project/qemu/-/issues/1961
>> which has a fix ready:
>> https://lore.kernel.org/qemu-devel/20231110170831.185001-1-richard.hender...@linaro.org/
>> 
>> Maybe wait the fix gets in first?
> 
> No, I applied Richard's patch, but the problem persists. Does this test
> still work for you?
 
 I bisected it to 1d4796cd008373 ("python/machine: use socketpair() for
 console connections"),
>>> 
>>> Maybe John (who wrote that commit) can help?
>> 
>> I find it hard to believe this commit is a direct root cause of the
>> problem since all it does is change the QEMU startup sequence so that
>> instead of QEMU listening for a monitor connection, it is given a
>> pre-opened monitor connection.
>> 
>> At the very most that should affect the startup timing a little.
>> 
>> I notice all the reverse debugging tests have a skip on gitlab
>> with a comment:
>> 
>># unidentified gitlab timeout problem
>> 
>> this makes be suspicious that John's patch has merely made this
>> (henceforth undiagnosed) timeout more likely to ocurr.
> 
> After an absolutely horrendous hours long debugging session I think
> I figured out the problem. The QEMU process is blocking in
> 
>qemu_chr_write_buffer
> 
> spinning in the loop on EAGAIN.
> 
> The Python  Machine() class has passed one of a pre-created socketpair
> FDs for the serial port chardev. The guest is trying to write to this
> and blocking.  Nothing in the Machine() class is reading from the
> other end of the serial port console.
> 
> 
> Before John's change, the serial port uses a chardev in server mode
> and crucially  'wait=off', and the Machine() class never opened the
> console socket unless the test case wanted to read from it.
> 
> IOW, QEMU had a background job setting there waiting for a connection
> that would never come.
> 
> As a result when QEMU started executing the guest, all the serial port
> writes get sent into to the void.
> 
> 
> So John's patch has had a semantic change in behaviour, because the
> console socket is permanently open, and thus socket buffers are liable
> to fill up.
> 
> As a demo I increased the socket buffers to 1MB and everything then
> succeeded.
> 
> @@ -357,6 +360,10 @@ def _pre_launch(self) -> None:
> 
> if self._console_set:
> self._cons_sock_pair = socket.socketpair()
> +self._cons_sock_pair[0].setsockopt(socket.SOL_SOCKET, 
> socket.SO_SNDBUF, 1024*1024);
> +self._cons_sock_pair[0].setsockopt(socket.SOL_SOCKET, 
> socket.SO_RCVBUF, 1024*1024);
> +self._cons_sock_pair[1].setsockopt(socket.SOL_SOCKET, 
> socket.SO_SNDBUF, 1024*1024);
> +self._cons_sock_pair[1].setsockopt(socket.SOL_SOCKET, 
> socket.SO_RCVBUF, 1024*1024);
> os.set_inheritable(self._cons_sock_pair[0].fileno(), True)
> 
> # NOTE: Make sure any opened resources are *definitely* freed in
> 
> 
> The Machine class doesn't know if anything will ever use the console,
> so as is the change is unsafe.
> 
> The original goal of John's change was to guarantee we capture early
> boot messages as some test need that.

As in 
https://gitlab.com/qemu-project/qemu/-/blob/master/tests/avocado/acpi-bits.py?ref_type=heads#L395
 ?
Some other tests do this too.

>  
> 
> I think we need to be able to have a flag to say whether the caller needs
> an "early console" facility, and only use the pre-opened FD passing for
> that case. Tests we need early console will have to ask for that guarantee
> explicitly.
> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v3 06/70] kvm: Introduce support for memory_attributes

[Xiaoyao Li]

On 11/15/2023 6:38 PM, Daniel P. Berrangé wrote:

On Wed, Nov 15, 2023 at 02:14:15AM -0500, Xiaoyao Li wrote:

Introduce the helper functions to set the attributes of a range of
memory to private or shared.

This is necessary to notify KVM the private/shared attribute of each gpa
range. KVM needs the information to decide the GPA needs to be mapped at
hva-based shared memory or guest_memfd based private memory.

Signed-off-by: Xiaoyao Li 
---
  accel/kvm/kvm-all.c  | 42 ++
  include/sysemu/kvm.h |  3 +++
  2 files changed, 45 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 69afeb47c9c0..76e2404d54d2 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -102,6 +102,7 @@ bool kvm_has_guest_debug;
  static int kvm_sstep_flags;
  static bool kvm_immediate_exit;
  static bool kvm_guest_memfd_supported;
+static uint64_t kvm_supported_memory_attributes;
  static hwaddr kvm_max_slot_size = ~0;
  
  static const KVMCapabilityInfo kvm_required_capabilites[] = {

@@ -1305,6 +1306,44 @@ void kvm_set_max_memslot_size(hwaddr max_slot_size)
  kvm_max_slot_size = max_slot_size;
  }
  
+static int kvm_set_memory_attributes(hwaddr start, hwaddr size, uint64_t attr)

+{
+struct kvm_memory_attributes attrs;
+int r;
+
+attrs.attributes = attr;
+attrs.address = start;
+attrs.size = size;
+attrs.flags = 0;
+
+r = kvm_vm_ioctl(kvm_state, KVM_SET_MEMORY_ATTRIBUTES, );
+if (r) {
+warn_report("%s: failed to set memory (0x%lx+%#zx) with attr 0x%lx error 
'%s'",
+ __func__, start, size, attr, strerror(errno));


This is an error condition rather than an warning condition.

Also again I think __func__ is generally not required in an error message,
if the error message text is suitably descriptive - applies to other
patches in this series too.


Get it.


+}
+return r;
+}
+
+int kvm_set_memory_attributes_private(hwaddr start, hwaddr size)
+{
+if (!(kvm_supported_memory_attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
+error_report("KVM doesn't support PRIVATE memory attribute\n");
+return -EINVAL;
+}
+
+return kvm_set_memory_attributes(start, size, 
KVM_MEMORY_ATTRIBUTE_PRIVATE);
+}
+
+int kvm_set_memory_attributes_shared(hwaddr start, hwaddr size)
+{
+if (!(kvm_supported_memory_attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
+error_report("KVM doesn't support PRIVATE memory attribute\n");
+return -EINVAL;
+}
+
+return kvm_set_memory_attributes(start, size, 0);
+}
+
  /* Called with KVMMemoryListener.slots_lock held */
  static void kvm_set_phys_mem(KVMMemoryListener *kml,
   MemoryRegionSection *section, bool add)
@@ -2440,6 +2479,9 @@ static int kvm_init(MachineState *ms)
  
  kvm_guest_memfd_supported = kvm_check_extension(s, KVM_CAP_GUEST_MEMFD);
  
+ret = kvm_check_extension(s, KVM_CAP_MEMORY_ATTRIBUTES);

+kvm_supported_memory_attributes = ret > 0 ? ret : 0;
+
  if (object_property_find(OBJECT(current_machine), "kvm-type")) {
  g_autofree char *kvm_type = 
object_property_get_str(OBJECT(current_machine),
  "kvm-type",
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index fedc28c7d17f..0e88958190a4 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -540,4 +540,7 @@ bool kvm_dirty_ring_enabled(void);
  uint32_t kvm_dirty_ring_size(void);
  
  int kvm_create_guest_memfd(uint64_t size, uint64_t flags, Error **errp);

+
+int kvm_set_memory_attributes_private(hwaddr start, hwaddr size);
+int kvm_set_memory_attributes_shared(hwaddr start, hwaddr size);
  #endif
--
2.34.1



With regards,
Daniel

Re: [PATCH v3 02/70] RAMBlock: Add support of KVM private guest memfd

[Xiaoyao Li]

On 11/15/2023 6:20 PM, Daniel P. Berrangé wrote:

On Wed, Nov 15, 2023 at 02:14:11AM -0500, Xiaoyao Li wrote:

Add KVM guest_memfd support to RAMBlock so both normal hva based memory
and kvm guest memfd based private memory can be associated in one RAMBlock.

Introduce new flag RAM_GUEST_MEMFD. When it's set, it calls KVM ioctl to
create private guest_memfd during RAMBlock setup.

Note, RAM_GUEST_MEMFD is supposed to be set for memory backends of
confidential guests, such as TDX VM. How and when to set it for memory
backends will be implemented in the following patches.

Introduce memory_region_has_guest_memfd() to query if the MemoryRegion has
KVM guest_memfd allocated.

Signed-off-by: Xiaoyao Li 
---
Changes in v3:
- rename gmem to guest_memfd;
- close(guest_memfd) when RAMBlock is released; (Daniel P. Berrangé)
- Suqash the patch that introduces memory_region_has_guest_memfd().
---
  accel/kvm/kvm-all.c | 24 
  include/exec/memory.h   | 13 +
  include/exec/ramblock.h |  1 +
  include/sysemu/kvm.h|  2 ++
  system/memory.c |  5 +
  system/physmem.c| 27 ---
  6 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index c1b40e873531..9f751d4971f8 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -101,6 +101,7 @@ bool kvm_msi_use_devid;
  bool kvm_has_guest_debug;
  static int kvm_sstep_flags;
  static bool kvm_immediate_exit;
+static bool kvm_guest_memfd_supported;
  static hwaddr kvm_max_slot_size = ~0;
  
  static const KVMCapabilityInfo kvm_required_capabilites[] = {

@@ -2397,6 +2398,8 @@ static int kvm_init(MachineState *ms)
  }
  s->as = g_new0(struct KVMAs, s->nr_as);
  
+kvm_guest_memfd_supported = kvm_check_extension(s, KVM_CAP_GUEST_MEMFD);

+
  if (object_property_find(OBJECT(current_machine), "kvm-type")) {
  g_autofree char *kvm_type = 
object_property_get_str(OBJECT(current_machine),
  "kvm-type",
@@ -4078,3 +4081,24 @@ void query_stats_schemas_cb(StatsSchemaList **result, 
Error **errp)
  query_stats_schema_vcpu(first_cpu, _args);
  }
  }
+
+int kvm_create_guest_memfd(uint64_t size, uint64_t flags, Error **errp)
+{
+int fd;
+struct kvm_create_guest_memfd guest_memfd = {
+.size = size,
+.flags = flags,
+};
+
+if (!kvm_guest_memfd_supported) {
+error_setg(errp, "KVM doesn't support guest memfd\n");
+return -EOPNOTSUPP;


Returning an errno value is unusual when we have an 'Error **errp' parameter
for reporting, and the following codepath merely returns -1, so this is
inconsistent. Just return -1 here too.


OK.


+}
+
+fd = kvm_vm_ioctl(kvm_state, KVM_CREATE_GUEST_MEMFD, _memfd);
+if (fd < 0) {
+error_setg_errno(errp, errno, "%s: error creating kvm guest memfd\n", 
__func__);


I'd prefer an explicit 'return -1' here, even though 'fd' is technically going
to be -1 already.

Also including __func__ in the error message is not really needed IMHO


OK


+}
+
+return fd;
+}
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 831f7c996d9d..f780367ab1bd 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -243,6 +243,9 @@ typedef struct IOMMUTLBEvent {
  /* RAM FD is opened read-only */
  #define RAM_READONLY_FD (1 << 11)
  
+/* RAM can be private that has kvm gmem backend */

+#define RAM_GUEST_MEMFD   (1 << 12)
+
  static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
 IOMMUNotifierFlag flags,
 hwaddr start, hwaddr end,
@@ -1702,6 +1705,16 @@ static inline bool memory_region_is_romd(MemoryRegion 
*mr)
   */
  bool memory_region_is_protected(MemoryRegion *mr);
  
+/**

+ * memory_region_has_guest_memfd: check whether a memory region has guest_memfd
+ * associated
+ *
+ * Returns %true if a memory region's ram_block has valid guest_memfd assigned.
+ *
+ * @mr: the memory region being queried
+ */
+bool memory_region_has_guest_memfd(MemoryRegion *mr);
+
  /**
   * memory_region_get_iommu: check whether a memory region is an iommu
   *
diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
index 69c6a5390293..0a17ba882729 100644
--- a/include/exec/ramblock.h
+++ b/include/exec/ramblock.h
@@ -41,6 +41,7 @@ struct RAMBlock {
  QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;
  int fd;
  uint64_t fd_offset;
+int guest_memfd;
  size_t page_size;
  /* dirty bitmap used during migration */
  unsigned long *bmap;
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index d61487816421..fedc28c7d17f 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -538,4 +538,6 @@ bool kvm_arch_cpu_check_are_resettable(void);
  bool kvm_dirty_ring_enabled(void);
  
  uint32_t kvm_dirty_ring_size(void);

+
+int 

Re: [PATCH v3 07/70] physmem: Relax the alignment check of host_startaddr in ram_block_discard_range()

[Xiaoyao Li]

On 11/16/2023 2:20 AM, David Hildenbrand wrote:

On 15.11.23 08:14, Xiaoyao Li wrote:

Commit d3a5038c461 ("exec: ram_block_discard_range") introduced
ram_block_discard_range() which grabs some code from
ram_discard_range(). However, during code movement, it changed alignment
check of host_startaddr from qemu_host_page_size to rb->page_size.

When ramblock is back'ed by hugepage, it requires the startaddr to be
huge page size aligned, which is a overkill. e.g., TDX's private-shared
page conversion is done at 4KB granularity. Shared page is discarded
when it gets converts to private and when shared page back'ed by
hugepage it is going to fail on this check.

So change to alignment check back to qemu_host_page_size.

Signed-off-by: Xiaoyao Li 
---
Changes in v3:
  - Newly added in v3;
---
  system/physmem.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/system/physmem.c b/system/physmem.c
index c56b17e44df6..8a4e42c7cf60 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3532,7 +3532,7 @@ int ram_block_discard_range(RAMBlock *rb, 
uint64_t start, size_t length)

  uint8_t *host_startaddr = rb->host + start;
-    if (!QEMU_PTR_IS_ALIGNED(host_startaddr, rb->page_size)) {
+    if (!QEMU_PTR_IS_ALIGNED(host_startaddr, qemu_host_page_size)) {


For your use cases, rb->page_size should always match qemu_host_page_size.

IIRC, we only set rb->page_size to different values for hugetlb. And 
guest_memfd does not support hugetlb.


Even if QEMU is using THP, rb->page_size should 4k.

Please elaborate how you can actually trigger that. From what I recall, 
guest_memfd is not compatible with hugetlb.


It's the shared memory that can be back'ed by hugetlb.

Later patch 9 introduces ram_block_convert_page(), which will discard 
shared memory when it gets converted to private. TD guest can request 
convert a 4K to private while the page is previously back'ed by hugetlb 
as 2M shared page.


And the check here makes perfect sense for existing callers of 
ram_block_discard_range(): you cannot partially zap a hugetlb page.

Re: [PATCH v3 04/70] HostMem: Add mechanism to opt in kvm guest memfd via MachineState

[Xiaoyao Li]

On 11/16/2023 2:14 AM, David Hildenbrand wrote:

On 15.11.23 08:14, Xiaoyao Li wrote:

Add a new member "require_guest_memfd" to memory backends. When it's set
to true, it enables RAM_GUEST_MEMFD in ram_flags, thus private kvm
guest_memfd will be allocated during RAMBlock allocation.

Memory backend's @require_guest_memfd is wired with @require_guest_memfd
field of MachineState. MachineState::require_guest_memfd is supposed to
be set by any VMs that requires KVM guest memfd as private memory, e.g.,
TDX VM.

Signed-off-by: Xiaoyao Li 


I'm confused, why do we need this if it's going to be the same for all 
memory backends right now?




I want to provide a elegant (in my sense) way to configure "the need of 
guest memfd" instead of checking x86machinestate->vm_type in physmem.c

Re: [PATCH v3 03/70] RAMBlock/guest_memfd: Enable KVM_GUEST_MEMFD_ALLOW_HUGEPAGE

[Xiaoyao Li]

On 11/16/2023 2:10 AM, David Hildenbrand wrote:

On 15.11.23 08:14, Xiaoyao Li wrote:

KVM allows KVM_GUEST_MEMFD_ALLOW_HUGEPAGE for guest memfd. When the
flag is set, KVM tries to allocate memory with transparent hugeapge at
first and falls back to non-hugepage on failure.

However, KVM defines one restriction that size must be hugepage size
aligned when KVM_GUEST_MEMFD_ALLOW_HUGEPAGE is set.

Signed-off-by: Xiaoyao Li 
---
v3:
  - New one in v3.
---
  system/physmem.c | 38 --
  1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/system/physmem.c b/system/physmem.c
index 0af2213cbd9c..c56b17e44df6 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -1803,6 +1803,40 @@ static void dirty_memory_extend(ram_addr_t 
old_ram_size,

  }
  }
+#ifdef CONFIG_KVM
+#define HPAGE_PMD_SIZE_PATH 
"/sys/kernel/mm/transparent_hugepage/hpage_pmd_size"

+#define DEFAULT_PMD_SIZE (1ul << 21)
+
+static uint32_t get_thp_size(void)
+{
+    gchar *content = NULL;
+    const char *endptr;
+    static uint64_t thp_size = 0;
+    uint64_t tmp;
+
+    if (thp_size != 0) {
+    return thp_size;
+    }
+
+    if (g_file_get_contents(HPAGE_PMD_SIZE_PATH, , NULL, 
NULL) &&

+    !qemu_strtou64(content, , 0, ) &&
+    (!endptr || *endptr == '\n')) {
+    /* Sanity-check the value and fallback to something 
reasonable. */

+    if (!tmp || !is_power_of_2(tmp)) {
+    warn_report("Read unsupported THP size: %" PRIx64, tmp);
+    } else {
+    thp_size = tmp;
+    }
+    }
+
+    if (!thp_size) {
+    thp_size = DEFAULT_PMD_SIZE;
+    }


... did you shamelessly copy that from hw/virtio/virtio-mem.c ? ;)


Get caught.


This should be factored out into a common helper.


Sure, will do it in next version.

Re: [PATCH v3 02/70] RAMBlock: Add support of KVM private guest memfd

[Xiaoyao Li]

On 11/16/2023 1:54 AM, David Hildenbrand wrote:

On 15.11.23 08:14, Xiaoyao Li wrote:

Add KVM guest_memfd support to RAMBlock so both normal hva based memory
and kvm guest memfd based private memory can be associated in one 
RAMBlock.


Introduce new flag RAM_GUEST_MEMFD. When it's set, it calls KVM ioctl to
create private guest_memfd during RAMBlock setup.

Note, RAM_GUEST_MEMFD is supposed to be set for memory backends of
confidential guests, such as TDX VM. How and when to set it for memory
backends will be implemented in the following patches.


Can you elaborate (and add to the patch description if there is good 
reason) why we need that flag and why we cannot simply rely on the VM 
type instead to decide whether to allocate a guest_memfd or not?




The reason is, relying on the VM type is sort of hack that we need to 
get the MachineState instance and retrieve the vm type info. I think 
it's better not to couple them.


More importantly, it's not flexible and extensible for future case that 
not all the memory need guest memfd.

RE: [PATCH 1/4] vfio/pci: Move VFIODevice initializations in vfio_instance_init

[Duan, Zhenzhong]

>-Original Message-
>From: Cédric Le Goater 
>Sent: Wednesday, November 15, 2023 9:12 PM
>Subject: Re: [PATCH 1/4] vfio/pci: Move VFIODevice initializations in
>vfio_instance_init
>
>On 11/15/23 09:32, Zhenzhong Duan wrote:
>> Some of the VFIODevice initializations is in vfio_realize,
>> move all of them in vfio_instance_init.
>>
>> No functional change intended.
>>
>> Suggested-by: Cédric Le Goater 
>> Signed-off-by: Zhenzhong Duan 
>> ---
>>   hw/vfio/pci.c | 10 ++
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index b23b492cce..5a2b7a2d6b 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -2969,9 +2969,6 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>>   if (vfio_device_get_name(vbasedev, errp)) {
>>   return;
>>   }
>> -vbasedev->ops = _pci_ops;
>> -vbasedev->type = VFIO_DEVICE_TYPE_PCI;
>> -vbasedev->dev = DEVICE(vdev);
>>
>>   /*
>>* Mediated devices *might* operate compatibly with discarding of RAM,
>but
>> @@ -3320,6 +3317,7 @@ static void vfio_instance_init(Object *obj)
>>   {
>>   PCIDevice *pci_dev = PCI_DEVICE(obj);
>>   VFIOPCIDevice *vdev = VFIO_PCI(obj);
>> +VFIODevice *vbasedev = >vbasedev;
>>
>>   device_add_bootindex_property(obj, >bootindex,
>> "bootindex", NULL,
>> @@ -3328,7 +3326,11 @@ static void vfio_instance_init(Object *obj)
>>   vdev->host.bus = ~0U;
>>   vdev->host.slot = ~0U;
>>   vdev->host.function = ~0U;
>> -vdev->vbasedev.fd = -1;
>> +
>> +vbasedev->type = VFIO_DEVICE_TYPE_PCI;
>> +vbasedev->ops = _pci_ops;
>> +vbasedev->dev = DEVICE(vdev);
>> +vbasedev->fd = -1;
>
>VFIODevice is similar to a base QOM parent. Could we introduce an helper
>routine like we did with vfio_device_set_fd() ?

Sure, will do.

Thanks
Zhenzhong

RE: [PATCH] tests/qtest: remove unused variables

[Zhang, Chen]

> -Original Message-
> From: qemu-devel-bounces+chen.zhang=intel@nongnu.org  devel-bounces+chen.zhang=intel@nongnu.org> On Behalf Of zhujun2
> Sent: Wednesday, November 15, 2023 4:00 PM
> To: th...@redhat.com
> Cc: lviv...@redhat.com; pbonz...@redhat.com; qemu-devel@nongnu.org;
> zhuj...@cmss.chinamobile.com
> Subject: [PATCH] tests/qtest: remove unused variables
> 
> These variables are never referenced in the code, just remove them
> 
> Signed-off-by: zhujun2 
> ---
>  tests/qtest/test-filter-mirror.c | 2 +-
>  tests/qtest/test-filter-redirector.c | 4 ++--
>  tests/qtest/virtio-net-test.c| 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qtest/test-filter-mirror.c 
> b/tests/qtest/test-filter-mirror.c
> index adeada3eb8..7aa81daa93 100644
> --- a/tests/qtest/test-filter-mirror.c
> +++ b/tests/qtest/test-filter-mirror.c
> @@ -60,7 +60,7 @@ static void test_mirror(void)
> 
>  g_assert_cmpint(len, ==, sizeof(send_buf));
>  recv_buf = g_malloc(len);
> -ret = recv(recv_sock[0], recv_buf, len, 0);
> +recv(recv_sock[0], recv_buf, len, 0);
>  g_assert_cmpstr(recv_buf, ==, send_buf);
> 
>  g_free(recv_buf);
> diff --git a/tests/qtest/test-filter-redirector.c b/tests/qtest/test-filter-
> redirector.c
> index e72e3b7873..e4dfeff2e0 100644
> --- a/tests/qtest/test-filter-redirector.c
> +++ b/tests/qtest/test-filter-redirector.c
> @@ -117,7 +117,7 @@ static void test_redirector_tx(void)
> 
>  g_assert_cmpint(len, ==, sizeof(send_buf));
>  recv_buf = g_malloc(len);
> -ret = recv(recv_sock, recv_buf, len, 0);
> +recv(recv_sock, recv_buf, len, 0);
>  g_assert_cmpstr(recv_buf, ==, send_buf);
> 
>  g_free(recv_buf);
> @@ -184,7 +184,7 @@ static void test_redirector_rx(void)
> 
>  g_assert_cmpint(len, ==, sizeof(send_buf));
>  recv_buf = g_malloc(len);
> -ret = recv(backend_sock[0], recv_buf, len, 0);
> +recv(backend_sock[0], recv_buf, len, 0);

It looks like add more check for the "ret" is better.
For example:
g_assert_cmpint(ret, ==, len);

Thanks
Chen

>  g_assert_cmpstr(recv_buf, ==, send_buf);
> 
>  close(send_sock);
> diff --git a/tests/qtest/virtio-net-test.c b/tests/qtest/virtio-net-test.c 
> index
> fab5dd8b05..26df5bbabe 100644
> --- a/tests/qtest/virtio-net-test.c
> +++ b/tests/qtest/virtio-net-test.c
> @@ -90,7 +90,7 @@ static void tx_test(QVirtioDevice *dev,
>  g_assert_cmpint(ret, ==, sizeof(len));
>  len = ntohl(len);
> 
> -ret = recv(socket, buffer, len, 0);
> +recv(socket, buffer, len, 0);
>  g_assert_cmpstr(buffer, ==, "TEST");  }
> 
> --
> 2.17.1
> 
> 
> 

RE: [PATCH v6 11/21] vfio/pci: Make vfio cdev pre-openable by passing a file handle

[Duan, Zhenzhong]

Hi Philippe,

>-Original Message-
>From: Cédric Le Goater 
>Sent: Wednesday, November 15, 2023 9:05 PM
>Subject: Re: [PATCH v6 11/21] vfio/pci: Make vfio cdev pre-openable by passing 
>a
>file handle
>
>On 11/15/23 13:09, Philippe Mathieu-Daudé wrote:
>> Hi Zhenzhong,
>>
>> On 14/11/23 11:09, Zhenzhong Duan wrote:
>>> This gives management tools like libvirt a chance to open the vfio
>>> cdev with privilege and pass FD to qemu. This way qemu never needs
>>> to have privilege to open a VFIO or iommu cdev node.
>>>
>>> Together with the earlier support of pre-opening /dev/iommu device,
>>> now we have full support of passing a vfio device to unprivileged
>>> qemu by management tool. This mode is no more considered for the
>>> legacy backend. So let's remove the "TODO" comment.
>>>
>>> Add helper functions vfio_device_set_fd() and vfio_device_get_name()
>>> to set fd and get device name, they will also be used by other vfio
>>> devices.
>>>
>>> There is no easy way to check if a device is mdev with FD passing,
>>> so fail the x-balloon-allowed check unconditionally in this case.
>>>
>>> There is also no easy way to get BDF as name with FD passing, so
>>> we fake a name by VFIO_FD[fd].
>>>
>>> Signed-off-by: Zhenzhong Duan 
>>> ---
>>> v6: simplify CONFIG_IOMMUFD checking code
>>>  introduce a helper vfio_device_set_fd
>>>
>>>   include/hw/vfio/vfio-common.h |  3 +++
>>>   hw/vfio/helpers.c | 44 +++
>>>   hw/vfio/iommufd.c | 12 ++
>>>   hw/vfio/pci.c | 28 --
>>>   4 files changed, 71 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>> index 3dac5c167e..567e5f7bea 100644
>>> --- a/include/hw/vfio/vfio-common.h
>>> +++ b/include/hw/vfio/vfio-common.h
>>> @@ -251,4 +251,7 @@ int
>vfio_devices_query_dirty_bitmap(VFIOContainerBase *bcontainer,
>>>   hwaddr size);
>>>   int vfio_get_dirty_bitmap(VFIOContainerBase *bcontainer, uint64_t iova,
>>>    uint64_t size, ram_addr_t ram_addr);
>>> +
>>
>> Please add bare documentation:
>>
>>    /* Returns 0 on success, or a negative errno. */
>>
>>> +int vfio_device_get_name(VFIODevice *vbasedev, Error **errp);

Will do, I'd like to wait a few days to collect more suggested changes and RB,
Then send all these updates to Cédric in once before he pushes this series to 
vfio-next.

>>
>> Functions taking an Error** param should return a boolean, so:
>>
>>    /* Return: true on success, else false setting @errp with error. */
>>
>>> +void vfio_device_set_fd(VFIODevice *vbasedev, const char *str, Error 
>>> **errp);
>>>   #endif /* HW_VFIO_VFIO_COMMON_H */
>>
>>
>>> @@ -609,3 +611,45 @@ bool vfio_has_region_cap(VFIODevice *vbasedev, int
>region, uint16_t cap_type)
>>>   return ret;
>>>   }
>>> +
>>> +int vfio_device_get_name(VFIODevice *vbasedev, Error **errp)
>>> +{
>>> +    struct stat st;
>>> +
>>> +    if (vbasedev->fd < 0) {
>>> +    if (stat(vbasedev->sysfsdev, ) < 0) {
>>> +    error_setg_errno(errp, errno, "no such host device");
>>> +    error_prepend(errp, VFIO_MSG_PREFIX, vbasedev->sysfsdev);
>>> +    return -errno;
>>> +    }
>>> +    /* User may specify a name, e.g: VFIO platform device */
>>> +    if (!vbasedev->name) {
>>> +    vbasedev->name = g_path_get_basename(vbasedev->sysfsdev);
>>> +    }
>>> +    } else {
>>> +    if (!vbasedev->iommufd) {
>>> +    error_setg(errp, "Use FD passing only with iommufd backend");
>>> +    return -EINVAL;
>>> +    }
>>> +    /*
>>> + * Give a name with fd so any function printing out vbasedev->name
>>> + * will not break.
>>> + */
>>> +    if (!vbasedev->name) {
>>> +    vbasedev->name = g_strdup_printf("VFIO_FD%d", vbasedev->fd);
>>> +    }
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +void vfio_device_set_fd(VFIODevice *vbasedev, const char *str, Error 
>>> **errp)
>>
>>     bool vfio_device_set_fd(..., Error **errp)
>>
>>> +{
>>> +    int fd = monitor_fd_param(monitor_cur(), str, errp);
>>> +
>>> +    if (fd < 0) {
>>> +    error_prepend(errp, "Could not parse remote object fd %s:", str);
>>> +    return;
>>
>>     return false;
>>
>>> +    }
>>> +    vbasedev->fd = fd;
>>
>>     return true;
>
>If we had a QOM base device object, vfio_device_set_fd() would be passed
>directly to object_class_property_add_str() which expects a :
>
>   void (*set)(Object *, const char *, Error **)
>
>I think it is fine to keep as it is. We might have a QOM base device object
>one day ! Minor anyway.
>
>Thanks,
>
>C.
>
>
>>
>>> +}
>>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>>> index 3eec428162..e08a217057 100644
>>> --- a/hw/vfio/iommufd.c
>>> +++ b/hw/vfio/iommufd.c
>>> @@ -326,11 +326,15 @@ static int iommufd_cdev_attach(const char *name,

[PATCH 09/19] qapi/schema: assert info is present when necessary

[John Snow]

QAPISchemaInfo is sometimes defined as an Optional field because
built-in definitions don't *have* a source definition. As a consequence,
there are a few places where we need to assert that it's present because
the root entity definition only suggests it's "Optional".

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 3308f334872..c9a194103e1 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -733,6 +733,7 @@ def describe(self, info):
 else:
 assert False
 
+assert info is not None
 if defined_in != info.defn_name:
 return "%s '%s' of %s '%s'" % (role, self.name, meta, defined_in)
 return "%s '%s'" % (role, self.name)
@@ -823,6 +824,7 @@ def __init__(self, name, info, doc, ifcond, features,
 self.coroutine = coroutine
 
 def check(self, schema):
+assert self.info is not None
 super().check(schema)
 if self._arg_type_name:
 arg_type = schema.resolve_type(
-- 
2.41.0

[PATCH 10/19] qapi/schema: make QAPISchemaArrayType.element_type non-Optional

[John Snow]

This field should always be present and defined. Change this to be a
runtime @property that can emit an error if it's called prior to
check().

This helps simplify typing by avoiding the need to interrogate the value
for None at multiple callsites.

RFC: Yes, this is a slightly different technique than the one I used for
QAPISchemaObjectTypeMember.type; I think I prefer this one as being a
little less hokey, but it is more SLOC. Dealer's choice for which style
wins out -- now you have an example of both.

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index c9a194103e1..462acb2bb61 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -366,7 +366,16 @@ def __init__(self, name, info, element_type):
 super().__init__(name, info, None)
 assert isinstance(element_type, str)
 self._element_type_name = element_type
-self.element_type = None
+self._element_type: Optional[QAPISchemaType] = None
+
+@property
+def element_type(self) -> QAPISchemaType:
+if self._element_type is None:
+raise RuntimeError(
+"QAPISchemaArray has no element_type until "
+"after check() has been run."
+)
+return self._element_type
 
 def need_has_if_optional(self):
 # When FOO is an array, we still need has_FOO to distinguish
@@ -375,7 +384,7 @@ def need_has_if_optional(self):
 
 def check(self, schema):
 super().check(schema)
-self.element_type = schema.resolve_type(
+self._element_type = schema.resolve_type(
 self._element_type_name, self.info,
 self.info and self.info.defn_meta)
 assert not isinstance(self.element_type, QAPISchemaArrayType)
-- 
2.41.0

[PATCH 05/19] qapi/schema: make c_type() and json_type() abstract methods

[John Snow]

These methods should always return a str, it's only the default abstract
implementation that doesn't. They can be marked "abstract" by raising
NotImplementedError(), which requires subclasses to override the method
with the proper return type.

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index c5fdd625452..4600a566005 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -233,8 +233,8 @@ def visit(self, visitor):
 class QAPISchemaType(QAPISchemaEntity):
 # Return the C type for common use.
 # For the types we commonly box, this is a pointer type.
-def c_type(self):
-pass
+def c_type(self) -> str:
+raise NotImplementedError()
 
 # Return the C type to be used in a parameter list.
 def c_param_type(self):
@@ -244,8 +244,8 @@ def c_param_type(self):
 def c_unboxed_type(self):
 return self.c_type()
 
-def json_type(self):
-pass
+def json_type(self) -> str:
+raise NotImplementedError()
 
 def alternate_qtype(self):
 json2qtype = {
-- 
2.41.0

[PATCH 16/19] qapi/schema: add type hints

[John Snow]

This patch only adds type hints, which aren't utilized at runtime and
don't change the behavior of this module in any way.

In a scant few locations, type hints are removed where no longer
necessary due to inference power from typing all of the rest of
creation; and any type hints that no longer need string quotes are
changed.

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 554 +
 1 file changed, 389 insertions(+), 165 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index ce5b01b3182..5d19b59def0 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -15,10 +15,20 @@
 # TODO catching name collisions in generated code would be nice
 # pylint: disable=too-many-lines
 
+from __future__ import annotations
+
 from collections import OrderedDict
 import os
 import re
-from typing import List, Optional, cast
+from typing import (
+Any,
+Callable,
+Dict,
+List,
+Optional,
+Union,
+cast,
+)
 
 from .common import (
 POINTER_SUFFIX,
@@ -30,39 +40,50 @@
 )
 from .error import QAPIError, QAPISemError, QAPISourceError
 from .expr import check_exprs
-from .parser import QAPIExpression, QAPISchemaParser
+from .parser import QAPIDoc, QAPIExpression, QAPISchemaParser
+from .source import QAPISourceInfo
 
 
 class QAPISchemaIfCond:
-def __init__(self, ifcond=None):
+def __init__(
+self,
+ifcond: Optional[Union[str, Dict[str, object]]] = None,
+) -> None:
 self.ifcond = ifcond
 
-def _cgen(self):
+def _cgen(self) -> str:
 return cgen_ifcond(self.ifcond)
 
-def gen_if(self):
+def gen_if(self) -> str:
 return gen_if(self._cgen())
 
-def gen_endif(self):
+def gen_endif(self) -> str:
 return gen_endif(self._cgen())
 
-def docgen(self):
+def docgen(self) -> str:
 return docgen_ifcond(self.ifcond)
 
-def is_present(self):
+def is_present(self) -> bool:
 return bool(self.ifcond)
 
 
 class QAPISchemaEntity:
-meta: Optional[str] = None
+meta: str
 
-def __init__(self, name: str, info, doc, ifcond=None, features=None):
+def __init__(
+self,
+name: str,
+info: Optional[QAPISourceInfo],
+doc: Optional[QAPIDoc],
+ifcond: Optional[QAPISchemaIfCond] = None,
+features: Optional[List[QAPISchemaFeature]] = None,
+):
 assert name is None or isinstance(name, str)
 for f in features or []:
 assert isinstance(f, QAPISchemaFeature)
 f.set_defined_in(name)
 self.name = name
-self._module = None
+self._module: Optional[QAPISchemaModule] = None
 # For explicitly defined entities, info points to the (explicit)
 # definition.  For builtins (and their arrays), info is None.
 # For implicitly defined entities, info points to a place that
@@ -74,102 +95,162 @@ def __init__(self, name: str, info, doc, ifcond=None, 
features=None):
 self.features = features or []
 self._checked = False
 
-def __repr__(self):
+def __repr__(self) -> str:
 if self.name is None:
 return "<%s at 0x%x>" % (type(self).__name__, id(self))
 return "<%s:%s at 0x%x>" % (type(self).__name__, self.name, id(self))
 
-def c_name(self):
+def c_name(self) -> str:
 return c_name(self.name)
 
-def check(self, schema):
+def check(self, schema: QAPISchema) -> None:
 # pylint: disable=unused-argument
 assert not self._checked
-seen = {}
+seen: Dict[str, QAPISchemaMember] = {}
 for f in self.features:
 f.check_clash(self.info, seen)
 self._checked = True
 
-def connect_doc(self, doc=None):
+def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None:
 doc = doc or self.doc
 if doc:
 for f in self.features:
 doc.connect_feature(f)
 
-def check_doc(self):
+def check_doc(self) -> None:
 if self.doc:
 self.doc.check()
 
-def _set_module(self, schema, info):
+def _set_module(
+self, schema: QAPISchema, info: Optional[QAPISourceInfo]
+) -> None:
 assert self._checked
 fname = info.fname if info else QAPISchemaModule.BUILTIN_MODULE_NAME
 self._module = schema.module_by_fname(fname)
 self._module.add_entity(self)
 
-def set_module(self, schema):
+def set_module(self, schema: QAPISchema) -> None:
 self._set_module(schema, self.info)
 
 @property
-def ifcond(self):
+def ifcond(self) -> QAPISchemaIfCond:
 assert self._checked
 return self._ifcond
 
-def is_implicit(self):
+def is_implicit(self) -> bool:
 return not self.info
 
-def visit(self, visitor):
+def visit(self, visitor: QAPISchemaVisitor) -> None:
 # pylint: disable=unused-argument
 assert self._checked
 
-def 

[PATCH 06/19] qapi/schema: adjust type narrowing for mypy's benefit

[John Snow]

We already take care to perform some type narrowing for arg_type and
ret_type, but not in a way where mypy can utilize the result. A simple
change to use a temporary variable helps the medicine go down.

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 4600a566005..a1094283828 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -825,13 +825,14 @@ def __init__(self, name, info, doc, ifcond, features,
 def check(self, schema):
 super().check(schema)
 if self._arg_type_name:
-self.arg_type = schema.resolve_type(
+arg_type = schema.resolve_type(
 self._arg_type_name, self.info, "command's 'data'")
-if not isinstance(self.arg_type, QAPISchemaObjectType):
+if not isinstance(arg_type, QAPISchemaObjectType):
 raise QAPISemError(
 self.info,
 "command's 'data' cannot take %s"
-% self.arg_type.describe())
+% arg_type.describe())
+self.arg_type = arg_type
 if self.arg_type.variants and not self.boxed:
 raise QAPISemError(
 self.info,
@@ -848,8 +849,7 @@ def check(self, schema):
 if self.name not in self.info.pragma.command_returns_exceptions:
 typ = self.ret_type
 if isinstance(typ, QAPISchemaArrayType):
-typ = self.ret_type.element_type
-assert typ
+typ = typ.element_type
 if not isinstance(typ, QAPISchemaObjectType):
 raise QAPISemError(
 self.info,
@@ -885,13 +885,14 @@ def __init__(self, name, info, doc, ifcond, features, 
arg_type, boxed):
 def check(self, schema):
 super().check(schema)
 if self._arg_type_name:
-self.arg_type = schema.resolve_type(
+typ = schema.resolve_type(
 self._arg_type_name, self.info, "event's 'data'")
-if not isinstance(self.arg_type, QAPISchemaObjectType):
+if not isinstance(typ, QAPISchemaObjectType):
 raise QAPISemError(
 self.info,
 "event's 'data' cannot take %s"
-% self.arg_type.describe())
+% typ.describe())
+self.arg_type = typ
 if self.arg_type.variants and not self.boxed:
 raise QAPISemError(
 self.info,
-- 
2.41.0

[PATCH 14/19] qapi/schema: assert QAPISchemaVariants are QAPISchemaObjectType

[John Snow]

I'm actually not too sure about this one, it seems to hold up at runtime
but instead of lying and coming up with an elaborate ruse as a commit
message I'm just going to admit I just cribbed my own notes from the
last time I typed schema.py and I no longer remember why or if this is
correct.

Cool!

With more seriousness, variants are only guaranteed to house a
QAPISchemaType as per the definition of QAPISchemaObjectTypeMember but
the only classes/types that have a check_clash method are descendents of
QAPISchemaMember and the QAPISchemaVariants class itself.

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 476b19aed61..ce5b01b3182 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -717,6 +717,7 @@ def check_clash(self, info, seen):
 for v in self.variants:
 # Reset seen map for each variant, since qapi names from one
 # branch do not affect another branch
+assert isinstance(v.type, QAPISchemaObjectType)  # I think, anyway?
 v.type.check_clash(info, dict(seen))
 
 
-- 
2.41.0

[PATCH 08/19] qapi/schema: add static typing and assertions to lookup_type()

[John Snow]

This function is a bit hard to type as-is; mypy needs some assertions to
assist with the type narrowing.

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index a1094283828..3308f334872 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -968,8 +968,12 @@ def lookup_entity(self, name, typ=None):
 return None
 return ent
 
-def lookup_type(self, name):
-return self.lookup_entity(name, QAPISchemaType)
+def lookup_type(self, name: str) -> Optional[QAPISchemaType]:
+typ = self.lookup_entity(name, QAPISchemaType)
+if typ is None:
+return None
+assert isinstance(typ, QAPISchemaType)
+return typ
 
 def resolve_type(self, name, info, what):
 typ = self.lookup_type(name)
-- 
2.41.0

[PATCH 04/19] qapi/schema: declare type for QAPISchemaObjectTypeMember.type

[John Snow]

declare, but don't initialize the type of "type" to be QAPISchemaType -
and allow the value to be initialized during check().

This avoids the need for several "assert type is not None" statements
littered throughout the code by asserting it "will always be set."

It's a little hokey, but it works -- at the expense of slightly
incorrect type information before check() is called, anyway. If this
field is accessed before it is initialized, you'll be treated to an
AttributeError exception.

Fixes stuff like this:

qapi/schema.py:657: error: "None" has no attribute "alternate_qtype"  
[attr-defined]
qapi/schema.py:662: error: "None" has no attribute "describe"  [attr-defined]

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 0fb44452dd5..c5fdd625452 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -771,7 +771,7 @@ def __init__(self, name, info, typ, optional, ifcond=None, 
features=None):
 assert isinstance(f, QAPISchemaFeature)
 f.set_defined_in(name)
 self._type_name = typ
-self.type = None
+self.type: QAPISchemaType  # set during check(). Kind of hokey.
 self.optional = optional
 self.features = features or []
 
-- 
2.41.0

[PATCH 07/19] qapi/introspect: assert schema.lookup_type did not fail

[John Snow]

lookup_type() is capable of returning None, but introspect.py isn't
prepared for that. (And rightly so, if these built-in types are absent,
something has gone hugely wrong.)

RFC: This is slightly cumbersome as-is, but a patch at the end of this series
tries to address it with some slightly slicker lookup functions that
don't need as much hand-holding.

Signed-off-by: John Snow 
---
 scripts/qapi/introspect.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 67c7d89aae0..42981bce163 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -227,10 +227,14 @@ def _use_type(self, typ: QAPISchemaType) -> str:
 
 # Map the various integer types to plain int
 if typ.json_type() == 'int':
-typ = self._schema.lookup_type('int')
+tmp = self._schema.lookup_type('int')
+assert tmp is not None
+typ = tmp
 elif (isinstance(typ, QAPISchemaArrayType) and
   typ.element_type.json_type() == 'int'):
-typ = self._schema.lookup_type('intList')
+tmp = self._schema.lookup_type('intList')
+assert tmp is not None
+typ = tmp
 # Add type to work queue if new
 if typ not in self._used_types:
 self._used_types.append(typ)
-- 
2.41.0

[PATCH 01/19] qapi/schema: fix QAPISchemaEntity.__repr__()

[John Snow]

This needs parentheses to work how you want it to:

>>> "%s-%s-%s" % 'a', 'b', 'c'
Traceback (most recent call last):
  File "", line 1, in 
TypeError: not enough arguments for format string

>>> "%s-%s-%s" % ('a', 'b', 'c')
'a-b-c'

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index d739e558e9e..c79747b2a15 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -76,7 +76,7 @@ def __init__(self, name: str, info, doc, ifcond=None, 
features=None):
 def __repr__(self):
 if self.name is None:
 return "<%s at 0x%x>" % (type(self).__name__, id(self))
-return "<%s:%s at 0x%x>" % type(self).__name__, self.name, id(self)
+return "<%s:%s at 0x%x>" % (type(self).__name__, self.name, id(self))
 
 def c_name(self):
 return c_name(self.name)
-- 
2.41.0

[PATCH 11/19] qapi/schema: fix QAPISchemaArrayType.check's call to resolve_type

[John Snow]

There's more conditionals in here than we can reasonably pack into a
terse little statement, so break it apart into something more explicit.

(When would a built-in array ever cause a QAPISemError? I don't know,
maybe never - but the type system wasn't happy all the same.)

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 462acb2bb61..164d86c4064 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -384,9 +384,16 @@ def need_has_if_optional(self):
 
 def check(self, schema):
 super().check(schema)
+
+if self.info:
+assert self.info.defn_meta  # guaranteed to be set by expr.py
+what = self.info.defn_meta
+else:
+what = 'built-in array'
+
 self._element_type = schema.resolve_type(
-self._element_type_name, self.info,
-self.info and self.info.defn_meta)
+self._element_type_name, self.info, what
+)
 assert not isinstance(self.element_type, QAPISchemaArrayType)
 
 def set_module(self, schema):
-- 
2.41.0

[PATCH 13/19] qapi/schema: fix typing for QAPISchemaVariants.tag_member

[John Snow]

There are two related changes here:

(1) We need to perform type narrowing for resolving the type of
tag_member during check(), and

(2) tag_member is a delayed initialization field, but we can hide it
behind a property that raises an Exception if it's called too
early. This simplifies the typing in quite a few places and avoids
needing to assert that the "tag_member is not None" at a dozen
callsites, which can be confusing and suggest the wrong thing to a
drive-by contributor.

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 200bc0730d6..476b19aed61 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -627,25 +627,39 @@ def __init__(self, tag_name, info, tag_member, variants):
 assert isinstance(v, QAPISchemaVariant)
 self._tag_name = tag_name
 self.info = info
-self.tag_member = tag_member
+self._tag_member: Optional[QAPISchemaObjectTypeMember] = tag_member
 self.variants = variants
 
+@property
+def tag_member(self) -> 'QAPISchemaObjectTypeMember':
+if self._tag_member is None:
+raise RuntimeError(
+"QAPISchemaVariants has no tag_member property until "
+"after check() has been run."
+)
+return self._tag_member
+
 def set_defined_in(self, name):
 for v in self.variants:
 v.set_defined_in(name)
 
 def check(self, schema, seen):
 if self._tag_name:  # union
-self.tag_member = seen.get(c_name(self._tag_name))
+# We need to narrow the member type:
+tmp = seen.get(c_name(self._tag_name))
+assert tmp is None or isinstance(tmp, QAPISchemaObjectTypeMember)
+self._tag_member = tmp
+
 base = "'base'"
 # Pointing to the base type when not implicit would be
 # nice, but we don't know it here
-if not self.tag_member or self._tag_name != self.tag_member.name:
+if not self._tag_member or self._tag_name != self._tag_member.name:
 raise QAPISemError(
 self.info,
 "discriminator '%s' is not a member of %s"
 % (self._tag_name, base))
 # Here we do:
+assert self.tag_member.defined_in
 base_type = schema.lookup_type(self.tag_member.defined_in)
 assert base_type
 if not base_type.is_implicit():
@@ -666,11 +680,13 @@ def check(self, schema, seen):
 "discriminator member '%s' of %s must not be conditional"
 % (self._tag_name, base))
 else:   # alternate
+assert self._tag_member
 assert isinstance(self.tag_member.type, QAPISchemaEnumType)
 assert not self.tag_member.optional
 assert not self.tag_member.ifcond.is_present()
 if self._tag_name:  # union
 # branches that are not explicitly covered get an empty type
+assert self.tag_member.defined_in
 cases = {v.name for v in self.variants}
 for m in self.tag_member.type.members:
 if m.name not in cases:
-- 
2.41.0

[PATCH 15/19] qapi/parser: demote QAPIExpression to Dict[str, Any]

[John Snow]

Dict[str, object] is a stricter type, but with the way that code is
currently arranged, it is infeasible to enforce this strictness.

In particular, although expr.py's entire raison d'être is normalization
and type-checking of QAPI Expressions, that type information is not
"remembered" in any meaningful way by mypy because each individual
expression is not downcast to a specific expression type that holds all
the details of each expression's unique form.

As a result, all of the code in schema.py that deals with actually
creating type-safe specialized structures has no guarantee (myopically)
that the data it is being passed is correct.

There are two ways to solve this:

(1) Re-assert that the incoming data is in the shape we expect it to be, or
(2) Disable type checking for this data.

(1) is appealing to my sense of strictness, but I gotta concede that it
is asinine to re-check the shape of a QAPIExpression in schema.py when
expr.py has just completed that work at length. The duplication of code
and the nightmare thought of needing to update both locations if and
when we change the shape of these structures makes me extremely
reluctant to go down this route.

(2) allows us the chance to miss updating types in the case that types
are updated in expr.py, but it *is* an awful lot simpler and,
importantly, gets us closer to type checking schema.py *at
all*. Something is better than nothing, I'd argue.

So, do the simpler dumber thing and worry about future strictness
improvements later.

Signed-off-by: John Snow 
---
 scripts/qapi/parser.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index bf31018aef0..b7f08cf36f2 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -19,6 +19,7 @@
 import re
 from typing import (
 TYPE_CHECKING,
+Any,
 Dict,
 List,
 Mapping,
@@ -43,7 +44,7 @@
 _ExprValue = Union[List[object], Dict[str, object], str, bool]
 
 
-class QAPIExpression(Dict[str, object]):
+class QAPIExpression(Dict[str, Any]):
 # pylint: disable=too-few-public-methods
 def __init__(self,
  data: Mapping[str, object],
-- 
2.41.0

[PATCH 02/19] qapi/schema: add pylint suppressions

[John Snow]

With this, pylint is happy with the file, so enable it in the
configuration.

Signed-off-by: John Snow 
---
 scripts/qapi/pylintrc  | 5 -
 scripts/qapi/schema.py | 4 
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc
index 90546df5345..aafddd3d8d0 100644
--- a/scripts/qapi/pylintrc
+++ b/scripts/qapi/pylintrc
@@ -1,10 +1,5 @@
 [MASTER]
 
-# Add files or directories matching the regex patterns to the ignore list.
-# The regex matches against base names, not paths.
-ignore-patterns=schema.py,
-
-
 [MESSAGES CONTROL]
 
 # Disable the message, report, category or checker with the given id(s). You
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index c79747b2a15..153e703e0ef 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -13,6 +13,7 @@
 # See the COPYING file in the top-level directory.
 
 # TODO catching name collisions in generated code would be nice
+# pylint: disable=too-many-lines
 
 from collections import OrderedDict
 import os
@@ -82,6 +83,7 @@ def c_name(self):
 return c_name(self.name)
 
 def check(self, schema):
+# pylint: disable=unused-argument
 assert not self._checked
 seen = {}
 for f in self.features:
@@ -116,6 +118,7 @@ def is_implicit(self):
 return not self.info
 
 def visit(self, visitor):
+# pylint: disable=unused-argument
 assert self._checked
 
 def describe(self):
@@ -134,6 +137,7 @@ def visit_module(self, name):
 pass
 
 def visit_needed(self, entity):
+# pylint: disable=unused-argument
 # Default to visiting everything
 return True
 
-- 
2.41.0

[PATCH 00/19] qapi: statically type schema.py

[John Snow]

Hi! This branch has some comical name like python-qapi-cleanup-pt6-v2
but, simply, it finishes what I started and statically types all of the
QAPI generator code.

GitLab CI: https://gitlab.com/jsnow/qemu/-/pipelines/1074124051

Patch 1 is an actual fix,
Patch 2 is a minor patch for pylint with no runtime effect,
Patches 3-15 fix individual typing issues that have some runtime effect.
Patch 16 adds typing information,
Patch 17 removes the schema.py exemption from the mypy conf,
Patches 18-19 are optional.

Most of the patches (expect 16) are very small. I'm sure we'll need to
rework parts of this, but hey, gotta start somewhere.

--js

John Snow (19):
  qapi/schema: fix QAPISchemaEntity.__repr__()
  qapi/schema: add pylint suppressions
  qapi/schema: name QAPISchemaInclude entities
  qapi/schema: declare type for QAPISchemaObjectTypeMember.type
  qapi/schema: make c_type() and json_type() abstract methods
  qapi/schema: adjust type narrowing for mypy's benefit
  qapi/introspect: assert schema.lookup_type did not fail
  qapi/schema: add static typing and assertions to lookup_type()
  qapi/schema: assert info is present when necessary
  qapi/schema: make QAPISchemaArrayType.element_type non-Optional
  qapi/schema: fix QAPISchemaArrayType.check's call to resolve_type
  qapi/schema: split "checked" field into "checking" and "checked"
  qapi/schema: fix typing for QAPISchemaVariants.tag_member
  qapi/schema: assert QAPISchemaVariants are QAPISchemaObjectType
  qapi/parser: demote QAPIExpression to Dict[str, Any]
  qapi/schema: add type hints
  qapi/schema: turn on mypy strictness
  qapi/schema: remove unnecessary asserts
  qapi/schema: refactor entity lookup helpers

 docs/sphinx/qapidoc.py |   2 +-
 scripts/qapi/mypy.ini  |   5 -
 scripts/qapi/parser.py |   3 +-
 scripts/qapi/pylintrc  |   5 -
 scripts/qapi/schema.py | 723 -
 5 files changed, 498 insertions(+), 240 deletions(-)

-- 
2.41.0

[PATCH 19/19] qapi/schema: refactor entity lookup helpers

[John Snow]

This is not a clear win, but I was confused and I couldn't help myself.

Before:

lookup_entity(self, name: str, typ: Optional[type] = None
  ) -> Optional[QAPISchemaEntity]: ...

lookup_type(self, name: str) -> Optional[QAPISchemaType]: ...

resolve_type(self, name: str, info: Optional[QAPISourceInfo],
 what: Union[str, Callable[[Optional[QAPISourceInfo]], str]]
 ) -> QAPISchemaType: ...

After:

get_entity(self, name: str) -> Optional[QAPISchemaEntity]: ...
get_typed_entity(self, name: str, typ: Type[_EntityType]
 ) -> Optional[_EntityType]: ...
lookup_type(self, name: str) -> QAPISchemaType: ...
resolve_type(self, name: str, info: Optional[QAPISourceInfo],
 what: Union[str, Callable[[Optional[QAPISourceInfo]], str]]
 ) -> QAPISchemaType: ...

In essence, any function that can return a None value becomes "get ..."
to encourage association with the dict.get() function which has the same
behavior. Any function named "lookup" or "resolve" by contrast is no
longer allowed to return a None value.

This means that any callers to resolve_type or lookup_type don't have to
check that the function worked, they can just assume it did.

Callers to resolve_type will be greeted with a QAPISemError if something
has gone wrong, as they have in the past. Callers to lookup_type will be
greeted with a KeyError if the entity does not exist, or a TypeError if
it does, but is the wrong type.

get_entity and get_typed_entity remain for any callers who are
specifically interested in the negative case. These functions have only
a single caller each.

Signed-off-by: John Snow 
---
 docs/sphinx/qapidoc.py |  2 +-
 scripts/qapi/introspect.py |  8 ++
 scripts/qapi/schema.py | 52 --
 3 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 8f3b9997a15..96deadbf7fc 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -508,7 +508,7 @@ def run(self):
 vis.visit_begin(schema)
 for doc in schema.docs:
 if doc.symbol:
-vis.symbol(doc, schema.lookup_entity(doc.symbol))
+vis.symbol(doc, schema.get_entity(doc.symbol))
 else:
 vis.freeform(doc)
 return vis.get_document_nodes()
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 42981bce163..67c7d89aae0 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -227,14 +227,10 @@ def _use_type(self, typ: QAPISchemaType) -> str:
 
 # Map the various integer types to plain int
 if typ.json_type() == 'int':
-tmp = self._schema.lookup_type('int')
-assert tmp is not None
-typ = tmp
+typ = self._schema.lookup_type('int')
 elif (isinstance(typ, QAPISchemaArrayType) and
   typ.element_type.json_type() == 'int'):
-tmp = self._schema.lookup_type('intList')
-assert tmp is not None
-typ = tmp
+typ = self._schema.lookup_type('intList')
 # Add type to work queue if new
 if typ not in self._used_types:
 self._used_types.append(typ)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index b5f377e68b8..5813136e78b 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -26,6 +26,8 @@
 Dict,
 List,
 Optional,
+Type,
+TypeVar,
 Union,
 cast,
 )
@@ -767,7 +769,6 @@ def check(
 # Here we do:
 assert self.tag_member.defined_in
 base_type = schema.lookup_type(self.tag_member.defined_in)
-assert base_type
 if not base_type.is_implicit():
 base = "base type '%s'" % self.tag_member.defined_in
 if not isinstance(self.tag_member.type, QAPISchemaEnumType):
@@ -,6 +1112,12 @@ def visit(self, visitor: QAPISchemaVisitor) -> None:
 self.arg_type, self.boxed)
 
 
+# Used for type-dependent type lookup return values.
+_EntityType = TypeVar(   # pylint: disable=invalid-name
+'_EntityType', bound=QAPISchemaEntity
+)
+
+
 class QAPISchema:
 def __init__(self, fname: str):
 self.fname = fname
@@ -1155,22 +1162,28 @@ def _def_entity(self, ent: QAPISchemaEntity) -> None:
 ent.info, "%s is already defined" % other_ent.describe())
 self._entity_dict[ent.name] = ent
 
-def lookup_entity(
+def get_entity(self, name: str) -> Optional[QAPISchemaEntity]:
+return self._entity_dict.get(name)
+
+def get_typed_entity(
 self,
 name: str,
-typ: Optional[type] = None,
-) -> Optional[QAPISchemaEntity]:
-ent = self._entity_dict.get(name)
-if typ and not isinstance(ent, typ):
-return None
+typ: Type[_EntityType]
+) -> Optional[_EntityType]:
+ent = 

[PATCH 17/19] qapi/schema: turn on mypy strictness

[John Snow]

This patch can be rolled in with the previous one once the series is
ready for merge, but for work-in-progress' sake, it's separate here.

Signed-off-by: John Snow 
---
 scripts/qapi/mypy.ini | 5 -
 1 file changed, 5 deletions(-)

diff --git a/scripts/qapi/mypy.ini b/scripts/qapi/mypy.ini
index 56e0dfb1327..8109470a031 100644
--- a/scripts/qapi/mypy.ini
+++ b/scripts/qapi/mypy.ini
@@ -2,8 +2,3 @@
 strict = True
 disallow_untyped_calls = False
 python_version = 3.8
-
-[mypy-qapi.schema]
-disallow_untyped_defs = False
-disallow_incomplete_defs = False
-check_untyped_defs = False
-- 
2.41.0

[PATCH 18/19] qapi/schema: remove unnecessary asserts

[John Snow]

With strict typing enabled, these runtime statements aren't necessary
anymore.

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 23 ---
 1 file changed, 23 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 5d19b59def0..b5f377e68b8 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -78,9 +78,7 @@ def __init__(
 ifcond: Optional[QAPISchemaIfCond] = None,
 features: Optional[List[QAPISchemaFeature]] = None,
 ):
-assert name is None or isinstance(name, str)
 for f in features or []:
-assert isinstance(f, QAPISchemaFeature)
 f.set_defined_in(name)
 self.name = name
 self._module: Optional[QAPISchemaModule] = None
@@ -145,7 +143,6 @@ def visit(self, visitor: QAPISchemaVisitor) -> None:
 assert self._checked
 
 def describe(self) -> str:
-assert self.meta
 return "%s '%s'" % (self.meta, self.name)
 
 
@@ -359,7 +356,6 @@ def check(self, schema: QAPISchema) -> None:
 f"feature '{feat.name}' is not supported for types")
 
 def describe(self) -> str:
-assert self.meta
 return "%s type '%s'" % (self.meta, self.name)
 
 
@@ -368,7 +364,6 @@ class QAPISchemaBuiltinType(QAPISchemaType):
 
 def __init__(self, name: str, json_type: str, c_type: str):
 super().__init__(name, None, None)
-assert not c_type or isinstance(c_type, str)
 assert json_type in ('string', 'number', 'int', 'boolean', 'null',
  'value')
 self._json_type_name = json_type
@@ -411,9 +406,7 @@ def __init__(
 ):
 super().__init__(name, info, doc, ifcond, features)
 for m in members:
-assert isinstance(m, QAPISchemaEnumMember)
 m.set_defined_in(name)
-assert prefix is None or isinstance(prefix, str)
 self.members = members
 self.prefix = prefix
 
@@ -456,7 +449,6 @@ def __init__(
 self, name: str, info: Optional[QAPISourceInfo], element_type: str
 ):
 super().__init__(name, info, None)
-assert isinstance(element_type, str)
 self._element_type_name = element_type
 self._element_type: Optional[QAPISchemaType] = None
 
@@ -517,7 +509,6 @@ def visit(self, visitor: QAPISchemaVisitor) -> None:
  self.element_type)
 
 def describe(self) -> str:
-assert self.meta
 return "%s type ['%s']" % (self.meta, self._element_type_name)
 
 
@@ -537,12 +528,9 @@ def __init__(
 # union has base, variants, and no local_members
 super().__init__(name, info, doc, ifcond, features)
 self.meta = 'union' if variants else 'struct'
-assert base is None or isinstance(base, str)
 for m in local_members:
-assert isinstance(m, QAPISchemaObjectTypeMember)
 m.set_defined_in(name)
 if variants is not None:
-assert isinstance(variants, QAPISchemaVariants)
 variants.set_defined_in(name)
 self._base_name = base
 self.base = None
@@ -666,7 +654,6 @@ def __init__(
 variants: QAPISchemaVariants,
 ):
 super().__init__(name, info, doc, ifcond, features)
-assert isinstance(variants, QAPISchemaVariants)
 assert variants.tag_member
 variants.set_defined_in(name)
 variants.tag_member.set_defined_in(self.name)
@@ -742,8 +729,6 @@ def __init__(
 assert bool(tag_member) != bool(tag_name)
 assert (isinstance(tag_name, str) or
 isinstance(tag_member, QAPISchemaObjectTypeMember))
-for v in variants:
-assert isinstance(v, QAPISchemaVariant)
 self._tag_name = tag_name
 self.info = info
 self._tag_member = tag_member
@@ -856,7 +841,6 @@ def __init__(
 info: Optional[QAPISourceInfo],
 ifcond: Optional[QAPISchemaIfCond] = None,
 ):
-assert isinstance(name, str)
 self.name = name
 self.info = info
 self.ifcond = ifcond or QAPISchemaIfCond()
@@ -924,7 +908,6 @@ def __init__(
 ):
 super().__init__(name, info, ifcond)
 for f in features or []:
-assert isinstance(f, QAPISchemaFeature)
 f.set_defined_in(name)
 self.features = features or []
 
@@ -953,10 +936,7 @@ def __init__(
 features: Optional[List[QAPISchemaFeature]] = None,
 ):
 super().__init__(name, info, ifcond)
-assert isinstance(typ, str)
-assert isinstance(optional, bool)
 for f in features or []:
-assert isinstance(f, QAPISchemaFeature)
 f.set_defined_in(name)
 self._type_name = typ
 self.type: QAPISchemaType  # set during check(). Kind of hokey.
@@ -1015,8 +995,6 @@ def __init__(
 coroutine: bool,
 ):
 super().__init__(name, info, doc, ifcond, features)
-assert not 

[PATCH 03/19] qapi/schema: name QAPISchemaInclude entities

[John Snow]

It simplifies typing to mandate that entities will always have a name;
to achieve this we can occasionally assign an internal name. This
alleviates errors such as:

qapi/schema.py:287: error: Argument 1 to "__init__" of
"QAPISchemaEntity" has incompatible type "None"; expected "str"
[arg-type]

Trying to fix it the other way by allowing entities to only have
optional names opens up a nightmare portal of whackamole to try and
audit that every other pathway doesn't actually pass a None name when we
expect it to; this is the simpler direction of consitifying the typing.

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 153e703e0ef..0fb44452dd5 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -220,7 +220,9 @@ def visit(self, visitor):
 
 class QAPISchemaInclude(QAPISchemaEntity):
 def __init__(self, sub_module, info):
-super().__init__(None, info, None)
+# Includes are internal entity objects; and may occur multiple times
+name = f"q_include_{info.fname}:{info.line}"
+super().__init__(name, info, None)
 self._sub_module = sub_module
 
 def visit(self, visitor):
-- 
2.41.0

[PATCH 12/19] qapi/schema: split "checked" field into "checking" and "checked"

[John Snow]

Differentiate between "actively in the process of checking" and
"checking has completed". This allows us to clean up the types of some
internal fields such as QAPISchemaObjectType's members field which
currently uses "None" as a canary for determining if check has
completed.

This simplifies the typing from a cumbersome Optional[List[T]] to merely
a List[T], which is more pythonic: it is safe to iterate over an empty
list with "for x in []" whereas with an Optional[List[T]] you have to
rely on the more cumbersome "if L: for x in L: ..."

RFC: are we guaranteed to have members here? can we just use "if
members" without adding the new field?

Signed-off-by: John Snow 
---
 scripts/qapi/schema.py | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 164d86c4064..200bc0730d6 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -18,7 +18,7 @@
 from collections import OrderedDict
 import os
 import re
-from typing import List, Optional
+from typing import List, Optional, cast
 
 from .common import (
 POINTER_SUFFIX,
@@ -447,22 +447,24 @@ def __init__(self, name, info, doc, ifcond, features,
 self.base = None
 self.local_members = local_members
 self.variants = variants
-self.members = None
+self.members: List[QAPISchemaObjectTypeMember] = []
+self._checking = False
 
 def check(self, schema):
 # This calls another type T's .check() exactly when the C
 # struct emitted by gen_object() contains that T's C struct
 # (pointers don't count).
-if self.members is not None:
-# A previous .check() completed: nothing to do
-return
-if self._checked:
+if self._checking:
 # Recursed: C struct contains itself
 raise QAPISemError(self.info,
"object %s contains itself" % self.name)
+if self._checked:
+# A previous .check() completed: nothing to do
+return
 
+self._checking = True
 super().check(schema)
-assert self._checked and self.members is None
+assert self._checked and not self.members
 
 seen = OrderedDict()
 if self._base_name:
@@ -479,13 +481,17 @@ def check(self, schema):
 for m in self.local_members:
 m.check(schema)
 m.check_clash(self.info, seen)
-members = seen.values()
+
+# check_clash is abstract, but local_members is asserted to be
+# List[QAPISchemaObjectTypeMember]. Cast to the narrower type.
+members = cast(List[QAPISchemaObjectTypeMember], list(seen.values()))
 
 if self.variants:
 self.variants.check(schema, seen)
 self.variants.check_clash(self.info, seen)
 
-self.members = members  # mark completed
+self.members = members
+self._checking = False  # mark completed
 
 # Check that the members of this type do not cause duplicate JSON members,
 # and update seen to track the members seen so far. Report any errors
-- 
2.41.0