date:20220603

From: Alex Bennée 

Signed-off-by: Alex Bennée 
Message-Id: <20220524154056.2896913-12-alex.ben...@linaro.org>
Signed-off-by: Thomas Huth 
---
 tests/qtest/vhost-user-test.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index a2cec87684..8bf390be20 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -524,14 +524,13 @@ static void chr_event(void *opaque, QEMUChrEvent event)
 
 static void test_server_create_chr(TestServer *server, const gchar *opt)
 {
-gchar *chr_path;
+g_autofree gchar *chr_path = g_strdup_printf("unix:%s%s",
+ server->socket_path, opt);
 Chardev *chr;
 
-chr_path = g_strdup_printf("unix:%s%s", server->socket_path, opt);
 chr = qemu_chr_new(server->chr_name, chr_path, server->context);
-g_free(chr_path);
+g_assert(chr);
 
-g_assert_nonnull(chr);
 qemu_chr_fe_init(&server->chr, chr, &error_abort);
 qemu_chr_fe_set_handlers(&server->chr, chr_can_read, chr_read,
  chr_event, NULL, server, server->context, true);
-- 
2.31.1

[PULL 01/12] s390: Typo fix FLOATING_POINT_SUPPPORT_ENH

From: "Dr. David Alan Gilbert" 

One less P needed.

Signed-off-by: Dr. David Alan Gilbert 
Message-Id: <20220523115123.150340-1-dgilb...@redhat.com>
Signed-off-by: Thomas Huth 
---
 target/s390x/cpu_features_def.h.inc | 2 +-
 target/s390x/gen-features.c | 6 +++---
 target/s390x/tcg/translate.c| 8 
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/target/s390x/cpu_features_def.h.inc 
b/target/s390x/cpu_features_def.h.inc
index e86662bb3b..3603e5fb12 100644
--- a/target/s390x/cpu_features_def.h.inc
+++ b/target/s390x/cpu_features_def.h.inc
@@ -58,7 +58,7 @@ DEF_FEAT(ENHANCED_MONITOR, "emon", STFL, 36, 
"Enhanced-monitor facility")
 DEF_FEAT(FLOATING_POINT_EXT, "fpe", STFL, 37, "Floating-point extension 
facility")
 DEF_FEAT(ORDER_PRESERVING_COMPRESSION, "opc", STFL, 38, "Order Preserving 
Compression facility")
 DEF_FEAT(SET_PROGRAM_PARAMETERS, "sprogp", STFL, 40, "Set-program-parameters 
facility")
-DEF_FEAT(FLOATING_POINT_SUPPPORT_ENH, "fpseh", STFL, 41, 
"Floating-point-support-enhancement facilities")
+DEF_FEAT(FLOATING_POINT_SUPPORT_ENH, "fpseh", STFL, 41, 
"Floating-point-support-enhancement facilities")
 DEF_FEAT(DFP, "dfp", STFL, 42, "DFP (decimal-floating-point) facility")
 DEF_FEAT(DFP_FAST, "dfphp", STFL, 43, "DFP (decimal-floating-point) facility 
has high performance")
 DEF_FEAT(PFPO, "pfpo", STFL, 44, "PFPO instruction")
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index c03ec2c9a9..ad140184b9 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -374,7 +374,7 @@ static uint16_t base_GEN10_GA1[] = {
 S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2,
 S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
 S390_FEAT_EXECUTE_EXT,
-S390_FEAT_FLOATING_POINT_SUPPPORT_ENH,
+S390_FEAT_FLOATING_POINT_SUPPORT_ENH,
 S390_FEAT_DFP,
 S390_FEAT_DFP_FAST,
 S390_FEAT_PFPO,
@@ -476,7 +476,7 @@ static uint16_t full_GEN9_GA2[] = {
 S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
 S390_FEAT_EXTRACT_CPU_TIME,
 S390_FEAT_COMPARE_AND_SWAP_AND_STORE,
-S390_FEAT_FLOATING_POINT_SUPPPORT_ENH,
+S390_FEAT_FLOATING_POINT_SUPPORT_ENH,
 S390_FEAT_DFP,
 };
 
@@ -700,7 +700,7 @@ static uint16_t qemu_V3_1[] = {
 S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
 S390_FEAT_EXECUTE_EXT,
 S390_FEAT_SET_PROGRAM_PARAMETERS,
-S390_FEAT_FLOATING_POINT_SUPPPORT_ENH,
+S390_FEAT_FLOATING_POINT_SUPPORT_ENH,
 S390_FEAT_STFLE_45,
 S390_FEAT_STFLE_49,
 S390_FEAT_LOCAL_TLB_CLEARING,
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index b40cb84bae..fd2433d625 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6185,17 +6185,17 @@ enum DisasInsnEnum {
 #define FAC_Z   S390_FEAT_ZARCH
 #define FAC_CASSS390_FEAT_COMPARE_AND_SWAP_AND_STORE
 #define FAC_DFP S390_FEAT_DFP
-#define FAC_DFPRS390_FEAT_FLOATING_POINT_SUPPPORT_ENH /* DFP-rounding 
*/
+#define FAC_DFPRS390_FEAT_FLOATING_POINT_SUPPORT_ENH /* DFP-rounding */
 #define FAC_DO  S390_FEAT_STFLE_45 /* distinct-operands */
 #define FAC_EE  S390_FEAT_EXECUTE_EXT
 #define FAC_EI  S390_FEAT_EXTENDED_IMMEDIATE
 #define FAC_FPE S390_FEAT_FLOATING_POINT_EXT
-#define FAC_FPSSH   S390_FEAT_FLOATING_POINT_SUPPPORT_ENH /* 
FPS-sign-handling */
-#define FAC_FPRGR   S390_FEAT_FLOATING_POINT_SUPPPORT_ENH /* 
FPR-GR-transfer */
+#define FAC_FPSSH   S390_FEAT_FLOATING_POINT_SUPPORT_ENH /* 
FPS-sign-handling */
+#define FAC_FPRGR   S390_FEAT_FLOATING_POINT_SUPPORT_ENH /* 
FPR-GR-transfer */
 #define FAC_GIE S390_FEAT_GENERAL_INSTRUCTIONS_EXT
 #define FAC_HFP_MA  S390_FEAT_HFP_MADDSUB
 #define FAC_HW  S390_FEAT_STFLE_45 /* high-word */
-#define FAC_I_SIM   S390_FEAT_FLOATING_POINT_SUPPPORT_ENH /* 
IEEE-exception-simulation */
+#define FAC_I_SIM   S390_FEAT_FLOATING_POINT_SUPPORT_ENH /* 
IEEE-exception-simulation */
 #define FAC_MIE S390_FEAT_STFLE_49 /* misc-instruction-extensions */
 #define FAC_LAT S390_FEAT_STFLE_49 /* load-and-trap */
 #define FAC_LOC S390_FEAT_STFLE_45 /* load/store on condition 1 */
-- 
2.31.1

[PULL 00/12] s390x and misc patches

The following changes since commit 1e62a82574fc28e64deca589a23cf55ada2e1a7d:

  Merge tag 'm68k-for-7.1-pull-request' of https://github.com/vivier/qemu-m68k 
into staging (2022-06-02 06:30:24 -0700)

are available in the Git repository at:

  https://gitlab.com/thuth/qemu.git tags/pull-request-2022-06-03

for you to fetch changes up to 707d93d4abc6485c638e2aecd93fbd904f2f6b3e:

  ui: Remove deprecated options "-sdl" and "-curses" (2022-06-03 08:03:28 +0200)


* s390x storage key improvements for KVM
* Some cosmetics for s390x
* Update MAINTAINERS entries
* Improve some spots wrt memory handling in the qtests
* Clean up the "-display sdl" parameter parsing


Alex Bennée (1):
  tests/qtest: use g_autofree for test_server_create_chr

Dr. David Alan Gilbert (1):
  s390: Typo fix FLOATING_POINT_SUPPPORT_ENH

Eric Farman (1):
  MAINTAINERS: Update s390 vhost entries

Gautam Agrawal (1):
  tests/tcg: Test overflow conditions

Hailiang Zhang (1):
  MAINTAINERS: Change my email address

Janis Schoetterl-Glausch (1):
  target/s390x: kvm: Honor storage keys during emulation

Miaoqian Lin (1):
  qtest/npcm7xx_pwm-test: Fix memory leak in mft_qom_set

Thomas Huth (4):
  hw/s390x/s390-virtio-ccw: Improve the machine description string
  ui: Remove deprecated parameters of the "-display sdl" option
  ui: Switch "-display sdl" to use the QAPI parser
  ui: Remove deprecated options "-sdl" and "-curses"

Wenchao Wang (1):
  MAINTAINERS: Update maintainers for Guest x86 HAXM CPUs

 docs/about/deprecated.rst   |  26 
 docs/about/removed-features.rst |  27 
 qapi/ui.json|  26 +++-
 include/sysemu/sysemu.h |   2 -
 target/s390x/cpu_features_def.h.inc |   2 +-
 hw/s390x/s390-virtio-ccw.c  |   2 +-
 softmmu/globals.c   |   2 -
 softmmu/vl.c| 128 +---
 target/s390x/gen-features.c |   6 +-
 target/s390x/kvm/kvm.c  |   9 +++
 target/s390x/tcg/translate.c|   8 +--
 tests/qtest/npcm7xx_pwm-test.c  |   3 +
 tests/qtest/vhost-user-test.c   |   7 +-
 tests/tcg/multiarch/overflow.c  |  58 
 ui/sdl2.c   |  10 +++
 MAINTAINERS |   6 +-
 qemu-options.hx |  56 ++--
 17 files changed, 151 insertions(+), 227 deletions(-)
 create mode 100644 tests/tcg/multiarch/overflow.c

[PULL 05/12] tests/tcg: Test overflow conditions

From: Gautam Agrawal 

Add a test to check for overflow conditions in s390x.
This patch is based on the following patches :
* https://git.qemu.org/?p=qemu.git;a=commitdiff;h=5a2e67a691501
* https://git.qemu.org/?p=qemu.git;a=commitdiff;h=fc6e0d0f2db51

Signed-off-by: Gautam Agrawal 
Message-Id: <20220531183524.40948-1-gautamnagra...@gmail.com>
[thuth: Move overflow.c to tests/tcg/multiarch/ to make it generic]
Signed-off-by: Thomas Huth 
---
 tests/tcg/multiarch/overflow.c | 58 ++
 1 file changed, 58 insertions(+)
 create mode 100644 tests/tcg/multiarch/overflow.c

diff --git a/tests/tcg/multiarch/overflow.c b/tests/tcg/multiarch/overflow.c
new file mode 100644
index 00..1c59c2cb70
--- /dev/null
+++ b/tests/tcg/multiarch/overflow.c
@@ -0,0 +1,58 @@
+#include 
+
+int overflow_add_32(int x, int y)
+{
+int res;
+return __builtin_add_overflow(x, y, &res);
+}
+
+int overflow_add_64(long long x, long long y)
+{
+long long res;
+return __builtin_add_overflow(x, y, &res);
+}
+
+int overflow_sub_32(int x, int y)
+{
+int res;
+return __builtin_sub_overflow(x, y, &res);
+}
+
+int overflow_sub_64(long long x, long long y)
+{
+long long res;
+return __builtin_sub_overflow(x, y, &res);
+}
+
+int a1_add = -2147483648;
+int b1_add = -2147483648;
+long long a2_add = -9223372036854775808ULL;
+long long b2_add = -9223372036854775808ULL;
+
+int a1_sub;
+int b1_sub = -2147483648;
+long long a2_sub = 0L;
+long long b2_sub = -9223372036854775808ULL;
+
+int main()
+{
+int ret = 0;
+
+if (!overflow_add_32(a1_add, b1_add)) {
+fprintf(stderr, "data overflow while adding 32 bits\n");
+ret = 1;
+}
+if (!overflow_add_64(a2_add, b2_add)) {
+fprintf(stderr, "data overflow while adding 64 bits\n");
+ret = 1;
+}
+if (!overflow_sub_32(a1_sub, b1_sub)) {
+fprintf(stderr, "data overflow while subtracting 32 bits\n");
+ret = 1;
+}
+if (!overflow_sub_64(a2_sub, b2_sub)) {
+fprintf(stderr, "data overflow while subtracting 64 bits\n");
+ret = 1;
+}
+return ret;
+}
-- 
2.31.1

[PULL 03/12] target/s390x: kvm: Honor storage keys during emulation

From: Janis Schoetterl-Glausch 

Storage key controlled protection is currently not honored when
emulating instructions.
If available, enable key protection for the MEM_OP ioctl, thereby
enabling it for the s390_cpu_virt_mem_* functions, when using kvm.
As a result, the emulation of the following instructions honors storage
keys:

* CLP
The Synch I/O CLP command would need special handling in order
to support storage keys, but is currently not supported.
* CHSC
Performing commands asynchronously would require special
handling, but commands are currently always synchronous.
* STSI
* TSCH
Must (and does) not change channel if terminated due to
protection.
* MSCH
Suppressed on protection, works because fetching instruction.
* SSCH
Suppressed on protection, works because fetching instruction.
* STSCH
* STCRW
Suppressed on protection, this works because no partial store is
possible, because the operand cannot span multiple pages.
* PCISTB
* MPCIFC
* STPCIFC

Signed-off-by: Janis Schoetterl-Glausch 
Message-Id: <20220506153956.2217601-3-s...@linux.ibm.com>
Signed-off-by: Thomas Huth 
---
 target/s390x/kvm/kvm.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 53098bf541..7bd8db0e7b 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -151,12 +151,15 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] 
= {
 static int cap_sync_regs;
 static int cap_async_pf;
 static int cap_mem_op;
+static int cap_mem_op_extension;
 static int cap_s390_irq;
 static int cap_ri;
 static int cap_hpage_1m;
 static int cap_vcpu_resets;
 static int cap_protected;
 
+static bool mem_op_storage_key_support;
+
 static int active_cmma;
 
 static int kvm_s390_query_mem_limit(uint64_t *memory_limit)
@@ -354,6 +357,8 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
 cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
 cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
+cap_mem_op_extension = kvm_check_extension(s, 
KVM_CAP_S390_MEM_OP_EXTENSION);
+mem_op_storage_key_support = cap_mem_op_extension > 0;
 cap_s390_irq = kvm_check_extension(s, KVM_CAP_S390_INJECT_IRQ);
 cap_vcpu_resets = kvm_check_extension(s, KVM_CAP_S390_VCPU_RESETS);
 cap_protected = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
@@ -842,6 +847,7 @@ int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, 
void *hostbuf,
: KVM_S390_MEMOP_LOGICAL_READ,
 .buf = (uint64_t)hostbuf,
 .ar = ar,
+.key = (cpu->env.psw.mask & PSW_MASK_KEY) >> PSW_SHIFT_KEY,
 };
 int ret;
 
@@ -851,6 +857,9 @@ int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, 
void *hostbuf,
 if (!hostbuf) {
 mem_op.flags |= KVM_S390_MEMOP_F_CHECK_ONLY;
 }
+if (mem_op_storage_key_support) {
+mem_op.flags |= KVM_S390_MEMOP_F_SKEY_PROTECTION;
+}
 
 ret = kvm_vcpu_ioctl(CPU(cpu), KVM_S390_MEM_OP, &mem_op);
 if (ret < 0) {
-- 
2.31.1

[PULL 12/12] ui: Remove deprecated options "-sdl" and "-curses"

We have "-sdl" and "-curses", but no "-gtk" and no "-cocoa" ...
these old-style options are rather confusing than helpful nowadays.
Now that the deprecation period is over, let's remove them, so we
get a cleaner interface (where "-display" is the only way to select
the user interface).

Message-Id: <20220519155625.1414365-4-th...@redhat.com>
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst   | 10 --
 docs/about/removed-features.rst | 10 ++
 softmmu/vl.c| 19 ---
 qemu-options.hx | 24 ++--
 4 files changed, 12 insertions(+), 51 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 562a133f18..e19bcba242 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -81,16 +81,6 @@ the process listing. This is replaced by the new 
``password-secret``
 option which lets the password be securely provided on the command
 line using a ``secret`` object instance.
 
-``-sdl`` (since 6.2)
-
-
-Use ``-display sdl`` instead.
-
-``-curses`` (since 6.2)
-'''
-
-Use ``-display curses`` instead.
-
 ``-watchdog`` (since 6.2)
 '
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 4c9e001c35..c7b9dadd5d 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -386,6 +386,16 @@ Use ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
 
 Use ``-display sdl,grab-mod=rctrl`` instead.
 
+``-sdl`` (removed in 7.1)
+'
+
+Use ``-display sdl`` instead.
+
+``-curses`` (removed in 7.1)
+
+
+Use ``-display curses`` instead.
+
 
 QEMU Machine Protocol (QMP) commands
 
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 484e9d9921..4c1e94b00e 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2800,16 +2800,6 @@ void qemu_init(int argc, char **argv, char **envp)
 nographic = true;
 dpy.type = DISPLAY_TYPE_NONE;
 break;
-case QEMU_OPTION_curses:
-warn_report("-curses is deprecated, "
-"use -display curses instead.");
-#ifdef CONFIG_CURSES
-dpy.type = DISPLAY_TYPE_CURSES;
-#else
-error_report("curses or iconv support is disabled");
-exit(1);
-#endif
-break;
 case QEMU_OPTION_portrait:
 graphic_rotate = 90;
 break;
@@ -3176,15 +3166,6 @@ void qemu_init(int argc, char **argv, char **envp)
 dpy.has_full_screen = true;
 dpy.full_screen = true;
 break;
-case QEMU_OPTION_sdl:
-warn_report("-sdl is deprecated, use -display sdl instead.");
-#ifdef CONFIG_SDL
-dpy.type = DISPLAY_TYPE_SDL;
-break;
-#else
-error_report("SDL support is disabled");
-exit(1);
-#endif
 case QEMU_OPTION_pidfile:
 pid_file = optarg;
 break;
diff --git a/qemu-options.hx b/qemu-options.hx
index 726e437a97..60cf188da4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1981,9 +1981,8 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
 , QEMU_ARCH_ALL)
 SRST
 ``-display type``
-Select type of display to use. This option is a replacement for the
-old style -sdl/-curses/... options. Use ``-display help`` to list
-the available display types. Valid values for type are
+Select type of display to use. Use ``-display help`` to list the available
+display types. Valid values for type are
 
 ``spice-app[,gl=on|off]``
 Start QEMU as a Spice server and launch the default Spice client
@@ -2085,25 +2084,6 @@ SRST
 Use C-a h for help on switching between the console and monitor.
 ERST
 
-DEF("curses", 0, QEMU_OPTION_curses,
-"-curses shorthand for -display curses\n",
-QEMU_ARCH_ALL)
-SRST
-``-curses``
-Normally, if QEMU is compiled with graphical window support, it
-displays output such as guest graphics, guest console, and the QEMU
-monitor in a window. With this option, QEMU can display the VGA
-output when in text mode using a curses/ncurses interface. Nothing
-is displayed in graphical mode.
-ERST
-
-DEF("sdl", 0, QEMU_OPTION_sdl,
-"-sdlshorthand for -display sdl\n", QEMU_ARCH_ALL)
-SRST
-``-sdl``
-Enable SDL.
-ERST
-
 #ifdef CONFIG_SPICE
 DEF("spice", HAS_ARG, QEMU_OPTION_spice,
 "-spice [port=port][,tls-port=secured-port][,x509-dir=]\n"
-- 
2.31.1

[PULL 06/12] MAINTAINERS: Change my email address

From: Hailiang Zhang 

The zhang.zhanghaili...@huawei.com email address has been
stopped. Change it to my new email address.

Signed-off-by: Hailiang Zhang 
Message-Id: <20211214075424.6920-1-zhanghaili...@xfusion.com>
Acked-by: Gonglei 
Acked-by: Zhang Chen 
Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7b7d3f9c02..ee9693dc3a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3102,7 +3102,7 @@ F: include/qemu/yank.h
 F: qapi/yank.json
 
 COLO Framework
-M: zhanghailiang 
+M: Hailiang Zhang 
 S: Maintained
 F: migration/colo*
 F: include/migration/colo.h
-- 
2.31.1

[PULL 02/12] hw/s390x/s390-virtio-ccw: Improve the machine description string

The machine name already contains the words "ccw" and "virtio", so
using "VirtIO-ccw" in the description likely does not really help
the average user to get an idea what this machine type is about.
Thus let's switch to "Virtual s390x machine" now, since "virtual
machine" should be a familiar term, and "s390x" signals that this
is about 64-bit guests (unlike S390 which could mean that it is
31-bit only).
Also expand "v" to "version", since this makes it easier to use
this macro also with non-numeric machine names in downstream.

Message-Id: <20220506065026.513590-1-th...@redhat.com>
Acked-by: Cornelia Huck 
Signed-off-by: Thomas Huth 
---
 hw/s390x/s390-virtio-ccw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 047cca0487..cc3097bfee 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -768,7 +768,7 @@ bool css_migration_enabled(void)
 { \
 MachineClass *mc = MACHINE_CLASS(oc); \
 ccw_machine_##suffix##_class_options(mc); \
-mc->desc = "VirtIO-ccw based S390 machine v" verstr;  \
+mc->desc = "Virtual s390x machine (version " verstr ")";  \
 if (latest) { \
 mc->alias = "s390-ccw-virtio";\
 mc->is_default = true;\
-- 
2.31.1

[PULL 04/12] MAINTAINERS: Update s390 vhost entries

From: Eric Farman 

Commit 7a523d96a0 ("virtio-ccw: move vhost_ccw_scsi to a separate file")
introduced a new file hw/s390x/vhost-scsi-ccw.c, which received a
couple comments [1][2] to update MAINTAINERS that were missed.

Fix that by making the vhost CCW entries a wildcard.

[1] 
https://lore.kernel.org/r/d8d2bbd5021076bdba444d31a6da74f507baede3.ca...@linux.ibm.com/
[2] https://lore.kernel.org/r/87k0c4gb9f@redhat.com/

Signed-off-by: Eric Farman 
Reviewed-by: Cornelia Huck 
Message-Id: <20220525145814.2750501-1-far...@linux.ibm.com>
Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 00dc4a8ecb..7b7d3f9c02 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2018,8 +2018,7 @@ M: Halil Pasic 
 M: Eric Farman 
 S: Supported
 F: hw/s390x/virtio-ccw*.[hc]
-F: hw/s390x/vhost-vsock-ccw.c
-F: hw/s390x/vhost-user-fs-ccw.c
+F: hw/s390x/vhost-*-ccw.c
 T: git https://gitlab.com/cohuck/qemu.git s390-next
 T: git https://github.com/borntraeger/qemu.git s390-next
 L: qemu-s3...@nongnu.org
-- 
2.31.1

[PULL 10/12] ui: Remove deprecated parameters of the "-display sdl" option

Dropping these deprecated parameters simplifies further refactoring
(e.g. QAPIfication is easier without underscores in the name).

Message-Id: <20220519155625.1414365-2-th...@redhat.com>
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Markus Armbruster 
Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst   | 16 -
 docs/about/removed-features.rst | 17 ++
 softmmu/vl.c| 41 +
 qemu-options.hx | 32 ++---
 4 files changed, 20 insertions(+), 86 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index a92ae0f162..562a133f18 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -81,22 +81,6 @@ the process listing. This is replaced by the new 
``password-secret``
 option which lets the password be securely provided on the command
 line using a ``secret`` object instance.
 
-``-display sdl,window_close=...`` (since 6.1)
-'
-
-Use ``-display sdl,window-close=...`` instead (i.e. with a minus instead of
-an underscore between "window" and "close").
-
-``-alt-grab`` and ``-display sdl,alt_grab=on`` (since 6.2)
-''
-
-Use ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
-
-``-ctrl-grab`` and ``-display sdl,ctrl_grab=on`` (since 6.2)
-
-
-Use ``-display sdl,grab-mod=rctrl`` instead.
-
 ``-sdl`` (since 6.2)
 
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index eb76974347..4c9e001c35 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -370,6 +370,23 @@ The ``opened=on`` option in the command line or QMP 
``object-add`` either had
 no effect (if ``opened`` was the last option) or caused errors.  The property
 is therefore useless and should simply be removed.
 
+``-display sdl,window_close=...`` (removed in 7.1)
+''
+
+Use ``-display sdl,window-close=...`` instead (i.e. with a minus instead of
+an underscore between "window" and "close").
+
+``-alt-grab`` and ``-display sdl,alt_grab=on`` (removed in 7.1)
+'''
+
+Use ``-display sdl,grab-mod=lshift-lctrl-lalt`` instead.
+
+``-ctrl-grab`` and ``-display sdl,ctrl_grab=on`` (removed in 7.1)
+'
+
+Use ``-display sdl,grab-mod=rctrl`` instead.
+
+
 QEMU Machine Protocol (QMP) commands
 
 
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 84a31eba76..57ab9d5322 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1079,32 +1079,7 @@ static void parse_display(const char *p)
 } else {
 goto invalid_sdl_args;
 }
-} else if (strstart(opts, ",alt_grab=", &nextopt)) {
-opts = nextopt;
-if (strstart(opts, "on", &nextopt)) {
-alt_grab = 1;
-} else if (strstart(opts, "off", &nextopt)) {
-alt_grab = 0;
-} else {
-goto invalid_sdl_args;
-}
-warn_report("alt_grab is deprecated, use grab-mod instead.");
-} else if (strstart(opts, ",ctrl_grab=", &nextopt)) {
-opts = nextopt;
-if (strstart(opts, "on", &nextopt)) {
-ctrl_grab = 1;
-} else if (strstart(opts, "off", &nextopt)) {
-ctrl_grab = 0;
-} else {
-goto invalid_sdl_args;
-}
-warn_report("ctrl_grab is deprecated, use grab-mod instead.");
-} else if (strstart(opts, ",window_close=", &nextopt) ||
-   strstart(opts, ",window-close=", &nextopt)) {
-if (strstart(opts, ",window_close=", NULL)) {
-warn_report("window_close with an underscore is 
deprecated,"
-" please use window-close instead.");
-}
+} else if (strstart(opts, ",window-close=", &nextopt)) {
 opts = nextopt;
 dpy.has_window_close = true;
 if (strstart(opts, "on", &nextopt)) {
@@ -1962,10 +1937,6 @@ static void qemu_create_early_backends(void)
 const bool use_gtk = false;
 #endif
 
-if ((alt_grab || ctrl_grab) && !use_sdl) {
-error_report("-alt-grab and -ctrl-grab are only valid "
- "for SDL, ignoring option");
-}
 if (dpy.has_window_close && !use_gtk && !use_sdl) {
 error_report("window-close is only valid for GTK and SDL, "
  "ignoring option");
@@ -3273,16 +3244,6 @@ void qemu_init(int argc, char **argv, char **envp)
 dpy.has_full_sc

[PULL 07/12] MAINTAINERS: Update maintainers for Guest x86 HAXM CPUs

From: Wenchao Wang 

Clean up the maintainer list.

Reviewed-by: Hang Yuan 
Signed-off-by: Wenchao Wang 
Message-Id: 

[thuth: Note: Colin Xu's address bounces]
Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index ee9693dc3a..5fe8f7eca2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -488,7 +488,6 @@ Guest CPU Cores (HAXM)
 -
 X86 HAXM CPUs
 M: Wenchao Wang 
-M: Colin Xu 
 L: haxm-t...@intel.com
 W: https://github.com/intel/haxm/issues
 S: Maintained
-- 
2.31.1

[PULL 08/12] qtest/npcm7xx_pwm-test: Fix memory leak in mft_qom_set

From: Miaoqian Lin 

g_strdup_printf() allocated memory for path, we should free it with
g_free() when no longer needed.

Signed-off-by: Miaoqian Lin 
Reviewed-by: Hao Wu 
Message-Id: <20220531080921.4704-1-linmq...@gmail.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/npcm7xx_pwm-test.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qtest/npcm7xx_pwm-test.c b/tests/qtest/npcm7xx_pwm-test.c
index c4a5fdcacd..e320a625c4 100644
--- a/tests/qtest/npcm7xx_pwm-test.c
+++ b/tests/qtest/npcm7xx_pwm-test.c
@@ -268,6 +268,9 @@ static void mft_qom_set(QTestState *qts, int index, const 
char *name,
 path, name, value);
 /* The qom set message returns successfully. */
 g_assert_true(qdict_haskey(response, "return"));
+
+qobject_unref(response);
+g_free(path);
 }
 
 static uint32_t get_pll(uint32_t con)
-- 
2.31.1

[PULL 11/12] ui: Switch "-display sdl" to use the QAPI parser

The "-display sdl" option still uses a hand-crafted parser for its
parameters since we didn't want to drag an interface we considered
somewhat flawed into the QAPI schema. Since the flaws are gone now,
it's time to QAPIfy.

This introduces the new "DisplaySDL" QAPI struct that is used to hold
the parameters that are unique to the SDL display. The only specific
parameter is currently "grab-mod" that is used to specify the required
modifier keys to escape from the mouse grabbing mode.

Message-Id: <20220519155625.1414365-3-th...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Thomas Huth 
---
 qapi/ui.json| 26 ++-
 include/sysemu/sysemu.h |  2 --
 softmmu/globals.c   |  2 --
 softmmu/vl.c| 70 +
 ui/sdl2.c   | 10 ++
 5 files changed, 36 insertions(+), 74 deletions(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 11a827d10f..413371d5e8 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1295,6 +1295,29 @@
   '*swap-opt-cmd': 'bool'
   } }
 
+##
+# @HotKeyMod:
+#
+# Set of modifier keys that need to be held for shortcut key actions.
+#
+# Since: 7.1
+##
+{ 'enum'  : 'HotKeyMod',
+  'data'  : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
+
+##
+# @DisplaySDL:
+#
+# SDL2 display options.
+#
+# @grab-mod:  Modifier keys that should be pressed together with the
+# "G" key to release the mouse grab.
+#
+# Since: 7.1
+##
+{ 'struct'  : 'DisplaySDL',
+  'data': { '*grab-mod'   : 'HotKeyMod' } }
+
 ##
 # @DisplayType:
 #
@@ -1374,7 +1397,8 @@
   'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
   'egl-headless': { 'type': 'DisplayEGLHeadless',
 'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
-  'dbus': { 'type': 'DisplayDBus', 'if': 'CONFIG_DBUS_DISPLAY' }
+  'dbus': { 'type': 'DisplayDBus', 'if': 'CONFIG_DBUS_DISPLAY' },
+  'sdl': { 'type': 'DisplaySDL', 'if': 'CONFIG_SDL' }
   }
 }
 
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b4030acd74..812f66a31a 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -42,8 +42,6 @@ extern int graphic_depth;
 extern int display_opengl;
 extern const char *keyboard_layout;
 extern int win2k_install_hack;
-extern int alt_grab;
-extern int ctrl_grab;
 extern int graphic_rotate;
 extern int old_param;
 extern uint8_t *boot_splash_filedata;
diff --git a/softmmu/globals.c b/softmmu/globals.c
index 916bc12e2b..527edbefdd 100644
--- a/softmmu/globals.c
+++ b/softmmu/globals.c
@@ -50,8 +50,6 @@ QEMUOptionRom option_rom[MAX_OPTION_ROMS];
 int nb_option_roms;
 int old_param;
 const char *qemu_name;
-int alt_grab;
-int ctrl_grab;
 unsigned int nb_prom_envs;
 const char *prom_envs[MAX_PROM_ENVS];
 uint8_t *boot_splash_filedata;
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 57ab9d5322..484e9d9921 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1056,75 +1056,7 @@ static void parse_display(const char *p)
 exit(0);
 }
 
-if (strstart(p, "sdl", &opts)) {
-/*
- * sdl DisplayType needs hand-crafted parser instead of
- * parse_display_qapi() due to some options not in
- * DisplayOptions, specifically:
- *   - ctrl_grab + alt_grab
- * They can't be moved into the QAPI since they use underscores,
- * thus they will get replaced by "grab-mod" in the long term
- */
-#if defined(CONFIG_SDL)
-dpy.type = DISPLAY_TYPE_SDL;
-while (*opts) {
-const char *nextopt;
-
-if (strstart(opts, ",grab-mod=", &nextopt)) {
-opts = nextopt;
-if (strstart(opts, "lshift-lctrl-lalt", &nextopt)) {
-alt_grab = 1;
-} else if (strstart(opts, "rctrl", &nextopt)) {
-ctrl_grab = 1;
-} else {
-goto invalid_sdl_args;
-}
-} else if (strstart(opts, ",window-close=", &nextopt)) {
-opts = nextopt;
-dpy.has_window_close = true;
-if (strstart(opts, "on", &nextopt)) {
-dpy.window_close = true;
-} else if (strstart(opts, "off", &nextopt)) {
-dpy.window_close = false;
-} else {
-goto invalid_sdl_args;
-}
-} else if (strstart(opts, ",show-cursor=", &nextopt)) {
-opts = nextopt;
-dpy.has_show_cursor = true;
-if (strstart(opts, "on", &nextopt)) {
-dpy.show_cursor = true;
-} else if (strstart(opts, "off", &nextopt)) {
-dpy.show_cursor = false;
-} else {
-goto invalid_sdl_args;
-}
-} else if (strstart(opts, ",gl=", &nextopt)) {
-opts = nextopt;
-dpy.has_gl = true;
-if (st

Re: [RFC PATCH 3/3] hw/openrisc: Add the OpenRISC virtual machine

2022-06-03 Thread Geert Uytterhoeven

Hi Stafford,

On Thu, Jun 2, 2022 at 9:59 PM Stafford Horne  wrote:
> On Thu, Jun 02, 2022 at 09:08:52PM +0200, Geert Uytterhoeven wrote:
> > On Thu, Jun 2, 2022 at 1:42 PM Joel Stanley  wrote:
> > > On Fri, 27 May 2022 at 17:27, Stafford Horne  wrote:
> > > > This patch add the OpenRISC virtual machine 'virt' for OpenRISC.  This
> > > > platform allows for a convenient CI platform for toolchain, software
> > > > ports and the OpenRISC linux kernel port.
> > > >
> > > > Much of this has been sourced from the m68k and riscv virt platforms.
> >
> > > I enabled the options:
> > >
> > > CONFIG_RTC_CLASS=y
> > > # CONFIG_RTC_SYSTOHC is not set
> > > # CONFIG_RTC_NVMEM is not set
> > > CONFIG_RTC_DRV_GOLDFISH=y
> > >
> > > But it didn't work. It seems the goldfish rtc model doesn't handle a
> > > big endian guest running on my little endian host.
> > >
> > > Doing this fixes it:
> > >
> > > -.endianness = DEVICE_NATIVE_ENDIAN,
> > > +.endianness = DEVICE_HOST_ENDIAN,
> > >
> > > [0.19] goldfish_rtc 96005000.rtc: registered as rtc0
> > > [0.19] goldfish_rtc 96005000.rtc: setting system clock to
> > > 2022-06-02T11:16:04 UTC (1654168564)
> > >
> > > But literally no other model in the tree does this, so I suspect it's
> > > not the right fix.
> >
> > Goldfish devices are supposed to be little endian.
> > Unfortunately m68k got this wrong, cfr.
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2e2ac4a3327479f7e2744cdd88a5c823f2057bad
> > Please don't duplicate this bad behavior for new architectures
>
> Thanks for the pointer, I just wired in the goldfish RTC because I wanted to
> play with it.  I was not attached to it. I can either remove it our find 
> another
> RTC.

Sorry for being too unclear: the mistake was not to use the Goldfish
RTC, but to make its register accesses big-endian.
Using Goldfish devices as little-endian devices should be fine.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [RFC PATCH v2 0/6] hw/i2c: i2c slave mode support

2022-06-03 Thread Cédric Le Goater


With this combined I am able to boot up Linux on an emulated Aspeed 2600
evaluation board and have the i2c echo device write into a Linux slave
EEPROM. Assuming the echo device is on address 0x42:

# echo slave-24c02 0x1064 > /sys/bus/i2c/devices/i2c-15/new_device
i2c i2c-15: new_device: Instantiated device slave-24c02 at 0x64
# i2cset -y 15 0x42 0x64 0x00 0xaa i
# hexdump /sys/bus/i2c/devices/15-1064/slave-eeprom
000 ffaa       
010        
*
100


I have started working on buildroot images  :

   https://github.com/legoater/buildroot/commits/aspeed

The resulting files are quite small :

 $ ll output/images/
 total 86040
 drwxr-xr-x 2 legoater legoater 4096 Jun  1 20:01 ./
 drwxrwxr-x 6 legoater legoater 4096 Jun  1 19:40 ../
 -rwxr-xr-x 1 legoater legoater36837 Jun  1 20:01 
aspeed-ast2600-evb.dtb*
 -rw-r--r-- 1 legoater legoater 67108864 Jun  1 20:01 flash.img
 -rw-r--r-- 1 legoater legoater  6682796 Jun  1 20:01 image.itb
 -rw-r--r-- 1 legoater legoater 1846 Jun  1 20:01 image.its
 -rw-r--r-- 1 legoater legoater  3168768 Jun  1 20:01 rootfs.cpio
 -rw-r--r-- 1 legoater legoater  1026660 Jun  1 20:01 rootfs.cpio.xz
 -rw-r--r-- 1 legoater legoater  3788800 Jun  1 20:01 rootfs.tar
 -rw-r--r-- 1 legoater legoater   653777 Jun  1 20:00 u-boot.bin
 -rw-r--r-- 1 legoater legoater  5617280 Jun  1 20:01 zImage

I will probably host them on GH and we could use them under avocado
to extend the tests.


They should boot real HW. I will submit the defconfigs to buildroot
after more tests and cleanups.



Nice!


Uploaded here :

https://github.com/legoater/qemu-aspeed-boot/tree/master/images/ast2600-evb/buildroot-2022.05-rc2


C.

Changes for building bits on newer gcc 9.4 compiler

2022-06-03 Thread Ani Sinha

Hi josh :
Here are the pull requests. Please feel free to review and merge:

Main bits module:
https://github.com/biosbits/bits/pull/13

Submodules:
https://github.com/biosbits/grub/pull/1
https://github.com/biosbits/python/pull/1
https://github.com/biosbits/libffi/pull/1
https://github.com/biosbits/fdlibm/pull/1

Thanks
ani

Re: [PATCH 1/1] hw: m25p80: add W# pin and SRWD bit for write protection

2022-06-03 Thread Cédric Le Goater


Hello Iris,

On 5/26/22 04:12, Iris Chen wrote:

From: Iris Chen 

Add the W# pin and SRWD bit which control the status register write
ability.


may be replace W# by WP# (for write protect)



Signed-off-by: Iris Chen 
---
  hw/block/m25p80.c | 72 +++
  tests/qtest/aspeed_smc-test.c | 62 ++
  2 files changed, 134 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 81ba3da4df..c845fa08d4 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -27,12 +27,14 @@
  #include "hw/qdev-properties.h"
  #include "hw/qdev-properties-system.h"
  #include "hw/ssi/ssi.h"
+#include "hw/irq.h"
  #include "migration/vmstate.h"
  #include "qemu/bitops.h"
  #include "qemu/log.h"
  #include "qemu/module.h"
  #include "qemu/error-report.h"
  #include "qapi/error.h"
+#include "qapi/visitor.h"
  #include "trace.h"
  #include "qom/object.h"
  
@@ -472,11 +474,13 @@ struct Flash {

  uint8_t spansion_cr2v;
  uint8_t spansion_cr3v;
  uint8_t spansion_cr4v;
+bool write_protect_pin;


I would call this attribute 'wp_level' since it is an assertion level
of the WP# pin.


  bool write_enable;
  bool four_bytes_address_mode;
  bool reset_enable;
  bool quad_enable;
  bool aai_enable;
+bool status_register_write_disabled;
  uint8_t ear;
  
  int64_t dirty_page;

@@ -723,6 +727,21 @@ static void complete_collecting_data(Flash *s)
  flash_erase(s, s->cur_addr, s->cmd_in_progress);
  break;
  case WRSR:
+/*
+ * If W# is low and status_register_write_disabled is high,
+ * status register writes are disabled.
+ * This is also called "hardware protected mode" (HPM). All other
+ * combinations of the two states are called "software protected mode"
+ * (SPM), and status register writes are permitted.
+ */
+if ((s->write_protect_pin == 0 && s->status_register_write_disabled)
+|| !s->write_enable) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "M25P80: Status register write is disabled!\n");
+break;
+}
+s->status_register_write_disabled = extract32(s->data[0], 7, 1);
+
  switch (get_man(s)) {
  case MAN_SPANSION:
  s->quad_enable = !!(s->data[1] & 0x02);
@@ -1195,6 +1214,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
  
  case RDSR:

  s->data[0] = (!!s->write_enable) << 1;
+s->data[0] |= (!!s->status_register_write_disabled) << 7;
+
  if (get_man(s) == MAN_MACRONIX || get_man(s) == MAN_ISSI) {
  s->data[0] |= (!!s->quad_enable) << 6;
  }
@@ -1484,6 +1505,15 @@ static uint32_t m25p80_transfer8(SSIPeripheral *ss, 
uint32_t tx)
  return r;
  }
  
+static void m25p80_write_protect_pin_irq_handler(void *opaque, int n, int level)

+{
+Flash *s = M25P80(opaque);
+bool wp = !!level;


this extra variable looks superfluous. minor


+/* W# is just a single pin. */
+assert(n == 0);
+s->write_protect_pin = wp;
+}
+
  static void m25p80_realize(SSIPeripheral *ss, Error **errp)
  {
  Flash *s = M25P80(ss);
@@ -1515,12 +1545,18 @@ static void m25p80_realize(SSIPeripheral *ss, Error 
**errp)
  s->storage = blk_blockalign(NULL, s->size);
  memset(s->storage, 0xFF, s->size);
  }
+
+qdev_init_gpio_in_named(DEVICE(s),
+m25p80_write_protect_pin_irq_handler, "W#", 1);
  }
  
  static void m25p80_reset(DeviceState *d)

  {
  Flash *s = M25P80(d);
  
+s->write_protect_pin = true;

+s->status_register_write_disabled = false;


Are we ok with these defaults ? We wouldn't want to break existing
implementation.



  reset_memory(s);
  }
  
@@ -1601,6 +1637,7 @@ static const VMStateDescription vmstate_m25p80 = {

  VMSTATE_UINT8(needed_bytes, Flash),
  VMSTATE_UINT8(cmd_in_progress, Flash),
  VMSTATE_UINT32(cur_addr, Flash),
+VMSTATE_BOOL(write_protect_pin, Flash),


and what about status_register_write_disabled ?


  VMSTATE_BOOL(write_enable, Flash),
  VMSTATE_BOOL(reset_enable, Flash),
  VMSTATE_UINT8(ear, Flash),
@@ -1622,6 +1659,38 @@ static const VMStateDescription vmstate_m25p80 = {
  }
  };
  
+static void m25p80_get_write_protect_pin(Object *obj,

+   Visitor *v,
+   const char *name,
+   void *opaque,
+   Error **errp)
+{
+Flash *s = M25P80(obj);
+bool value;
+
+value = s->write_protect_pin;
+
+visit_type_bool(v, name, &value, errp);
+}
+
+static void m25p80_set_write_protect_pin(Object *obj,
+   Visitor *v,
+   const char *name,
+   void *opaque,
+

[PATCH] microvm: turn off io reservations for pcie root ports

2022-06-03 Thread Gerd Hoffmann

The pcie host bridge has no io window on microvm,
so io reservations will not work.

Signed-off-by: Gerd Hoffmann 
---
 hw/i386/microvm.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 4b3b1dd262f1..f01d972f5d28 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -757,6 +757,12 @@ static void microvm_class_init(ObjectClass *oc, void *data)
 "Set off to disable adding virtio-mmio devices to the kernel cmdline");
 
 machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
+
+/*
+ * pcie host bridge (gpex) on microvm has no io address window,
+ * so reserving io space is not going to work.  Turn it off.
+ */
+object_register_sugar_prop("pcie-root-port", "io-reserve", "0", true);
 }
 
 static const TypeInfo microvm_machine_info = {
-- 
2.36.1

[PATCH] tests/avocado: Prefer max cpu type when using AArch64 virt machine

The max cpu type is the best default cpu type for tests to use
when specifying the cpu type for AArch64 mach-virt. Switch all
tests to it.

Cc: Alex Bennée 
Signed-off-by: Andrew Jones 
---
 tests/avocado/replay_kernel.py | 2 +-
 tests/avocado/reverse_debugging.py | 2 +-
 tests/avocado/tcg_plugins.py   | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py
index 0b2b0dc692b1..c19022ea977d 100644
--- a/tests/avocado/replay_kernel.py
+++ b/tests/avocado/replay_kernel.py
@@ -147,7 +147,7 @@ def test_aarch64_virt(self):
 """
 :avocado: tags=arch:aarch64
 :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
 """
 kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
   '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
diff --git a/tests/avocado/reverse_debugging.py 
b/tests/avocado/reverse_debugging.py
index d2921e70c3b4..d6a6d7277235 100644
--- a/tests/avocado/reverse_debugging.py
+++ b/tests/avocado/reverse_debugging.py
@@ -198,7 +198,7 @@ def test_aarch64_virt(self):
 """
 :avocado: tags=arch:aarch64
 :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
 """
 kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
   '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
diff --git a/tests/avocado/tcg_plugins.py b/tests/avocado/tcg_plugins.py
index 642d2e49e305..2bbf62f5036e 100644
--- a/tests/avocado/tcg_plugins.py
+++ b/tests/avocado/tcg_plugins.py
@@ -68,7 +68,7 @@ def test_aarch64_virt_insn(self):
 :avocado: tags=accel:tcg
 :avocado: tags=arch:aarch64
 :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
 """
 kernel_path = self._grab_aarch64_kernel()
 kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
@@ -94,7 +94,7 @@ def test_aarch64_virt_insn_icount(self):
 :avocado: tags=accel:tcg
 :avocado: tags=arch:aarch64
 :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
 """
 kernel_path = self._grab_aarch64_kernel()
 kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
@@ -120,7 +120,7 @@ def test_aarch64_virt_mem_icount(self):
 :avocado: tags=accel:tcg
 :avocado: tags=arch:aarch64
 :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
 """
 kernel_path = self._grab_aarch64_kernel()
 kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
-- 
2.34.3

Re: [PATCH] tests/avocado: Prefer max cpu type when using AArch64 virt machine

On Fri, Jun 03, 2022 at 11:25:05AM +0200, Andrew Jones wrote:
> The max cpu type is the best default cpu type for tests to use
> when specifying the cpu type for AArch64 mach-virt. Switch all
> tests to it.

Hmm, looking at tests in tests/qtest and tests/vm I see cortex-a57
is still used for a default choice. I should probably post another
patch changing those to max as well.

Thanks,
drew

> 
> Cc: Alex Bennée 
> Signed-off-by: Andrew Jones 
> ---
>  tests/avocado/replay_kernel.py | 2 +-
>  tests/avocado/reverse_debugging.py | 2 +-
>  tests/avocado/tcg_plugins.py   | 6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py
> index 0b2b0dc692b1..c19022ea977d 100644
> --- a/tests/avocado/replay_kernel.py
> +++ b/tests/avocado/replay_kernel.py
> @@ -147,7 +147,7 @@ def test_aarch64_virt(self):
>  """
>  :avocado: tags=arch:aarch64
>  :avocado: tags=machine:virt
> -:avocado: tags=cpu:cortex-a53
> +:avocado: tags=cpu:max
>  """
>  kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
>
> '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
> diff --git a/tests/avocado/reverse_debugging.py 
> b/tests/avocado/reverse_debugging.py
> index d2921e70c3b4..d6a6d7277235 100644
> --- a/tests/avocado/reverse_debugging.py
> +++ b/tests/avocado/reverse_debugging.py
> @@ -198,7 +198,7 @@ def test_aarch64_virt(self):
>  """
>  :avocado: tags=arch:aarch64
>  :avocado: tags=machine:virt
> -:avocado: tags=cpu:cortex-a53
> +:avocado: tags=cpu:max
>  """
>  kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
>
> '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
> diff --git a/tests/avocado/tcg_plugins.py b/tests/avocado/tcg_plugins.py
> index 642d2e49e305..2bbf62f5036e 100644
> --- a/tests/avocado/tcg_plugins.py
> +++ b/tests/avocado/tcg_plugins.py
> @@ -68,7 +68,7 @@ def test_aarch64_virt_insn(self):
>  :avocado: tags=accel:tcg
>  :avocado: tags=arch:aarch64
>  :avocado: tags=machine:virt
> -:avocado: tags=cpu:cortex-a53
> +:avocado: tags=cpu:max
>  """
>  kernel_path = self._grab_aarch64_kernel()
>  kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> @@ -94,7 +94,7 @@ def test_aarch64_virt_insn_icount(self):
>  :avocado: tags=accel:tcg
>  :avocado: tags=arch:aarch64
>  :avocado: tags=machine:virt
> -:avocado: tags=cpu:cortex-a53
> +:avocado: tags=cpu:max
>  """
>  kernel_path = self._grab_aarch64_kernel()
>  kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> @@ -120,7 +120,7 @@ def test_aarch64_virt_mem_icount(self):
>  :avocado: tags=accel:tcg
>  :avocado: tags=arch:aarch64
>  :avocado: tags=machine:virt
> -:avocado: tags=cpu:cortex-a53
> +:avocado: tags=cpu:max
>  """
>  kernel_path = self._grab_aarch64_kernel()
>  kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> -- 
> 2.34.3
>

Re: [PATCH] tests/avocado: Prefer max cpu type when using AArch64 virt machine

2022-06-03 Thread Cornelia Huck

On Fri, Jun 03 2022, Andrew Jones  wrote:

> The max cpu type is the best default cpu type for tests to use
> when specifying the cpu type for AArch64 mach-virt. Switch all
> tests to it.
>
> Cc: Alex Bennée 
> Signed-off-by: Andrew Jones 
> ---
>  tests/avocado/replay_kernel.py | 2 +-
>  tests/avocado/reverse_debugging.py | 2 +-
>  tests/avocado/tcg_plugins.py   | 6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)

Is there anything in the boot_xen tests that requires cortex-a57, or
should they be switched to max as well?

Re: [PATCH] tests/avocado: Prefer max cpu type when using AArch64 virt machine

On Fri, Jun 03, 2022 at 12:07:13PM +0200, Cornelia Huck wrote:
> On Fri, Jun 03 2022, Andrew Jones  wrote:
> 
> > The max cpu type is the best default cpu type for tests to use
> > when specifying the cpu type for AArch64 mach-virt. Switch all
> > tests to it.
> >
> > Cc: Alex Bennée 
> > Signed-off-by: Andrew Jones 
> > ---
> >  tests/avocado/replay_kernel.py | 2 +-
> >  tests/avocado/reverse_debugging.py | 2 +-
> >  tests/avocado/tcg_plugins.py   | 6 +++---
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> Is there anything in the boot_xen tests that requires cortex-a57, or
> should they be switched to max as well?
>

You're right. I was ignoring xen tests for no good reason. They're
simply using TCG mach-virt with a "default" cpu type too.

I'll spin a v2 that changes everything in tests/ which uses mach-
virt and appears to be want a default cpu type.

Thanks,
drew

Re: [PATCH v2 00/10] Random cleanup patches

On Sat, May 21, 2022 at 11:55 AM Mark Cave-Ayland <
mark.cave-ayl...@ilande.co.uk> wrote:

> On 20/05/2022 19:00, Bernhard Beschow wrote:
>
> > v2:
> > * Omit removal of isa_connect_gpio_out() (Mark)
> >
> > v1:
> > This patch series contains random cleanups that I made while studying
> the code.
> >
> > Bernhard Beschow (10):
> >hw: Reuse TYPE_I8042 define
> >hw/audio/cs4231a: Const'ify global tables
> >hw/i386/pc: Unexport PC_CPU_MODEL_IDS macro
> >hw/i386/pc: Unexport functions used only internally
> >hw/i386/pc: Remove orphan declarations
> >hw/ppc/e500: Remove unused BINARY_DEVICE_TREE_FILE
> >hw/net/fsl_etsec/etsec: Remove obsolete and unused etsec_create()
> >accel/tcg/cpu-exec: Unexport dump_drift_info()
> >accel/tcg: Inline dump_opcount_info() and remove it
> >docs/devel: Fix link to developer mailing lists
> >
> >   accel/tcg/cpu-exec.c  |  4 ++--
> >   accel/tcg/translate-all.c |  5 -
> >   docs/devel/submitting-a-patch.rst |  6 +++---
> >   hw/audio/cs4231a.c|  8 
> >   hw/i386/pc.c  | 17 +
> >   hw/net/fsl_etsec/etsec.c  | 23 ---
> >   hw/net/fsl_etsec/etsec.h  |  7 ---
> >   hw/ppc/e500.c |  1 -
> >   hw/sparc64/sun4u.c|  2 +-
> >   include/exec/cpu-all.h|  3 ---
> >   include/hw/i386/pc.h  | 14 --
> >   11 files changed, 23 insertions(+), 67 deletions(-)
>
> In general these changes look okay, so I'd be fine to give an:
>
> Acked-by: Mark Cave-Ayland 
>
> for those I haven't already given a Reviewed-by tag for.
>
> Laurent, are you happy to take these patches with their current tags via
> qemu-trivial? Or would you prefer an extra set of eyes on the two
> accel/tcg ones first?
>

Ping


> ATB,
>
> Mark.
>

[PATCH v2] tests: Prefer max cpu type when using AArch64 virt machine

The max cpu type is the best default cpu type for tests to use
when specifying the cpu type for AArch64 mach-virt. Switch all
tests to it.

Cc: Alex Bennée 
Signed-off-by: Andrew Jones 
---
 tests/avocado/boot_xen.py  |  6 +++---
 tests/avocado/replay_kernel.py |  2 +-
 tests/avocado/reverse_debugging.py |  2 +-
 tests/avocado/tcg_plugins.py   |  6 +++---
 tests/qtest/bios-tables-test.c | 12 ++--
 tests/qtest/machine-none-test.c|  4 ++--
 tests/vm/aarch64vm.py  |  2 +-
 tests/vm/ubuntu.aarch64|  2 +-
 8 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/tests/avocado/boot_xen.py b/tests/avocado/boot_xen.py
index fc2faeedb559..899c396bd55c 100644
--- a/tests/avocado/boot_xen.py
+++ b/tests/avocado/boot_xen.py
@@ -66,7 +66,7 @@ def test_arm64_xen_411_and_dom0(self):
 """
 :avocado: tags=arch:aarch64
 :avocado: tags=accel:tcg
-:avocado: tags=cpu:cortex-a57
+:avocado: tags=cpu:max
 :avocado: tags=machine:virt
 """
 
@@ -84,7 +84,7 @@ def test_arm64_xen_414_and_dom0(self):
 """
 :avocado: tags=arch:aarch64
 :avocado: tags=accel:tcg
-:avocado: tags=cpu:cortex-a57
+:avocado: tags=cpu:max
 :avocado: tags=machine:virt
 """
 
@@ -102,7 +102,7 @@ def test_arm64_xen_415_and_dom0(self):
 """
 :avocado: tags=arch:aarch64
 :avocado: tags=accel:tcg
-:avocado: tags=cpu:cortex-a57
+:avocado: tags=cpu:max
 :avocado: tags=machine:virt
 """
 
diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py
index 0b2b0dc692b1..c19022ea977d 100644
--- a/tests/avocado/replay_kernel.py
+++ b/tests/avocado/replay_kernel.py
@@ -147,7 +147,7 @@ def test_aarch64_virt(self):
 """
 :avocado: tags=arch:aarch64
 :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
 """
 kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
   '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
diff --git a/tests/avocado/reverse_debugging.py 
b/tests/avocado/reverse_debugging.py
index d2921e70c3b4..d6a6d7277235 100644
--- a/tests/avocado/reverse_debugging.py
+++ b/tests/avocado/reverse_debugging.py
@@ -198,7 +198,7 @@ def test_aarch64_virt(self):
 """
 :avocado: tags=arch:aarch64
 :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
 """
 kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
   '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
diff --git a/tests/avocado/tcg_plugins.py b/tests/avocado/tcg_plugins.py
index 642d2e49e305..2bbf62f5036e 100644
--- a/tests/avocado/tcg_plugins.py
+++ b/tests/avocado/tcg_plugins.py
@@ -68,7 +68,7 @@ def test_aarch64_virt_insn(self):
 :avocado: tags=accel:tcg
 :avocado: tags=arch:aarch64
 :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
 """
 kernel_path = self._grab_aarch64_kernel()
 kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
@@ -94,7 +94,7 @@ def test_aarch64_virt_insn_icount(self):
 :avocado: tags=accel:tcg
 :avocado: tags=arch:aarch64
 :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
 """
 kernel_path = self._grab_aarch64_kernel()
 kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
@@ -120,7 +120,7 @@ def test_aarch64_virt_mem_icount(self):
 :avocado: tags=accel:tcg
 :avocado: tags=arch:aarch64
 :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
 """
 kernel_path = self._grab_aarch64_kernel()
 kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index a4a46e97f0b8..7c3a58a97460 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1270,7 +1270,7 @@ static void test_acpi_virt_tcg_memhp(void)
 
 data.variant = ".memhp";
 test_acpi_one(" -machine nvdimm=on"
-  " -cpu cortex-a57"
+  " -cpu max"
   " -m 256M,slots=3,maxmem=1G"
   " -object memory-backend-ram,id=ram0,size=128M"
   " -object memory-backend-ram,id=ram1,size=128M"
@@ -1363,7 +1363,7 @@ static void test_acpi_virt_tcg_numamem(void)
 };
 
 data.variant = ".numamem";
-test_acpi_one(" -cpu cortex-a57"
+test_acpi_one(" -cpu max"
   " -object memory-backend-ram,id=ram0,size=128M"
   " -numa node,memdev=ram0",
   &data);
@@ -1397,7 +1397,7 @@ static void test_acpi_virt_tcg_pxb(void)

Re: [PATCH v6 5/8] libvduse: Add VDUSE (vDPA Device in Userspace) library

2022-06-03 Thread Maxime Coquelin


Hi Yongji,

On 5/23/22 10:46, Xie Yongji wrote:

VDUSE [1] is a linux framework that makes it possible to implement
software-emulated vDPA devices in userspace. This adds a library
as a subproject to help implementing VDUSE backends in QEMU.

[1] https://www.kernel.org/doc/html/latest/userspace-api/vduse.html

Signed-off-by: Xie Yongji 
---
  MAINTAINERS |5 +
  meson.build |   15 +
  meson_options.txt   |2 +
  scripts/meson-buildoptions.sh   |3 +
  subprojects/libvduse/include/atomic.h   |1 +
  subprojects/libvduse/include/compiler.h |1 +
  subprojects/libvduse/libvduse.c | 1167 +++
  subprojects/libvduse/libvduse.h |  235 
  subprojects/libvduse/linux-headers/linux|1 +
  subprojects/libvduse/meson.build|   10 +
  subprojects/libvduse/standard-headers/linux |1 +
  11 files changed, 1441 insertions(+)
  create mode 12 subprojects/libvduse/include/atomic.h
  create mode 12 subprojects/libvduse/include/compiler.h
  create mode 100644 subprojects/libvduse/libvduse.c
  create mode 100644 subprojects/libvduse/libvduse.h
  create mode 12 subprojects/libvduse/linux-headers/linux
  create mode 100644 subprojects/libvduse/meson.build
  create mode 12 subprojects/libvduse/standard-headers/linux



...


diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
new file mode 100644
index 00..fa4822b9a9
--- /dev/null
+++ b/subprojects/libvduse/libvduse.c
@@ -0,0 +1,1167 @@


...


+
+int vduse_dev_destroy(VduseDev *dev)
+{
+int ret = 0;
+
+free(dev->vqs);
+if (dev->fd > 0) {


if (dev->fd >= 0) {


+close(dev->fd);
+dev->fd = -1;
+}
+if (dev->ctrl_fd > 0) {


if (dev->ctrl_fd >= 0) {


+if (ioctl(dev->ctrl_fd, VDUSE_DESTROY_DEV, dev->name)) {
+ret = -errno;
+}
+close(dev->ctrl_fd);
+dev->ctrl_fd = -1;
+}
+free(dev->name);
+free(dev);
+
+return ret;
+}


Maxime

[PATCH v3 1/3] target/riscv: Reorganize riscv_cpu_properties

Because many developers introduced new properties in various ways, the
entire riscv_cpu_properties block is getting too complex.

This commit reorganizes riscv_cpu_properties for clarity on future.

Signed-off-by: Tsukasa OI 
---
 target/riscv/cpu.c | 64 +++---
 1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index a91253d4bd..3f21563f2d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -840,7 +840,7 @@ static void riscv_cpu_init(Object *obj)
 }
 
 static Property riscv_cpu_properties[] = {
-/* Defaults for standard extensions */
+/* Base ISA and single-letter standard extensions */
 DEFINE_PROP_BOOL("i", RISCVCPU, cfg.ext_i, true),
 DEFINE_PROP_BOOL("e", RISCVCPU, cfg.ext_e, false),
 DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, false),
@@ -853,29 +853,17 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
 DEFINE_PROP_BOOL("v", RISCVCPU, cfg.ext_v, false),
 DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, true),
-DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
-DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
+
+/* Standard unprivileged extensions */
 DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
+DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
+
 DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
 DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
-DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
-DEFINE_PROP_BOOL("Zve64f", RISCVCPU, cfg.ext_zve64f, false),
-DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
-DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
-DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
-
-DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
-DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
-DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
-DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
-
-DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
-DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, RISCV_CPU_MARCHID),
-DEFINE_PROP_UINT64("mimpid", RISCVCPU, cfg.mimpid, RISCV_CPU_MIMPID),
-
-DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
-DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
-DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
+DEFINE_PROP_BOOL("zfinx", RISCVCPU, cfg.ext_zfinx, false),
+DEFINE_PROP_BOOL("zdinx", RISCVCPU, cfg.ext_zdinx, false),
+DEFINE_PROP_BOOL("zhinx", RISCVCPU, cfg.ext_zhinx, false),
+DEFINE_PROP_BOOL("zhinxmin", RISCVCPU, cfg.ext_zhinxmin, false),
 
 DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
 DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
@@ -884,6 +872,7 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("zbkc", RISCVCPU, cfg.ext_zbkc, false),
 DEFINE_PROP_BOOL("zbkx", RISCVCPU, cfg.ext_zbkx, false),
 DEFINE_PROP_BOOL("zbs", RISCVCPU, cfg.ext_zbs, true),
+
 DEFINE_PROP_BOOL("zk", RISCVCPU, cfg.ext_zk, false),
 DEFINE_PROP_BOOL("zkn", RISCVCPU, cfg.ext_zkn, false),
 DEFINE_PROP_BOOL("zknd", RISCVCPU, cfg.ext_zknd, false),
@@ -895,10 +884,31 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("zksh", RISCVCPU, cfg.ext_zksh, false),
 DEFINE_PROP_BOOL("zkt", RISCVCPU, cfg.ext_zkt, false),
 
-DEFINE_PROP_BOOL("zdinx", RISCVCPU, cfg.ext_zdinx, false),
-DEFINE_PROP_BOOL("zfinx", RISCVCPU, cfg.ext_zfinx, false),
-DEFINE_PROP_BOOL("zhinx", RISCVCPU, cfg.ext_zhinx, false),
-DEFINE_PROP_BOOL("zhinxmin", RISCVCPU, cfg.ext_zhinxmin, false),
+DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
+DEFINE_PROP_BOOL("Zve64f", RISCVCPU, cfg.ext_zve64f, false),
+
+/* Standard supervisor-level extensions */
+DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
+DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
+DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
+
+/* Base features */
+DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
+DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
+DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
+DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+
+/* ISA specification / extension versions */
+DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
+DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
+
+/* CPU parameters */
+DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
+DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, RISCV_CPU_MARCHID),
+DEFINE_PROP_UINT64("mimpid", RISCVCPU, cfg.mimpid, RISCV_CPU_MIMPID),
+DEFINE_PROP_UINT64("resetvec", RISCVCPU, cfg.resetvec, DEFAULT_RSTVEC),
+DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
+DEFINE_PROP_

[PATCH v3 3/3] target/riscv: Deprecate capitalized property names

This commit adds a deprecation note of capitalized property names of
RISC-V CPU to documentation.

Signed-off-by: Tsukasa OI 
---
 docs/about/deprecated.rst | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index a92ae0f162..cfc9adcd4b 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -300,6 +300,16 @@ Options are:
 Device options
 --
 
+CPU options
+'''
+
+Capitalized property names on RISC-V ``-cpu`` (since 7.1)
+^
+
+Using capitalized RISC-V CPU property names like ``-cpu rv64,Counters=on`` is
+deprecated.  Use lowercase names instead (e.g. ``-cpu rv64,counters=on``).
+
+
 Emulated device options
 '''
 
-- 
2.34.1

[PATCH v3 0/3] target/riscv: Make CPU property names lowercase (w/ capitalized aliases)

v1: 
v2: 
v2.1: 

Hello,

This is v3 of the patch to RISC-V CPU property names.
See v1 for background.

[Changes: PATCH v2 -> PATCH v3]

-   Rebased to newer master
-   Fixed a typo
-   Clarified intent of PATCH 1/3




Tsukasa OI (3):
  target/riscv: Reorganize riscv_cpu_properties
  target/riscv: Make CPU property names lowercase
  target/riscv: Deprecate capitalized property names

 docs/about/deprecated.rst | 10 +
 target/riscv/cpu.c| 79 +--
 2 files changed, 61 insertions(+), 28 deletions(-)


base-commit: c9641eb422905cc0804a7e310269abf09543cce8
-- 
2.34.1

[PATCH v3 2/3] target/riscv: Make CPU property names lowercase

Many CPU properties for RISC-V are in lowercase except those with
"capitalized" (or CamelCase) names:

-   Counters
-   Zifencei
-   Zicsr
-   Zfh
-   Zfhmin
-   Zve32f
-   Zve64f

This commit makes lowercase names primary but keeps capitalized names
as aliases (for backward compatibility, but with deprecated status).

Signed-off-by: Tsukasa OI 
---
 target/riscv/cpu.c | 27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3f21563f2d..83262586e4 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -840,6 +840,10 @@ static void riscv_cpu_init(Object *obj)
 }
 
 static Property riscv_cpu_properties[] = {
+/*
+ * Names for ISA extensions and features should be in lowercase.
+ */
+
 /* Base ISA and single-letter standard extensions */
 DEFINE_PROP_BOOL("i", RISCVCPU, cfg.ext_i, true),
 DEFINE_PROP_BOOL("e", RISCVCPU, cfg.ext_e, false),
@@ -855,11 +859,11 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, true),
 
 /* Standard unprivileged extensions */
-DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
-DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
+DEFINE_PROP_BOOL("zicsr", RISCVCPU, cfg.ext_icsr, true),
+DEFINE_PROP_BOOL("zifencei", RISCVCPU, cfg.ext_ifencei, true),
 
-DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
-DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
+DEFINE_PROP_BOOL("zfh", RISCVCPU, cfg.ext_zfh, false),
+DEFINE_PROP_BOOL("zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
 DEFINE_PROP_BOOL("zfinx", RISCVCPU, cfg.ext_zfinx, false),
 DEFINE_PROP_BOOL("zdinx", RISCVCPU, cfg.ext_zdinx, false),
 DEFINE_PROP_BOOL("zhinx", RISCVCPU, cfg.ext_zhinx, false),
@@ -884,8 +888,8 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("zksh", RISCVCPU, cfg.ext_zksh, false),
 DEFINE_PROP_BOOL("zkt", RISCVCPU, cfg.ext_zkt, false),
 
-DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
-DEFINE_PROP_BOOL("Zve64f", RISCVCPU, cfg.ext_zve64f, false),
+DEFINE_PROP_BOOL("zve32f", RISCVCPU, cfg.ext_zve32f, false),
+DEFINE_PROP_BOOL("zve64f", RISCVCPU, cfg.ext_zve64f, false),
 
 /* Standard supervisor-level extensions */
 DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
@@ -893,7 +897,7 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
 
 /* Base features */
-DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
+DEFINE_PROP_BOOL("counters", RISCVCPU, cfg.ext_counters, true),
 DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
 DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
 DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
@@ -922,6 +926,15 @@ static Property riscv_cpu_properties[] = {
 /* Other options */
 DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, 
false),
 
+/* Capitalized aliases (deprecated and will be removed) */
+DEFINE_PROP("Counters", RISCVCPU, cfg.ext_counters, qdev_prop_bool, bool),
+DEFINE_PROP("Zifencei", RISCVCPU, cfg.ext_ifencei, qdev_prop_bool, bool),
+DEFINE_PROP("Zicsr", RISCVCPU, cfg.ext_icsr, qdev_prop_bool, bool),
+DEFINE_PROP("Zfh", RISCVCPU, cfg.ext_zfh, qdev_prop_bool, bool),
+DEFINE_PROP("Zfhmin", RISCVCPU, cfg.ext_zfhmin, qdev_prop_bool, bool),
+DEFINE_PROP("Zve32f", RISCVCPU, cfg.ext_zve32f, qdev_prop_bool, bool),
+DEFINE_PROP("Zve64f", RISCVCPU, cfg.ext_zve64f, qdev_prop_bool, bool),
+
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.34.1

Re: [PATCH v2] tests: Prefer max cpu type when using AArch64 virt machine

2022-06-03 Thread Cornelia Huck

On Fri, Jun 03 2022, Andrew Jones  wrote:

> The max cpu type is the best default cpu type for tests to use
> when specifying the cpu type for AArch64 mach-virt. Switch all
> tests to it.
>
> Cc: Alex Bennée 
> Signed-off-by: Andrew Jones 
> ---
>  tests/avocado/boot_xen.py  |  6 +++---
>  tests/avocado/replay_kernel.py |  2 +-
>  tests/avocado/reverse_debugging.py |  2 +-
>  tests/avocado/tcg_plugins.py   |  6 +++---
>  tests/qtest/bios-tables-test.c | 12 ++--
>  tests/qtest/machine-none-test.c|  4 ++--
>  tests/vm/aarch64vm.py  |  2 +-
>  tests/vm/ubuntu.aarch64|  2 +-
>  8 files changed, 18 insertions(+), 18 deletions(-)

Reviewed-by: Cornelia Huck

Re: [PATCH v10 13/14] vfio-user: handle device interrupts

2022-06-03 Thread John Johnson



> On Jun 1, 2022, at 1:26 PM, Alex Williamson  
> wrote:
> 
> On Wed, 1 Jun 2022 17:00:54 +
> Jag Raman  wrote:
>> 
>> Hi Alex,
>> 
>> Just to add some more detail, the emulated PCI device in QEMU presently
>> maintains a MSIx table (PCIDevice->msix_table) and Pending Bit Array. In the
>> present VFIO PCI device implementation, QEMU leverages the same
>> MSIx table for interrupt masking/unmasking. The backend PCI device (such as
>> the passthru device) always thinks that the interrupt is unmasked and lets
>> QEMU manage masking.
>> 
>> Whereas in the vfio-user case, the client additionally pushes a copy of
>> emulated PCI device’s table downstream to the remote device. We did this
>> to allow a small set of devices (such as e1000e) to clear the
>> PBA (msix_clr_pending()). Secondly, the remote device uses its copy of the
>> MSIx table to determine if interrupt should be triggered - this would prevent
>> an interrupt from being sent to the client unnecessarily if it's masked.
>> 
>> We are wondering if pushing the MSIx table to the remote device and
>> reading PBA from it would diverge from the VFIO protocol specification?
>> 
>> From your comment, I understand it’s similar to VFIO protocol because VFIO
>> clients could mask an interrupt using VFIO_DEVICE_SET_IRQS ioctl +
>> VFIO_IRQ_SET_ACTION_MASK / _UNMASK flags. I observed that QEMU presently
>> does not use this approach and the kernel does not support it for MSI.
> 
> I believe the SET_IRQS ioctl definition is pre-enabled to support
> masking and unmasking, we've just lacked kernel support to mask at the
> device which leads to the hybrid approach we have today.  Our intention
> would be to use the current uAPI, to provide that masking support, at
> which point we'd leave the PBA mapped to the device.
> 

The reason I didn’t use SET_IRQS was the kernel implementation
didn’t, and I wanted to avoid having the two behave differently.  If we
want to go down the modal path, then if we use SET_IRQS to mask interrupts
at the source, we don’t need to emulate masking by changing the interrupt
eventfd from KVM to QEMU when the interrupt is masked.  Do you want that
change as well?

JJ

[PATCH] gitlab-ci: Fix the build-cfi-aarch64 and build-cfi-ppc64-s390x jobs

The job definitions recently got a second "variables:" section by
accident and thus are failing now if one tries to run them. Merge
the two sections into one again to fix the issue.

And while we're at it, bump the timeout here (70 minutes are currently
not enough for the aarch64 job). The jobs are marked as manual anyway,
so if the user starts them, they want to see their result for sure and
then it's annoying if the job timeouts too early.

Fixes: e312d1fdbb ("gitlab: convert build/container jobs to .base_job_template")
Signed-off-by: Thomas Huth 
---
 I wonder whether we should remove the build-cfi-aarch64 job instead.
 When I tried to run it during the past months, it was always failing
 for me. This time, I tried to bump the timeout while I was at it,
 and it takes longer than 80 minutes here to finish - so I asume
 nobody ever ran this successfully in the last months... Is anybody
 using this job at all? I think if we want to have CFI coverage here,
 it should get replaced by a custom runner job that runs on a more
 beefy machine... (the ppc64-s390x job is fine by the way, it often
 only runs a little bit longer than 60 minutes - I still bumped the
 timeout here, too, just to be on the safe side)

 .gitlab-ci.d/buildtest.yml | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index ecac3ec50c..baaa0ebb87 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -355,16 +355,15 @@ build-cfi-aarch64:
   --enable-safe-stack --enable-slirp=git
 TARGETS: aarch64-softmmu
 MAKE_CHECK_ARGS: check-build
-  timeout: 70m
-  artifacts:
-expire_in: 2 days
-paths:
-  - build
-  variables:
 # FIXME: This job is often failing, likely due to out-of-memory problems in
 # the constrained containers of the shared runners. Thus this is marked as
 # skipped until the situation has been solved.
 QEMU_JOB_SKIPPED: 1
+  timeout: 90m
+  artifacts:
+expire_in: 2 days
+paths:
+  - build
 
 check-cfi-aarch64:
   extends: .native_test_job_template
@@ -396,16 +395,15 @@ build-cfi-ppc64-s390x:
   --enable-safe-stack --enable-slirp=git
 TARGETS: ppc64-softmmu s390x-softmmu
 MAKE_CHECK_ARGS: check-build
-  timeout: 70m
-  artifacts:
-expire_in: 2 days
-paths:
-  - build
-  variables:
 # FIXME: This job is often failing, likely due to out-of-memory problems in
 # the constrained containers of the shared runners. Thus this is marked as
 # skipped until the situation has been solved.
 QEMU_JOB_SKIPPED: 1
+  timeout: 80m
+  artifacts:
+expire_in: 2 days
+paths:
+  - build
 
 check-cfi-ppc64-s390x:
   extends: .native_test_job_template
-- 
2.31.1

Debian MinGW cross compilation (was: Re: [PULL 2/3] qga-win32: Add support for NVME but type)


On 24/05/2022 15.38, Daniel P. Berrangé wrote:

On Tue, May 24, 2022 at 03:28:37PM +0200, Thomas Huth wrote:

...


Daniel, do you remember whether we supported Debian for MinGW
cross-compilation in the past?


At one time we used to have Debian with the 3rd party 'mxe' builds
of mingw added. It broke periodically and we deleted it in the
end. It wasn't adding value over what Fedora mingw could provide
as both more or less tracked the same versions of software in
their mingw packages.


I wonder whether anybody still tried to compile with this mxe repo in recent 
times...?
Should we adjust our support statement and just mention Fedora there? 
Otherwise we should maybe explicitly mention MXE there next to "Debian", 
too, so that people don't get the impression that QEMU can be compiled with 
a vanilla MinGW installation on Debian?


 Thomas

Re: Debian MinGW cross compilation (was: Re: [PULL 2/3] qga-win32: Add support for NVME but type)

2022-06-03 Thread Stefan Weil via


Am 03.06.22 um 14:56 schrieb Thomas Huth:


On 24/05/2022 15.38, Daniel P. Berrangé wrote:

On Tue, May 24, 2022 at 03:28:37PM +0200, Thomas Huth wrote:

...


Daniel, do you remember whether we supported Debian for MinGW
cross-compilation in the past?


At one time we used to have Debian with the 3rd party 'mxe' builds
of mingw added. It broke periodically and we deleted it in the
end. It wasn't adding value over what Fedora mingw could provide
as both more or less tracked the same versions of software in
their mingw packages.


I wonder whether anybody still tried to compile with this mxe repo in 
recent times...?
Should we adjust our support statement and just mention Fedora there? 
Otherwise we should maybe explicitly mention MXE there next to 
"Debian", too, so that people don't get the impression that QEMU can 
be compiled with a vanilla MinGW installation on Debian?


 Thomas



My QEMU for Windows builds are all done on Debian. They use the cross 
tools which are provided in the normal Debian distribution. I don't use 
the (few) cross libraries from Debian.


Until end of last year, I added library packages from Cygwin (plus a few 
self compiled libraries, for example for braille support). See 
https://qemu.weilnetz.de/debian/.


In 2022 I switched to using the library packages from msys (I still have 
to write some documentation for that).


Stefan

Re: Debian MinGW cross compilation (was: Re: [PULL 2/3] qga-win32: Add support for NVME but type)


On 03/06/2022 15.09, Stefan Weil wrote:

Am 03.06.22 um 14:56 schrieb Thomas Huth:


On 24/05/2022 15.38, Daniel P. Berrangé wrote:

On Tue, May 24, 2022 at 03:28:37PM +0200, Thomas Huth wrote:

...


Daniel, do you remember whether we supported Debian for MinGW
cross-compilation in the past?


At one time we used to have Debian with the 3rd party 'mxe' builds
of mingw added. It broke periodically and we deleted it in the
end. It wasn't adding value over what Fedora mingw could provide
as both more or less tracked the same versions of software in
their mingw packages.


I wonder whether anybody still tried to compile with this mxe repo in 
recent times...?
Should we adjust our support statement and just mention Fedora there? 
Otherwise we should maybe explicitly mention MXE there next to "Debian", 
too, so that people don't get the impression that QEMU can be compiled 
with a vanilla MinGW installation on Debian?


 Thomas



My QEMU for Windows builds are all done on Debian. They use the cross tools 
which are provided in the normal Debian distribution. I don't use the (few) 
cross libraries from Debian.


Until end of last year, I added library packages from Cygwin (plus a few 
self compiled libraries, for example for braille support). See 
https://qemu.weilnetz.de/debian/.


In 2022 I switched to using the library packages from msys (I still have to 
write some documentation for that).


Ok, thanks for the info. Seems like there are multiple ways to get the 
missing packages for the MinGW installation on Debian, so let's simply keep 
the support statement in the current shape.


 Thomas

about the current status of Multi-process QEMU / out-of-process emulation

2022-06-03 Thread Yu Zhang

Hi All,

I saw that you authored the QEMU page for "Multi-process QEMU". (
https://www.qemu.org/docs/master/system/multi-process.html)

I'm interested in this feature, but feel a little confused with the command
line:

+  /usr/bin/qemu-system-x86_64\
+  -machine x-remote  \
+  -device lsi53c895a,id=lsi0 \
+  -drive id=drive_image2,file=/build/ol7-nvme-test-1.qcow2   \
+  -device scsi-hd,id=drive2,drive=drive_image2,bus=lsi0.0,scsi-id=0  \
+  -object x-remote-object,id=robj1,devid=lsi1,fd=4,
(https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg02887.html)

It seems that the man page of qemu command contains no parameter and option
yet for this feature. May I know whether is it still in experimental
stage? And even a few more questions:

- Is "x-remote" a standalone machine type for creating the orchestrator?
- Can each device has a dedicated emulation process or shares one process
for emulating multiple devices?
- Are there more command line examples illustrating the combination of
orchestrator, remote emulation process, memory-backend-memfd and
x-pci-proxy-dev?

Thank you very much
Kind regard

Yu Zhang @ IONOS Compute Platform
03.06.2022

Re: [PATCH 3/3] capstone: Remove the capstone submodule


On 6/2/22 22:21, Thomas Huth wrote:
So is capstone disassembly better now with Ubuntu 20.04 or should we still revert the 
submodule removal?


It's better, yes.  At least it's giving me disassembly of the system registers.


Also, if libvixl is so bad, why do we still have that in the repo?


Well, we just removed 3 other old disassemblers -- I think libvixl can be next.


r~

Re: [PATCH v2] tests: Prefer max cpu type when using AArch64 virt machine

2022-06-03 Thread Gavin Shan


On 6/3/22 7:18 PM, Andrew Jones wrote:

The max cpu type is the best default cpu type for tests to use
when specifying the cpu type for AArch64 mach-virt. Switch all
tests to it.

Cc: Alex Bennée 
Signed-off-by: Andrew Jones 
---
  tests/avocado/boot_xen.py  |  6 +++---
  tests/avocado/replay_kernel.py |  2 +-
  tests/avocado/reverse_debugging.py |  2 +-
  tests/avocado/tcg_plugins.py   |  6 +++---
  tests/qtest/bios-tables-test.c | 12 ++--
  tests/qtest/machine-none-test.c|  4 ++--
  tests/vm/aarch64vm.py  |  2 +-
  tests/vm/ubuntu.aarch64|  2 +-
  8 files changed, 18 insertions(+), 18 deletions(-)



Reviewed-by: Gavin Shan 


diff --git a/tests/avocado/boot_xen.py b/tests/avocado/boot_xen.py
index fc2faeedb559..899c396bd55c 100644
--- a/tests/avocado/boot_xen.py
+++ b/tests/avocado/boot_xen.py
@@ -66,7 +66,7 @@ def test_arm64_xen_411_and_dom0(self):
  """
  :avocado: tags=arch:aarch64
  :avocado: tags=accel:tcg
-:avocado: tags=cpu:cortex-a57
+:avocado: tags=cpu:max
  :avocado: tags=machine:virt
  """
  
@@ -84,7 +84,7 @@ def test_arm64_xen_414_and_dom0(self):

  """
  :avocado: tags=arch:aarch64
  :avocado: tags=accel:tcg
-:avocado: tags=cpu:cortex-a57
+:avocado: tags=cpu:max
  :avocado: tags=machine:virt
  """
  
@@ -102,7 +102,7 @@ def test_arm64_xen_415_and_dom0(self):

  """
  :avocado: tags=arch:aarch64
  :avocado: tags=accel:tcg
-:avocado: tags=cpu:cortex-a57
+:avocado: tags=cpu:max
  :avocado: tags=machine:virt
  """
  
diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py

index 0b2b0dc692b1..c19022ea977d 100644
--- a/tests/avocado/replay_kernel.py
+++ b/tests/avocado/replay_kernel.py
@@ -147,7 +147,7 @@ def test_aarch64_virt(self):
  """
  :avocado: tags=arch:aarch64
  :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
  """
  kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'

'/linux/releases/29/Everything/aarch64/os/images/pxeboot'
diff --git a/tests/avocado/reverse_debugging.py 
b/tests/avocado/reverse_debugging.py
index d2921e70c3b4..d6a6d7277235 100644
--- a/tests/avocado/reverse_debugging.py
+++ b/tests/avocado/reverse_debugging.py
@@ -198,7 +198,7 @@ def test_aarch64_virt(self):
  """
  :avocado: tags=arch:aarch64
  :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
  """
  kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'

'/linux/releases/29/Everything/aarch64/os/images/pxeboot'
diff --git a/tests/avocado/tcg_plugins.py b/tests/avocado/tcg_plugins.py
index 642d2e49e305..2bbf62f5036e 100644
--- a/tests/avocado/tcg_plugins.py
+++ b/tests/avocado/tcg_plugins.py
@@ -68,7 +68,7 @@ def test_aarch64_virt_insn(self):
  :avocado: tags=accel:tcg
  :avocado: tags=arch:aarch64
  :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
  """
  kernel_path = self._grab_aarch64_kernel()
  kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
@@ -94,7 +94,7 @@ def test_aarch64_virt_insn_icount(self):
  :avocado: tags=accel:tcg
  :avocado: tags=arch:aarch64
  :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
  """
  kernel_path = self._grab_aarch64_kernel()
  kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
@@ -120,7 +120,7 @@ def test_aarch64_virt_mem_icount(self):
  :avocado: tags=accel:tcg
  :avocado: tags=arch:aarch64
  :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
  """
  kernel_path = self._grab_aarch64_kernel()
  kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index a4a46e97f0b8..7c3a58a97460 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1270,7 +1270,7 @@ static void test_acpi_virt_tcg_memhp(void)
  
  data.variant = ".memhp";

  test_acpi_one(" -machine nvdimm=on"
-  " -cpu cortex-a57"
+  " -cpu max"
" -m 256M,slots=3,maxmem=1G"
" -object memory-backend-ram,id=ram0,size=128M"
" -object memory-backend-ram,id=ram1,size=128M"
@@ -1363,7 +1363,7 @@ static void test_acpi_virt_tcg_numamem(void)
  };
  
  data.variant = ".numamem";

-test_acpi_one(" -cpu cortex-a57"
+test_acpi_one(" -cpu max"
" -object memory-backend-ram,id=ram0,size=128M"

Re: [PATCH] tests/avocado: Prefer max cpu type when using AArch64 virt machine


On 6/3/22 02:25, Andrew Jones wrote:

The max cpu type is the best default cpu type for tests to use
when specifying the cpu type for AArch64 mach-virt. Switch all
tests to it.


This won't work without further changes.


@@ -147,7 +147,7 @@ def test_aarch64_virt(self):
  """
  :avocado: tags=arch:aarch64
  :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
  """
  kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'

'/linux/releases/29/Everything/aarch64/os/images/pxeboot'


For a release this old, we'll see the kernel bugs wrt FEAT_LPA/FEAT_LPA2.
See 11593544df6f ("tests/avocado: update aarch64_virt test to exercise -cpu 
max")


r~

Re: [PATCH v2] tests: Prefer max cpu type when using AArch64 virt machine


On 6/3/22 04:18, Andrew Jones wrote:

The max cpu type is the best default cpu type for tests to use
when specifying the cpu type for AArch64 mach-virt. Switch all
tests to it.

Cc: Alex Bennée 
Signed-off-by: Andrew Jones 


For avoidance of doubt, copying v1 comment to v2:


diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py
index 0b2b0dc692b1..c19022ea977d 100644
--- a/tests/avocado/replay_kernel.py
+++ b/tests/avocado/replay_kernel.py
@@ -147,7 +147,7 @@ def test_aarch64_virt(self):
  """
  :avocado: tags=arch:aarch64
  :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
  """
  kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'

'/linux/releases/29/Everything/aarch64/os/images/pxeboot'


This will *not* work without further changes.  Fedora 29 will is old, and will not boot a 
cpu with FEAT_LPA2 enabled.


See 11593544df6f ("tests/avocado: update aarch64_virt test to exercise -cpu 
max")

Which makes me wonder if you've actually tested this?


r~

Re: [PATCH] target/ppc: fix vbpermd in big endian hosts


On 6/1/22 05:53, matheus.fe...@eldorado.org.br wrote:

From: Matheus Ferst

The extract64 arguments are not endian dependent as they are only used
for bitwise operations. The current behavior in little-endian hosts is
correct; since the indexes in VRB are in PowerISA-ordering, we should
always invert the value before calling extract64. Also, using the VsrD
macro, we can have a single EXTRACT_BIT definition for big and
little-endian with the correct behavior.

Signed-off-by: Matheus Ferst
---
Found this bug while refactoring VECTOR_FOR_INORDER_I uses. The
complete patch series will also use Vsr[DB] instead of VBPERM[DQ]_INDEX,
but it will need more testing. For now, we're just changing what is
necessary to fix the instruction.
---
  target/ppc/int_helper.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH] tests/avocado: Prefer max cpu type when using AArch64 virt machine

On Fri, Jun 03, 2022 at 06:56:41AM -0700, Richard Henderson wrote:
> On 6/3/22 02:25, Andrew Jones wrote:
> > The max cpu type is the best default cpu type for tests to use
> > when specifying the cpu type for AArch64 mach-virt. Switch all
> > tests to it.
> 
> This won't work without further changes.
> 
> > @@ -147,7 +147,7 @@ def test_aarch64_virt(self):
> >   """
> >   :avocado: tags=arch:aarch64
> >   :avocado: tags=machine:virt
> > -:avocado: tags=cpu:cortex-a53
> > +:avocado: tags=cpu:max
> >   """
> >   kernel_url = 
> > ('https://archives.fedoraproject.org/pub/archive/fedora'
> > 
> > '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
> 
> For a release this old, we'll see the kernel bugs wrt FEAT_LPA/FEAT_LPA2.
> See 11593544df6f ("tests/avocado: update aarch64_virt test to exercise -cpu 
> max")

Thanks Richard. How about for each test with guest kernels which don't
work, I add a comment about LPA2 and use '-cpu max,lpa2=off' instead?

drew

Re: [PATCH 3/3] capstone: Remove the capstone submodule


On 03/06/2022 15.48, Richard Henderson wrote:

On 6/2/22 22:21, Thomas Huth wrote:
So is capstone disassembly better now with Ubuntu 20.04 or should we still 
revert the submodule removal?


It's better, yes.  At least it's giving me disassembly of the system registers.


Also, if libvixl is so bad, why do we still have that in the repo?


Well, we just removed 3 other old disassemblers -- I think libvixl can be next.


I thought there was a reason for keeping vixl around ... but seems my memory 
is wrong here. After searching a little bit, I only found this here:


 https://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg03416.html

So seems like there was already the idea of removing vixl four years ago... 
I'll send a patch to remove it.


 Thomas

Re: [PATCH v2] tests: Prefer max cpu type when using AArch64 virt machine

On Fri, Jun 03, 2022 at 07:04:01AM -0700, Richard Henderson wrote:
> On 6/3/22 04:18, Andrew Jones wrote:
> > The max cpu type is the best default cpu type for tests to use
> > when specifying the cpu type for AArch64 mach-virt. Switch all
> > tests to it.
> > 
> > Cc: Alex Bennée 
> > Signed-off-by: Andrew Jones 
> 
> For avoidance of doubt, copying v1 comment to v2:
> 
> > diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py
> > index 0b2b0dc692b1..c19022ea977d 100644
> > --- a/tests/avocado/replay_kernel.py
> > +++ b/tests/avocado/replay_kernel.py
> > @@ -147,7 +147,7 @@ def test_aarch64_virt(self):
> >   """
> >   :avocado: tags=arch:aarch64
> >   :avocado: tags=machine:virt
> > -:avocado: tags=cpu:cortex-a53
> > +:avocado: tags=cpu:max
> >   """
> >   kernel_url = 
> > ('https://archives.fedoraproject.org/pub/archive/fedora'
> > 
> > '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
> 
> This will *not* work without further changes.  Fedora 29 will is old, and
> will not boot a cpu with FEAT_LPA2 enabled.
> 
> See 11593544df6f ("tests/avocado: update aarch64_virt test to exercise -cpu 
> max")
> 
> Which makes me wonder if you've actually tested this?

I only did 'make check'. I'll also do 'make check-avocado' for v3.

Thanks,
drew

Re: [PULL 00/12] s390x and misc patches


On 6/2/22 23:58, Thomas Huth wrote:

The following changes since commit 1e62a82574fc28e64deca589a23cf55ada2e1a7d:

   Merge tag 'm68k-for-7.1-pull-request' of https://github.com/vivier/qemu-m68k 
into staging (2022-06-02 06:30:24 -0700)

are available in the Git repository at:

   https://gitlab.com/thuth/qemu.git tags/pull-request-2022-06-03

for you to fetch changes up to 707d93d4abc6485c638e2aecd93fbd904f2f6b3e:

   ui: Remove deprecated options "-sdl" and "-curses" (2022-06-03 08:03:28 
+0200)


* s390x storage key improvements for KVM
* Some cosmetics for s390x
* Update MAINTAINERS entries
* Improve some spots wrt memory handling in the qtests
* Clean up the "-display sdl" parameter parsing


Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/7.1 as 
appropriate.


r~





Alex Bennée (1):
   tests/qtest: use g_autofree for test_server_create_chr

Dr. David Alan Gilbert (1):
   s390: Typo fix FLOATING_POINT_SUPPPORT_ENH

Eric Farman (1):
   MAINTAINERS: Update s390 vhost entries

Gautam Agrawal (1):
   tests/tcg: Test overflow conditions

Hailiang Zhang (1):
   MAINTAINERS: Change my email address

Janis Schoetterl-Glausch (1):
   target/s390x: kvm: Honor storage keys during emulation

Miaoqian Lin (1):
   qtest/npcm7xx_pwm-test: Fix memory leak in mft_qom_set

Thomas Huth (4):
   hw/s390x/s390-virtio-ccw: Improve the machine description string
   ui: Remove deprecated parameters of the "-display sdl" option
   ui: Switch "-display sdl" to use the QAPI parser
   ui: Remove deprecated options "-sdl" and "-curses"

Wenchao Wang (1):
   MAINTAINERS: Update maintainers for Guest x86 HAXM CPUs

  docs/about/deprecated.rst   |  26 
  docs/about/removed-features.rst |  27 
  qapi/ui.json|  26 +++-
  include/sysemu/sysemu.h |   2 -
  target/s390x/cpu_features_def.h.inc |   2 +-
  hw/s390x/s390-virtio-ccw.c  |   2 +-
  softmmu/globals.c   |   2 -
  softmmu/vl.c| 128 +---
  target/s390x/gen-features.c |   6 +-
  target/s390x/kvm/kvm.c  |   9 +++
  target/s390x/tcg/translate.c|   8 +--
  tests/qtest/npcm7xx_pwm-test.c  |   3 +
  tests/qtest/vhost-user-test.c   |   7 +-
  tests/tcg/multiarch/overflow.c  |  58 
  ui/sdl2.c   |  10 +++
  MAINTAINERS |   6 +-
  qemu-options.hx |  56 ++--
  17 files changed, 151 insertions(+), 227 deletions(-)
  create mode 100644 tests/tcg/multiarch/overflow.c

Re: [PATCH] tests/avocado: Prefer max cpu type when using AArch64 virt machine


On 6/3/22 08:05, Andrew Jones wrote:

On Fri, Jun 03, 2022 at 06:56:41AM -0700, Richard Henderson wrote:

On 6/3/22 02:25, Andrew Jones wrote:

The max cpu type is the best default cpu type for tests to use
when specifying the cpu type for AArch64 mach-virt. Switch all
tests to it.


This won't work without further changes.


@@ -147,7 +147,7 @@ def test_aarch64_virt(self):
   """
   :avocado: tags=arch:aarch64
   :avocado: tags=machine:virt
-:avocado: tags=cpu:cortex-a53
+:avocado: tags=cpu:max
   """
   kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
 
'/linux/releases/29/Everything/aarch64/os/images/pxeboot'


For a release this old, we'll see the kernel bugs wrt FEAT_LPA/FEAT_LPA2.
See 11593544df6f ("tests/avocado: update aarch64_virt test to exercise -cpu 
max")


Thanks Richard. How about for each test with guest kernels which don't
work, I add a comment about LPA2 and use '-cpu max,lpa2=off' instead?


I would prefer to test new kernels with max, where we might actually test the newer 
features.  Perhaps keep one older kernel test around, to make sure we don't regress, but 
don't pretend that it can test more than what it was built for.



r~

Fwd: about the current status of Multi-process QEMU / out-of-process emulation

2022-06-03 Thread Yu Zhang

Hello Dongli, Elena, John, and Jagannathan,

I'm interested in the "multi-process QEMU" feature and got the kind reply
by Mr. Vivier that I may contact you for this.
On one of the QEMU docs [1] I saw the command line:

+  /usr/bin/qemu-system-x86_64\
+  -machine x-remote  \
+  -device lsi53c895a,id=lsi0 \
+  -drive id=drive_image2,file=/build/ol7-nvme-test-1.qcow2   \
+  -device scsi-hd,id=drive2,drive=drive_image2,bus=lsi0.0,scsi-id=0  \
+  -object x-remote-object,id=robj1,devid=lsi1,fd=4,

It seems that the man page of qemu contains no parameter and option yet for
this feature. The qemu docs, such as [2][3][4] are either not up-to-date or
"doesn't reflect the current status of the implementation".
So may I know whether is it still in experimental stage or mature enough
for use? And even a few further questions:

- When creating the orchestrator, can we specify a machine type such as
pc-i440fx-7.0 for -machine?
- Can each device has a dedicated emulation process or shares one process
for emulating multiple devices?
- Can we find more command line examples showing the combination of
orchestrator, remote emulation process, memory-backend-memfd and
x-pci-proxy-dev?

Thank you very much and all the best

Yu Zhang
03.06.2022

[1] https://www.qemu.org/docs/master/system/multi-process.html
[2] https://wiki.qemu.org/Features/MultiProcessQEMU
[3]
https://lxr.missinglinkelectronics.com/qemu+v7.0.0/docs/devel/multi-process.rst
[4] https://qemu.readthedocs.io/en/latest/devel/multi-process.html

-- Forwarded message -
From: Laurent Vivier 
Date: Fri, Jun 3, 2022 at 4:14 PM
Subject: Re: about the current status of Multi-process QEMU /
out-of-process emulation
To: Yu Zhang 


Hi Yu,

I'm not the author of this documentation, only the person that has merged
the last change in the repo.

According to the logs you should contact Dongli Zhang <
dongli.zh...@oracle.com>, Elena Ufimtseva
, John G Johnson  or
Jagannathan Raman
 .

Thanks,
Laurent

Le 03/06/2022 à 12:17, Yu Zhang a écrit :
> Dear Mr. Vivier,
>
> I saw that you authored the QEMU page for "Multi-process QEMU".
> (https://www.qemu.org/docs/master/system/multi-process.html
> )
>
> I'm interested in this feature, but feel a little confused with the
command line:
>
> +  /usr/bin/qemu-system-x86_64
 \
> +  -machine x-remote
 \
> +  -device lsi53c895a,id=lsi0
\
> +  -drive id=drive_image2,file=/build/ol7-nvme-test-1.qcow2
\
> +  -device scsi-hd,id=drive2,drive=drive_image2,bus=lsi0.0,scsi-id=0
 \
> +  -object x-remote-object,id=robj1,devid=lsi1,fd=4,
>
> It seems that the man page of qemu command contains no parameter and
option yet for this feature.
> May I know whether is it still in experimental stage? And even a few more
questions:
>
> - Is "x-remote" a standalone machine type for creating the orchestrator?
> - Can each device has a dedicated emulation process or shares one process
for emulating multiple
> devices?
> - Can I find more command line examples illustrating the combination of
orchestrator, remote
> emulation process, memory-backend-memfd and x-pci-proxy-dev?
>
> Thank you very much
> Kind regard
>
> Yu Zhang
> 03.06.2022

Re: [PATCH v6 02/18] job.h: categorize fields in struct Job

Am 14.03.2022 um 14:36 hat Emanuele Giuseppe Esposito geschrieben:
> Categorize the fields in struct Job to understand which ones
> need to be protected by the job mutex and which don't.
> 
> Signed-off-by: Emanuele Giuseppe Esposito 

I suppose it might be a result of moving things back and forth between
patches, but this patch doesn't really define separate categories.

>  include/qemu/job.h | 59 ++
>  1 file changed, 34 insertions(+), 25 deletions(-)
> 
> diff --git a/include/qemu/job.h b/include/qemu/job.h
> index d1192ffd61..86ec46c09e 100644
> --- a/include/qemu/job.h
> +++ b/include/qemu/job.h
> @@ -40,27 +40,50 @@ typedef struct JobTxn JobTxn;
>   * Long-running operation.
>   */
>  typedef struct Job {
> +
> +/* Fields set at initialization (job_create), and never modified */

This is clearly a comment starting a category, but I can't see any other
comment indicating that another category would start.

>  /** The ID of the job. May be NULL for internal jobs. */
>  char *id;
>  
> -/** The type of this job. */
> +/**
> + * The type of this job.
> + * All callbacks are called with job_mutex *not* held.
> + */
>  const JobDriver *driver;
>  
> -/** Reference count of the block job */
> -int refcnt;
> -
> -/** Current state; See @JobStatus for details. */
> -JobStatus status;
> -
> -/** AioContext to run the job coroutine in */
> -AioContext *aio_context;
> -
>  /**
>   * The coroutine that executes the job.  If not NULL, it is reentered 
> when
>   * busy is false and the job is cancelled.
> + * Initialized in job_start()
>   */
>  Coroutine *co;
>  
> +/** True if this job should automatically finalize itself */
> +bool auto_finalize;
> +
> +/** True if this job should automatically dismiss itself */
> +bool auto_dismiss;
> +
> +/** The completion function that will be called when the job completes.  
> */
> +BlockCompletionFunc *cb;
> +
> +/** The opaque value that is passed to the completion function.  */
> +void *opaque;
> +
> +/* ProgressMeter API is thread-safe */
> +ProgressMeter progress;
> +
> +

And the end of the series, this is where the cutoff is and the rest is:

/** Protected by job_mutex */

With this in mind, it seems correct to me that everything above progress
is indeed never changed after creating the job. Of course, it's hard to
tell without looking at the final result, so if you have to respin for
some reason, it would be good to mark the end of the section more
clearly for the intermediate state to make sense.

Kevin

[RFC PATCH v5 3/4] target/riscv: smstateen check for fcsr

If smstateen is implemented and sstateen0.fcsr is clear
then the floating point operations must return illegal
instruction exception.

Signed-off-by: Mayuresh Chitale 
---
 target/riscv/csr.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ae91ae1f7e..8bbbed38ff 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -77,6 +77,10 @@ static RISCVException fs(CPURISCVState *env, int csrno)
 !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
 return RISCV_EXCP_ILLEGAL_INST;
 }
+
+if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
+return smstateen_acc_ok(env, PRV_U, SMSTATEEN0_FCSR);
+}
 #endif
 return RISCV_EXCP_NONE;
 }
@@ -1700,6 +1704,10 @@ static RISCVException write_mstateen(CPURISCVState *env, 
int csrno,
(1UL << SMSTATEEN0_HSENVCFG);
 
 reg = &env->mstateen[csrno - CSR_MSTATEEN0];
+if (riscv_has_ext(env, RVF)) {
+wr_mask |= 1UL << SMSTATEEN0_FCSR;
+}
+
 write_smstateen(env, reg, wr_mask, new_val);
 
 return RISCV_EXCP_NONE;
@@ -1724,6 +1732,10 @@ static RISCVException write_mstateenh(CPURISCVState 
*env, int csrno,
 reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
 val = (uint64_t)new_val << 32;
 val |= *reg & 0x;
+if (riscv_has_ext(env, RVF)) {
+wr_mask |= 1UL << SMSTATEEN0_FCSR;
+}
+
 write_smstateen(env, reg, wr_mask, val);
 
 return RISCV_EXCP_NONE;
@@ -1745,6 +1757,10 @@ static RISCVException write_hstateen(CPURISCVState *env, 
int csrno,
(1UL << SMSTATEEN0_HSENVCFG);
 int index = csrno - CSR_HSTATEEN0;
 
+if (riscv_has_ext(env, RVF)) {
+wr_mask |= 1UL << SMSTATEEN0_FCSR;
+}
+
 reg = &env->hstateen[index];
 wr_mask &= env->mstateen[index];
 write_smstateen(env, reg, wr_mask, new_val);
@@ -1769,6 +1785,10 @@ static RISCVException write_hstateenh(CPURISCVState 
*env, int csrno,
 uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
(1UL << SMSTATEEN0_HSENVCFG);
 
+if (riscv_has_ext(env, RVF)) {
+wr_mask |= 1UL << SMSTATEEN0_FCSR;
+}
+
 reg = &env->hstateen[index];
 val = (uint64_t)new_val << 32;
 val |= *reg & 0x;
@@ -1794,6 +1814,10 @@ static RISCVException write_sstateen(CPURISCVState *env, 
int csrno,
 int index = csrno - CSR_SSTATEEN0;
 bool virt = riscv_cpu_virt_enabled(env);
 
+if (riscv_has_ext(env, RVF)) {
+wr_mask |= 1UL << SMSTATEEN0_FCSR;
+}
+
 reg = &env->sstateen[index];
 if (virt) {
 wr_mask &= env->mstateen[index];
-- 
2.25.1

[RFC PATCH v5 0/4] RISC-V Smstateen support

This series adds support for the Smstateen specification which provides
a mechanism plug potential covert channels which are opened by extensions
that add to processor state that may not get context-switched. Currently
access to AIA registers, *envcfg registers and floating point(fcsr) is
controlled via smstateen.

These patches can also be found on riscv_smstateen_v5 branch at:
https://github.com/mdchitale/qemu.git

This series depends on the following series from Anup:
https://lists.nongnu.org/archive/html/qemu-devel/2022-05/msg05231.html

Changes in v5:
- Fix the order in which smstateen extension is added to the isa_edata_arr as
described in rule #3 the comment.

Changes in v4:
- Fix build issue with riscv32/riscv64-linux-user targets

Changes in v3:
- Fix coding style issues
- Fix *stateen0h index calculation

Changes in v2:
- Make h/s/envcfg bits in m/h/stateen registers as writeable by default.

Anup Patel (1):
  target/riscv: Force disable extensions if priv spec version does not
match

Mayuresh Chitale (4):
  target/riscv: Add smstateen support
  target/riscv: smstateen check for h/senvcfg
  target/riscv: smstateen check for fcsr
  target/riscv: smstateen check for AIA/IMSIC

 target/riscv/cpu.c  |   2 +
 target/riscv/cpu.h  |   4 +
 target/riscv/cpu_bits.h |  36 +++
 target/riscv/csr.c  | 555 +++-
 target/riscv/machine.c  |  21 ++
 5 files changed, 615 insertions(+), 3 deletions(-)

-- 
2.25.1

[RFC PATCH v5 1/4] target/riscv: Add smstateen support

Smstateen extension specifies a mechanism to close
the potential covert channels that could cause security issues.

This patch adds the CSRs defined in the specification and
the corresponding predicates and read/write functions.

Signed-off-by: Mayuresh Chitale 
---
 target/riscv/cpu.c  |   2 +
 target/riscv/cpu.h  |   4 +
 target/riscv/cpu_bits.h |  36 +++
 target/riscv/csr.c  | 210 
 target/riscv/machine.c  |  21 
 5 files changed, 273 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e6e878ceb3..2d65ccd90f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -913,6 +913,7 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
 DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
 DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen, false),
 
 DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
 DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
@@ -1104,6 +1105,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char 
**isa_str, int max_str_len)
 ISA_EDATA_ENTRY(zve64f, ext_zve64f),
 ISA_EDATA_ENTRY(zhinx, ext_zhinx),
 ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
+ISA_EDATA_ENTRY(smstateen, ext_smstateen),
 ISA_EDATA_ENTRY(svinval, ext_svinval),
 ISA_EDATA_ENTRY(svnapot, ext_svnapot),
 ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f08c3e8813..1c269b77bd 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -327,6 +327,9 @@ struct CPUArchState {
 
 /* CSRs for execution enviornment configuration */
 uint64_t menvcfg;
+uint64_t mstateen[SMSTATEEN_MAX_COUNT];
+uint64_t hstateen[SMSTATEEN_MAX_COUNT];
+uint64_t sstateen[SMSTATEEN_MAX_COUNT];
 target_ulong senvcfg;
 uint64_t henvcfg;
 #endif
@@ -411,6 +414,7 @@ struct RISCVCPUConfig {
 bool ext_zhinxmin;
 bool ext_zve32f;
 bool ext_zve64f;
+bool ext_smstateen;
 
 uint32_t mvendorid;
 uint64_t marchid;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 4a55c6a709..2a3ef26d21 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -208,6 +208,12 @@
 /* Supervisor Configuration CSRs */
 #define CSR_SENVCFG 0x10A
 
+/* Supervisor state CSRs */
+#define CSR_SSTATEEN0   0x10C
+#define CSR_SSTATEEN1   0x10D
+#define CSR_SSTATEEN2   0x10E
+#define CSR_SSTATEEN3   0x10F
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH0x140
 #define CSR_SEPC0x141
@@ -257,6 +263,16 @@
 #define CSR_HENVCFG 0x60A
 #define CSR_HENVCFGH0x61A
 
+/* Hypervisor state CSRs */
+#define CSR_HSTATEEN0   0x60C
+#define CSR_HSTATEEN0H  0x61C
+#define CSR_HSTATEEN1   0x60D
+#define CSR_HSTATEEN1H  0x61D
+#define CSR_HSTATEEN2   0x60E
+#define CSR_HSTATEEN2H  0x61E
+#define CSR_HSTATEEN3   0x60F
+#define CSR_HSTATEEN3H  0x61F
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS0x200
 #define CSR_VSIE0x204
@@ -304,6 +320,26 @@
 #define CSR_MENVCFG 0x30A
 #define CSR_MENVCFGH0x31A
 
+/* Machine state CSRs */
+#define CSR_MSTATEEN0   0x30C
+#define CSR_MSTATEEN0H  0x31C
+#define CSR_MSTATEEN1   0x30D
+#define CSR_MSTATEEN1H  0x31D
+#define CSR_MSTATEEN2   0x30E
+#define CSR_MSTATEEN2H  0x31E
+#define CSR_MSTATEEN3   0x30F
+#define CSR_MSTATEEN3H  0x31F
+
+/* Common defines for all smstateen */
+#define SMSTATEEN_MAX_COUNT 4
+#define SMSTATEEN0_CS   0
+#define SMSTATEEN0_FCSR 0
+#define SMSTATEEN0_IMSIC58
+#define SMSTATEEN0_AIA  59
+#define SMSTATEEN0_SVSLCT   60
+#define SMSTATEEN0_HSENVCFG 62
+#define SMSTATEEN_STATEN63
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG 0x747
 #define CSR_MSECCFGH0x757
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 409a209f14..324fefce59 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -247,6 +247,42 @@ static RISCVException hmode32(CPURISCVState *env, int 
csrno)
 
 }
 
+static RISCVException mstateen(CPURISCVState *env, int csrno)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+
+if (!cpu->cfg.ext_smstateen) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+return any(env, csrno);
+}
+
+static RISCVException hstateen(CPURISCVState *env, int csrno)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+
+if (!cpu->cfg.ext_smstateen) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+return hmode(env, csrno);
+}
+
+static RISCVException sstateen(CPURISCVState *env, int csrno)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+
+if (!cpu->cfg.ext_smstateen) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+return smode(env,

[RFC PATCH v5 4/4] target/riscv: smstateen check for AIA/IMSIC

If smstateen is implemented then accesses to AIA
registers CSRS, IMSIC CSRs and other IMSIC registers
is controlled by setting of corresponding bits in
mstateen/hstateen registers. Otherwise an illegal
instruction trap or virtual instruction trap is
generated.

Signed-off-by: Mayuresh Chitale 
---
 target/riscv/csr.c | 253 -
 1 file changed, 248 insertions(+), 5 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 8bbbed38ff..213b3c17ff 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -39,6 +39,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 }
 
 /* Predicates */
+#if !defined(CONFIG_USER_ONLY)
 static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
 {
 CPUState *cs = env_cpu(env);
@@ -49,7 +50,6 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, 
int mode, int bit)
 return RISCV_EXCP_NONE;
 }
 
-#if !defined(CONFIG_USER_ONLY)
 if (!(env->mstateen[0] & 1UL << bit)) {
 return RISCV_EXCP_ILLEGAL_INST;
 }
@@ -65,11 +65,57 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env, 
int mode, int bit)
 return RISCV_EXCP_ILLEGAL_INST;
 }
 }
-#endif
-
 return RISCV_EXCP_NONE;
 }
 
+static RISCVException smstateen_aia_acc_ok(CPURISCVState *env, int csrno)
+{
+int bit, mode;
+
+switch (csrno) {
+case CSR_SSETEIPNUM:
+case CSR_SCLREIPNUM:
+case CSR_SSETEIENUM:
+case CSR_SCLREIENUM:
+case CSR_STOPEI:
+case CSR_VSSETEIPNUM:
+case CSR_VSCLREIPNUM:
+case CSR_VSSETEIENUM:
+case CSR_VSCLREIENUM:
+case CSR_VSTOPEI:
+case CSR_HSTATUS:
+mode = PRV_S;
+bit = SMSTATEEN0_IMSIC;
+break;
+
+case CSR_SIEH:
+case CSR_SIPH:
+case CSR_HVIPH:
+case CSR_HVICTL:
+case CSR_HVIPRIO1:
+case CSR_HVIPRIO2:
+case CSR_HVIPRIO1H:
+case CSR_HVIPRIO2H:
+case CSR_VSIEH:
+case CSR_VSIPH:
+mode = PRV_S;
+bit = SMSTATEEN0_AIA;
+break;
+
+case CSR_SISELECT:
+case CSR_VSISELECT:
+mode = PRV_S;
+bit = SMSTATEEN0_SVSLCT;
+break;
+
+default:
+return RISCV_EXCP_NONE;
+}
+
+return smstateen_acc_ok(env, mode, bit);
+}
+#endif
+
 static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
@@ -1130,6 +1176,13 @@ static int rmw_xiselect(CPURISCVState *env, int csrno, 
target_ulong *val,
 target_ulong new_val, target_ulong wr_mask)
 {
 target_ulong *iselect;
+RISCVException ret;
+
+/* Check if smstateen is enabled and this access is allowed */
+ret = smstateen_aia_acc_ok(env, csrno);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
 
 /* Translate CSR number for VS-mode */
 csrno = aia_xlate_vs_csrno(env, csrno);
@@ -1212,7 +1265,9 @@ static int rmw_xireg(CPURISCVState *env, int csrno, 
target_ulong *val,
 bool virt;
 uint8_t *iprio;
 int ret = -EINVAL;
-target_ulong priv, isel, vgein;
+target_ulong priv, isel, vgein = 0;
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
 
 /* Translate CSR number for VS-mode */
 csrno = aia_xlate_vs_csrno(env, csrno);
@@ -1241,11 +1296,20 @@ static int rmw_xireg(CPURISCVState *env, int csrno, 
target_ulong *val,
 };
 
 /* Find the selected guest interrupt file */
-vgein = (virt) ? get_field(env->hstatus, HSTATUS_VGEIN) : 0;
+if (virt) {
+if (!cpu->cfg.ext_smstateen ||
+(env->hstateen[0] & 1UL << SMSTATEEN0_IMSIC)) {
+vgein = get_field(env->hstatus, HSTATUS_VGEIN);
+}
+}
 
 if (ISELECT_IPRIO0 <= isel && isel <= ISELECT_IPRIO15) {
 /* Local interrupt priority registers not available for VS-mode */
 if (!virt) {
+if (priv == PRV_S && cpu->cfg.ext_smstateen &&
+!(env->hstateen[0] & 1UL << SMSTATEEN0_AIA)) {
+goto done;
+}
 ret = rmw_iprio(riscv_cpu_mxl_bits(env),
 isel, iprio, val, new_val, wr_mask,
 (priv == PRV_M) ? IRQ_M_EXT : IRQ_S_EXT);
@@ -1279,6 +1343,13 @@ static int rmw_xsetclreinum(CPURISCVState *env, int 
csrno, target_ulong *val,
 int ret = -EINVAL;
 bool set, pend, virt;
 target_ulong priv, isel, vgein, xlen, nval, wmask;
+RISCVException excp;
+
+/* Check if smstateen is enabled and this access is allowed */
+excp = smstateen_aia_acc_ok(env, csrno);
+if (excp != RISCV_EXCP_NONE) {
+return excp;
+}
 
 /* Translate CSR number for VS-mode */
 csrno = aia_xlate_vs_csrno(env, csrno);
@@ -1397,6 +1468,13 @@ static int rmw_xtopei(CPURISCVState *env, int csrno, 
target_ulong *val,
 bool virt;
 int ret = -EINVAL;
 target_ulong priv, vgein;
+RISCVException excp;
+
+/* Check if smstateen is enabled and this access is allowed */
+excp = smsta

[RFC PATCH v5 2/4] target/riscv: smstateen check for h/senvcfg

Accesses to henvcfg, henvcfgh and senvcfg are allowed
only if corresponding bit in mstateen0/hstateen0 is
enabled. Otherwise an illegal instruction trap is
generated.

Signed-off-by: Mayuresh Chitale 
---
 target/riscv/csr.c | 84 ++
 1 file changed, 78 insertions(+), 6 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 324fefce59..ae91ae1f7e 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -39,6 +39,37 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 }
 
 /* Predicates */
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+bool virt = riscv_cpu_virt_enabled(env);
+
+if (!cpu->cfg.ext_smstateen) {
+return RISCV_EXCP_NONE;
+}
+
+#if !defined(CONFIG_USER_ONLY)
+if (!(env->mstateen[0] & 1UL << bit)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+if (virt) {
+if (!(env->hstateen[0] & 1UL << bit)) {
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+}
+}
+
+if (mode == PRV_U) {
+if (!(env->sstateen[0] & 1UL << bit)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+}
+#endif
+
+return RISCV_EXCP_NONE;
+}
+
 static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
@@ -1557,6 +1588,13 @@ static RISCVException write_menvcfgh(CPURISCVState *env, 
int csrno,
 static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
  target_ulong *val)
 {
+RISCVException ret;
+
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
 *val = env->senvcfg;
 return RISCV_EXCP_NONE;
 }
@@ -1565,15 +1603,27 @@ static RISCVException write_senvcfg(CPURISCVState *env, 
int csrno,
   target_ulong val)
 {
 uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
+RISCVException ret;
 
-env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
 
+env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
 return RISCV_EXCP_NONE;
 }
 
 static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
  target_ulong *val)
 {
+RISCVException ret;
+
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
 *val = env->henvcfg;
 return RISCV_EXCP_NONE;
 }
@@ -1582,6 +1632,12 @@ static RISCVException write_henvcfg(CPURISCVState *env, 
int csrno,
   target_ulong val)
 {
 uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
+RISCVException ret;
+
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
 
 if (riscv_cpu_mxl(env) == MXL_RV64) {
 mask |= HENVCFG_PBMTE | HENVCFG_STCE;
@@ -1595,6 +1651,13 @@ static RISCVException write_henvcfg(CPURISCVState *env, 
int csrno,
 static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
  target_ulong *val)
 {
+RISCVException ret;
+
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
 *val = env->henvcfg >> 32;
 return RISCV_EXCP_NONE;
 }
@@ -1604,9 +1667,14 @@ static RISCVException write_henvcfgh(CPURISCVState *env, 
int csrno,
 {
 uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
 uint64_t valh = (uint64_t)val << 32;
+RISCVException ret;
 
-env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
 
+env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
 return RISCV_EXCP_NONE;
 }
 
@@ -1628,7 +1696,8 @@ static RISCVException write_mstateen(CPURISCVState *env, 
int csrno,
  target_ulong new_val)
 {
 uint64_t *reg;
-uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+   (1UL << SMSTATEEN0_HSENVCFG);
 
 reg = &env->mstateen[csrno - CSR_MSTATEEN0];
 write_smstateen(env, reg, wr_mask, new_val);
@@ -1649,7 +1718,8 @@ static RISCVException write_mstateenh(CPURISCVState *env, 
int csrno,
 {
 uint64_t *reg;
 uint64_t val;
-uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+   (1UL << SMSTATEEN0_HSENVCFG);
 
 reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
 val = (uint64_t)new_val << 32;
@@ -1671,7 +1741,8 @@ static RISCVException write_hstateen(CPURISCVState *env, 
int csrno,

Re: [PATCH] gitlab-ci: Fix the build-cfi-aarch64 and build-cfi-ppc64-s390x jobs

On 6/3/22 05:48, Thomas Huth wrote:

The job definitions recently got a second "variables:" section by
accident and thus are failing now if one tries to run them. Merge
the two sections into one again to fix the issue.

And while we're at it, bump the timeout here (70 minutes are currently
not enough for the aarch64 job). The jobs are marked as manual anyway,
so if the user starts them, they want to see their result for sure and
then it's annoying if the job timeouts too early.

Fixes: e312d1fdbb ("gitlab: convert build/container jobs to .base_job_template")
Signed-off-by: Thomas Huth
---
I wonder whether we should remove the build-cfi-aarch64 job instead.
When I tried to run it during the past months, it was always failing
for me. This time, I tried to bump the timeout while I was at it,
and it takes longer than 80 minutes here to finish - so I asume
nobody ever ran this successfully in the last months... Is anybody
using this job at all? I think if we want to have CFI coverage here,
it should get replaced by a custom runner job that runs on a more
beefy machine... (the ppc64-s390x job is fine by the way, it often
only runs a little bit longer than 60 minutes - I still bumped the
timeout here, too, just to be on the safe side)

Acked-by: Richard Henderson

I think it might be useful to extend the other s390x jobs a bit too. The last couple of
fails have the test *nearly* completing. E.g. your most recent pr:

https://gitlab.com/qemu-project/qemu/-/jobs/2544009687

Whether that indicates we've a speed regression, or host loading, or simply changes to the
testsuite, I don't know.

Re: Changes for building bits on newer gcc 9.4 compiler

2022-06-03 Thread Ani Sinha

On an additional note, my changes are not backward compatible with
older compiler. The build will break when built with a centos 7
docker/vm/host:

/home/anisinha/workspace/bits/build/grub/grub-core/contrib-deps/python/Modules/_ctypes/_ctypes.c:
In function '_build_callargs':
/home/anisinha/workspace/bits/build/grub/grub-core/contrib-deps/python/Modules/_ctypes/_ctypes.c:3707:6:
error: empty declaration [-Werror]
  __attribute__ ((fallthrough));
  ^
/home/anisinha/workspace/bits/build/grub/grub-core/contrib-deps/python/Modules/_ctypes/_ctypes.c:3707:6:
error: ISO C90 forbids mixed declarations and code
[-Werror=declaration-after-statement]
/home/anisinha/workspace/bits/build/grub/grub-core/contrib-deps/python/Modules/_ctypes/_ctypes.c:
At top level:
cc1: error: unrecognized command line option
"-Wno-shift-negative-value" [-Werror]
cc1: error: unrecognized command line option "-Wno-cast-function-type" [-Werror]
cc1: error: unrecognized command line option
"-Wno-address-of-packed-member" [-Werror]
cc1: error: unrecognized command line option
"-Wno-discarded-array-qualifiers" [-Werror]

If fixing this is essential, we can ifdef some of these changes
between compiler version checks.

On Fri, Jun 3, 2022 at 2:06 PM Ani Sinha  wrote:
>
> Hi josh :
> Here are the pull requests. Please feel free to review and merge:
>
> Main bits module:
> https://github.com/biosbits/bits/pull/13
>
> Submodules:
> https://github.com/biosbits/grub/pull/1
> https://github.com/biosbits/python/pull/1
> https://github.com/biosbits/libffi/pull/1
> https://github.com/biosbits/fdlibm/pull/1
>
> Thanks
> ani

Re: [PATCH v6 05/18] job.h: add _locked duplicates for job API functions called with and without job_mutex

Am 14.03.2022 um 14:36 hat Emanuele Giuseppe Esposito geschrieben:
> In preparation to the job_lock/unlock usage, create _locked
> duplicates of some functions, since they will be sometimes called with
> job_mutex held (mostly within job.c),
> and sometimes without (mostly from JobDrivers using the job API).
> 
> Therefore create a _locked version of such function, so that it
> can be used in both cases.
> 
> List of functions duplicated as _locked:
> job_is_ready (both versions are public)
> job_is_completed (both versions are public)
> job_is_cancelled (_locked version is public, needed by mirror.c)
> job_pause_point (_locked version is static, purely done to simplify the code)
> job_cancel_requested (_locked version is static)
> 
> Note: at this stage, job_{lock/unlock} and job lock guard macros
> are *nop*.
> 
> Signed-off-by: Emanuele Giuseppe Esposito 
> ---
>  include/qemu/job.h | 25 +---
>  job.c  | 48 --
>  2 files changed, 64 insertions(+), 9 deletions(-)
> 
> diff --git a/include/qemu/job.h b/include/qemu/job.h
> index 6000463126..aa33d091b1 100644
> --- a/include/qemu/job.h
> +++ b/include/qemu/job.h
> @@ -473,21 +473,40 @@ const char *job_type_str(const Job *job);
>  /** Returns true if the job should not be visible to the management layer. */
>  bool job_is_internal(Job *job);
>  
> -/** Returns whether the job is being cancelled. */
> +/**
> + * Returns whether the job is being cancelled.
> + * Called with job_mutex *not* held.
> + */
>  bool job_is_cancelled(Job *job);
>  
> +/** Just like job_is_cancelled, but called between job_lock and job_unlock */
> +bool job_is_cancelled_locked(Job *job);
> +
>  /**
>   * Returns whether the job is scheduled for cancellation (at an
>   * indefinite point).
> + * Called with job_mutex *not* held.
>   */
>  bool job_cancel_requested(Job *job);
>  
> -/** Returns whether the job is in a completed state. */
> +/**
> + * Returns whether the job is in a completed state.
> + * Called with job_mutex *not* held.
> + */
>  bool job_is_completed(Job *job);
>  
> -/** Returns whether the job is ready to be completed. */
> +/** Same as job_is_completed(), but assumes job_lock is held. */
> +bool job_is_completed_locked(Job *job);

Any reason why this comment is phrased differently than for
job_is_cancelled_locked()? I don't mind which one we use, but if they
should express the same thing, it would be better to have the same
wording. If they should express different things, it need to be clearer
what they are.

Also, I assume job_mutex is meant because job_lock() is a function, not
the lock that is held.

> +/**
> + * Returns whether the job is ready to be completed.
> + * Called with job_mutex *not* held.
> + */
>  bool job_is_ready(Job *job);
>  
> +/** Same as job_is_ready(), but assumes job_lock is held. */
> +bool job_is_ready_locked(Job *job);

Same as above.

>  /**
>   * Request @job to pause at the next pause point. Must be paired with
>   * job_resume(). If the job is supposed to be resumed by user action, call

Kevin

Re: [PATCH] gitlab-ci: Fix the build-cfi-aarch64 and build-cfi-ppc64-s390x jobs


On 03/06/2022 18.17, Richard Henderson wrote:

On 6/3/22 05:48, Thomas Huth wrote:

The job definitions recently got a second "variables:" section by
accident and thus are failing now if one tries to run them. Merge
the two sections into one again to fix the issue.

And while we're at it, bump the timeout here (70 minutes are currently
not enough for the aarch64 job). The jobs are marked as manual anyway,
so if the user starts them, they want to see their result for sure and
then it's annoying if the job timeouts too early.

Fixes: e312d1fdbb ("gitlab: convert build/container jobs to 
.base_job_template")

Signed-off-by: Thomas Huth 
---
  I wonder whether we should remove the build-cfi-aarch64 job instead.
  When I tried to run it during the past months, it was always failing
  for me. This time, I tried to bump the timeout while I was at it,
  and it takes longer than 80 minutes here to finish - so I asume
  nobody ever ran this successfully in the last months... Is anybody
  using this job at all? I think if we want to have CFI coverage here,
  it should get replaced by a custom runner job that runs on a more
  beefy machine... (the ppc64-s390x job is fine by the way, it often
  only runs a little bit longer than 60 minutes - I still bumped the
  timeout here, too, just to be on the safe side)


Acked-by: Richard Henderson 

I think it might be useful to extend the other s390x jobs a bit too.  The 
last couple of fails have the test *nearly* completing.  E.g. your most 
recent pr:


https://gitlab.com/qemu-project/qemu/-/jobs/2544009687


These tests are running on the custom s390x runner machine - I don't have 
access to that one, i.e. I also do not have any means to test changes here 
--> it would be great if that change could be done by somebody who has 
access to that machine... Peter? Christian?


 Thomas

Re: [PATCH v6 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)


On 6/1/22 03:25, Xiaojuan Yang wrote:

+static uint64_t extioi_readw(void *opaque, hwaddr addr, unsigned size)
+{
+LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+unsigned long offset = addr & 0x;
+uint32_t index, cpu, ret = 0;
+
+switch (offset) {
+case EXTIOI_NODETYPE_START ... EXTIOI_NODETYPE_END - 1:
+index = (offset - EXTIOI_NODETYPE_START) >> 2;
+ret = s->nodetype[index];
+break;
+case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1:
+index = offset - EXTIOI_IPMAP_START;
+ret = *(uint32_t *)&s->ipmap[index];


This...


+break;
+case EXTIOI_ENABLE_START ... EXTIOI_ENABLE_END - 1:
+index = (offset - EXTIOI_ENABLE_START) >> 2;
+ret = s->enable[index];
+break;
+case EXTIOI_BOUNCE_START ... EXTIOI_BOUNCE_END - 1:
+index = (offset - EXTIOI_BOUNCE_START) >> 2;
+ret = s->bounce[index];
+break;
+case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
+index = ((offset - EXTIOI_COREISR_START) & 0x1f) >> 2;
+cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
+ret = s->coreisr[cpu][index];
+break;
+case EXTIOI_COREMAP_START ... EXTIOI_COREMAP_END - 1:
+index = offset - EXTIOI_COREMAP_START;
+ret = *(uint32_t *)&s->coremap[index];


... and this are points of concern.  You can't simply re-interpret an array of uint8_t as 
uint32_t without running into host endian issues.


I wonder why you've declared them as uint8_t at all?  Both read and write use this cast. 
Was this some attempt to avoid


s->coremap[index / 4]?

or what?


+static const VMStateDescription vmstate_loongarch_extioi = {
+.name = TYPE_LOONGARCH_EXTIOI,
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32_ARRAY(bounce, LoongArchExtIOI, EXTIOI_IRQS_GROUP_COUNT),
+VMSTATE_UINT32_2DARRAY(coreisr, LoongArchExtIOI, LOONGARCH_MAX_VCPUS,
+   EXTIOI_IRQS_GROUP_COUNT),
+VMSTATE_UINT32_ARRAY(nodetype, LoongArchExtIOI,
+ EXTIOI_IRQS_NODETYPE_COUNT / 2),
+VMSTATE_UINT32_ARRAY(enable, LoongArchExtIOI, EXTIOI_IRQS / 32),
+VMSTATE_UINT32_ARRAY(isr, LoongArchExtIOI, EXTIOI_IRQS / 32),
+VMSTATE_UINT8_ARRAY(ipmap, LoongArchExtIOI, EXTIOI_IRQS_IPMAP_SIZE),
+VMSTATE_UINT8_ARRAY(coremap, LoongArchExtIOI, EXTIOI_IRQS),
+VMSTATE_END_OF_LIST()
+}


Missing the sw_* members.


r~

Re: [PATCH v6 06/18] jobs: protect jobs with job_lock/unlock

Am 14.03.2022 um 14:36 hat Emanuele Giuseppe Esposito geschrieben:
> Introduce the job locking mechanism through the whole job API,
> following the comments  in job.h and requirements of job-monitor
> (like the functions in job-qmp.c, assume lock is held) and
> job-driver (like in mirror.c and all other JobDriver, lock is not held).
> 
> Use the _locked helpers introduced before to differentiate
> between functions called with and without job_mutex.
> This only applies to function that are called under both
> cases, all the others will be renamed later.
> 
> job_{lock/unlock} is independent from real_job_{lock/unlock}.
> 
> Note: at this stage, job_{lock/unlock} and job lock guard macros
> are *nop*.
> 
> Signed-off-by: Emanuele Giuseppe Esposito 
> ---
>  block.c |  18 ---
>  block/replication.c |   8 ++-
>  blockdev.c  |  17 --
>  blockjob.c  |  56 +---
>  job-qmp.c   |   2 +
>  job.c   | 125 +++-
>  monitor/qmp-cmds.c  |   6 ++-
>  qemu-img.c  |  41 +--
>  8 files changed, 187 insertions(+), 86 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 718e4cae8b..5dc46fde11 100644
> --- a/block.c
> +++ b/block.c
> @@ -4978,7 +4978,9 @@ static void bdrv_close(BlockDriverState *bs)
>  
>  void bdrv_close_all(void)
>  {
> -assert(job_next(NULL) == NULL);
> +WITH_JOB_LOCK_GUARD() {
> +assert(job_next(NULL) == NULL);
> +}
>  GLOBAL_STATE_CODE();

This series seems really hard to review patch by patch, in this case
because I would have to know whether you intended job_next() to be
called with the lock held or not. Nothing in job.h indicates either way
at this point in the series.

Patch 11 answers this by actually renaming it job_next_locked(), but
always having to refer to the final state after the whole series is
applied is really not how things should work. We're splitting the work
into individual patches so that the state after each single patch makes
sense on its own. Otherwise the whole series could as well be a single
patch. :-(

So I'd argue that patch 11 should probably come before this one.

Anyway, I guess I'll try to make my way to the end of the series quickly
and then somehow try to verify whatever the state is then.

Kevin

Re: [PATCH v6 38/43] hw/loongarch: Add LoongArch ls7a rtc device support


On 6/1/22 03:25, Xiaojuan Yang wrote:

This patch add ls7a rtc device support.

Signed-off-by: Xiaojuan Yang 
Signed-off-by: Song Gao 
---
  MAINTAINERS|   1 +
  hw/loongarch/Kconfig   |   1 +
  hw/loongarch/loongson3.c   |   3 +
  hw/rtc/Kconfig |   3 +
  hw/rtc/ls7a_rtc.c  | 528 +
  hw/rtc/meson.build |   1 +
  include/hw/pci-host/ls7a.h |   4 +
  7 files changed, 541 insertions(+)
  create mode 100644 hw/rtc/ls7a_rtc.c


Reviewed-by: Richard Henderson 

r~



diff --git a/MAINTAINERS b/MAINTAINERS
index c22f95fb6a..9f4ee7978d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1137,6 +1137,7 @@ F: include/hw/loongarch/virt.h
  F: include/hw/intc/loongarch_*.h
  F: hw/intc/loongarch_*.c
  F: include/hw/pci-host/ls7a.h
+F: hw/rtc/ls7a_rtc.c
  
  M68K Machines

  -
diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig
index 8552ff4bee..35b6680772 100644
--- a/hw/loongarch/Kconfig
+++ b/hw/loongarch/Kconfig
@@ -13,3 +13,4 @@ config LOONGARCH_VIRT
  select LOONGARCH_PCH_PIC
  select LOONGARCH_PCH_MSI
  select LOONGARCH_EXTIOI
+select LS7A_RTC
diff --git a/hw/loongarch/loongson3.c b/hw/loongarch/loongson3.c
index 7bc17113dc..95984c9086 100644
--- a/hw/loongarch/loongson3.c
+++ b/hw/loongarch/loongson3.c
@@ -97,6 +97,9 @@ static void loongarch_devices_init(DeviceState *pch_pic)
   * Create some unimplemented devices to emulate this.
   */
  create_unimplemented_device("pci-dma-cfg", 0x1001041c, 0x4);
+sysbus_create_simple("ls7a_rtc", LS7A_RTC_REG_BASE,
+ qdev_get_gpio_in(pch_pic,
+ LS7A_RTC_IRQ - PCH_PIC_IRQ_OFFSET));
  }
  
  static void loongarch_irq_init(LoongArchMachineState *lams)

diff --git a/hw/rtc/Kconfig b/hw/rtc/Kconfig
index 730c272bc5..d0d8dda084 100644
--- a/hw/rtc/Kconfig
+++ b/hw/rtc/Kconfig
@@ -27,3 +27,6 @@ config SUN4V_RTC
  
  config GOLDFISH_RTC

  bool
+
+config LS7A_RTC
+bool
diff --git a/hw/rtc/ls7a_rtc.c b/hw/rtc/ls7a_rtc.c
new file mode 100644
index 00..fe6710310f
--- /dev/null
+++ b/hw/rtc/ls7a_rtc.c
@@ -0,0 +1,528 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Loongarch LS7A Real Time Clock emulation
+ *
+ * Copyright (C) 2021 Loongson Technology Corporation Limited
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+#include "include/hw/register.h"
+#include "qemu/timer.h"
+#include "sysemu/sysemu.h"
+#include "qemu/cutils.h"
+#include "qemu/log.h"
+#include "migration/vmstate.h"
+#include "hw/misc/unimp.h"
+#include "sysemu/rtc.h"
+#include "hw/registerfields.h"
+
+#define SYS_TOYTRIM0x20
+#define SYS_TOYWRITE0  0x24
+#define SYS_TOYWRITE1  0x28
+#define SYS_TOYREAD0   0x2C
+#define SYS_TOYREAD1   0x30
+#define SYS_TOYMATCH0  0x34
+#define SYS_TOYMATCH1  0x38
+#define SYS_TOYMATCH2  0x3C
+#define SYS_RTCCTRL0x40
+#define SYS_RTCTRIM0x60
+#define SYS_RTCWRTIE0  0x64
+#define SYS_RTCREAD0   0x68
+#define SYS_RTCMATCH0  0x6C
+#define SYS_RTCMATCH1  0x70
+#define SYS_RTCMATCH2  0x74
+
+#define LS7A_RTC_FREQ 32768
+#define TIMER_NUMS3
+/*
+ * Shift bits and filed mask
+ */
+
+FIELD(TOY, MON, 26, 6)
+FIELD(TOY, DAY, 21, 5)
+FIELD(TOY, HOUR, 16, 5)
+FIELD(TOY, MIN, 10, 6)
+FIELD(TOY, SEC, 4, 6)
+FIELD(TOY, MSEC, 0, 4)
+
+FIELD(TOY_MATCH, YEAR, 26, 6)
+FIELD(TOY_MATCH, MON, 22, 4)
+FIELD(TOY_MATCH, DAY, 17, 5)
+FIELD(TOY_MATCH, HOUR, 12, 5)
+FIELD(TOY_MATCH, MIN, 6, 6)
+FIELD(TOY_MATCH, SEC, 0, 6)
+
+FIELD(RTC_CTRL, RTCEN, 13, 1)
+FIELD(RTC_CTRL, TOYEN, 11, 1)
+FIELD(RTC_CTRL, EO, 8, 1)
+
+#define TYPE_LS7A_RTC "ls7a_rtc"
+OBJECT_DECLARE_SIMPLE_TYPE(LS7ARtcState, LS7A_RTC)
+
+struct LS7ARtcState {
+SysBusDevice parent_obj;
+
+MemoryRegion iomem;
+/*
+ * Needed to preserve the tick_count across migration, even if the
+ * absolute value of the rtc_clock is different on the source and
+ * destination.
+ */
+int64_t offset_toy;
+int64_t offset_rtc;
+uint64_t save_toy_mon;
+uint64_t save_toy_year;
+uint64_t save_rtc;
+int64_t data;
+int tidx;
+uint32_t toymatch[3];
+uint32_t toytrim;
+uint32_t cntrctl;
+uint32_t rtctrim;
+uint32_t rtccount;
+uint32_t rtcmatch[3];
+QEMUTimer *toy_timer[TIMER_NUMS];
+QEMUTimer *rtc_timer[TIMER_NUMS];
+qemu_irq irq;
+};
+
+/* switch nanoseconds time to rtc ticks */
+static inline uint64_t ls7a_rtc_ticks(void)
+{
+return qemu_clock_get_ns(rtc_clock) * LS7A_RTC_FREQ / 
NANOSECONDS_PER_SECOND;
+}
+
+/* switch rtc ticks to nanoseconds */
+static inline uint64_t ticks_to_ns(uint64_t ticks)
+{
+return ticks * NANOSECONDS_PER_SECOND / LS7A_RTC_FREQ;
+}
+
+static inline bool toy_enabled(LS7ARtcState *s)
+{
+return FIELD_EX32(s->cntrctl, RTC_CTRL, TOYEN) &&
+   FIELD_EX32(s->cntrctl, RTC_CTRL, EO);
+}
+
+static inline bool rtc_enabled(LS7ARtcState *s)
+{

Re: [PATCH v6 40/43] hw/loongarch: Add LoongArch power manager support


On 6/1/22 03:25, Xiaojuan Yang wrote:

Signed-off-by: Xiaojuan Yang 
Signed-off-by: Song Gao 
---
  hw/loongarch/loongson3.c | 45 +++-
  1 file changed, 44 insertions(+), 1 deletion(-)



Acked-by: Richard Henderson 


+#define PM_BASE 0x1008
+#define PM_SIZE 0x100
+#define PM_CTRL 0x10
+
+static uint64_t loongarch_virt_pm_read(void *opaque, hwaddr addr, unsigned 
size)
+{
+return 0;
+}
+
+static void loongarch_virt_pm_write(void *opaque, hwaddr addr,
+   uint64_t val, unsigned size)
+{
+if (addr != PM_CTRL) {
+return;
+}
+
+switch (val) {
+case 0x00:
+qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+return;
+case 0xff:
+qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+return;
+default:
+return;
+}
+}


It would be nice to add a comment that this is a placeholder for missing ACPI, and will 
eventually be replaced.



r~

Re: [PATCH v6 15/18] job: detect change of aiocontext within job coroutine

Am 14.03.2022 um 14:37 hat Emanuele Giuseppe Esposito geschrieben:
> From: Paolo Bonzini 
> 
> We want to make sure access of job->aio_context is always done
> under either BQL or job_mutex. The problem is that using
> aio_co_enter(job->aiocontext, job->co) in job_start and job_enter_cond
> makes the coroutine immediately resume, so we can't hold the job lock.
> And caching it is not safe either, as it might change.
> 
> job_start is under BQL, so it can freely read job->aiocontext, but
> job_enter_cond is not. In order to fix this, use aio_co_wake():
> the advantage is that it won't use job->aiocontext, but the
> main disadvantage is that it won't be able to detect a change of
> job AioContext.
> 
> Calling bdrv_try_set_aio_context() will issue the following calls
> (simplified):
> * in terms of  bdrv callbacks:
>   .drained_begin -> .set_aio_context -> .drained_end
> * in terms of child_job functions:
>   child_job_drained_begin -> child_job_set_aio_context -> 
> child_job_drained_end
> * in terms of job functions:
>   job_pause_locked -> job_set_aio_context -> job_resume_locked
> 
> We can see that after setting the new aio_context, job_resume_locked
> calls again job_enter_cond, which then invokes aio_co_wake(). But
> while job->aiocontext has been set in job_set_aio_context,
> job->co->ctx has not changed, so the coroutine would be entering in
> the wrong aiocontext.
> 
> Using aio_co_schedule in job_resume_locked() might seem as a valid
> alternative, but the problem is that the bh resuming the coroutine
> is not scheduled immediately, and if in the meanwhile another
> bdrv_try_set_aio_context() is run (see test_propagate_mirror() in
> test-block-iothread.c), we would have the first schedule in the
> wrong aiocontext, and the second set of drains won't even manage
> to schedule the coroutine, as job->busy would still be true from
> the previous job_resume_locked().
> 
> The solution is to stick with aio_co_wake(), but then detect every time
> the coroutine resumes back from yielding if job->aio_context
> has changed. If so, we can reschedule it to the new context.
> 
> Check for the aiocontext change in job_do_yield_locked because:
> 1) aio_co_reschedule_self requires to be in the running coroutine
> 2) since child_job_set_aio_context allows changing the aiocontext only
>while the job is paused, this is the exact place where the coroutine
>resumes, before running JobDriver's code.
> 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Paolo Bonzini 
> ---
>  job.c | 24 +---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/job.c b/job.c
> index 89c0e6bed9..10a5981748 100644
> --- a/job.c
> +++ b/job.c
> @@ -543,11 +543,12 @@ void job_enter_cond_locked(Job *job, bool(*fn)(Job 
> *job))
>  return;
>  }
>  
> -assert(!job->deferred_to_main_loop);

Why doesn't this assertion hold true any more?

>  timer_del(&job->sleep_timer);
>  job->busy = true;
>  real_job_unlock();
> -aio_co_enter(job->aio_context, job->co);
> +job_unlock();
> +aio_co_wake(job->co);
> +job_lock();
>  }
>  
>  void job_enter(Job *job)
> @@ -568,6 +569,8 @@ void job_enter(Job *job)
>   */
>  static void coroutine_fn job_do_yield_locked(Job *job, uint64_t ns)
>  {
> +AioContext *next_aio_context;
> +
>  real_job_lock();
>  if (ns != -1) {
>  timer_mod(&job->sleep_timer, ns);
> @@ -579,6 +582,20 @@ static void coroutine_fn job_do_yield_locked(Job *job, 
> uint64_t ns)
>  qemu_coroutine_yield();
>  job_lock();
>  
> +next_aio_context = job->aio_context;
> +/*
> + * Coroutine has resumed, but in the meanwhile the job AioContext
> + * might have changed via bdrv_try_set_aio_context(), so we need to move
> + * the coroutine too in the new aiocontext.
> + */
> +while (qemu_get_current_aio_context() != next_aio_context) {
> +job_unlock();
> +aio_co_reschedule_self(next_aio_context);
> +job_lock();
> +next_aio_context = job->aio_context;
> +}
> +
> +

Extra empty line.

>  /* Set by job_enter_cond_locked() before re-entering the coroutine.  */
>  assert(job->busy);
>  }
> @@ -680,7 +697,6 @@ void job_resume_locked(Job *job)
>  if (job->pause_count) {
>  return;
>  }
> -
>  /* kick only if no timer is pending */
>  job_enter_cond_locked(job, job_timer_not_pending_locked);
>  }

This hunk looks unrelated.

Kevin

Re: [PATCH] disas: Remove libvixl disassembler


On 6/3/22 09:42, Thomas Huth wrote:

The disassembly via capstone should be superiour to our old vixl
sources nowadays, so let's finally cut this old disassembler out
of the QEMU source tree.

Signed-off-by: Thomas Huth 
---
  See also the discussions here:
  - https://lists.gnu.org/archive/html/qemu-devel/2022-06/msg00428.html
  - https://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg03416.html


Reviewed-by: Richard Henderson 
Tested-by: Richard Henderson 


r~

Re: [PATCH] disas: Remove libvixl disassembler


On 03/06/2022 19.26, Claudio Fontana wrote:

On 6/3/22 18:42, Thomas Huth wrote:

The disassembly via capstone should be superiour to our old vixl
sources nowadays, so let's finally cut this old disassembler out
of the QEMU source tree.

Signed-off-by: Thomas Huth 


agreed, one thought: at the time I added this thing, I had to add C++ 
compilation support,
maybe something we can now drop if there are no more C++ users?


I thought about that, too, but we still have disas/nanomips.cpp left and the 
Windows-related files in qga/vss-win32/* . And I think Paolo was considering 
to use C++ for coroutine fixes - not sure whether that still is planned, though.


 Thomas

Re: about the current status of Multi-process QEMU / out-of-process emulation

2022-06-03 Thread Jag Raman

On Jun 3, 2022, at 11:34 AM, Yu Zhang 
mailto:yu.zh...@ionos.com>> wrote:

Hello Dongli, Elena, John, and Jagannathan,

I'm interested in the "multi-process QEMU" feature and got the kind reply by 
Mr. Vivier that I may contact you for this.
On one of the QEMU docs [1] I saw the command line:

+  /usr/bin/qemu-system-x86_64\
+  -machine x-remote  \
+  -device lsi53c895a,id=lsi0 \
+  -drive id=drive_image2,file=/build/ol7-nvme-test-1.qcow2   \
+  -device scsi-hd,id=drive2,drive=drive_image2,bus=lsi0.0,scsi-id=0  \
+  -object x-remote-object,id=robj1,devid=lsi1,fd=4,

It seems that the man page of qemu contains no parameter and option yet for 
this feature. The qemu docs, such as [2][3][4] are either not up-to-date or 
"doesn't reflect the current status of the implementation".
So may I know whether is it still in experimental stage or mature enough for 
use? And even a few further questions:

Hello Yu,

We are working on vfio-user for QEMU which would supersede multi-process QEMU.
The vfio-user feature is currently under review for merging with QEMU. We would 
drop
multi-process QEMU support after the vfio-user changes are merged.

We use the following repo for testing vfio-user before sending the patches for
review. You may be interested in checking it out:
https://github.com/oracle/qemu/tree/master

- When creating the orchestrator, can we specify a machine type such as 
pc-i440fx-7.0 for -machine?

For vfio-user, the machine type on the remote QEMU process (server) is
always “x-remote”. The client QEMU could be of any machine type.

- Can each device has a dedicated emulation process or shares one process for 
emulating multiple devices?

Each device could be running in a dedicated process, or multiple
devices could share one process.

- Can we find more command line examples showing the combination of 
orchestrator, remote emulation process, memory-backend-memfd and 
x-pci-proxy-dev?

For vfio-user, we could give you a heads up once they are merged into QEMU. We
are using the following for testing our changes, which you could checkout
in the meanwhile:
scripts/vfiouser-launcher.py

Could you please give us more details about what you’re trying to do? Which
devices are you trying to emulate in the remote process?

Thank you!
--
Jag

Thank you very much and all the best

Yu Zhang
03.06.2022

[1] https://www.qemu.org/docs/master/system/multi-process.html
[2] https://wiki.qemu.org/Features/MultiProcessQEMU
[3] 
https://lxr.missinglinkelectronics.com/qemu+v7.0.0/docs/devel/multi-process.rst
[4] https://qemu.readthedocs.io/en/latest/devel/multi-process.html

-- Forwarded message -
From: Laurent Vivier mailto:laur...@vivier.eu>>
Date: Fri, Jun 3, 2022 at 4:14 PM
Subject: Re: about the current status of Multi-process QEMU / out-of-process 
emulation
To: Yu Zhang mailto:yu.zh...@ionos.com>>

Hi Yu,

I'm not the author of this documentation, only the person that has merged the 
last change in the repo.

According to the logs you should contact Dongli Zhang 
mailto:dongli.zh...@oracle.com>>, Elena Ufimtseva
mailto:elena.ufimts...@oracle.com>>, John G Johnson 
mailto:john.g.john...@oracle.com>> or Jagannathan 
Raman
mailto:jag.ra...@oracle.com>> .

Thanks,
Laurent

Le 03/06/2022 à 12:17, Yu Zhang a écrit :
> Dear Mr. Vivier,
>
> I saw that you authored the QEMU page for "Multi-process QEMU".
> (https://www.qemu.org/docs/master/system/multi-process.html
> )
>
> I'm interested in this feature, but feel a little confused with the command 
> line:
>
> +  /usr/bin/qemu-system-x86_64\
> +  -machine x-remote  \
> +  -device lsi53c895a,id=lsi0 \
> +  -drive id=drive_image2,file=/build/ol7-nvme-test-1.qcow2   \
> +  -device scsi-hd,id=drive2,drive=drive_image2,bus=lsi0.0,scsi-id=0  \
> +  -object x-remote-object,id=robj1,devid=lsi1,fd=4,
>
> It seems that the man page of qemu command contains no parameter and option 
> yet for this feature.
> May I know whether is it still in experimental stage? And even a few more 
> questions:
>
> - Is "x-remote" a standalone machine type for creating the orchestrator?
> - Can each device has a dedicated emulation process or shares one process for 
> emulating multiple
> devices?
> - Can I find more command line examples illustrating the combination of 
> orchestrator, remote
> emulation process, memory-backend-memfd and x-pci-proxy-dev?
>
> Thank you very much
> Kind regard
>
> Yu Zhang
> 03.06.2022

Re: [PATCH RESEND v3 1/8] target/ppc: Implemented vector divide instructions


On 5/25/22 06:49, Lucas Mateus Castro(alqotel) wrote:

From: "Lucas Mateus Castro (alqotel)"

Implement the following PowerISA v3.1 instructions:
vdivsw: Vector Divide Signed Word
vdivuw: Vector Divide Unsigned Word
vdivsd: Vector Divide Signed Doubleword
vdivud: Vector Divide Unsigned Doubleword

Signed-off-by: Lucas Mateus Castro (alqotel)
---
  target/ppc/insn32.decode|  7 +++
  target/ppc/translate/vmx-impl.c.inc | 85 +
  2 files changed, 92 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH RESEND v3 3/8] target/ppc: Implemented vector divide extended word


On 5/25/22 06:49, Lucas Mateus Castro(alqotel) wrote:

From: "Lucas Mateus Castro (alqotel)"

Implement the following PowerISA v3.1 instructions:
vdivesw: Vector Divide Extended Signed Word
vdiveuw: Vector Divide Extended Unsigned Word

Signed-off-by: Lucas Mateus Castro (alqotel)
---
  target/ppc/insn32.decode|  3 ++
  target/ppc/translate/vmx-impl.c.inc | 48 +
  2 files changed, 51 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH RESEND v3 7/8] target/ppc: Implemented vector module word/doubleword


On 5/25/22 06:49, Lucas Mateus Castro(alqotel) wrote:

From: "Lucas Mateus Castro (alqotel)"

Implement the following PowerISA v3.1 instructions:
vmodsw: Vector Modulo Signed Word
vmoduw: Vector Modulo Unsigned Word
vmodsd: Vector Modulo Signed Doubleword
vmodud: Vector Modulo Unsigned Doubleword

Signed-off-by: Lucas Mateus Castro (alqotel)
---
  target/ppc/insn32.decode|  5 +
  target/ppc/translate/vmx-impl.c.inc | 10 ++
  2 files changed, 15 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH RESEND v3 4/8] host-utils: Implemented unsigned 256-by-128 division


On 5/25/22 06:49, Lucas Mateus Castro(alqotel) wrote:

From: "Lucas Mateus Castro (alqotel)"

Based on already existing QEMU implementation, created an unsigned 256
bit by 128 bit division needed to implement the vector divide extended
unsigned instruction from PowerISA3.1

Signed-off-by: Lucas Mateus Castro (alqotel)
---
This patch had received Reviewed-by by Richard Henderson pending on the
placemente of clz128 being moved to int128.h, but clz128 ended up being changed
to accommodate to int128.h (i.e. the lack of clz64), so out of precaution I'd
like to request a review of the clz128 implementation
---
  include/qemu/host-utils.h |   2 +
  include/qemu/int128.h |  38 +++
  util/host-utils.c | 129 ++
  3 files changed, 169 insertions(+)


Reviewed-by: Richard Henderson 

r~

[PATCH v4 00/11] QOM'ify PIIX southbridge creation

v4:
* Rebase onto 
https://patchew.org/QEMU/20220530112718.26582-1-philippe.mathieu.da...@gmail.com/
* Cosmetics (fix typo, omit "include") (Mark, Philippe)
* Split piix3 and piix4 (Philippe)
* s/Found-by/Reported-by/ (Philippe)
* Don't alias smbus (Mark)

v3:
* Rebase onto 'hw/acpi/piix4: remove legacy piix4_pm_init() function' (Mark) [1]
* Use embedded structs for touched PCI devices (Mark)
* Fix piix4's rtc embedded struct to be initialized by
  object_initialize_child() (Peter) [2]

Testing done:

1)
`make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
Result: All pass.

2)
* `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
* `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
  debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`

In both cases the system booted successfully and it was possible to shut down
the system using the `poweroff` command.

v2:
* Preserve `DeviceState *` as return value of piix4_create() (Mark)
* Aggregate all type name movements into first commit (Mark)
* Have piix4 southbridge rather than malta board instantiate piix4 pm (me)

Testing done:

1)
`make check-avocado` for --target-list=x86_64-softmmu,mips-softmmu
Result: All pass.

2)
Modify pci_piix3_realize() to start with
error_setg(errp, "This is a test");
Then start `qemu-system-x86_64 -M pc -m 1G -accel kvm -cpu host -cdrom
archlinux-2022.05.01-x86_64.iso`.
Result: qemu-system-x86_64 aborts with: "This is a test"

v1:
The piix3 and piix4 southbridge devices still rely on create() functions which
are deprecated. This series resolves these functions piece by piece to
modernize the code.

Both devices are modified in lockstep where possible to provide more context.

Testing done:
* `qemu-system-x86_64 -M pc -m 2G -cdrom archlinux-2022.05.01-x86_64.iso`
* `qemu-system-mipsel -M malta -kernel vmlinux-3.2.0-4-4kc-malta -hda
  debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=tty0"`

In both cases the system booted successfully and it was possible to shut down
the system using the `poweroff` command.

[1] https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg05686.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01128.html

Bernhard Beschow (11):
  hw/southbridge/piix: Aggregate all PIIX southbridge type names
  hw/isa/piix4: Use object_initialize_child() for embedded struct
  hw/isa/piix4: Move pci_map_irq_fn' near pci_set_irq_fn
  hw/isa/piix4: QOM'ify PCI device creation and wiring
  hw/isa/piix4: Factor out ISABus retrieval from piix4_create()
  hw/isa/piix4: QOM'ify PIIX4 PM creation
  hw/isa/piix4: Inline and remove piix4_create()
  hw/isa/piix3: Move pci_map_irq_fn near pci_set_irq_fn
  hw/isa/piix3: QOM'ify PCI device creation and wiring
  hw/isa/piix3: Factor out ISABus retrieval from piix3_create()
  hw/isa/piix3: Inline and remove piix3_create()

 hw/i386/pc_piix.c |   7 +-
 hw/isa/piix3.c|  98 +++-
 hw/isa/piix4.c| 119 +-
 hw/mips/malta.c   |   9 ++-
 include/hw/isa/isa.h  |   2 -
 include/hw/southbridge/piix.h |   6 +-
 6 files changed, 128 insertions(+), 113 deletions(-)

-- 
2.36.1

[PATCH v4 03/11] hw/isa/piix4: Move pci_map_irq_fn' near pci_set_irq_fn

The pci_map_irq_fn was implemented below type_init() which made it
inaccessible to QOM functions. So move it up.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/isa/piix4.c | 50 +-
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 1d04fb6a55..18aa24424f 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -74,6 +74,31 @@ static void piix4_set_irq(void *opaque, int irq_num, int 
level)
 }
 }
 
+static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+{
+int slot;
+
+slot = PCI_SLOT(pci_dev->devfn);
+
+switch (slot) {
+/* PIIX4 USB */
+case 10:
+return 3;
+/* AMD 79C973 Ethernet */
+case 11:
+return 1;
+/* Crystal 4281 Sound */
+case 12:
+return 2;
+/* PCI slot 1 to 4 */
+case 18 ... 21:
+return ((slot - 18) + irq_num) & 0x03;
+/* Unknown device, don't do any translation */
+default:
+return irq_num;
+}
+}
+
 static void piix4_isa_reset(DeviceState *dev)
 {
 PIIX4State *d = PIIX4_PCI_DEVICE(dev);
@@ -266,31 +291,6 @@ static void piix4_register_types(void)
 
 type_init(piix4_register_types)
 
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
-{
-int slot;
-
-slot = PCI_SLOT(pci_dev->devfn);
-
-switch (slot) {
-/* PIIX4 USB */
-case 10:
-return 3;
-/* AMD 79C973 Ethernet */
-case 11:
-return 1;
-/* Crystal 4281 Sound */
-case 12:
-return 2;
-/* PCI slot 1 to 4 */
-case 18 ... 21:
-return ((slot - 18) + irq_num) & 0x03;
-/* Unknown device, don't do any translation */
-default:
-return irq_num;
-}
-}
-
 DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
 {
 PIIX4State *s;
-- 
2.36.1

[PATCH v4 01/11] hw/southbridge/piix: Aggregate all PIIX southbridge type names

TYPE_PIIX3_PCI_DEVICE resides there as already, so add the remaining
ones, too.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/isa/piix3.c| 3 ---
 include/hw/isa/isa.h  | 2 --
 include/hw/southbridge/piix.h | 4 
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index dab901c9ad..d96ce2b788 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -35,9 +35,6 @@
 
 #define XEN_PIIX_NUM_PIRQS  128ULL
 
-#define TYPE_PIIX3_DEVICE "PIIX3"
-#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
-
 static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
 {
 qemu_set_irq(piix3->pic[pic_irq],
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 034d706ba1..e9fa2f5cea 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -144,6 +144,4 @@ static inline ISABus *isa_bus_from_device(ISADevice *d)
 return ISA_BUS(qdev_get_parent_bus(DEVICE(d)));
 }
 
-#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
-
 #endif
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 976b4da582..3b97186f75 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -64,6 +64,10 @@ typedef struct PIIXState PIIX3State;
 DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
  TYPE_PIIX3_PCI_DEVICE)
 
+#define TYPE_PIIX3_DEVICE "PIIX3"
+#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
+#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
+
 PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus);
 
 DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus);
-- 
2.36.1

[PATCH v4 05/11] hw/isa/piix4: Factor out ISABus retrieval from piix4_create()

Modernizes the code.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/isa/piix4.c| 6 +-
 hw/mips/malta.c   | 3 ++-
 include/hw/southbridge/piix.h | 2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 058bebb5e2..96df21a610 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -312,7 +312,7 @@ static void piix4_register_types(void)
 
 type_init(piix4_register_types)
 
-DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
+DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus)
 {
 PCIDevice *pci;
 DeviceState *dev;
@@ -322,10 +322,6 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus 
**isa_bus, I2CBus **smbus)
   TYPE_PIIX4_PCI_DEVICE);
 dev = DEVICE(pci);
 
-if (isa_bus) {
-*isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
-}
-
 if (smbus) {
 pci = pci_new(devfn + 3, TYPE_PIIX4_PM);
 qdev_prop_set_uint32(DEVICE(pci), "smb_io_base", 0x1100);
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 9ffdc5b8f1..e446b25ad0 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1399,7 +1399,8 @@ void mips_malta_init(MachineState *machine)
 empty_slot_init("GT64120", 0, 0x2000);
 
 /* Southbridge */
-dev = piix4_create(pci_bus, &isa_bus, &smbus);
+dev = piix4_create(pci_bus, &smbus);
+isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
 
 /* Interrupt controller */
 qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq);
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 3b97186f75..dab5c9704e 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -70,6 +70,6 @@ DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
 
 PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus);
 
-DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus);
+DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus);
 
 #endif
-- 
2.36.1

[PATCH v4 10/11] hw/isa/piix3: Factor out ISABus retrieval from piix3_create()

Modernizes the code.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Mark Cave-Ayland 
---
 hw/i386/pc_piix.c | 3 ++-
 hw/isa/piix3.c| 3 +--
 include/hw/southbridge/piix.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d6668b7c06..c884d1a489 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -207,9 +207,10 @@ static void pc_init1(MachineState *machine,
   pci_memory, ram_memory);
 pcms->bus = pci_bus;
 
-piix3 = piix3_create(pci_bus, &isa_bus);
+piix3 = piix3_create(pci_bus);
 piix3->pic = x86ms->gsi;
 piix3_devfn = piix3->dev.devfn;
+isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
 } else {
 pci_bus = NULL;
 i440fx_state = NULL;
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index de532cc692..c6ff7795f4 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -400,7 +400,7 @@ static void piix3_register_types(void)
 
 type_init(piix3_register_types)
 
-PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus)
+PIIX3State *piix3_create(PCIBus *pci_bus)
 {
 PIIX3State *piix3;
 PCIDevice *pci_dev;
@@ -409,7 +409,6 @@ PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus)
 
 pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
 piix3 = PIIX3_PCI_DEVICE(pci_dev);
-*isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
 
 return piix3;
 }
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 9a2dd93c2d..f805fb8683 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -68,6 +68,6 @@ DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
 #define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
 #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
 
-PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus);
+PIIX3State *piix3_create(PCIBus *pci_bus);
 
 #endif
-- 
2.36.1

[PATCH v4 06/11] hw/isa/piix4: QOM'ify PIIX4 PM creation

Just like the real hardware, create the PIIX4 ACPI controller as part of
the PIIX4 southbridge. This also mirrors how the IDE and USB functions
are already created.

Signed-off-by: Bernhard Beschow 
---
 hw/isa/piix4.c| 24 +---
 hw/mips/malta.c   |  5 -
 include/hw/southbridge/piix.h |  2 +-
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 96df21a610..d97b245df3 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -49,6 +49,7 @@ struct PIIX4State {
 RTCState rtc;
 PCIIDEState ide;
 UHCIState uhci;
+PIIX4PMState pm;
 /* Reset Control Register */
 MemoryRegion rcr_mem;
 uint8_t rcr;
@@ -261,6 +262,13 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
 return;
 }
 
+/* ACPI controller */
+qdev_prop_set_int32(DEVICE(&s->pm), "addr", dev->devfn + 3);
+if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
+return;
+}
+qdev_connect_gpio_out(DEVICE(&s->pm), 0, s->isa[9]);
+
 pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
 }
 
@@ -271,6 +279,10 @@ static void piix4_init(Object *obj)
 object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
 object_initialize_child(obj, "ide", &s->ide, "piix4-ide");
 object_initialize_child(obj, "uhci", &s->uhci, "piix4-usb-uhci");
+
+object_initialize_child(obj, "pm", &s->pm, TYPE_PIIX4_PM);
+qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", 0x1100);
+qdev_prop_set_bit(DEVICE(&s->pm), "smm-enabled", 0);
 }
 
 static void piix4_class_init(ObjectClass *klass, void *data)
@@ -312,7 +324,7 @@ static void piix4_register_types(void)
 
 type_init(piix4_register_types)
 
-DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus)
+DeviceState *piix4_create(PCIBus *pci_bus)
 {
 PCIDevice *pci;
 DeviceState *dev;
@@ -322,15 +334,5 @@ DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus)
   TYPE_PIIX4_PCI_DEVICE);
 dev = DEVICE(pci);
 
-if (smbus) {
-pci = pci_new(devfn + 3, TYPE_PIIX4_PM);
-qdev_prop_set_uint32(DEVICE(pci), "smb_io_base", 0x1100);
-qdev_prop_set_bit(DEVICE(pci), "smm-enabled", 0);
-pci_realize_and_unref(pci, pci_bus, &error_fatal);
-qdev_connect_gpio_out(DEVICE(pci), 0,
-  qdev_get_gpio_in_named(dev, "isa", 9));
-*smbus = I2C_BUS(qdev_get_child_bus(DEVICE(pci), "i2c"));
-}
-
 return dev;
 }
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index e446b25ad0..be9f26d841 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1238,6 +1238,7 @@ void mips_malta_init(MachineState *machine)
 int be;
 MaltaState *s;
 DeviceState *dev;
+DeviceState *pm_dev;
 
 s = MIPS_MALTA(qdev_new(TYPE_MIPS_MALTA));
 sysbus_realize_and_unref(SYS_BUS_DEVICE(s), &error_fatal);
@@ -1399,8 +1400,10 @@ void mips_malta_init(MachineState *machine)
 empty_slot_init("GT64120", 0, 0x2000);
 
 /* Southbridge */
-dev = piix4_create(pci_bus, &smbus);
+dev = piix4_create(pci_bus);
 isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
+pm_dev = DEVICE(object_resolve_path_component(OBJECT(dev), "pm"));
+smbus = I2C_BUS(qdev_get_child_bus(pm_dev, "i2c"));
 
 /* Interrupt controller */
 qdev_connect_gpio_out_named(dev, "intr", 0, i8259_irq);
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index dab5c9704e..2357ce0287 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -70,6 +70,6 @@ DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
 
 PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus);
 
-DeviceState *piix4_create(PCIBus *pci_bus, I2CBus **smbus);
+DeviceState *piix4_create(PCIBus *pci_bus);
 
 #endif
-- 
2.36.1

[PATCH v4 08/11] hw/isa/piix3: Move pci_map_irq_fn near pci_set_irq_fn

The pci_map_irq_fn was implemented below type_init() which made it
inaccessible to QOM functions. So move it up.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/isa/piix3.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index d96ce2b788..c7a9014c3f 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -78,6 +78,17 @@ static void piix3_set_irq(void *opaque, int pirq, int level)
 piix3_set_irq_level(piix3, pirq, level);
 }
 
+/*
+ * Return the global irq number corresponding to a given device irq
+ * pin. We could also use the bus number to have a more precise mapping.
+ */
+static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
+{
+int slot_addend;
+slot_addend = PCI_SLOT(pci_dev->devfn) - 1;
+return (pci_intx + slot_addend) & 3;
+}
+
 static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
 {
 PIIX3State *piix3 = opaque;
@@ -350,17 +361,6 @@ static void piix3_register_types(void)
 
 type_init(piix3_register_types)
 
-/*
- * Return the global irq number corresponding to a given device irq
- * pin. We could also use the bus number to have a more precise mapping.
- */
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
-{
-int slot_addend;
-slot_addend = PCI_SLOT(pci_dev->devfn) - 1;
-return (pci_intx + slot_addend) & 3;
-}
-
 PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus)
 {
 PIIX3State *piix3;
-- 
2.36.1

[PATCH v4 02/11] hw/isa/piix4: Use object_initialize_child() for embedded struct

Reported-by: Peter Maydell 
Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/isa/piix4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 9a6d981037..1d04fb6a55 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -224,7 +224,7 @@ static void piix4_init(Object *obj)
 {
 PIIX4State *s = PIIX4_PCI_DEVICE(obj);
 
-object_initialize(&s->rtc, sizeof(s->rtc), TYPE_MC146818_RTC);
+object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
 }
 
 static void piix4_class_init(ObjectClass *klass, void *data)
-- 
2.36.1

[PATCH v4 04/11] hw/isa/piix4: QOM'ify PCI device creation and wiring

PCI interrupt wiring and device creation were performed in create()
functions which are obsolete. Move these tasks into QOM functions to
modernize the code.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/isa/piix4.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 18aa24424f..058bebb5e2 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -35,6 +35,7 @@
 #include "hw/rtc/mc146818rtc.h"
 #include "hw/ide/pci.h"
 #include "hw/acpi/piix4.h"
+#include "hw/usb/hcd-uhci.h"
 #include "migration/vmstate.h"
 #include "sysemu/reset.h"
 #include "sysemu/runstate.h"
@@ -46,6 +47,8 @@ struct PIIX4State {
 qemu_irq *isa;
 
 RTCState rtc;
+PCIIDEState ide;
+UHCIState uhci;
 /* Reset Control Register */
 MemoryRegion rcr_mem;
 uint8_t rcr;
@@ -205,6 +208,7 @@ static const MemoryRegionOps piix4_rcr_ops = {
 static void piix4_realize(PCIDevice *dev, Error **errp)
 {
 PIIX4State *s = PIIX4_PCI_DEVICE(dev);
+PCIBus *pci_bus = pci_get_bus(dev);
 ISABus *isa_bus;
 qemu_irq *i8259_out_irq;
 
@@ -243,6 +247,21 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
 return;
 }
 s->rtc.irq = isa_get_irq(ISA_DEVICE(&s->rtc), s->rtc.isairq);
+
+/* IDE */
+qdev_prop_set_int32(DEVICE(&s->ide), "addr", dev->devfn + 1);
+if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
+return;
+}
+pci_ide_create_devs(PCI_DEVICE(&s->ide));
+
+/* USB */
+qdev_prop_set_int32(DEVICE(&s->uhci), "addr", dev->devfn + 2);
+if (!qdev_realize(DEVICE(&s->uhci), BUS(pci_bus), errp)) {
+return;
+}
+
+pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
 }
 
 static void piix4_init(Object *obj)
@@ -250,6 +269,8 @@ static void piix4_init(Object *obj)
 PIIX4State *s = PIIX4_PCI_DEVICE(obj);
 
 object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
+object_initialize_child(obj, "ide", &s->ide, "piix4-ide");
+object_initialize_child(obj, "uhci", &s->uhci, "piix4-usb-uhci");
 }
 
 static void piix4_class_init(ObjectClass *klass, void *data)
@@ -293,7 +314,6 @@ type_init(piix4_register_types)
 
 DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
 {
-PIIX4State *s;
 PCIDevice *pci;
 DeviceState *dev;
 int devfn = PCI_DEVFN(10, 0);
@@ -301,15 +321,11 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus 
**isa_bus, I2CBus **smbus)
 pci = pci_create_simple_multifunction(pci_bus, devfn,  true,
   TYPE_PIIX4_PCI_DEVICE);
 dev = DEVICE(pci);
-s = PIIX4_PCI_DEVICE(pci);
+
 if (isa_bus) {
 *isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
 }
 
-pci = pci_create_simple(pci_bus, devfn + 1, "piix4-ide");
-pci_ide_create_devs(pci);
-
-pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci");
 if (smbus) {
 pci = pci_new(devfn + 3, TYPE_PIIX4_PM);
 qdev_prop_set_uint32(DEVICE(pci), "smb_io_base", 0x1100);
@@ -320,7 +336,5 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus 
**isa_bus, I2CBus **smbus)
 *smbus = I2C_BUS(qdev_get_child_bus(DEVICE(pci), "i2c"));
 }
 
-pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
-
 return dev;
 }
-- 
2.36.1

[PATCH v4 09/11] hw/isa/piix3: QOM'ify PCI device creation and wiring

PCI interrupt wiring was performed in create() functions which are
obsolete. Move these tasks into QOM functions to modernize the code.

In order to avoid duplicate checking for xen_enabled() the realize
methods are now split.

Signed-off-by: Bernhard Beschow 
---
 hw/isa/piix3.c | 67 +-
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index c7a9014c3f..de532cc692 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -24,6 +24,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/range.h"
+#include "qapi/error.h"
 #include "hw/southbridge/piix.h"
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
@@ -277,7 +278,7 @@ static const MemoryRegionOps rcr_ops = {
 .endianness = DEVICE_LITTLE_ENDIAN
 };
 
-static void piix3_realize(PCIDevice *dev, Error **errp)
+static void pci_piix3_realize(PCIDevice *dev, Error **errp)
 {
 PIIX3State *d = PIIX3_PCI_DEVICE(dev);
 
@@ -302,7 +303,6 @@ static void pci_piix3_class_init(ObjectClass *klass, void 
*data)
 dc->desc= "ISA bridge";
 dc->vmsd= &vmstate_piix3;
 dc->hotpluggable   = false;
-k->realize  = piix3_realize;
 k->vendor_id= PCI_VENDOR_ID_INTEL;
 /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
 k->device_id= PCI_DEVICE_ID_INTEL_82371SB_0;
@@ -326,11 +326,28 @@ static const TypeInfo piix3_pci_type_info = {
 },
 };
 
