[PULL 00/20] Misc fixes for 2024-08-20

[PULL 00/20] Misc fixes for 2024-08-20

[Philippe Mathieu-Daudé]

The following changes since commit ecdfa31beb1f7616091bedba79dfdf9ee525ed9d:

  Merge tag 'pull-request-2024-08-16' of https://gitlab.com/thuth/qemu into staging (2024-08-16 18:18:27 +1000)

are available in the Git repository at:

  https://github.com/philmd/qemu.git tags/hw-misc-20240820

for you to fetch changes up to 87e012f29f2e47dcd8c385ff8bb8188f9e06d4ea:

  crypto/tlscredspsk: Free username on finalize (2024-08-20 00:49:14 +0200)

Ignored checkpatch warning:

  WARNING: line over 80 characters
  #115: FILE: target/mips/tcg/sysemu/tlb_helper.c:713:
  +    MemOp native_op = (((env->CP0_PWSize >> CP0PS_PS) & 1) == 0) ? MO_32 : MO_64;

----------------------------------------------------------------
Various fixes

- Null pointer dereference in IPI IOCSR (Jiaxun)
- Correct '-smbios type=4' in man page (Heinrich)
- Use correct MMU index in MIPS get_pte (Phil)
- Reset MPQEMU remote message using device_cold_reset (Peter)
- Update linux-user MIPS CPU list (Phil)
- Do not let exec_command read console if no pattern to wait for (Nick)
- Remove shadowed declaration warning (Pierrick)
- Restrict STQF opcode to SPARC V9 (Richard)
- Add missing Kconfig dependency for POWERNV ISA serial port (Bernhard)
- Do not allow vmport device without i8042 PS/2 controller (Kamil)
- Fix QCryptoTLSCredsPSK leak (Peter)

----------------------------------------------------------------

Bernhard Beschow (1):
  hw/ppc/Kconfig: Add missing SERIAL_ISA dependency to POWERNV machine

Heinrich Schuchardt (1):
  qemu-options.hx: correct formatting -smbios type=4

Jiaxun Yang (2):
  hw/mips/loongson3_virt: Store core_iocsr into LoongsonMachineState
  hw/mips/loongson3_virt: Fix condition of IPI IOCSR connection

Kamil Szczęk (2):
  hw/i386/pc: Unify vmport=auto handling
  hw/i386/pc: Ensure vmport prerequisites are fulfilled

Nicholas Piggin (2):
  tests/avocado: exec_command should not consume console output
  tests/avocado: Mark ppc_hv_tests.py as non-flaky after fixed console
    interaction

Peter Maydell (3):
  hw/dma/xilinx_axidma: Use semicolon at end of statement, not comma
  hw/remote/message.c: Don't directly invoke DeviceClass:reset
  crypto/tlscredspsk: Free username on finalize

Philippe Mathieu-Daudé (7):
  target/mips: Pass page table entry size as MemOp to get_pte()
  target/mips: Use correct MMU index in get_pte()
  target/mips: Load PTE as DATA
  linux-user/mips: Do not try to use removed R5900 CPU
  linux-user/mips: Select Octeon68XX CPU for Octeon binaries
  linux-user/mips: Select MIPS64R2-generic for Rel2 binaries
  linux-user/mips: Select Loongson CPU for Loongson binaries

Pierrick Bouvier (1):
  contrib/plugins/execlog: Fix shadowed declaration warning

Richard Henderson (1):
  target/sparc: Restrict STQF to sparcv9

 linux-user/mips/target_elf.h           |  3 --
 linux-user/mips64/target_elf.h         | 24 +++++++--
 target/sparc/insns.decode              |  2 +-
 contrib/plugins/execlog.c              |  4 +-
 crypto/tlscredspsk.c                   |  1 +
 hw/dma/xilinx_axidma.c                 |  2 +-
 hw/i386/pc.c                           | 14 +++++-
 hw/i386/pc_piix.c                      |  5 --
 hw/i386/pc_q35.c                       |  5 --
 hw/mips/loongson3_virt.c               |  5 +-
 hw/remote/message.c                    |  5 +-
 target/mips/tcg/sysemu/tlb_helper.c    | 69 +++++++++++++-------------
 target/sparc/translate.c               |  2 +-
 hw/ppc/Kconfig                         |  1 +
 qemu-options.hx                        |  6 +--
 tests/avocado/avocado_qemu/__init__.py |  7 +++
 tests/avocado/ppc_hv_tests.py          |  1 -
 17 files changed, 89 insertions(+), 67 deletions(-)

-- 
2.45.2

[PULL 01/20] hw/mips/loongson3_virt: Store core_iocsr into LoongsonMachineState

[Philippe Mathieu-Daudé]

From: Jiaxun Yang <jiaxun.yang@flygoat.com>

Link: https://lore.kernel.org/qemu-devel/972034d6-23b3-415a-b401-b8bc1cc515c9@linaro.org/
Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240621-loongson3-ipi-follow-v2-1-848eafcbb67e@flygoat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/mips/loongson3_virt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index 408e3d7054..27a85e3614 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -97,6 +97,7 @@ struct LoongsonMachineState {
     MemoryRegion *pio_alias;
     MemoryRegion *mmio_alias;
     MemoryRegion *ecam_alias;
+    MemoryRegion *core_iocsr[LOONGSON_MAX_VCPUS];
 };
 typedef struct LoongsonMachineState LoongsonMachineState;
 
@@ -493,6 +494,7 @@ static void mips_loongson3_virt_init(MachineState *machine)
     const char *kernel_filename = machine->kernel_filename;
     const char *initrd_filename = machine->initrd_filename;
     ram_addr_t ram_size = machine->ram_size;
+    LoongsonMachineState *s = LOONGSON_MACHINE(machine);
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *bios = g_new(MemoryRegion, 1);
@@ -586,6 +588,7 @@ static void mips_loongson3_virt_init(MachineState *machine)
                                      iocsr, 0, UINT32_MAX);
             memory_region_add_subregion(&MIPS_CPU(cpu)->env.iocsr.mr,
                                         0, core_iocsr);