+static void piix3_realize(PCIDevice *dev, Error **errp)
+{
+ERRP_GUARD();
+PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
+PCIBus *pci_bus = pci_get_bus(dev);
+
+pci_piix3_realize(dev, errp);
+if (*errp) {
+return;
+}
+
+pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq,
+ piix3, PIIX_NUM_PIRQS);
+pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
+};
+
 static void piix3_class_init(ObjectClass *klass, void *data)
 {
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
 k->config_write = piix3_write_config;
+k->realize = piix3_realize;
 }
 
 static const TypeInfo piix3_info = {
@@ -339,11 +356,33 @@ static const TypeInfo piix3_info = {
 .class_init= piix3_class_init,
 };
 
+static void piix3_xen_realize(PCIDevice *dev, Error **errp)
+{
+ERRP_GUARD();
+PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
+PCIBus *pci_bus = pci_get_bus(dev);
+
+pci_piix3_realize(dev, errp);
+if (*errp) {
+return;
+}
+
+/*
+ * Xen supports additional interrupt routes from the PCI devices to
+ * the IOAPIC: the four pins of each PCI device on the bus are also
+ * connected to the IOAPIC directly.
+ * These additional routes can be discovered through ACPI.
+ */
+pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq,
+ piix3, XEN_PIIX_NUM_PIRQS);
+};
+
 static void piix3_xen_class_init(ObjectClass *klass, void *data)
 {
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
 k->config_write = piix3_write_config_xen;
+k->realize = piix3_xen_realize;
 };
 
 static const TypeInfo piix3_xen_info = {
@@ -365,27 +404,11 @@ PIIX3State *piix3_create(PCIBus *pci_bus, ISABus 
**isa_bus)
 {
 PIIX3State *piix3;
 PCIDevice *pci_dev;
+const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
+ : TYPE_PIIX3_DEVICE;
 
-/*
- * Xen supports additional interrupt routes from the PCI devices to
- * the IOAPIC: the four pins of each PCI device on the bus are also
- * connected to the IOAPIC directly.
- * These additional routes can be discovered through ACPI.
- */
-if (xen_enabled()) {
-pci_dev = pci_create_simple_multifunction(pci_bus, -1, true,
-  TYPE_PIIX3_XEN_DEVICE);
-piix3 = PIIX3_PCI_DEVICE(pci_dev);
-pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq,
- piix3, XEN_PIIX_NUM_PIRQS);
-} else {
-pci_dev = pci_create_simple_multifunction(pci_bus, -1, true,
-  TYPE_PIIX3_DEVICE);
-piix3 = PIIX3_PCI_DEVICE(pci_dev);
-pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq,
- piix3, PIIX_NUM_PIRQS);
-pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
-}
+pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
+piix3 = PIIX3_PCI_DEVICE(pci_dev);
 *isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
 
 return piix3;
-- 
2.36.1

[PATCH v4 11/11] hw/isa/piix3: Inline and remove piix3_create()

During the previous changesets piix3_create() became a trivial
wrapper around more generic functions. Modernize the code.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/i386/pc_piix.c |  6 +-
 hw/isa/piix3.c| 13 -
 include/hw/southbridge/piix.h |  2 --
 3 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c884d1a489..b58fbd4815 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -197,6 +197,9 @@ static void pc_init1(MachineState *machine,
 
 if (pcmc->pci_enabled) {
 PIIX3State *piix3;
+PCIDevice *pci_dev;
+const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
+ : TYPE_PIIX3_DEVICE;
 
 pci_bus = i440fx_init(host_type,
   pci_type,
@@ -207,7 +210,8 @@ static void pc_init1(MachineState *machine,
   pci_memory, ram_memory);
 pcms->bus = pci_bus;
 
-piix3 = piix3_create(pci_bus);
+pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
+piix3 = PIIX3_PCI_DEVICE(pci_dev);
 piix3->pic = x86ms->gsi;
 piix3_devfn = piix3->dev.devfn;
 isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index c6ff7795f4..01c376b39a 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -399,16 +399,3 @@ static void piix3_register_types(void)
 }
 
 type_init(piix3_register_types)
-
-PIIX3State *piix3_create(PCIBus *pci_bus)
-{
-PIIX3State *piix3;
-PCIDevice *pci_dev;
-const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
- : TYPE_PIIX3_DEVICE;
-
-pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
-piix3 = PIIX3_PCI_DEVICE(pci_dev);
-
-return piix3;
-}
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index f805fb8683..2693778b23 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -68,6 +68,4 @@ DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
 #define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
 #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
 
-PIIX3State *piix3_create(PCIBus *pci_bus);
-
 #endif
-- 
2.36.1

[PATCH v4 07/11] hw/isa/piix4: Inline and remove piix4_create()

During the previous changesets piix4_create() became a trivial
wrapper around more generic functions. Modernize the code.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/isa/piix4.c| 13 -
 hw/mips/malta.c   |  5 -
 include/hw/southbridge/piix.h |  2 --
 3 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index d97b245df3..15f344dbb7 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -323,16 +323,3 @@ static void piix4_register_types(void)
 }
 
 type_init(piix4_register_types)
-
-DeviceState *piix4_create(PCIBus *pci_bus)
-{
-PCIDevice *pci;
-DeviceState *dev;
-int devfn = PCI_DEVFN(10, 0);
-
-pci = pci_create_simple_multifunction(pci_bus, devfn,  true,
-  TYPE_PIIX4_PCI_DEVICE);
-dev = DEVICE(pci);
-
-return dev;
-}
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index be9f26d841..7a0ec513b0 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1237,6 +1237,7 @@ void mips_malta_init(MachineState *machine)
 int fl_idx = 0;
 int be;
 MaltaState *s;
+PCIDevice *piix4;
 DeviceState *dev;
 DeviceState *pm_dev;
 
@@ -1400,7 +1401,9 @@ void mips_malta_init(MachineState *machine)
 empty_slot_init("GT64120", 0, 0x2000);
 
 /* Southbridge */
-dev = piix4_create(pci_bus);
+piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
+TYPE_PIIX4_PCI_DEVICE);
+dev = DEVICE(piix4);
 isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
 pm_dev = DEVICE(object_resolve_path_component(OBJECT(dev), "pm"));
 smbus = I2C_BUS(qdev_get_child_bus(pm_dev, "i2c"));
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 2357ce0287..9a2dd93c2d 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -70,6 +70,4 @@ DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
 
 PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus);
 