+            s->core_iocsr[i] = core_iocsr;
         }
 
         if (node > 0) {
-- 
2.45.2

[PULL 02/20] hw/mips/loongson3_virt: Fix condition of IPI IOCSR connection

[Philippe Mathieu-Daudé]

From: Jiaxun Yang <jiaxun.yang@flygoat.com>

>>>     CID 1547264:  Null pointer dereferences  (REVERSE_INULL)
>>>     Null-checking "ipi" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.

Resolves: Coverity CID 1547264
Link: https://lore.kernel.org/qemu-devel/752417ad-ab72-4fed-8d1f-af41f15bc225@app.fastmail.com/
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240621-loongson3-ipi-follow-v2-2-848eafcbb67e@flygoat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/mips/loongson3_virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index 27a85e3614..2067b4fecb 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -574,7 +574,7 @@ static void mips_loongson3_virt_init(MachineState *machine)
         cpu_mips_clock_init(cpu);
         qemu_register_reset(main_cpu_reset, cpu);
 
-        if (ipi) {
+        if (!kvm_enabled()) {
             hwaddr base = ((hwaddr)node << 44) + virt_memmap[VIRT_IPI].base;
             base += core * 0x100;
             qdev_connect_gpio_out(ipi, i, cpu->env.irq[6]);
-- 
2.45.2

[PULL 03/20] qemu-options.hx: correct formatting -smbios type=4

[Philippe Mathieu-Daudé]

From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

processor-family and processor-id can be assigned independently.

Add missing brackets.

Fixes: b5831d79671c ("smbios: add processor-family option")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240729204816.11905-1-heinrich.schuchardt@canonical.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 qemu-options.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index cee0da2014..d99084a5ee 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2704,7 +2704,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
     "                specify SMBIOS type 3 fields\n"
     "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n"
     "              [,asset=str][,part=str][,max-speed=%d][,current-speed=%d]\n"
-    "              [,processor-family=%d,processor-id=%d]\n"
+    "              [,processor-family=%d][,processor-id=%d]\n"
     "                specify SMBIOS type 4 fields\n"
     "-smbios type=8[,external_reference=str][,internal_reference=str][,connector_type=%d][,port_type=%d]\n"
     "                specify SMBIOS type 8 fields\n"
-- 
2.45.2

[PULL 04/20] target/mips: Pass page table entry size as MemOp to get_pte()

[Philippe Mathieu-Daudé]

In order to simplify the next commit, pass the PTE size as MemOp.

Rename:

  native_shift -> native_op
  directory_shift -> directory_mop
  leaf_shift -> leaf_mop

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240814090452.2591-2-philmd@linaro.org>
---
 target/mips/tcg/sysemu/tlb_helper.c | 58 ++++++++++++++---------------
 1 file changed, 27 insertions(+), 31 deletions(-)

diff --git a/target/mips/tcg/sysemu/tlb_helper.c b/target/mips/tcg/sysemu/tlb_helper.c
index 3ba6d369a6..60147ba0af 100644
--- a/target/mips/tcg/sysemu/tlb_helper.c
+++ b/target/mips/tcg/sysemu/tlb_helper.c
@@ -592,13 +592,13 @@ static void raise_mmu_exception(CPUMIPSState *env, target_ulong address,
  * resulting in a TLB or XTLB Refill exception.
  */
 
-static bool get_pte(CPUMIPSState *env, uint64_t vaddr, int entry_size,
+static bool get_pte(CPUMIPSState *env, uint64_t vaddr, MemOp op,
         uint64_t *pte)
 {
-    if ((vaddr & ((entry_size >> 3) - 1)) != 0) {
+    if ((vaddr & (memop_size(op) - 1)) != 0) {
         return false;
     }
-    if (entry_size == 64) {
+    if (op == MO_64) {
         *pte = cpu_ldq_code(env, vaddr);
     } else {
         *pte = cpu_ldl_code(env, vaddr);
@@ -607,8 +607,9 @@ static bool get_pte(CPUMIPSState *env, uint64_t vaddr, int entry_size,
 }
 
 static uint64_t get_tlb_entry_layout(CPUMIPSState *env, uint64_t entry,
-        int entry_size, int ptei)
+                                     MemOp op, int ptei)
 {
+    unsigned entry_size = memop_size(op) << 3;
     uint64_t result = entry;
     uint64_t rixi;
     if (ptei > entry_size) {
@@ -624,14 +625,12 @@ static uint64_t get_tlb_entry_layout(CPUMIPSState *env, uint64_t entry,
 static int walk_directory(CPUMIPSState *env, uint64_t *vaddr,
         int directory_index, bool *huge_page, bool *hgpg_directory_hit,
         uint64_t *pw_entrylo0, uint64_t *pw_entrylo1,
-        unsigned directory_shift, unsigned leaf_shift, int ptw_mmu_idx)
+        MemOp directory_mop, MemOp leaf_mop, int ptw_mmu_idx)
 {
     int dph = (env->CP0_PWCtl >> CP0PC_DPH) & 0x1;
     int psn = (env->CP0_PWCtl >> CP0PC_PSN) & 0x3F;
     int hugepg = (env->CP0_PWCtl >> CP0PC_HUGEPG) & 0x1;
     int pf_ptew = (env->CP0_PWField >> CP0PF_PTEW) & 0x3F;
-    uint32_t direntry_size = 1 << (directory_shift + 3);
-    uint32_t leafentry_size = 1 << (leaf_shift + 3);
     uint64_t entry;
     uint64_t paddr;
     int prot;
@@ -643,14 +642,14 @@ static int walk_directory(CPUMIPSState *env, uint64_t *vaddr,
         /* wrong base address */
         return 0;
     }
-    if (!get_pte(env, *vaddr, direntry_size, &entry)) {
+    if (!get_pte(env, *vaddr, directory_mop, &entry)) {
         return 0;
     }
 
     if ((entry & (1 << psn)) && hugepg) {
         *huge_page = true;
         *hgpg_directory_hit = true;
-        entry = get_tlb_entry_layout(env, entry, leafentry_size, pf_ptew);
+        entry = get_tlb_entry_layout(env, entry, leaf_mop, pf_ptew);
         w = directory_index - 1;
         if (directory_index & 0x1) {
             /* Generate adjacent page from same PTE for odd TLB page */
@@ -658,7 +657,7 @@ static int walk_directory(CPUMIPSState *env, uint64_t *vaddr,
             *pw_entrylo0 = entry & ~lsb; /* even page */
             *pw_entrylo1 = entry | lsb; /* odd page */
         } else if (dph) {
-            int oddpagebit = 1 << leaf_shift;
+            int oddpagebit = 1 << leaf_mop;
             uint64_t vaddr2 = *vaddr ^ oddpagebit;
             if (*vaddr & oddpagebit) {
                 *pw_entrylo1 = entry;
@@ -669,10 +668,10 @@ static int walk_directory(CPUMIPSState *env, uint64_t *vaddr,
                                      ptw_mmu_idx) != TLBRET_MATCH) {
                 return 0;
             }
-            if (!get_pte(env, vaddr2, leafentry_size, &entry)) {
+            if (!get_pte(env, vaddr2, leaf_mop, &entry)) {
                 return 0;
             }
-            entry = get_tlb_entry_layout(env, entry, leafentry_size, pf_ptew);
+            entry = get_tlb_entry_layout(env, entry, leaf_mop, pf_ptew);
             if (*vaddr & oddpagebit) {
                 *pw_entrylo0 = entry;
             } else {
@@ -711,7 +710,7 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
 
     /* Native pointer size */
     /*For the 32-bit architectures, this bit is fixed to 0.*/
-    int native_shift = (((env->CP0_PWSize >> CP0PS_PS) & 1) == 0) ? 2 : 3;
+    MemOp native_op = (((env->CP0_PWSize >> CP0PS_PS) & 1) == 0) ? MO_32 : MO_64;
 
     /* Indices from PWField */
     int pf_gdw = (env->CP0_PWField >> CP0PF_GDW) & 0x3F;
@@ -728,11 +727,10 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
 
     /* Other HTW configs */
     int hugepg = (env->CP0_PWCtl >> CP0PC_HUGEPG) & 0x1;
-    unsigned directory_shift, leaf_shift;
+    MemOp directory_mop, leaf_mop;
 
     /* Offsets into tables */
     unsigned goffset, uoffset, moffset, ptoffset0, ptoffset1;
-    uint32_t leafentry_size;
 
     /* Starting address - Page Table Base */
     uint64_t vaddr = env->CP0_PWBase;
@@ -759,23 +757,21 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
     }
 
     /* HTW Shift values (depend on entry size) */
-    directory_shift = (hugepg && (ptew == 1)) ? native_shift + 1 : native_shift;
-    leaf_shift = (ptew == 1) ? native_shift + 1 : native_shift;
+    directory_mop = (hugepg && (ptew == 1)) ? native_op + 1 : native_op;
+    leaf_mop = (ptew == 1) ? native_op + 1 : native_op;
 
-    goffset = gindex << directory_shift;
-    uoffset = uindex << directory_shift;
-    moffset = mindex << directory_shift;
-    ptoffset0 = (ptindex >> 1) << (leaf_shift + 1);
-    ptoffset1 = ptoffset0 | (1 << (leaf_shift));
-
-    leafentry_size = 1 << (leaf_shift + 3);
+    goffset = gindex << directory_mop;
+    uoffset = uindex << directory_mop;
+    moffset = mindex << directory_mop;
+    ptoffset0 = (ptindex >> 1) << (leaf_mop + 1);
+    ptoffset1 = ptoffset0 | (1 << (leaf_mop));
 
     /* Global Directory */
     if (gdw > 0) {
         vaddr |= goffset;
         switch (walk_directory(env, &vaddr, pf_gdw, &huge_page, &hgpg_gdhit,
                                &pw_entrylo0, &pw_entrylo1,
-                               directory_shift, leaf_shift, ptw_mmu_idx))
+                               directory_mop, leaf_mop, ptw_mmu_idx))
         {
         case 0:
             return false;
@@ -792,7 +788,7 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
         vaddr |= uoffset;
         switch (walk_directory(env, &vaddr, pf_udw, &huge_page, &hgpg_udhit,
                                &pw_entrylo0, &pw_entrylo1,
-                               directory_shift, leaf_shift, ptw_mmu_idx))
+                               directory_mop, leaf_mop, ptw_mmu_idx))
         {
         case 0:
             return false;
@@ -809,7 +805,7 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
         vaddr |= moffset;
         switch (walk_directory(env, &vaddr, pf_mdw, &huge_page, &hgpg_mdhit,
                                &pw_entrylo0, &pw_entrylo1,
-                               directory_shift, leaf_shift, ptw_mmu_idx))
+                               directory_mop, leaf_mop, ptw_mmu_idx))
         {
         case 0:
             return false;
@@ -827,10 +823,10 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
                              ptw_mmu_idx) != TLBRET_MATCH) {
         return false;
     }
-    if (!get_pte(env, vaddr, leafentry_size, &dir_entry)) {
+    if (!get_pte(env, vaddr, leaf_mop, &dir_entry)) {
         return false;
     }
-    dir_entry = get_tlb_entry_layout(env, dir_entry, leafentry_size, pf_ptew);
+    dir_entry = get_tlb_entry_layout(env, dir_entry, leaf_mop, pf_ptew);
     pw_entrylo0 = dir_entry;
 
     /* Leaf Level Page Table - Second half of PTE pair */
@@ -839,10 +835,10 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
                              ptw_mmu_idx) != TLBRET_MATCH) {
         return false;
     }
-    if (!get_pte(env, vaddr, leafentry_size, &dir_entry)) {
+    if (!get_pte(env, vaddr, leaf_mop, &dir_entry)) {
         return false;
     }
-    dir_entry = get_tlb_entry_layout(env, dir_entry, leafentry_size, pf_ptew);
+    dir_entry = get_tlb_entry_layout(env, dir_entry, leaf_mop, pf_ptew);
     pw_entrylo1 = dir_entry;
 
 refill:
-- 
2.45.2

[PULL 05/20] target/mips: Use correct MMU index in get_pte()

[Philippe Mathieu-Daudé]

When refactoring page_table_walk_refill() in commit 4e999bf419
we missed the indirect call to cpu_mmu_index() in get_pte():

  page_table_walk_refill()
  -> get_pte()
     -> cpu_ld[lq]_code()
        -> cpu_mmu_index()

Since we don't mask anymore the modes in hflags, cpu_mmu_index()
can return UM or SM, while we only expect KM or ERL.

Fix by propagating ptw_mmu_idx to get_pte(), and use the
cpu_ld/st_code_mmu() API with the correct MemOpIdx.

Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Reported-by: Waldemar Brodkorb <wbx@uclibc-ng.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2470
Fixes: 4e999bf419 ("target/mips: Pass ptw_mmu_idx down from mips_cpu_tlb_fill")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20240814090452.2591-3-philmd@linaro.org>
---
 target/mips/tcg/sysemu/tlb_helper.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/target/mips/tcg/sysemu/tlb_helper.c b/target/mips/tcg/sysemu/tlb_helper.c
index 60147ba0af..0e94e00a5f 100644
--- a/target/mips/tcg/sysemu/tlb_helper.c
+++ b/target/mips/tcg/sysemu/tlb_helper.c
@@ -593,16 +593,21 @@ static void raise_mmu_exception(CPUMIPSState *env, target_ulong address,
  */
 
 static bool get_pte(CPUMIPSState *env, uint64_t vaddr, MemOp op,
-        uint64_t *pte)
+                    uint64_t *pte, unsigned ptw_mmu_idx)
 {
+    MemOpIdx oi;
+
     if ((vaddr & (memop_size(op) - 1)) != 0) {
         return false;
     }
+
+    oi = make_memop_idx(op | MO_TE, ptw_mmu_idx);
     if (op == MO_64) {
-        *pte = cpu_ldq_code(env, vaddr);
+        *pte = cpu_ldq_code_mmu(env, vaddr, oi, 0);
     } else {
-        *pte = cpu_ldl_code(env, vaddr);
+        *pte = cpu_ldl_code_mmu(env, vaddr, oi, 0);
     }
+
     return true;
 }
 
@@ -642,7 +647,7 @@ static int walk_directory(CPUMIPSState *env, uint64_t *vaddr,
         /* wrong base address */
         return 0;
     }
-    if (!get_pte(env, *vaddr, directory_mop, &entry)) {
+    if (!get_pte(env, *vaddr, directory_mop, &entry, ptw_mmu_idx)) {
         return 0;
     }
 
@@ -668,7 +673,7 @@ static int walk_directory(CPUMIPSState *env, uint64_t *vaddr,
                                      ptw_mmu_idx) != TLBRET_MATCH) {
                 return 0;
             }
-            if (!get_pte(env, vaddr2, leaf_mop, &entry)) {
+            if (!get_pte(env, vaddr2, leaf_mop, &entry, ptw_mmu_idx)) {
                 return 0;
             }
             entry = get_tlb_entry_layout(env, entry, leaf_mop, pf_ptew);
@@ -823,7 +828,7 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
                              ptw_mmu_idx) != TLBRET_MATCH) {
         return false;
     }
-    if (!get_pte(env, vaddr, leaf_mop, &dir_entry)) {
+    if (!get_pte(env, vaddr, leaf_mop, &dir_entry, ptw_mmu_idx)) {
         return false;
     }
     dir_entry = get_tlb_entry_layout(env, dir_entry, leaf_mop, pf_ptew);
@@ -835,7 +840,7 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
                              ptw_mmu_idx) != TLBRET_MATCH) {
         return false;
     }
-    if (!get_pte(env, vaddr, leaf_mop, &dir_entry)) {
+    if (!get_pte(env, vaddr, leaf_mop, &dir_entry, ptw_mmu_idx)) {
         return false;
     }
     dir_entry = get_tlb_entry_layout(env, dir_entry, leaf_mop, pf_ptew);
-- 
2.45.2

[PULL 06/20] target/mips: Load PTE as DATA

[Philippe Mathieu-Daudé]

PTE is not CODE so load it as normal DATA access.

Fixes: 074cfcb4da ("Implement hardware page table walker for MIPS32")
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20240814090452.2591-4-philmd@linaro.org>
---
 target/mips/tcg/sysemu/tlb_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/mips/tcg/sysemu/tlb_helper.c b/target/mips/tcg/sysemu/tlb_helper.c
index 0e94e00a5f..3836137750 100644
--- a/target/mips/tcg/sysemu/tlb_helper.c
+++ b/target/mips/tcg/sysemu/tlb_helper.c
@@ -603,9 +603,9 @@ static bool get_pte(CPUMIPSState *env, uint64_t vaddr, MemOp op,
 
     oi = make_memop_idx(op | MO_TE, ptw_mmu_idx);
     if (op == MO_64) {
-        *pte = cpu_ldq_code_mmu(env, vaddr, oi, 0);
+        *pte = cpu_ldq_mmu(env, vaddr, oi, 0);
     } else {
-        *pte = cpu_ldl_code_mmu(env, vaddr, oi, 0);
+        *pte = cpu_ldl_mmu(env, vaddr, oi, 0);
     }
 
     return true;
-- 
2.45.2

[PULL 07/20] hw/dma/xilinx_axidma: Use semicolon at end of statement, not comma

[Philippe Mathieu-Daudé]

From: Peter Maydell <peter.maydell@linaro.org>

In axidma_class_init() we accidentally used a comma at the end of
a statement rather than a semicolon. This has no ill effects, but
it's obviously not intended and it means that Coccinelle scripts
for instance will fail to match on the two statements. Use a
semicolon instead.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20240813165250.2717650-6-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/dma/xilinx_axidma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index c9cfc3169b..7707634253 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -626,7 +626,7 @@ static void axidma_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->realize = xilinx_axidma_realize,
+    dc->realize = xilinx_axidma_realize;
     dc->reset = xilinx_axidma_reset;
     device_class_set_props(dc, axidma_properties);
 }
-- 
2.45.2

[PULL 08/20] hw/remote/message.c: Don't directly invoke DeviceClass:reset

[Philippe Mathieu-Daudé]

From: Peter Maydell <peter.maydell@linaro.org>

Directly invoking the DeviceClass::reset method is a bad idea,
because if the device is using three-phase reset then it relies on
transitional reset machinery which is likely to disappear at some
point.

Reset the device in the standard way, by calling device_cold_reset().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240813165250.2717650-7-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/remote/message.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/remote/message.c b/hw/remote/message.c
index 50f6bf2d49..38ae6c75b4 100644
--- a/hw/remote/message.c
+++ b/hw/remote/message.c
@@ -215,13 +215,10 @@ fail:
 static void process_device_reset_msg(QIOChannel *ioc, PCIDevice *dev,
                                      Error **errp)
 {
-    DeviceClass *dc = DEVICE_GET_CLASS(dev);
     DeviceState *s = DEVICE(dev);
     MPQemuMsg ret = { 0 };
 
-    if (dc->reset) {
-        dc->reset(s);
-    }
+    device_cold_reset(s);
 
     ret.cmd = MPQEMU_CMD_RET;
 
-- 
2.45.2

[PULL 09/20] linux-user/mips: Do not try to use removed R5900 CPU

[Philippe Mathieu-Daudé]

R5900 emulation was removed in commit 823f2897bd.
Remove it from ELF parsing in order to avoid:

  $ qemu-mipsn32 ./test5900
  qemu-mipsn32: unable to find CPU model 'R5900'

This reverts commit 4d9e5a0eb7df6e98ac6cf5e16029f35dd05b9537.

Fixes: 823f2897bd ("target/mips: Disable R5900 support")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240814133928.6746-2-philmd@linaro.org>
---
 linux-user/mips/target_elf.h   | 3 ---
 linux-user/mips64/target_elf.h | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/linux-user/mips/target_elf.h b/linux-user/mips/target_elf.h
index b965e86b2b..71a32315a8 100644
--- a/linux-user/mips/target_elf.h
+++ b/linux-user/mips/target_elf.h
@@ -12,9 +12,6 @@ static inline const char *cpu_get_model(uint32_t eflags)
     if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_32R6) {
         return "mips32r6-generic";
     }
-    if ((eflags & EF_MIPS_MACH) == EF_MIPS_MACH_5900) {
-        return "R5900";
-    }
     if (eflags & EF_MIPS_NAN2008) {
         return "P5600";
     }
diff --git a/linux-user/mips64/target_elf.h b/linux-user/mips64/target_elf.h
index 5f2f2df29f..ec55d8542a 100644
--- a/linux-user/mips64/target_elf.h
+++ b/linux-user/mips64/target_elf.h
@@ -12,9 +12,6 @@ static inline const char *cpu_get_model(uint32_t eflags)
     if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_64R6) {
         return "I6400";
     }
-    if ((eflags & EF_MIPS_MACH) == EF_MIPS_MACH_5900) {
-        return "R5900";
-    }
     return "5KEf";
 }
 #endif
-- 
2.45.2

[PULL 10/20] linux-user/mips: Select Octeon68XX CPU for Octeon binaries

[Philippe Mathieu-Daudé]

The Octeon68XX CPU is available since commit 9a6046a655
("target/mips: introduce Cavium Octeon CPU model").

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1722
Reported-by: Johnathan Hữu Trí <nhtri2003@gmail.com>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240814133928.6746-3-philmd@linaro.org>
---
 linux-user/mips64/target_elf.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/linux-user/mips64/target_elf.h b/linux-user/mips64/target_elf.h
index ec55d8542a..ce6fb6541e 100644
--- a/linux-user/mips64/target_elf.h
+++ b/linux-user/mips64/target_elf.h
@@ -9,6 +9,14 @@
 #define MIPS64_TARGET_ELF_H
 static inline const char *cpu_get_model(uint32_t eflags)
 {
+    switch (eflags & EF_MIPS_MACH) {
+    case EF_MIPS_MACH_OCTEON:
+    case EF_MIPS_MACH_OCTEON2:
+    case EF_MIPS_MACH_OCTEON3:
+        return "Octeon68XX";
+    default:
+        break;
+    }
     if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_64R6) {
         return "I6400";
     }
-- 
2.45.2

[PULL 11/20] linux-user/mips: Select MIPS64R2-generic for Rel2 binaries

[Philippe Mathieu-Daudé]

Cc: YunQiang Su <syq@debian.org>
Reported-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240814133928.6746-4-philmd@linaro.org>
---
 linux-user/mips64/target_elf.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/linux-user/mips64/target_elf.h b/linux-user/mips64/target_elf.h
index ce6fb6541e..a3a8b2e385 100644
--- a/linux-user/mips64/target_elf.h
+++ b/linux-user/mips64/target_elf.h
@@ -17,8 +17,13 @@ static inline const char *cpu_get_model(uint32_t eflags)
     default:
         break;
     }
-    if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_64R6) {
+    switch (eflags & EF_MIPS_ARCH) {
+    case EF_MIPS_ARCH_64R6:
         return "I6400";
+    case EF_MIPS_ARCH_64R2:
+        return "MIPS64R2-generic";
+    default:
+        break;
     }
     return "5KEf";
 }
-- 
2.45.2

[PULL 12/20] linux-user/mips: Select Loongson CPU for Loongson binaries

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20240814133928.6746-5-philmd@linaro.org>
---
 linux-user/mips64/target_elf.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/linux-user/mips64/target_elf.h b/linux-user/mips64/target_elf.h
index a3a8b2e385..502af9d278 100644
--- a/linux-user/mips64/target_elf.h
+++ b/linux-user/mips64/target_elf.h
@@ -14,6 +14,12 @@ static inline const char *cpu_get_model(uint32_t eflags)
     case EF_MIPS_MACH_OCTEON2:
     case EF_MIPS_MACH_OCTEON3:
         return "Octeon68XX";
+    case EF_MIPS_MACH_LS2E:
+        return "Loongson-2E";
+    case EF_MIPS_MACH_LS2F:
+        return "Loongson-2F";
+    case EF_MIPS_MACH_LS3A:
+        return "Loongson-3A1000";
     default:
         break;
     }
-- 
2.45.2

[PULL 13/20] tests/avocado: exec_command should not consume console output

[Philippe Mathieu-Daudé]

From: Nicholas Piggin <npiggin@gmail.com>

_console_interaction reads data from the console even when there is only
an input string to send, and no output data to wait on. This can cause
lines to be missed by wait_for_console_pattern calls that follows an
exec_command. Fix this by not reading the console if there is no pattern
to wait for.

This solves occasional hangs in ppc_hv_tests.py, usually when run on KVM
hosts that are fast enough to output important lines quickly enough to be
consumed by exec_command, so they get missed by subsequent wait for
pattern calls.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20240805232814.267843-2-npiggin@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/avocado/avocado_qemu/__init__.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
index a3da2a96bb..ef935614cf 100644
--- a/tests/avocado/avocado_qemu/__init__.py
+++ b/tests/avocado/avocado_qemu/__init__.py
@@ -135,6 +135,13 @@ def _console_interaction(test, success_message, failure_message,
             vm.console_socket.sendall(send_string.encode())
             if not keep_sending:
                 send_string = None # send only once
+
+        # Only consume console output if waiting for something
+        if success_message is None and failure_message is None:
+            if send_string is None:
+                break
+            continue
+
         try:
             msg = console.readline().decode().strip()
         except UnicodeDecodeError:
-- 
2.45.2

[PULL 14/20] tests/avocado: Mark ppc_hv_tests.py as non-flaky after fixed console interaction

[Philippe Mathieu-Daudé]

From: Nicholas Piggin <npiggin@gmail.com>

Now that exec_command doesn't incorrectly consume console output,
and guest time is set correctly, ppc_hv_tests.py is working more
reliably. Try marking it non-flaky.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Message-ID: <20240805232814.267843-3-npiggin@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/avocado/ppc_hv_tests.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/avocado/ppc_hv_tests.py b/tests/avocado/ppc_hv_tests.py
index bf8822bb97..0e83bbac71 100644
--- a/tests/avocado/ppc_hv_tests.py
+++ b/tests/avocado/ppc_hv_tests.py
@@ -45,7 +45,6 @@ def missing_deps():
 # QEMU already installed and use that.
 # XXX: The order of these tests seems to matter, see git blame.
 @skipIf(missing_deps(), 'dependencies (%s) not installed' % ','.join(deps))
-@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test sometimes gets stuck due to console handling problem')
 @skipUnless(os.getenv('AVOCADO_ALLOW_LARGE_STORAGE'), 'storage limited')
 @skipUnless(os.getenv('SPEED') == 'slow', 'runtime limited')
 class HypervisorTest(QemuSystemTest):
-- 
2.45.2

[PULL 15/20] contrib/plugins/execlog: Fix shadowed declaration warning

[Philippe Mathieu-Daudé]

From: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Found on debian stable.

../contrib/plugins/execlog.c: In function ‘vcpu_tb_trans’:
../contrib/plugins/execlog.c:236:22: error: declaration of ‘n’ shadows a previous local [-Werror=shadow=local]
  236 |             for (int n = 0; n < all_reg_names->len; n++) {
      |                      ^
../contrib/plugins/execlog.c:184:12: note: shadowed declaration is here
  184 |     size_t n = qemu_plugin_tb_n_insns(tb);
      |

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20240814233645.944327-2-pierrick.bouvier@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 contrib/plugins/execlog.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index 1c1601cc0b..d67d010761 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -181,8 +181,8 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
     bool check_regs_this = rmatches;
     bool check_regs_next = false;
 
-    size_t n = qemu_plugin_tb_n_insns(tb);
-    for (size_t i = 0; i < n; i++) {
+    size_t n_insns = qemu_plugin_tb_n_insns(tb);
+    for (size_t i = 0; i < n_insns; i++) {
         char *insn_disas;
         uint64_t insn_vaddr;
 
-- 
2.45.2

[PULL 16/20] target/sparc: Restrict STQF to sparcv9

[Philippe Mathieu-Daudé]

From: Richard Henderson <richard.henderson@linaro.org>

Prior to sparcv9, the same encoding was STDFQ.

Cc: qemu-stable@nongnu.org
Fixes: 06c060d9e5b ("target/sparc: Move simple fp load/store to decodetree")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240816072311.353234-2-richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sparc/insns.decode | 2 +-
 target/sparc/translate.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode
index fbcb4f7aef..5fd478191a 100644
--- a/target/sparc/insns.decode
+++ b/target/sparc/insns.decode
@@ -644,7 +644,7 @@ STF         11 ..... 100100 ..... . .............          @r_r_ri_na
 STFSR       11 00000 100101 ..... . .............          @n_r_ri
 STXFSR      11 00001 100101 ..... . .............          @n_r_ri
 {
-  STQF      11 ..... 100110 ..... . .............          @q_r_ri_na
+  STQF      11 ..... 100110 ..... . .............          @q_r_ri_na # v9
   STDFQ     11 ----- 100110 ----- - -------------
 }
 STDF        11 ..... 100111 ..... . .............          @d_r_ri_na
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 113639083b..c803e8d1ba 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -4521,7 +4521,7 @@ static bool do_st_fpr(DisasContext *dc, arg_r_r_ri_asi *a, MemOp sz)
 
 TRANS(STF, ALL, do_st_fpr, a, MO_32)
 TRANS(STDF, ALL, do_st_fpr, a, MO_64)
-TRANS(STQF, ALL, do_st_fpr, a, MO_128)
+TRANS(STQF, 64, do_st_fpr, a, MO_128)
 
 TRANS(STFA, 64, do_st_fpr, a, MO_32)
 TRANS(STDFA, 64, do_st_fpr, a, MO_64)
-- 
2.45.2

[PULL 17/20] hw/ppc/Kconfig: Add missing SERIAL_ISA dependency to POWERNV machine

[Philippe Mathieu-Daudé]

From: Bernhard Beschow <shentey@gmail.com>

The machine calls serial_hds_isa_init() which is provided by serial-isa.c,
guarded by SERIAL_ISA.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240814181534.218964-4-shentey@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index c235519881..5addad1124 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -39,6 +39,7 @@ config POWERNV
     select PCI_POWERNV
     select PCA9552
     select PCA9554
+    select SERIAL_ISA
     select SSI
     select SSI_M25P80
     select PNV_SPI
-- 
2.45.2

[PULL 18/20] hw/i386/pc: Unify vmport=auto handling

[Philippe Mathieu-Daudé]

From: Kamil Szczęk <kamil@szczek.dev>

The code which translates vmport=auto to on/off is currently separate
for each PC machine variant, while being functionally equivalent.
This moves the translation into a shared initialization function, while
also tightening the enum assertion.

Signed-off-by: Kamil Szczęk <kamil@szczek.dev>
Reviewed-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <v8pz1uwgIYWkidgZK-o8H-qJvnSyl0641XVmNO43Qls307AA3QRPuad_py6xGe0JAxB6yDEe76oZ8tau_n-2Y6sJBCKzCujNbEUUFhd-ahI=@szczek.dev>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c      | 5 +++++
 hw/i386/pc_piix.c | 5 -----
 hw/i386/pc_q35.c  | 5 -----
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c74931d577..72229a24ff 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1217,6 +1217,11 @@ void pc_basic_device_init(struct PCMachineState *pcms,
         isa_realize_and_unref(pcms->pcspk, isa_bus, &error_fatal);
     }
 
+    assert(pcms->vmport >= 0 && pcms->vmport < ON_OFF_AUTO__MAX);
+    if (pcms->vmport == ON_OFF_AUTO_AUTO) {
+        pcms->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
+    }
+
     /* Super I/O */
     pc_superio_init(isa_bus, create_fdctrl, pcms->i8042_enabled,
                     pcms->vmport != ON_OFF_AUTO_ON);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d9e69243b4..347afa4c37 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -310,11 +310,6 @@ static void pc_init1(MachineState *machine, const char *pci_type)
 
     pc_vga_init(isa_bus, pcmc->pci_enabled ? pcms->pcibus : NULL);
 
-    assert(pcms->vmport != ON_OFF_AUTO__MAX);
-    if (pcms->vmport == ON_OFF_AUTO_AUTO) {
-        pcms->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
-    }
-
     /* init basic PC hardware */
     pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
                          !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 9d108b194e..f2d8edfa84 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -276,11 +276,6 @@ static void pc_q35_init(MachineState *machine)
         x86_register_ferr_irq(x86ms->gsi[13]);
     }
 
-    assert(pcms->vmport != ON_OFF_AUTO__MAX);
-    if (pcms->vmport == ON_OFF_AUTO_AUTO) {
-        pcms->vmport = ON_OFF_AUTO_ON;
-    }
-
     /* init basic PC hardware */
     pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc, !mc->no_floppy,
                          0xff0104);
-- 
2.45.2

Re: [PULL 18/20] hw/i386/pc: Unify vmport=auto handling

[Philippe Mathieu-Daudé]

Hi Kamil,

On 20/8/24 00:51, Philippe Mathieu-Daudé wrote:
> From: Kamil Szczęk <kamil@szczek.dev>
> 
> The code which translates vmport=auto to on/off is currently separate
> for each PC machine variant, while being functionally equivalent.
> This moves the translation into a shared initialization function, while
> also tightening the enum assertion.
> 
> Signed-off-by: Kamil Szczęk <kamil@szczek.dev>
> Reviewed-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-ID: <v8pz1uwgIYWkidgZK-o8H-qJvnSyl0641XVmNO43Qls307AA3QRPuad_py6xGe0JAxB6yDEe76oZ8tau_n-2Y6sJBCKzCujNbEUUFhd-ahI=@szczek.dev>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/i386/pc.c      | 5 +++++
>   hw/i386/pc_piix.c | 5 -----
>   hw/i386/pc_q35.c  | 5 -----
>   3 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c74931d577..72229a24ff 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1217,6 +1217,11 @@ void pc_basic_device_init(struct PCMachineState *pcms,
>           isa_realize_and_unref(pcms->pcspk, isa_bus, &error_fatal);
>       }
>   
> +    assert(pcms->vmport >= 0 && pcms->vmport < ON_OFF_AUTO__MAX);

Coverity reported:

 >>>     CID 1559533:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
 >>>     "pcms->vmport >= 0" is always true regardless of the values of 
its operands. This occurs as the logical first operand of "&&".

QAPI enums are unsigned because they start at 0, see:
https://www.qemu.org/docs/master/devel/qapi-code-gen.html#enumeration-types

   The generated C enumeration constants have values 0, 1, …, N-1
   (in QAPI schema order), where N is the number of values. There
   is an additional enumeration constant PREFIX__MAX with value N.

Could you post a patch to address this issue?

Thanks,

Phil.

> +    if (pcms->vmport == ON_OFF_AUTO_AUTO) {
> +        pcms->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
> +    }
> +
>       /* Super I/O */
>       pc_superio_init(isa_bus, create_fdctrl, pcms->i8042_enabled,
>                       pcms->vmport != ON_OFF_AUTO_ON);
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d9e69243b4..347afa4c37 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -310,11 +310,6 @@ static void pc_init1(MachineState *machine, const char *pci_type)
>   
>       pc_vga_init(isa_bus, pcmc->pci_enabled ? pcms->pcibus : NULL);
>   
> -    assert(pcms->vmport != ON_OFF_AUTO__MAX);
> -    if (pcms->vmport == ON_OFF_AUTO_AUTO) {
> -        pcms->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
> -    }
> -
>       /* init basic PC hardware */
>       pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
>                            !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 9d108b194e..f2d8edfa84 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -276,11 +276,6 @@ static void pc_q35_init(MachineState *machine)
>           x86_register_ferr_irq(x86ms->gsi[13]);
>       }
>   
> -    assert(pcms->vmport != ON_OFF_AUTO__MAX);
> -    if (pcms->vmport == ON_OFF_AUTO_AUTO) {
> -        pcms->vmport = ON_OFF_AUTO_ON;
> -    }
> -
>       /* init basic PC hardware */
>       pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc, !mc->no_floppy,
>                            0xff0104);

Re: [PULL 18/20] hw/i386/pc: Unify vmport=auto handling

[Kamil Szczęk]

On Tuesday, August 20th, 2024 at 21:48, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
> 
> Hi Kamil,
> 
> On 20/8/24 00:51, Philippe Mathieu-Daudé wrote:
> 
> > From: Kamil Szczęk kamil@szczek.dev
> > 
> > The code which translates vmport=auto to on/off is currently separate
> > for each PC machine variant, while being functionally equivalent.
> > This moves the translation into a shared initialization function, while
> > also tightening the enum assertion.
> > 
> > Signed-off-by: Kamil Szczęk kamil@szczek.dev
> > Reviewed-by: Bernhard Beschow shentey@gmail.com
> > Reviewed-by: Philippe Mathieu-Daudé philmd@linaro.org
> > Message-ID: v8pz1uwgIYWkidgZK-o8H-qJvnSyl0641XVmNO43Qls307AA3QRPuad_py6xGe0JAxB6yDEe76oZ8tau_n-2Y6sJBCKzCujNbEUUFhd-ahI=@szczek.dev
> > Signed-off-by: Philippe Mathieu-Daudé philmd@linaro.org
> > ---
> > hw/i386/pc.c | 5 +++++
> > hw/i386/pc_piix.c | 5 -----
> > hw/i386/pc_q35.c | 5 -----
> > 3 files changed, 5 insertions(+), 10 deletions(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index c74931d577..72229a24ff 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1217,6 +1217,11 @@ void pc_basic_device_init(struct PCMachineState *pcms,
> > isa_realize_and_unref(pcms->pcspk, isa_bus, &error_fatal);
> > }
> > 
> > + assert(pcms->vmport >= 0 && pcms->vmport < ON_OFF_AUTO__MAX);
> 
> 
> Coverity reported:
> 
> > CID 1559533: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
> > "pcms->vmport >= 0" is always true regardless of the values of
> > its operands. This occurs as the logical first operand of "&&".
> 
> QAPI enums are unsigned because they start at 0, see:
> https://www.qemu.org/docs/master/devel/qapi-code-gen.html#enumeration-types
> 
> The generated C enumeration constants have values 0, 1, …, N-1
> (in QAPI schema order), where N is the number of values. There
> is an additional enumeration constant PREFIX__MAX with value N.

Oh, and here I thought I was being smart with modifying this assert :D

> 
> Could you post a patch to address this issue?
> 

Will do shortly. Although, I've looked around the codebase and found a few more instances of this pattern.

"assert\(.*>= *0.*__MAX" yields the following results:

job.c
> assert(s1 >= 0 && s1 < JOB_STATUS__MAX);
> assert(verb >= 0 && verb < JOB_VERB__MAX);

blkdebug.c
> assert((int)event >= 0 && event < BLKDBG__MAX);

pc.c
> assert(pcms->vmport >= 0 && pcms->vmport < ON_OFF_AUTO__MAX);

options.c
> assert(mode >= 0 && mode < MIG_MODE__MAX);

savevm.c
> assert(capability >= 0 && capability < MIGRATION_CAPABILITY__MAX);

Does coverity also complain about those? If so, should I address all of them or keep it minimal?

Also, just as a test I added a single line of code before the assert:

pcms->vmport = -1;

And, to my surprise, it compiled successfully without any warning and as expected, aborted on the assert:

qemu-system-x86_64: ../hw/i386/pc.c:1225: pc_basic_device_init: Assertion 'pcms->vmport >= 0 && pcms->vmport < ON_OFF_AUTO__MAX' failed.

Is this expected behavior?

> Thanks,
> 
> Phil.
> 
> > + if (pcms->vmport == ON_OFF_AUTO_AUTO) {
> > + pcms->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
> > + }
> > +
> > /* Super I/O */
> > pc_superio_init(isa_bus, create_fdctrl, pcms->i8042_enabled,
> > pcms->vmport != ON_OFF_AUTO_ON);
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index d9e69243b4..347afa4c37 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -310,11 +310,6 @@ static void pc_init1(MachineState *machine, const char *pci_type)
> > 
> > pc_vga_init(isa_bus, pcmc->pci_enabled ? pcms->pcibus : NULL);
> > 
> > - assert(pcms->vmport != ON_OFF_AUTO__MAX);
> > - if (pcms->vmport == ON_OFF_AUTO_AUTO) {
> > - pcms->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
> > - }
> > -
> > /* init basic PC hardware */
> > pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
> > !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 9d108b194e..f2d8edfa84 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -276,11 +276,6 @@ static void pc_q35_init(MachineState *machine)
> > x86_register_ferr_irq(x86ms->gsi[13]);
> > }
> > 
> > - assert(pcms->vmport != ON_OFF_AUTO__MAX);
> > - if (pcms->vmport == ON_OFF_AUTO_AUTO) {
> > - pcms->vmport = ON_OFF_AUTO_ON;
> > - }
> > -
> > /* init basic PC hardware */
> > pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc, !mc->no_floppy,
> > 0xff0104);

Re: [PULL 18/20] hw/i386/pc: Unify vmport=auto handling

[Richard Henderson]

On 8/21/24 06:32, Kamil Szczęk wrote:
> Also, just as a test I added a single line of code before the assert:
> 
> pcms->vmport = -1;
> 
> And, to my surprise, it compiled successfully without any warning and as expected, aborted on the assert:
> 
> qemu-system-x86_64: ../hw/i386/pc.c:1225: pc_basic_device_init: Assertion 'pcms->vmport >= 0 && pcms->vmport < ON_OFF_AUTO__MAX' failed.
> 
> Is this expected behavior?

Yes.

The underlying integral type for enum in C is implementation defined.
It can and does vary between compilers, leading to this sort of thing.

The only reasonable fix is

   (unsigned)foo < max

But you could also question whether the assert is really useful.


r~

Re: [PULL 18/20] hw/i386/pc: Unify vmport=auto handling

[Kamil Szczęk]

On Wednesday, August 21st, 2024 at 00:45, Richard Henderson <richard.henderson@linaro.org> wrote:
> On 8/21/24 06:32, Kamil Szczęk wrote:
> 
> > Also, just as a test I added a single line of code before the assert:
> > 
> > pcms->vmport = -1;
> > 
> > And, to my surprise, it compiled successfully without any warning and as expected, aborted on the assert:
> > 
> > qemu-system-x86_64: ../hw/i386/pc.c:1225: pc_basic_device_init: Assertion 'pcms->vmport >= 0 && pcms->vmport < ON_OFF_AUTO__MAX' failed.
> > 
> > Is this expected behavior?
> 
> 
> Yes.
> 
> The underlying integral type for enum in C is implementation defined.
> It can and does vary between compilers, leading to this sort of thing.
> 
> The only reasonable fix is
> 
> (unsigned)foo < max

Fair enough, just posted a patch.

[PULL 19/20] hw/i386/pc: Ensure vmport prerequisites are fulfilled

[Philippe Mathieu-Daudé]

From: Kamil Szczęk <kamil@szczek.dev>

Since commit 4ccd5fe22feb95137d325f422016a6473541fe9f ('pc: add option
to disable PS/2 mouse/keyboard'), the vmport will not be created unless
the i8042 PS/2 controller is enabled. To avoid confusion, let's fail if
vmport was explicitly requested, but the i8042 controller is disabled.
This also changes the behavior of vmport=auto to take i8042 controller
availability into account.

Signed-off-by: Kamil Szczęk <kamil@szczek.dev>
Reviewed-by: Bernhard Beschow <shentey@gmail.com>
Message-ID: <0MS3y5E-hHqODIhiuFxmCnIrXd612JIGq31UuMsz4KGCKZ_wWuF-PHGKTRSGS0nWaPEddOdF4YOczHdgorulECPo792OhWov7O9BBF6UMX4=@szczek.dev>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c    | 11 ++++++++---
 qemu-options.hx |  4 ++--
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 72229a24ff..7779c88a91 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1075,7 +1075,7 @@ static const MemoryRegionOps ioportF0_io_ops = {
 };
 
 static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
-                            bool create_i8042, bool no_vmport)
+                            bool create_i8042, bool no_vmport, Error **errp)
 {
     int i;
     DriveInfo *fd[MAX_FD];
@@ -1100,6 +1100,10 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
     }
 
     if (!create_i8042) {
+        if (!no_vmport) {
+            error_setg(errp,
+                       "vmport requires the i8042 controller to be enabled");
+        }
         return;
     }
 
@@ -1219,12 +1223,13 @@ void pc_basic_device_init(struct PCMachineState *pcms,
 
     assert(pcms->vmport >= 0 && pcms->vmport < ON_OFF_AUTO__MAX);
     if (pcms->vmport == ON_OFF_AUTO_AUTO) {
-        pcms->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
+        pcms->vmport = (xen_enabled() || !pcms->i8042_enabled)
+            ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
     }
 
     /* Super I/O */
     pc_superio_init(isa_bus, create_fdctrl, pcms->i8042_enabled,
-                    pcms->vmport != ON_OFF_AUTO_ON);
+                    pcms->vmport != ON_OFF_AUTO_ON, &error_fatal);
 }
 
 void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus)
diff --git a/qemu-options.hx b/qemu-options.hx
index d99084a5ee..d94e2cbbae 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -68,8 +68,8 @@ SRST
 
     ``vmport=on|off|auto``
         Enables emulation of VMWare IO port, for vmmouse etc. auto says
-        to select the value based on accel. For accel=xen the default is
-        off otherwise the default is on.
+        to select the value based on accel and i8042. For accel=xen or
+        i8042=off the default is off otherwise the default is on.
 
     ``dump-guest-core=on|off``
         Include guest memory in a core dump. The default is on.
-- 
2.45.2

[PULL 20/20] crypto/tlscredspsk: Free username on finalize

[Philippe Mathieu-Daudé]

From: Peter Maydell <peter.maydell@linaro.org>

When the creds->username property is set we allocate memory
for it in qcrypto_tls_creds_psk_prop_set_username(), but
we never free this when the QCryptoTLSCredsPSK is destroyed.
Free the memory in finalize.

This fixes a LeakSanitizer complaint in migration-test:

$ (cd build/asan; ASAN_OPTIONS="fast_unwind_on_malloc=0" QTEST_QEMU_BINARY=./qemu-system-x86_64 ./tests/qtest/migration-test --tap -k -p /x86_64/migration/precopy/unix/tls/psk)

=================================================================
==3867512==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 5 byte(s) in 1 object(s) allocated from:
    #0 0x5624e5c99dee in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qemu-system-x86_64+0x218edee) (BuildId: a9e623fa1009a9435c0142c037cd7b8c1ad04ce3)
    #1 0x7fb199ae9738 in g_malloc debian/build/deb/../../../glib/gmem.c:128:13
    #2 0x7fb199afe583 in g_strdup debian/build/deb/../../../glib/gstrfuncs.c:361:17
    #3 0x5624e82ea919 in qcrypto_tls_creds_psk_prop_set_username /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../crypto/tlscredspsk.c:255:23
    #4 0x5624e812c6b5 in property_set_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/object.c:2277:5
    #5 0x5624e8125ce5 in object_property_set /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/object.c:1463:5
    #6 0x5624e8136e7c in object_set_properties_from_qdict /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/object_interfaces.c:55:14
    #7 0x5624e81372d2 in user_creatable_add_type /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/object_interfaces.c:112:5
    #8 0x5624e8137964 in user_creatable_add_qapi /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/object_interfaces.c:157:11
    #9 0x5624e891ba3c in qmp_object_add /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/qom-qmp-cmds.c:227:5
    #10 0x5624e8af9118 in qmp_marshal_object_add /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qapi/qapi-commands-qom.c:337:5
    #11 0x5624e8bd1d49 in do_qmp_dispatch_bh /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qapi/qmp-dispatch.c:128:5
    #12 0x5624e8cb2531 in aio_bh_call /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/async.c:171:5
    #13 0x5624e8cb340c in aio_bh_poll /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/async.c:218:13
    #14 0x5624e8c0be98 in aio_dispatch /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/aio-posix.c:423:5
    #15 0x5624e8cba3ce in aio_ctx_dispatch /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/async.c:360:5
    #16 0x7fb199ae0d3a in g_main_dispatch debian/build/deb/../../../glib/gmain.c:3419:28
    #17 0x7fb199ae0d3a in g_main_context_dispatch debian/build/deb/../../../glib/gmain.c:4137:7
    #18 0x5624e8cbe1d9 in glib_pollfds_poll /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/main-loop.c:287:9
    #19 0x5624e8cbcb13 in os_host_main_loop_wait /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/main-loop.c:310:5
    #20 0x5624e8cbc6dc in main_loop_wait /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/main-loop.c:589:11
    #21 0x5624e6f3f917 in qemu_main_loop /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../system/runstate.c:801:9
    #22 0x5624e893379c in qemu_default_main /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../system/main.c:37:14
    #23 0x5624e89337e7 in main /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../system/main.c:48:12
    #24 0x7fb197972d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #25 0x7fb197972e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #26 0x5624e5c16fa4 in _start (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qemu-system-x86_64+0x210bfa4) (BuildId: a9e623fa1009a9435c0142c037cd7b8c1ad04ce3)

SUMMARY: AddressSanitizer: 5 byte(s) leaked in 1 allocation(s).

Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20240819145021.38524-1-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 crypto/tlscredspsk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crypto/tlscredspsk.c b/crypto/tlscredspsk.c
index 546cad1c5a..0d6b71a37c 100644
--- a/crypto/tlscredspsk.c
+++ b/crypto/tlscredspsk.c
@@ -243,6 +243,7 @@ qcrypto_tls_creds_psk_finalize(Object *obj)
     QCryptoTLSCredsPSK *creds = QCRYPTO_TLS_CREDS_PSK(obj);
 
     qcrypto_tls_creds_psk_unload(creds);
+    g_free(creds->username);
 }
 
 static void
-- 
2.45.2

Re: [PULL 00/20] Misc fixes for 2024-08-20

[Richard Henderson]

On 8/20/24 08:50, Philippe Mathieu-Daudé wrote:
> The following changes since commit ecdfa31beb1f7616091bedba79dfdf9ee525ed9d:
> 
>    Merge tag 'pull-request-2024-08-16' ofhttps://gitlab.com/thuth/qemu into staging (2024-08-16 18:18:27 +1000)
> 
> are available in the Git repository at:
> 
>    https://github.com/philmd/qemu.git tags/hw-misc-20240820
> 
> for you to fetch changes up to 87e012f29f2e47dcd8c385ff8bb8188f9e06d4ea:
> 
>    crypto/tlscredspsk: Free username on finalize (2024-08-20 00:49:14 +0200)
> 
> Ignored checkpatch warning:
> 
>    WARNING: line over 80 characters
>    #115: FILE: target/mips/tcg/sysemu/tlb_helper.c:713:
>    +    MemOp native_op = (((env->CP0_PWSize >> CP0PS_PS) & 1) == 0) ? MO_32 : MO_64;
> 
> ----------------------------------------------------------------
> Various fixes
> 
> - Null pointer dereference in IPI IOCSR (Jiaxun)
> - Correct '-smbios type=4' in man page (Heinrich)
> - Use correct MMU index in MIPS get_pte (Phil)
> - Reset MPQEMU remote message using device_cold_reset (Peter)
> - Update linux-user MIPS CPU list (Phil)
> - Do not let exec_command read console if no pattern to wait for (Nick)
> - Remove shadowed declaration warning (Pierrick)
> - Restrict STQF opcode to SPARC V9 (Richard)
> - Add missing Kconfig dependency for POWERNV ISA serial port (Bernhard)
> - Do not allow vmport device without i8042 PS/2 controller (Kamil)
> - Fix QCryptoTLSCredsPSK leak (Peter)


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

r~