-DeviceState *piix4_create(PCIBus *pci_bus);
-
 #endif
-- 
2.36.1

Re: [PATCH] hw/nvme: Fix deallocate when metadata is present

2022-06-03 Thread Keith Busch

On Fri, Jun 03, 2022 at 01:14:40PM -0600, Jonathan Derrick wrote:
> When metadata is present in the namespace and deallocates are issued, the 
> first
> deallocate could fail to zero the block range, resulting in another
> deallocation to be issued. Normally after the deallocation completes and the
> range is checked for zeroes, a deallocation is then issued for the metadata
> space. In the failure case where the range is not zeroed, deallocation is
> reissued for the block range (and followed with metadata deallocation), but 
> the
> original range deallocation task will also issue a metadata deallocation:
> 
> nvme_dsm_cb()
>   *range deallocation*
>   nvme_dsm_md_cb()
> if (nvme_block_status_all()) (range deallocation failure)
>   nvme_dsm_cb()
>   *range deallocation*
> nvme_dsm_md_cb()
>   if (nvme_block_status_all()) (no failure)
>   *metadata deallocation*
> *metadata deallocation*
> 
> This sequence results in reentry of nvme_dsm_cb() before the metadata has been
> deallocated. During reentry, the metadata is deallocated in the reentrant 
> task.
> nvme_dsm_bh() is called which deletes and sets iocb->bh to NULL. When reentry
> returns from nvme_dsm_cb(), metadata deallocation takes place again, and
> results in a null pointer dereference on the iocb->bh:

Nice, thank you for the detailed analysis. Patch looks good.

Reviewed-by: Keith Busch

[PULL 03/11] hw/nvme: fix copy cmd for pi enabled namespaces

From: Dmitry Tikhov 

Current implementation have problem in the read part of copy command.
Because there is no metadata mangling before nvme_dif_check invocation,
reftag error could be thrown for blocks of namespace that have not been
previously written to.

Signed-off-by: Dmitry Tikhov 
Reviewed-by: Klaus Jensen 
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ctrl.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 74540a03d518..08574c4dcbc8 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -2787,6 +2787,10 @@ static void nvme_copy_in_completed_cb(void *opaque, int 
ret)
 size_t mlen = nvme_m2b(ns, nlb);
 uint8_t *mbounce = iocb->bounce + nvme_l2b(ns, nlb);
 
+status = nvme_dif_mangle_mdata(ns, mbounce, mlen, slba);
+if (status) {
+goto invalid;
+}
 status = nvme_dif_check(ns, iocb->bounce, len, mbounce, mlen, prinfor,
 slba, apptag, appmask, &reftag);
 if (status) {
-- 
2.36.1

[PULL 00/11] hw/nvme updates

From: Klaus Jensen 

Hi Peter,

The following changes since commit 70e975203f366f2f30daaeb714bb852562b7b72f:

  Merge tag 'pull-request-2022-06-03' of https://gitlab.com/thuth/qemu into 
staging (2022-06-03 06:43:38 -0700)

are available in the Git repository at:

  git://git.infradead.org/qemu-nvme.git tags/nvme-next-pull-request

for you to fetch changes up to d7fe639cabf778903f6cab23ff58c905c71375ec:

  hw/nvme: add new command abort case (2022-06-03 21:48:24 +0200)


hw/nvme updates



Dmitry Tikhov (4):
  hw/nvme: fix narrowing conversion
  hw/nvme: add missing return statement
  hw/nvme: fix copy cmd for pi enabled namespaces
  hw/nvme: add new command abort case

Klaus Jensen (7):
  hw/nvme: fix smart aen
  hw/nvme: enforce common serial per subsystem
  hw/nvme: do not auto-generate eui64
  hw/nvme: do not auto-generate uuid
  hw/nvme: do not report null uuid
  hw/nvme: bump firmware revision
  hw/nvme: deprecate the use-intel-id compatibility parameter

 docs/about/deprecated.rst | 15 +++
 hw/core/machine.c |  1 +
 hw/nvme/ctrl.c| 26 ++
 hw/nvme/dif.c |  5 +
 hw/nvme/ns.c  |  9 +
 hw/nvme/nvme.h|  1 +
 hw/nvme/subsys.c  |  7 +++
 7 files changed, 48 insertions(+), 16 deletions(-)

-- 
2.36.1

[PULL 01/11] hw/nvme: fix narrowing conversion

From: Dmitry Tikhov 

Since nlbas is of type int, it does not work with large namespace size
values, e.g., 9 TB size of file backing namespace and 8 byte metadata
with 4096 bytes lbasz gives negative nlbas value, which is later
promoted to negative int64_t type value and results in negative
ns->moff which breaks namespace

Signed-off-by: Dmitry Tikhov 
Reviewed-by: Klaus Jensen 
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ns.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 324f53ea0cd1..af6504fad2d8 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -29,7 +29,8 @@ void nvme_ns_init_format(NvmeNamespace *ns)
 {
 NvmeIdNs *id_ns = &ns->id_ns;
 BlockDriverInfo bdi;
-int npdg, nlbas, ret;
+int npdg, ret;
+int64_t nlbas;
 
 ns->lbaf = id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(id_ns->flbas)];
 ns->lbasz = 1 << ns->lbaf.ds;
@@ -42,7 +43,7 @@ void nvme_ns_init_format(NvmeNamespace *ns)
 id_ns->ncap = id_ns->nsze;
 id_ns->nuse = id_ns->ncap;
 
-ns->moff = (int64_t)nlbas << ns->lbaf.ds;
+ns->moff = nlbas << ns->lbaf.ds;
 
 npdg = ns->blkconf.discard_granularity / ns->lbasz;
 
-- 
2.36.1

[PULL 06/11] hw/nvme: do not auto-generate eui64

From: Klaus Jensen 

We cannot provide auto-generated unique or persistent namespace
identifiers (EUI64, NGUID, UUID) easily. Since 6.1, namespaces have been
assigned a generated EUI64 of the form "52:54:00:".
This is will be unique within a QEMU instance, but not globally.

Revert that this is assigned automatically and immediately deprecate the
compatibility parameter. Users can opt-in to this with the
`eui64-default=on` device parameter or set it explicitly with
`eui64=UINT64`.

Cc: libvir-l...@redhat.com
Reviewed-by: Christoph Hellwig 
Signed-off-by: Klaus Jensen 
---
 docs/about/deprecated.rst | 7 +++
 hw/core/machine.c | 1 +
 hw/nvme/ns.c  | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index e19bcba24240..47a8628b5601 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -296,6 +296,13 @@ contains native support for this feature and thus use of 
the option
 ROM approach is obsolete. The native SeaBIOS support can be activated
 by using ``-machine graphics=off``.
 
+``-device nvme-ns,eui64-default=on|off`` (since 7.1)
+
+
+In QEMU versions 6.1, 6.2 and 7.0, the ``nvme-ns`` generates an EUI-64
+identifer that is not globally unique. If an EUI-64 identifer is required, the
+user must set it explicitly using the ``nvme-ns`` device parameter ``eui64``.
+
 
 Block device options
 
diff --git a/hw/core/machine.c b/hw/core/machine.c
index bb0dc8f6a93d..c53548d0b138 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -43,6 +43,7 @@
 
 GlobalProperty hw_compat_7_0[] = {
 { "arm-gicv3-common", "force-8-bit-prio", "on" },
+{ "nvme-ns", "eui64-default", "on"},
 };
 const size_t hw_compat_7_0_len = G_N_ELEMENTS(hw_compat_7_0);
 
diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index af6504fad2d8..06a04131f192 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -641,7 +641,7 @@ static Property nvme_ns_props[] = {
 DEFINE_PROP_SIZE("zoned.zrwas", NvmeNamespace, params.zrwas, 0),
 DEFINE_PROP_SIZE("zoned.zrwafg", NvmeNamespace, params.zrwafg, -1),
 DEFINE_PROP_BOOL("eui64-default", NvmeNamespace, params.eui64_default,
- true),
+ false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.36.1

[PULL 05/11] hw/nvme: enforce common serial per subsystem

From: Klaus Jensen 

The Identify Controller Serial Number (SN) is the serial number for the
NVM subsystem and must be the same across all controller in the NVM
subsystem.

Enforce this.

Reviewed-by: Christoph Hellwig 
Reviewed-by: Keith Busch 
Signed-off-by: Klaus Jensen 
---
 hw/nvme/nvme.h   | 1 +
 hw/nvme/subsys.c | 7 +++
 2 files changed, 8 insertions(+)

diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 677381932569..e41771604f59 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -48,6 +48,7 @@ typedef struct NvmeSubsystem {
 DeviceState parent_obj;
 NvmeBus bus;
 uint8_t subnqn[256];
+char*serial;
 
 NvmeCtrl  *ctrls[NVME_MAX_CONTROLLERS];
 NvmeNamespace *namespaces[NVME_MAX_NAMESPACES + 1];
diff --git a/hw/nvme/subsys.c b/hw/nvme/subsys.c
index fb58d639504e..691a90d20947 100644
--- a/hw/nvme/subsys.c
+++ b/hw/nvme/subsys.c
@@ -27,6 +27,13 @@ int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
 return -1;
 }
 
+if (!subsys->serial) {
+subsys->serial = g_strdup(n->params.serial);
+} else if (strcmp(subsys->serial, n->params.serial)) {
+error_setg(errp, "invalid controller serial");
+return -1;
+}
+
 subsys->ctrls[cntlid] = n;
 
 for (nsid = 1; nsid < ARRAY_SIZE(subsys->namespaces); nsid++) {
-- 
2.36.1

[PULL 04/11] hw/nvme: fix smart aen

From: Klaus Jensen 

Pass the right constant to nvme_smart_event(). The NVME_AER* values hold
the bit position in the SMART byte, not the shifted value that we expect
it to be in nvme_smart_event().

Fixes: c62720f137df ("hw/block/nvme: trigger async event during injecting smart 
warning")
Acked-by: zhenwei pi 
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 08574c4dcbc8..a2f6069f7fe1 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -5325,7 +5325,7 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest 
*req)
 
 if ((n->temperature >= n->features.temp_thresh_hi) ||
 (n->temperature <= n->features.temp_thresh_low)) {
-nvme_smart_event(n, NVME_AER_INFO_SMART_TEMP_THRESH);
+nvme_smart_event(n, NVME_SMART_TEMPERATURE);
 }
 
 break;
-- 
2.36.1

[PULL 02/11] hw/nvme: add missing return statement

From: Dmitry Tikhov 

Since there is no return after nvme_dsm_cb invocation, metadata
associated with non-zero block range is currently zeroed. Also this
behaviour leads to segfault since we schedule iocb->bh two times.
First when entering nvme_dsm_cb with iocb->idx == iocb->nr and
second because of missing return on call stack unwinding by calling
blk_aio_pwrite_zeroes and subsequent nvme_dsm_cb callback.

Fixes: d7d1474fd85d ("hw/nvme: reimplement dsm to allow cancellation")
Signed-off-by: Dmitry Tikhov 
Reviewed-by: Klaus Jensen 
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ctrl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 03760ddeae8c..74540a03d518 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -2372,6 +2372,7 @@ static void nvme_dsm_md_cb(void *opaque, int ret)
 }
 
 nvme_dsm_cb(iocb, 0);
+return;
 }
 
 iocb->aiocb = blk_aio_pwrite_zeroes(ns->blkconf.blk, nvme_moff(ns, slba),
-- 
2.36.1

[PULL 11/11] hw/nvme: add new command abort case

From: Dmitry Tikhov 

NVMe command set specification for end-to-end data protection formatted
namespace states:

o If the Reference Tag Check bit of the PRCHK field is set to ‘1’ and
  the namespace is formatted for Type 3 protection, then the
  controller:
  ▪ should not compare the protection Information Reference Tag
field to the computed reference tag; and
  ▪ may ignore the ILBRT and EILBRT fields. If a command is
aborted as a result of the Reference Tag Check bit of the
PRCHK field being set to ‘1’, then that command should be
aborted with a status code of Invalid Protection Information,
but may be aborted with a status code of Invalid Field in
Command.

Currently qemu compares reftag in the nvme_dif_prchk function whenever
Reference Tag Check bit is set in the command. For type 3 namespaces
however, caller of nvme_dif_prchk - nvme_dif_check does not increment
reftag for each subsequent logical block. That way commands incorporating
more than one logical block for type 3 formatted namespaces with reftag
check bit set, always fail with End-to-end Reference Tag Check Error.
Comply with spec by handling case of set Reference Tag Check
bit in the type 3 formatted namespace.

Fixes: 146f720c5563 ("hw/block/nvme: end-to-end data protection")
Signed-off-by: Dmitry Tikhov 
Signed-off-by: Klaus Jensen 
---
 hw/nvme/dif.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/nvme/dif.c b/hw/nvme/dif.c
index 62d885f83ea4..63c44c86ab55 100644
--- a/hw/nvme/dif.c
+++ b/hw/nvme/dif.c
@@ -26,6 +26,11 @@ uint16_t nvme_check_prinfo(NvmeNamespace *ns, uint8_t 
prinfo, uint64_t slba,
 return NVME_INVALID_PROT_INFO | NVME_DNR;
 }
 
+if ((NVME_ID_NS_DPS_TYPE(ns->id_ns.dps) == NVME_ID_NS_DPS_TYPE_3) &&
+(prinfo & NVME_PRINFO_PRCHK_REF)) {
+return NVME_INVALID_PROT_INFO;
+}
+
 return NVME_SUCCESS;
 }
 
-- 
2.36.1

[PULL 07/11] hw/nvme: do not auto-generate uuid

From: Klaus Jensen 

Do not default to generate an UUID for namespaces if it is not
explicitly specified.

This is a technically a breaking change in behavior. However, since the
UUID changes on every VM launch, it is not spec compliant and is of
little use since the UUID cannot be used reliably anyway and the
behavior prior to this patch must be considered buggy.

Reviewed-by: Keith Busch 
Reviewed-by: Christoph Hellwig 
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ns.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 06a04131f192..1b9c9d11567f 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -614,7 +614,7 @@ static Property nvme_ns_props[] = {
 DEFINE_PROP_BOOL("detached", NvmeNamespace, params.detached, false),
 DEFINE_PROP_BOOL("shared", NvmeNamespace, params.shared, true),
 DEFINE_PROP_UINT32("nsid", NvmeNamespace, params.nsid, 0),
-DEFINE_PROP_UUID("uuid", NvmeNamespace, params.uuid),
+DEFINE_PROP_UUID_NODEFAULT("uuid", NvmeNamespace, params.uuid),
 DEFINE_PROP_UINT64("eui64", NvmeNamespace, params.eui64, 0),
 DEFINE_PROP_UINT16("ms", NvmeNamespace, params.ms, 0),
 DEFINE_PROP_UINT8("mset", NvmeNamespace, params.mset, 0),
-- 
2.36.1

[PULL 08/11] hw/nvme: do not report null uuid

From: Klaus Jensen 

Do not report the "null uuid" (all zeros) in the namespace
identification descriptors.

Reported-by: Luis Chamberlain 
Reported-by: Christoph Hellwig 
Reviewed-by: Christoph Hellwig 
Reviewed-by: Keith Busch 
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ctrl.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index a2f6069f7fe1..909e357a7eb9 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -4955,16 +4955,13 @@ static uint16_t nvme_identify_ns_descr_list(NvmeCtrl 
*n, NvmeRequest *req)
 return NVME_INVALID_FIELD | NVME_DNR;
 }
 
-/*
- * If the EUI-64 field is 0 and the NGUID field is 0, the namespace must
- * provide a valid Namespace UUID in the Namespace Identification 
Descriptor
- * data structure. QEMU does not yet support setting NGUID.
- */
-uuid.hdr.nidt = NVME_NIDT_UUID;
-uuid.hdr.nidl = NVME_NIDL_UUID;
-memcpy(uuid.v, ns->params.uuid.data, NVME_NIDL_UUID);
-memcpy(pos, &uuid, sizeof(uuid));
-pos += sizeof(uuid);
+if (!qemu_uuid_is_null(&ns->params.uuid)) {
+uuid.hdr.nidt = NVME_NIDT_UUID;
+uuid.hdr.nidl = NVME_NIDL_UUID;
+memcpy(uuid.v, ns->params.uuid.data, NVME_NIDL_UUID);
+memcpy(pos, &uuid, sizeof(uuid));
+pos += sizeof(uuid);
+}
 
 if (ns->params.eui64) {
 eui64.hdr.nidt = NVME_NIDT_EUI64;
-- 
2.36.1

Re: [PATCH] hw/nvme: Fix deallocate when metadata is present

On Jun  3 13:14, Jonathan Derrick wrote:
> When metadata is present in the namespace and deallocates are issued, the 
> first
> deallocate could fail to zero the block range, resulting in another
> deallocation to be issued. Normally after the deallocation completes and the
> range is checked for zeroes, a deallocation is then issued for the metadata
> space. In the failure case where the range is not zeroed, deallocation is
> reissued for the block range (and followed with metadata deallocation), but 
> the
> original range deallocation task will also issue a metadata deallocation:
> 
> nvme_dsm_cb()
>   *range deallocation*
>   nvme_dsm_md_cb()
> if (nvme_block_status_all()) (range deallocation failure)
>   nvme_dsm_cb()
>   *range deallocation*
> nvme_dsm_md_cb()
>   if (nvme_block_status_all()) (no failure)
>   *metadata deallocation*
> *metadata deallocation*
> 
> This sequence results in reentry of nvme_dsm_cb() before the metadata has been
> deallocated. During reentry, the metadata is deallocated in the reentrant 
> task.
> nvme_dsm_bh() is called which deletes and sets iocb->bh to NULL. When reentry
> returns from nvme_dsm_cb(), metadata deallocation takes place again, and
> results in a null pointer dereference on the iocb->bh:
> 
> BH deletion:
> #0  nvme_dsm_bh (opaque=0x55ef893e2f10) at ../hw/nvme/ctrl.c:2316
> #1  0x55ef868eb333 in aio_bh_call (bh=0x55ef8a441b30) at 
> ../util/async.c:141
> #2  0x55ef868eb441 in aio_bh_poll (ctx=0x55ef892c6e40) at 
> ../util/async.c:169
> #3  0x55ef868d2789 in aio_dispatch (ctx=0x55ef892c6e40) at 
> ../util/aio-posix.c:415
> #4  0x55ef868eb896 in aio_ctx_dispatch (source=0x55ef892c6e40, 
> callback=0x0, user_data=0x0) at ../util/async.c:311
> #5  0x7f5bfe4ab17d in g_main_context_dispatch () at 
> /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #6  0x55ef868fcd98 in glib_pollfds_poll () at ../util/main-loop.c:232
> #7  0x55ef868fce16 in os_host_main_loop_wait (timeout=0) at 
> ../util/main-loop.c:255
> #8  0x55ef868fcf27 in main_loop_wait (nonblocking=0) at 
> ../util/main-loop.c:531
> #9  0x55ef864a2442 in qemu_main_loop () at ../softmmu/runstate.c:726
> #10 0x55ef860f957a in main (argc=29, argv=0x7ffdc9705508, 
> envp=0x7ffdc97055f8) at ../softmmu/main.c:50
> 
> nvme_dsm_cb() called for metadata after nvme_dsm_bh() completes from 
> reentrant task:
> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> 0x55ef868eb07c in aio_bh_enqueue (bh=0x0, new_flags=2) at 
> ../util/async.c:70
> 70  AioContext *ctx = bh->ctx;
> (gdb) backtrace
> #0  0x55ef868eb07c in aio_bh_enqueue (bh=0x0, new_flags=2) at 
> ../util/async.c:70
> #1  0x55ef868eb4cf in qemu_bh_schedule (bh=0x0) at ../util/async.c:186
> #2  0x55ef862db21e in nvme_dsm_cb (opaque=0x55ef897b41a0, ret=0) at 
> ../hw/nvme/ctrl.c:2423
> #3  0x55ef8665a662 in blk_aio_complete (acb=0x55ef89c6d8c0) at 
> ../block/block-backend.c:1419
> #4  0x55ef8665a940 in blk_aio_write_entry (opaque=0x55ef89c6d8c0) at 
> ../block/block-backend.c:1486
> #5  0x55ef868edcf2 in coroutine_trampoline (i0=-536848976, i1=32602) at 
> ../util/coroutine-ucontext.c:173
> #6  0x7f5bfe0bc510 in __start_context () at 
> ../sysdeps/unix/sysv/linux/x86_64/__start_context.S:91
> #7  0x7f5bf757bb40 in  ()
> #8  0x in  ()
> 
> The fix is to return when an nvme_dsm_cb() is reentered due to failure to
> deallocate the block range, so that metadata deallocate is then only issued in
> the reentrant task and prevent doing it again when the reentrant task returns
> to the original task.
> 
> Reproduction steps (with emulated namespace):
> nvme format --lbaf=1 -f /dev/nvme0n1
> mkfs.ext4 /dev/nvme0n1
> mkfs.ext4 -F /dev/nvme0n1
> 
> Signed-off-by: Francis Pravin AntonyX Michael Raj 
> 
> Signed-off-by: Michael Kropaczek 
> Signed-off-by: Jonathan Derrick 
> ---
>  hw/nvme/ctrl.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 03760ddeae..74540a03d5 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -2372,6 +2372,7 @@ static void nvme_dsm_md_cb(void *opaque, int ret)
>  }
>  
>  nvme_dsm_cb(iocb, 0);
> +return;
>  }
>  
>  iocb->aiocb = blk_aio_pwrite_zeroes(ns->blkconf.blk, nvme_moff(ns, slba),
> -- 
> 2.25.1
> 

Yeah, thanks for the elaborate analysis!

A fix for this has been lingering in nvme-next. I sincerely apologize
for not sending the pull request to master earlier, which might have
saved you the trouble of tracking this down.

I just sent the pull request to Peter.


signature.asc
Description: PGP signature

[PULL 09/11] hw/nvme: bump firmware revision

From: Klaus Jensen 

The Linux kernel quirks the QEMU NVMe controller pretty heavily because
of the namespace identifier mess. Since this is now fixed, bump the
firmware revision number to allow the quirk to be disabled for this
revision.

As of now, bump the firmware revision number to be equal to the QEMU
release version number.

Reviewed-by: Keith Busch 
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 909e357a7eb9..1e6e0fcad918 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6713,7 +6713,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
 id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
 strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
-strpadcpy((char *)id->fr, sizeof(id->fr), "1.0", ' ');
+strpadcpy((char *)id->fr, sizeof(id->fr), QEMU_VERSION, ' ');
 strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' ');
 
 id->cntlid = cpu_to_le16(n->cntlid);
-- 
2.36.1

[PULL 10/11] hw/nvme: deprecate the use-intel-id compatibility parameter

From: Klaus Jensen 

Since version 5.2 commit 6eb7a071292a ("hw/block/nvme: change controller
pci id"), the emulated NVMe controller has defaulted to a non-Intel PCI
identifier.

Deprecate the compatibility parameter so we can get rid of it once and
for all.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Klaus Jensen 
---
 docs/about/deprecated.rst | 8 
 1 file changed, 8 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 47a8628b5601..aa2e32020707 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -303,6 +303,14 @@ In QEMU versions 6.1, 6.2 and 7.0, the ``nvme-ns`` 
generates an EUI-64
 identifer that is not globally unique. If an EUI-64 identifer is required, the
 user must set it explicitly using the ``nvme-ns`` device parameter ``eui64``.
 
+``-device nvme,use-intel-id=on|off`` (since 7.1)
+
+
+The ``nvme`` device originally used a PCI Vendor/Device Identifier combination
+from Intel that was not properly allocated. Since version 5.2, the controller
+has used a properly allocated identifier. Deprecate the ``use-intel-id``
+machine compatibility parameter.
+
 
 Block device options
 
-- 
2.36.1

Re: [PATCH v2] hw/nvme: clean up CC register write logic

On Jun  1 15:28, Lukasz Maniak wrote:
> On Wed, May 25, 2022 at 09:35:24AM +0200, Klaus Jensen wrote:
> > 
> > +stl_le_p(&n->bar.intms, 0);
> > +stl_le_p(&n->bar.intmc, 0);
> > +stl_le_p(&n->bar.cc, 0);
> 
> Looks fine, though it seems the NVMe spec says the above registers
> should be cleared during each reset for VF as well.
> 

Aren't the values of all other registers than CSTS just undefined? (NVMe
v2.0b, Section 8.26.3)


signature.asc
Description: PGP signature

[PATCH] Revert "hw/block/nvme: add support for sgl bit bucket descriptor"

From: Klaus Jensen 

This reverts commit d97eee64fef35655bd06f5c44a07fdb83a6274ae.

The emulated controller correctly accounts for not including bit buckets
in the controller-to-host data transfer, however it doesn't correctly
account for the holes for the on-disk data offsets.

Reported-by: Keith Busch 
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ctrl.c | 29 ++---
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 03760ddeae8c..958363e70cfe 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -808,10 +808,6 @@ static uint16_t nvme_map_sgl_data(NvmeCtrl *n, NvmeSg *sg,
 uint8_t type = NVME_SGL_TYPE(segment[i].type);
 
 switch (type) {
-case NVME_SGL_DESCR_TYPE_BIT_BUCKET:
-if (cmd->opcode == NVME_CMD_WRITE) {
-continue;
-}
 case NVME_SGL_DESCR_TYPE_DATA_BLOCK:
 break;
 case NVME_SGL_DESCR_TYPE_SEGMENT:
@@ -844,10 +840,6 @@ static uint16_t nvme_map_sgl_data(NvmeCtrl *n, NvmeSg *sg,
 
 trans_len = MIN(*len, dlen);
 
-if (type == NVME_SGL_DESCR_TYPE_BIT_BUCKET) {
-goto next;
-}
-
 addr = le64_to_cpu(segment[i].addr);
 
 if (UINT64_MAX - addr < dlen) {
@@ -859,7 +851,6 @@ static uint16_t nvme_map_sgl_data(NvmeCtrl *n, NvmeSg *sg,
 return status;
 }
 
-next:
 *len -= trans_len;
 }
 
@@ -917,8 +908,7 @@ static uint16_t nvme_map_sgl(NvmeCtrl *n, NvmeSg *sg, 
NvmeSglDescriptor sgl,
 seg_len = le32_to_cpu(sgld->len);
 
 /* check the length of the (Last) Segment descriptor */
-if ((!seg_len || seg_len & 0xf) &&
-(NVME_SGL_TYPE(sgld->type) != NVME_SGL_DESCR_TYPE_BIT_BUCKET)) {
+if (!seg_len || seg_len & 0xf) {
 return NVME_INVALID_SGL_SEG_DESCR | NVME_DNR;
 }
 
@@ -956,26 +946,20 @@ static uint16_t nvme_map_sgl(NvmeCtrl *n, NvmeSg *sg, 
NvmeSglDescriptor sgl,
 last_sgld = &segment[nsgld - 1];
 
 /*
- * If the segment ends with a Data Block or Bit Bucket Descriptor Type,
- * then we are done.
+ * If the segment ends with a Data Block, then we are done.
  */
-switch (NVME_SGL_TYPE(last_sgld->type)) {
-case NVME_SGL_DESCR_TYPE_DATA_BLOCK:
-case NVME_SGL_DESCR_TYPE_BIT_BUCKET:
+if (NVME_SGL_TYPE(last_sgld->type) == NVME_SGL_DESCR_TYPE_DATA_BLOCK) {
 status = nvme_map_sgl_data(n, sg, segment, nsgld, &len, cmd);
 if (status) {
 goto unmap;
 }
 
 goto out;
-
-default:
-break;
 }
 
 /*
- * If the last descriptor was not a Data Block or Bit Bucket, then the
- * current segment must not be a Last Segment.
+ * If the last descriptor was not a Data Block, then the current
+ * segment must not be a Last Segment.
  */
 if (NVME_SGL_TYPE(sgld->type) == NVME_SGL_DESCR_TYPE_LAST_SEGMENT) {
 status = NVME_INVALID_SGL_SEG_DESCR | NVME_DNR;
@@ -6773,8 +6757,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 id->vwc = NVME_VWC_NSID_BROADCAST_SUPPORT | NVME_VWC_PRESENT;
 
 id->ocfs = cpu_to_le16(NVME_OCFS_COPY_FORMAT_0 | NVME_OCFS_COPY_FORMAT_1);
-id->sgls = cpu_to_le32(NVME_CTRL_SGLS_SUPPORT_NO_ALIGN |
-   NVME_CTRL_SGLS_BITBUCKET);
+id->sgls = cpu_to_le32(NVME_CTRL_SGLS_SUPPORT_NO_ALIGN);
 
 nvme_init_subnqn(n);
 
-- 
2.36.1

Re: [PATCH v2 04/16] ppc/pnv: change PnvPHB3 to be a PnvPHB backend

2022-06-03 Thread Daniel Henrique Barboza





On 6/2/22 04:56, Mark Cave-Ayland wrote:

On 31/05/2022 22:49, Daniel Henrique Barboza wrote:


We need a handful of changes that needs to be done in a single swoop to
turn PnvPHB3 into a PnvPHB backend.

In the PnvPHB3, since the PnvPHB device implements PCIExpressHost and
will hold the PCI bus, change PnvPHB3 parent to TYPE_DEVICE. There are a
couple of instances in pnv_phb3.c that needs to access the PCI bus, so a
phb_base pointer is added to allow access to the parent PnvPHB. The
PnvPHB3 root port will now be connected to a PnvPHB object.

In pnv.c, the powernv8 machine chip8 will now hold an array of PnvPHB
objects.  pnv_get_phb3_child() needs to be adapted to return the PnvPHB3
backend from the PnvPHB child. A global property is added in
pnv_machine_power8_class_init() to ensure that all PnvPHBs are created
with phb->version = 3.

After all these changes we're still able to boot a powernv8 machine with
default settings. The real gain will come with user created PnvPHB
devices, coming up next.

Signed-off-by: Daniel Henrique Barboza 
---
  hw/pci-host/pnv_phb3.c | 29 -
  hw/ppc/pnv.c   | 21 +
  include/hw/pci-host/pnv_phb3.h |  5 -
  include/hw/ppc/pnv.h   |  3 ++-
  4 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 60584e2aae..a39aa0e8c4 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -11,6 +11,7 @@
  #include "qapi/visitor.h"
  #include "qapi/error.h"
  #include "hw/pci-host/pnv_phb3_regs.h"
+#include "hw/pci-host/pnv_phb.h"
  #include "hw/pci-host/pnv_phb3.h"
  #include "hw/pci/pcie_host.h"
  #include "hw/pci/pcie_port.h"
@@ -26,7 +27,7 @@
  static PCIDevice *pnv_phb3_find_cfg_dev(PnvPHB3 *phb)
  {
-    PCIHostState *pci = PCI_HOST_BRIDGE(phb);
+    PCIHostState *pci = PCI_HOST_BRIDGE(phb->phb_base);
  uint64_t addr = phb->regs[PHB_CONFIG_ADDRESS >> 3];
  uint8_t bus, devfn;
@@ -590,7 +591,7 @@ void pnv_phb3_reg_write(void *opaque, hwaddr off, uint64_t 
val, unsigned size)
  uint64_t pnv_phb3_reg_read(void *opaque, hwaddr off, unsigned size)
  {
  PnvPHB3 *phb = opaque;
-    PCIHostState *pci = PCI_HOST_BRIDGE(phb);
+    PCIHostState *pci = PCI_HOST_BRIDGE(phb->phb_base);
  uint64_t val;
  if ((off & 0xfffc) == PHB_CONFIG_DATA) {
@@ -1057,8 +1058,6 @@ static void pnv_phb3_realize(DeviceState *dev, Error 
**errp)
    "phb3-regs", 0x1000);
  pnv_phb3_bus_init(dev, phb);
-
-    pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), TYPE_PNV_PHB3_ROOT_PORT);
  }
  void pnv_phb3_update_regions(PnvPHB3 *phb)
@@ -1083,38 +1082,26 @@ void pnv_phb3_update_regions(PnvPHB3 *phb)
  pnv_phb3_check_all_m64s(phb);
  }
-static const char *pnv_phb3_root_bus_path(PCIHostState *host_bridge,
-  PCIBus *rootbus)
-{
-    PnvPHB3 *phb = PNV_PHB3(host_bridge);
-
-    snprintf(phb->bus_path, sizeof(phb->bus_path), "00%02x:%02x",
- phb->chip_id, phb->phb_id);
-    return phb->bus_path;
-}
-
  static Property pnv_phb3_properties[] = {
  DEFINE_PROP_UINT32("index", PnvPHB3, phb_id, 0),
  DEFINE_PROP_UINT32("chip-id", PnvPHB3, chip_id, 0),
  DEFINE_PROP_LINK("chip", PnvPHB3, chip, TYPE_PNV_CHIP, PnvChip *),
+    DEFINE_PROP_LINK("phb-base", PnvPHB3, phb_base, TYPE_PNV_PHB, PnvPHB 
*),
  DEFINE_PROP_END_OF_LIST(),
  };
  static void pnv_phb3_class_init(ObjectClass *klass, void *data)
  {
-    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
  DeviceClass *dc = DEVICE_CLASS(klass);
-    hc->root_bus_path = pnv_phb3_root_bus_path;
  dc->realize = pnv_phb3_realize;
  device_class_set_props(dc, pnv_phb3_properties);
-    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
  dc->user_creatable = false;
  }
  static const TypeInfo pnv_phb3_type_info = {
  .name  = TYPE_PNV_PHB3,
-    .parent    = TYPE_PCIE_HOST_BRIDGE,
+    .parent = TYPE_DEVICE,
  .instance_size = sizeof(PnvPHB3),
  .class_init    = pnv_phb3_class_init,
  .instance_init = pnv_phb3_instance_init,
@@ -1146,11 +1133,11 @@ static void pnv_phb3_root_port_realize(DeviceState 
*dev, Error **errp)
  PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
  PCIDevice *pci = PCI_DEVICE(dev);
  PCIBus *bus = pci_get_bus(pci);
-    PnvPHB3 *phb = NULL;
+    PnvPHB *phb = NULL;
  Error *local_err = NULL;
-    phb = (PnvPHB3 *) object_dynamic_cast(OBJECT(bus->qbus.parent),
-  TYPE_PNV_PHB3);
+    phb = (PnvPHB *) object_dynamic_cast(OBJECT(bus->qbus.parent),
+ TYPE_PNV_PHB);


I realize that this has come from existing code, however these days there 
generally isn't a good reason for anything to access bus->qbus directly (and C 
casts without a QOM macro often need a closer look). I'm also not convinced by the 
use of object_dynamic_cas

Re: [PATCH] hw/nvme: clear aen mask on reset

On May 12 11:30, Klaus Jensen wrote:
> From: Klaus Jensen 
> 
> The internally maintained AEN mask is not cleared on reset. Fix this.
> 
> Signed-off-by: Klaus Jensen 
> ---
>  hw/nvme/ctrl.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 1e6e0fcad918..4c8200dfb859 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -5889,6 +5889,7 @@ static void nvme_ctrl_reset(NvmeCtrl *n)
>  }
>  
>  n->aer_queued = 0;
> +n->aer_mask = 0;
>  n->outstanding_aers = 0;
>  n->qs_created = false;
>  }
> -- 
> 2.36.0
> 

Gentle bump.


signature.asc
Description: PGP signature

Re: [PATCH v2] hw/nvme: allow to pass a memory backend object for the CMB