Re: [PULL 03/20] nubus-device: expose separate super slot memory region

[Laurent Vivier]

Le 02/10/2021 à 12:33, Peter Maydell a écrit :
> On Wed, 29 Sept 2021 at 10:49, Laurent Vivier  wrote:
>>
>> From: Mark Cave-Ayland 
>>
>> According to "Designing Cards and Drivers for the Macintosh Family" each 
>> physical
>> nubus slot can access 2 separate address ranges: a super slot memory region 
>> which
>> is 256MB and a standard slot memory region which is 16MB.
>>
>> Currently a Nubus device uses the physical slot number to determine whether 
>> it is
>> using a standard slot memory region or a super slot memory region rather than
>> exposing both memory regions for use as required.
> 
> 
>> +/* Super */
>> +slot_offset = nd->slot * NUBUS_SUPER_SLOT_SIZE;
> 
> Hi; Coverity thinks this multiply might overflow, because
> we're calculating a hw_addr (64-bits) but the multiply is only
> done at 32-bits. Adding an explicit cast or using 'ULL' in the
> constant #define rather than just 'U' would fix this.
> This is CID 1464070.
> 

I'm wondering if adding "assert(nd->slot < NUBUS_SUPER_SLOT_NB)" would help 
coverity to avoid the
error without using 64bit arithmetic?


>> +
>> +name = g_strdup_printf("nubus-super-slot-%x", nd->slot);
>> +memory_region_init(&nd->super_slot_mem, OBJECT(dev), name,
>> +   NUBUS_SUPER_SLOT_SIZE);
>> +memory_region_add_subregion(&nubus->super_slot_io, slot_offset,
>> +&nd->super_slot_mem);
>> +g_free(name);
>> +
>> +/* Normal */
>> +slot_offset = nd->slot * NUBUS_SLOT_SIZE;
> 
> Same with this one.

assert(nb->slot < NUBUS_SLOT_NB)

> thanks
> -- PMM
> 

Laurent

[PATCH] docs: build-system: rename "default-configs" to "configs"

[Kashyap Chamarthy]

Commit 812b31d3f9 (configs: rename default-configs to configs and
reorganise, 2021-07-07) did the rename.

Reflect that update also in the documentation.

Signed-off-by: Kashyap Chamarthy 
---
 docs/devel/build-system.rst | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst
index 0f636d620e..bfa5250802 100644
--- a/docs/devel/build-system.rst
+++ b/docs/devel/build-system.rst
@@ -250,7 +250,7 @@ Target-dependent emulator sourcesets:
   Each emulator also includes sources for files in the ``hw/`` and ``target/``
   subdirectories.  The subdirectory used for each emulator comes
   from the target's definition of ``TARGET_BASE_ARCH`` or (if missing)
-  ``TARGET_ARCH``, as found in ``default-configs/targets/*.mak``.
+  ``TARGET_ARCH``, as found in ``configs/targets/*.mak``.
 
   Each subdirectory in ``hw/`` adds one sourceset to the ``hw_arch`` 
dictionary,
   for example::
@@ -307,8 +307,8 @@ Utility sourcesets:
 The following files concur in the definition of which files are linked
 into each emulator:
 
-``default-configs/devices/*.mak``
-  The files under ``default-configs/devices/`` control the boards and devices
+``configs/devices/*.mak``
+  The files under ``configs/devices/`` control the boards and devices
   that are built into each QEMU system emulation targets. They merely contain
   a list of config variable definitions such as::
 
@@ -317,11 +317,11 @@ into each emulator:
 CONFIG_XLNX_VERSAL=y
 
 ``*/Kconfig``
-  These files are processed together with ``default-configs/devices/*.mak`` and
+  These files are processed together with ``configs/devices/*.mak`` and
   describe the dependencies between various features, subsystems and
   device models.  They are described in :ref:`kconfig`
 
-``default-configs/targets/*.mak``
+``configs/targets/*.mak``
   These files mostly define symbols that appear in the ``*-config-target.h``
   file for each emulator [#cfgtarget]_.  However, the ``TARGET_ARCH``
   and ``TARGET_BASE_ARCH`` will also be used to select the ``hw/`` and
@@ -460,7 +460,7 @@ Built by Meson:
   TARGET-NAME is again the name of a system or userspace emulator. The
   config-devices.mak file is automatically generated by make using the
   scripts/make_device_config.sh program, feeding it the
-  default-configs/$TARGET-NAME file as input.
+  configs/$TARGET-NAME file as input.
 
 ``config-host.h``, ``$TARGET-NAME/config-target.h``, 
``$TARGET-NAME/config-devices.h``
   These files are used by source code to determine what features
-- 
2.31.1

Re: [PATCH] docs: build-system: rename "default-configs" to "configs"

[Philippe Mathieu-Daudé]

On 10/4/21 09:12, Kashyap Chamarthy wrote:
> Commit 812b31d3f9 (configs: rename default-configs to configs and
> reorganise, 2021-07-07) did the rename.
> 
> Reflect that update also in the documentation.
> 
> Signed-off-by: Kashyap Chamarthy 
> ---
>  docs/devel/build-system.rst | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v4 0/4] softmmu/memory_mapping: optimize dump/tpm for virtio-mem

[David Hildenbrand]

On 02.10.21 11:55, Paolo Bonzini wrote:

On 27/07/21 10:25, David Hildenbrand wrote:

Minor fixes and cleanups, followed by an optimization for virtio-mem
regarding guest dumps and tpm.

virtio-mem logically plugs/unplugs memory within a sparse memory region
and notifies via the RamDiscardMgr interface when parts become
plugged (populated) or unplugged (discarded).

Currently, guest_phys_blocks_append() appends the whole (sparse)
virtio-mem managed region and therefore tpm code might zero the hole
region and dump code will dump the whole region. Let's only add logically
plugged (populated) parts of that region, skipping over logically
unplugged (discarded) parts by reusing the RamDiscardMgr infrastructure
introduced to handle virtio-mem + VFIO properly.


Queued, thanks.



Thanks Paolo!

--
Thanks,

David / dhildenb

Re: [PATCH v2 1/2] tcg: add dup_const_tl wrapper

[Philippe Mathieu-Daudé]

On 10/3/21 23:42, Philipp Tomsich wrote:
> dup_const always generates a uint64_t, which may exceed the size of a
> target_long (generating warnings with recent-enough compilers).
> 
> To ensure that we can use dup_const both for 64bit and 32bit targets,
> this adds dup_const_tl, which either maps back to dup_const (for 64bit
> targets) or provides a similar implementation using 32bit constants.
> 
> Signed-off-by: Philipp Tomsich 
> 
> ---
> 
> Changes in v2:
> - Changed dup_const_tl to enforce the sanity check with
>   qemu_build_not_reached as requested in the review.
> 
>  include/tcg/tcg.h | 12 
>  1 file changed, 12 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé 

[PULL 05/26] linux-user/arm: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Mirror what the kernel does in arch/arm/kernel/signal.h,
using the old sigframe struct in the rt sigframe struct.

Update the trampoline code to match the kernel: this uses
sp-relative accesses rather than pc-relative.

Copy the code into frame->retcode from the trampoline page.
This minimises the different cases wrt arm vs thumb vs fdpic.

Signed-off-by: Richard Henderson 
Reviewed-by: Peter Maydell 
Message-Id: <20210929130553.121567-6-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/arm/signal.c| 179 -
 linux-user/arm/target_signal.h |   2 +
 2 files changed, 110 insertions(+), 71 deletions(-)

diff --git a/linux-user/arm/signal.c b/linux-user/arm/signal.c
index ed7d1d80bb9c..df9f8e8eb200 100644
--- a/linux-user/arm/signal.c
+++ b/linux-user/arm/signal.c
@@ -99,43 +99,21 @@ struct sigframe
 struct rt_sigframe
 {
 struct target_siginfo info;
-struct target_ucontext uc;
-abi_ulong retcode[4];
+struct sigframe sig;
 };
 
-/*
- * For ARM syscalls, we encode the syscall number into the instruction.
- */
-#define SWI_SYS_SIGRETURN   (0xef00|(TARGET_NR_sigreturn + 
ARM_SYSCALL_BASE))
-#define SWI_SYS_RT_SIGRETURN(0xef00|(TARGET_NR_rt_sigreturn + 
ARM_SYSCALL_BASE))
-
-/*
- * For Thumb syscalls, we pass the syscall number via r7.  We therefore
- * need two 16-bit instructions.
- */
-#define SWI_THUMB_SIGRETURN (0xdf00 << 16 | 0x2700 | (TARGET_NR_sigreturn))
-#define SWI_THUMB_RT_SIGRETURN  (0xdf00 << 16 | 0x2700 | 
(TARGET_NR_rt_sigreturn))
-
-static const abi_ulong retcodes[4] = {
-SWI_SYS_SIGRETURN,  SWI_THUMB_SIGRETURN,
-SWI_SYS_RT_SIGRETURN,   SWI_THUMB_RT_SIGRETURN
-};
+static abi_ptr sigreturn_fdpic_tramp;
 
 /*
- * Stub needed to make sure the FD register (r9) contains the right
- * value.
+ * Up to 3 words of 'retcode' in the sigframe are code,
+ * with retcode[3] being used by fdpic for the function descriptor.
+ * This code is not actually executed, but is retained for ABI compat.
+ *
+ * We will create a table of 8 retcode variants in the sigtramp page.
+ * Let each table entry use 3 words.
  */
-static const unsigned long sigreturn_fdpic_codes[3] = {
-0xe59fc004, /* ldr r12, [pc, #4] to read function descriptor */
-0xe59c9004, /* ldr r9, [r12, #4] to setup GOT */
-0xe59cf000  /* ldr pc, [r12] to jump into restorer */
-};
-
-static const unsigned long sigreturn_fdpic_thumb_codes[3] = {
-0xc008f8df, /* ldr r12, [pc, #8] to read function descriptor */
-0x9004f8dc, /* ldr r9, [r12, #4] to setup GOT */
-0xf000f8dc  /* ldr pc, [r12] to jump into restorer */
-};
+#define RETCODE_WORDS  3
+#define RETCODE_BYTES  (RETCODE_WORDS * 4)
 
 static inline int valid_user_regs(CPUARMState *regs)
 {
@@ -183,15 +161,15 @@ get_sigframe(struct target_sigaction *ka, CPUARMState 
*regs, int framesize)
 }
 
 static int
-setup_return(CPUARMState *env, struct target_sigaction *ka,
- abi_ulong *rc, abi_ulong frame_addr, int usig, abi_ulong rc_addr)
+setup_return(CPUARMState *env, struct target_sigaction *ka, int usig,
+ struct sigframe *frame, abi_ulong sp_addr)
 {
 abi_ulong handler = 0;
 abi_ulong handler_fdpic_GOT = 0;
 abi_ulong retcode;
-
-int thumb;
+int thumb, retcode_idx;
 int is_fdpic = info_is_fdpic(((TaskState *)thread_cpu->opaque)->info);
+bool copy_retcode;
 
 if (is_fdpic) {
 /* In FDPIC mode, ka->_sa_handler points to a function
@@ -208,6 +186,7 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
 }
 
 thumb = handler & 1;
+retcode_idx = thumb + (ka->sa_flags & TARGET_SA_SIGINFO ? 2 : 0);
 
 uint32_t cpsr = cpsr_read(env);
 
@@ -225,44 +204,29 @@ setup_return(CPUARMState *env, struct target_sigaction 
*ka,
 
 if (ka->sa_flags & TARGET_SA_RESTORER) {
 if (is_fdpic) {
-/* For FDPIC we ensure that the restorer is called with a
- * correct r9 value.  For that we need to write code on
- * the stack that sets r9 and jumps back to restorer
- * value.
- */
-if (thumb) {
-__put_user(sigreturn_fdpic_thumb_codes[0], rc);
-__put_user(sigreturn_fdpic_thumb_codes[1], rc + 1);
-__put_user(sigreturn_fdpic_thumb_codes[2], rc + 2);
-__put_user((abi_ulong)ka->sa_restorer, rc + 3);
-} else {
-__put_user(sigreturn_fdpic_codes[0], rc);
-__put_user(sigreturn_fdpic_codes[1], rc + 1);
-__put_user(sigreturn_fdpic_codes[2], rc + 2);
-__put_user((abi_ulong)ka->sa_restorer, rc + 3);
-}
-
-retcode = rc_addr + thumb;
+__put_user((abi_ulong)ka->sa_restorer, &frame->retcode[3]);
+retcode = (sigreturn_fdpic_tramp +
+   retcode_idx * RETCODE_BYTES + thumb);
+copy_retcode = true;

[PULL 02/26] linux-user/aarch64: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the rt signal trampoline.
Use it when the guest does not use SA_RESTORER.

Reviewed-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-3-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/aarch64/signal.c| 34 ++
 linux-user/aarch64/target_signal.h |  2 ++
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
index 49025648cb4f..29c52db3f130 100644
--- a/linux-user/aarch64/signal.c
+++ b/linux-user/aarch64/signal.c
@@ -109,7 +109,6 @@ struct target_rt_sigframe {
 struct target_rt_frame_record {
 uint64_t fp;
 uint64_t lr;
-uint32_t tramp[2];
 };
 
 static void target_setup_general_frame(struct target_rt_sigframe *sf,
@@ -461,9 +460,9 @@ static void target_setup_frame(int usig, struct 
target_sigaction *ka,
 layout.total_size = MAX(layout.total_size,
 sizeof(struct target_rt_sigframe));
 
-/* Reserve space for the return code.  On a real system this would
- * be within the VDSO.  So, despite the name this is not a "real"
- * record within the frame.
+/*
+ * Reserve space for the standard frame unwind pair: fp, lr.
+ * Despite the name this is not a "real" record within the frame.
  */
 fr_ofs = layout.total_size;
 layout.total_size += sizeof(struct target_rt_frame_record);
@@ -496,15 +495,7 @@ static void target_setup_frame(int usig, struct 
target_sigaction *ka,
 if (ka->sa_flags & TARGET_SA_RESTORER) {
 return_addr = ka->sa_restorer;
 } else {
-/*
- * mov x8,#__NR_rt_sigreturn; svc #0
- * Since these are instructions they need to be put as little-endian
- * regardless of target default or current CPU endianness.
- */
-__put_user_e(0xd2801168, &fr->tramp[0], le);
-__put_user_e(0xd401, &fr->tramp[1], le);
-return_addr = frame_addr + fr_ofs
-+ offsetof(struct target_rt_frame_record, tramp);
+return_addr = default_rt_sigreturn;
 }
 env->xregs[0] = usig;
 env->xregs[29] = frame_addr + fr_ofs;
@@ -577,3 +568,20 @@ long do_sigreturn(CPUARMState *env)
 {
 return do_rt_sigreturn(env);
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 8, 0);
+assert(tramp != NULL);
+
+/*
+ * mov x8,#__NR_rt_sigreturn; svc #0
+ * Since these are instructions they need to be put as little-endian
+ * regardless of target default or current CPU endianness.
+ */
+__put_user_e(0xd2801168, &tramp[0], le);
+__put_user_e(0xd401, &tramp[1], le);
+
+default_rt_sigreturn = sigtramp_page;
+unlock_user(tramp, sigtramp_page, 8);
+}
diff --git a/linux-user/aarch64/target_signal.h 
b/linux-user/aarch64/target_signal.h
index 18013e1b2350..7580d99403cb 100644
--- a/linux-user/aarch64/target_signal.h
+++ b/linux-user/aarch64/target_signal.h
@@ -25,4 +25,6 @@ typedef struct target_sigaltstack {
 #define TARGET_SEGV_MTESERR  9  /* Synchronous ARM MTE exception */
 
 #define TARGET_ARCH_HAS_SETUP_FRAME
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* AARCH64_TARGET_SIGNAL_H */
-- 
2.31.1

Re: [PATCH v2 2/2] target/riscv: Use dup_const_tl in orc.b to legalise truncation of constant

[Philippe Mathieu-Daudé]

On 10/3/21 23:42, Philipp Tomsich wrote:
> We need to use the newly introduced dup_const_tl in orc.b to legalise
> the truncation (to target_long) of the constant generated with dup_const.
> 
> Signed-off-by: Philipp Tomsich 
> Reviewed-by: Richard Henderson 
> ---
> 
> (no changes since v1)
> 
>  target/riscv/insn_trans/trans_rvb.c.inc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé 

[PULL 01/26] linux-user: Add infrastructure for a signal trampoline page

[Laurent Vivier]

From: Richard Henderson 

Allocate a page to hold the signal trampoline(s).
Invoke a guest-specific hook to fill in the contents
of the page before marking it read-execute again.

Reviewed-by: Max Filippov 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-2-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/elfload.c   | 18 ++
 linux-user/signal-common.h |  6 ++
 linux-user/signal.c|  3 +++
 3 files changed, 27 insertions(+)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 5f9e2141ad1e..459a26ef1d93 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -7,6 +7,7 @@
 
 #include "qemu.h"
 #include "user-internals.h"
+#include "signal-common.h"
 #include "loader.h"
 #include "user-mmap.h"
 #include "disas/disas.h"
@@ -17,6 +18,7 @@
 #include "qemu/units.h"
 #include "qemu/selfmap.h"
 #include "qapi/error.h"
+#include "target_signal.h"
 
 #ifdef _ARCH_PPC64
 #undef ARCH_DLINFO
@@ -28,6 +30,10 @@
 #undef ELF_ARCH
 #endif
 
+#ifndef TARGET_ARCH_HAS_SIGTRAMP_PAGE
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
+#endif
+
 #define ELF_OSABI   ELFOSABI_SYSV
 
 /* from personality.h */
@@ -3249,6 +3255,18 @@ int load_elf_binary(struct linux_binprm *bprm, struct 
image_info *info)
 #endif
 }
 
+/*
+ * TODO: load a vdso, which would also contain the signal trampolines.
+ * Otherwise, allocate a private page to hold them.
+ */
+if (TARGET_ARCH_HAS_SIGTRAMP_PAGE) {
+abi_ulong tramp_page = target_mmap(0, TARGET_PAGE_SIZE,
+   PROT_READ | PROT_WRITE,
+   MAP_PRIVATE | MAP_ANON, -1, 0);
+setup_sigtramp(tramp_page);
+target_mprotect(tramp_page, TARGET_PAGE_SIZE, PROT_READ | PROT_EXEC);
+}
+
 bprm->p = create_elf_tables(bprm->p, bprm->argc, bprm->envc, &elf_ex,
 info, (elf_interpreter ? &interp_info : NULL));
 info->start_stack = bprm->p;
diff --git a/linux-user/signal-common.h b/linux-user/signal-common.h
index 79511becb4e7..7457f8025c47 100644
--- a/linux-user/signal-common.h
+++ b/linux-user/signal-common.h
@@ -20,6 +20,12 @@
 #ifndef SIGNAL_COMMON_H
 #define SIGNAL_COMMON_H
 
+/* Fallback addresses into sigtramp page. */
+extern abi_ulong default_sigreturn;
+extern abi_ulong default_rt_sigreturn;
+
+void setup_sigtramp(abi_ulong tramp_page);
+
 int on_sig_stack(unsigned long sp);
 int sas_ss_flags(unsigned long sp);
 abi_ulong target_sigsp(abi_ulong sp, struct target_sigaction *ka);
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 203821645509..14d8fdfde152 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -35,6 +35,9 @@ static struct target_sigaction sigact_table[TARGET_NSIG];
 static void host_signal_handler(int host_signum, siginfo_t *info,
 void *puc);
 
+/* Fallback addresses into sigtramp page. */
+abi_ulong default_sigreturn;
+abi_ulong default_rt_sigreturn;
 
 /*
  * System includes define _NSIG as SIGRTMAX + 1,
-- 
2.31.1

[PULL 00/26] Linux user for 6.2 patches

[Laurent Vivier]

The following changes since commit bb4aa8f59e18412cff0d69f14aee7abba153161a:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210930' 
into staging (2021-09-30 21:16:54 +0100)

are available in the Git repository at:

  git://github.com/vivier/qemu.git tags/linux-user-for-6.2-pull-request

for you to fetch changes up to efee71c8ca181d4f5b2211736b38a74a2a223375:

  tests/tcg/multiarch: Re-enable signals test for most guests (2021-10-01 
12:03:48 +0200)


Pull request linux-user 20211004

Move signal trampolines to new page



Richard Henderson (26):
  linux-user: Add infrastructure for a signal trampoline page
  linux-user/aarch64: Implement setup_sigtramp
  linux-user/arm: Drop v1 signal frames
  linux-user/arm: Drop "_v2" from symbols in signal.c
  linux-user/arm: Implement setup_sigtramp
  linux-user/alpha: Implement setup_sigtramp
  linux-user/cris: Implement setup_sigtramp
  linux-user/hexagon: Implement setup_sigtramp
  linux-user/hppa: Document non-use of setup_sigtramp
  linux-user/i386: Implement setup_sigtramp
  linux-user/x86_64: Raise SIGSEGV if SA_RESTORER not set
  linux-user/m68k: Implement setup_sigtramp
  linux-user/microblaze: Implement setup_sigtramp
  linux-user/mips: Tidy install_sigtramp
  linux-user/mips: Implement setup_sigtramp
  linux-user/nios2: Document non-use of setup_sigtramp
  linux-user/openrisc: Implement setup_sigtramp
  linux-user/ppc: Simplify encode_trampoline
  linux-user/ppc: Implement setup_sigtramp
  linux-user/riscv: Implement setup_sigtramp
  linux-user/s390x: Implement setup_sigtramp
  linux-user/sh4: Implement setup_sigtramp
  linux-user/sparc: Implement setup_sigtramp
  linux-user/xtensa: Implement setup_sigtramp
  linux-user: Remove default for TARGET_ARCH_HAS_SIGTRAMP_PAGE
  tests/tcg/multiarch: Re-enable signals test for most guests

 linux-user/aarch64/signal.c   |  34 +-
 linux-user/aarch64/target_signal.h|   2 +
 linux-user/alpha/signal.c |  34 +-
 linux-user/alpha/target_signal.h  |   1 +
 linux-user/arm/signal.c   | 472 --
 linux-user/arm/target_signal.h|   2 +
 linux-user/cris/signal.c  |  29 +-
 linux-user/cris/target_signal.h   |   2 +
 linux-user/elfload.c  |  14 +
 linux-user/hexagon/signal.c   |  19 +-
 linux-user/hexagon/target_signal.h|   2 +
 linux-user/hppa/target_signal.h   |  14 +
 linux-user/i386/signal.c  |  65 ++--
 linux-user/i386/target_signal.h   |   2 +
 linux-user/m68k/signal.c  |  47 ++-
 linux-user/m68k/target_signal.h   |   2 +
 linux-user/microblaze/signal.c|  24 +-
 linux-user/microblaze/target_signal.h |   2 +
 linux-user/mips/signal.c  |  39 ++-
 linux-user/mips/target_signal.h   |   1 +
 linux-user/mips64/target_signal.h |   2 +
 linux-user/nios2/target_signal.h  |   3 +
 linux-user/openrisc/signal.c  |  22 +-
 linux-user/openrisc/target_signal.h   |   2 +
 linux-user/ppc/signal.c   |  40 +--
 linux-user/ppc/target_signal.h|   2 +
 linux-user/riscv/signal.c |  22 +-
 linux-user/riscv/target_signal.h  |   2 +
 linux-user/s390x/signal.c |  24 +-
 linux-user/s390x/target_signal.h  |   2 +
 linux-user/sh4/signal.c   |  40 ++-
 linux-user/sh4/target_signal.h|   2 +
 linux-user/signal-common.h|   6 +
 linux-user/signal.c   |   3 +
 linux-user/sparc/signal.c |  40 ++-
 linux-user/sparc/target_signal.h  |   4 +
 linux-user/x86_64/target_signal.h |   3 +
 linux-user/xtensa/signal.c|  56 +--
 linux-user/xtensa/target_signal.h |   2 +
 tests/tcg/hppa/Makefile.target|   7 +
 tests/tcg/i386/Makefile.target|   3 -
 tests/tcg/multiarch/Makefile.target   |   8 -
 tests/tcg/sh4/Makefile.target |   7 +
 43 files changed, 559 insertions(+), 550 deletions(-)

-- 
2.31.1

[PULL 03/26] linux-user/arm: Drop v1 signal frames

[Laurent Vivier]

From: Richard Henderson 

Version 2 signal frames are used from 2.6.12 and since cbc14e6f286,
we have set UNAME_MINIMUM_RELEASE to 2.6.32.

Suggested-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-4-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/arm/signal.c | 220 +---
 1 file changed, 4 insertions(+), 216 deletions(-)

diff --git a/linux-user/arm/signal.c b/linux-user/arm/signal.c
index ed144f9455d2..d0940bab479c 100644
--- a/linux-user/arm/signal.c
+++ b/linux-user/arm/signal.c
@@ -46,14 +46,6 @@ struct target_sigcontext {
 abi_ulong fault_address;
 };
 
-struct target_ucontext_v1 {
-abi_ulong tuc_flags;
-abi_ulong tuc_link;
-target_stack_t tuc_stack;
-struct target_sigcontext tuc_mcontext;
-target_sigset_t  tuc_sigmask;   /* mask last for extensibility */
-};
-
 struct target_ucontext_v2 {
 abi_ulong tuc_flags;
 abi_ulong tuc_link;
@@ -98,28 +90,12 @@ struct target_iwmmxt_sigframe {
 #define TARGET_VFP_MAGIC 0x56465001
 #define TARGET_IWMMXT_MAGIC 0x12ef842a
 
-struct sigframe_v1
-{
-struct target_sigcontext sc;
-abi_ulong extramask[TARGET_NSIG_WORDS-1];
-abi_ulong retcode[4];
-};
-
 struct sigframe_v2
 {
 struct target_ucontext_v2 uc;
 abi_ulong retcode[4];
 };
 
-struct rt_sigframe_v1
-{
-abi_ulong pinfo;
-abi_ulong puc;
-struct target_siginfo info;
-struct target_ucontext_v1 uc;
-abi_ulong retcode[4];
-};
-
 struct rt_sigframe_v2
 {
 struct target_siginfo info;
@@ -363,37 +339,6 @@ static void setup_sigframe_v2(struct target_ucontext_v2 
*uc,
 }
 }
 
-/* compare linux/arch/arm/kernel/signal.c:setup_frame() */
-static void setup_frame_v1(int usig, struct target_sigaction *ka,
-   target_sigset_t *set, CPUARMState *regs)
-{
-struct sigframe_v1 *frame;
-abi_ulong frame_addr = get_sigframe(ka, regs, sizeof(*frame));
-int i;
-
-trace_user_setup_frame(regs, frame_addr);
-if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
-goto sigsegv;
-}
-
-setup_sigcontext(&frame->sc, regs, set->sig[0]);
-
-for(i = 1; i < TARGET_NSIG_WORDS; i++) {
-__put_user(set->sig[i], &frame->extramask[i - 1]);
-}
-
-if (setup_return(regs, ka, frame->retcode, frame_addr, usig,
- frame_addr + offsetof(struct sigframe_v1, retcode))) {
-goto sigsegv;
-}
-
-unlock_user_struct(frame, frame_addr, 1);
-return;
-sigsegv:
-unlock_user_struct(frame, frame_addr, 1);
-force_sigsegv(usig);
-}
-
 static void setup_frame_v2(int usig, struct target_sigaction *ka,
target_sigset_t *set, CPUARMState *regs)
 {
@@ -422,60 +367,7 @@ sigsegv:
 void setup_frame(int usig, struct target_sigaction *ka,
  target_sigset_t *set, CPUARMState *regs)
 {
-if (get_osversion() >= 0x020612) {
-setup_frame_v2(usig, ka, set, regs);
-} else {
-setup_frame_v1(usig, ka, set, regs);
-}
-}
-
-/* compare linux/arch/arm/kernel/signal.c:setup_rt_frame() */
-static void setup_rt_frame_v1(int usig, struct target_sigaction *ka,
-  target_siginfo_t *info,
-  target_sigset_t *set, CPUARMState *env)
-{
-struct rt_sigframe_v1 *frame;
-abi_ulong frame_addr = get_sigframe(ka, env, sizeof(*frame));
-struct target_sigaltstack stack;
-int i;
-abi_ulong info_addr, uc_addr;
-
-trace_user_setup_rt_frame(env, frame_addr);
-if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
-goto sigsegv;
-}
-
-info_addr = frame_addr + offsetof(struct rt_sigframe_v1, info);
-__put_user(info_addr, &frame->pinfo);
-uc_addr = frame_addr + offsetof(struct rt_sigframe_v1, uc);
-__put_user(uc_addr, &frame->puc);
-tswap_siginfo(&frame->info, info);
-
-/* Clear all the bits of the ucontext we don't use.  */
-memset(&frame->uc, 0, offsetof(struct target_ucontext_v1, tuc_mcontext));
-
-memset(&stack, 0, sizeof(stack));
-target_save_altstack(&stack, env);
-memcpy(&frame->uc.tuc_stack, &stack, sizeof(stack));
-
-setup_sigcontext(&frame->uc.tuc_mcontext, env, set->sig[0]);
-for(i = 0; i < TARGET_NSIG_WORDS; i++) {
-__put_user(set->sig[i], &frame->uc.tuc_sigmask.sig[i]);
-}
-
-if (setup_return(env, ka, frame->retcode, frame_addr, usig,
- frame_addr + offsetof(struct rt_sigframe_v1, retcode))) {
-goto sigsegv;
-}
-
-env->regs[1] = info_addr;
-env->regs[2] = uc_addr;
-
-unlock_user_struct(frame, frame_addr, 1);
-return;
-sigsegv:
-unlock_user_struct(frame, frame_addr, 1);
-force_sigsegv(usig);
+setup_frame_v2(usig, ka, set, regs);
 }
 
 static void setup_rt_frame_v2(int usig, struct target_sigaction *ka,
@@ -516,11 +408,7 @@ void setup_rt_frame(int usig, struct target_sigac

[PULL 16/26] linux-user/nios2: Document non-use of setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-17-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/nios2/target_signal.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux-user/nios2/target_signal.h b/linux-user/nios2/target_signal.h
index aebf749f1278..fe266c4c51dc 100644
--- a/linux-user/nios2/target_signal.h
+++ b/linux-user/nios2/target_signal.h
@@ -19,4 +19,7 @@ typedef struct target_sigaltstack {
 
 #include "../generic/signal.h"
 
+/* Nios2 uses a fixed address on the kuser page for sigreturn. */
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
+
 #endif /* NIOS2_TARGET_SIGNAL_H */
-- 
2.31.1

[PULL 11/26] linux-user/x86_64: Raise SIGSEGV if SA_RESTORER not set

[Laurent Vivier]

From: Richard Henderson 

This has been a fixme for some time.  The effect of
returning -EFAULT from the kernel code is to raise SIGSEGV.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-12-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/i386/signal.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/linux-user/i386/signal.c b/linux-user/i386/signal.c
index b38b5f108eaf..433efa3d693b 100644
--- a/linux-user/i386/signal.c
+++ b/linux-user/i386/signal.c
@@ -421,19 +421,18 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 
 /* Set up to return from userspace.  If provided, use a stub
already in userspace.  */
-#ifndef TARGET_X86_64
 if (ka->sa_flags & TARGET_SA_RESTORER) {
 __put_user(ka->sa_restorer, &frame->pretcode);
 } else {
+#ifdef TARGET_X86_64
+/* For x86_64, SA_RESTORER is required ABI.  */
+goto give_sigsegv;
+#else
 /* This is no longer used, but is retained for ABI compatibility. */
 install_rt_sigtramp(frame->retcode);
 __put_user(default_rt_sigreturn, &frame->pretcode);
-}
-#else
-/* XXX: Would be slightly better to return -EFAULT here if test fails
-   assert(ka->sa_flags & TARGET_SA_RESTORER); */
-__put_user(ka->sa_restorer, &frame->pretcode);
 #endif
+}
 
 /* Set up registers for signal handler */
 env->regs[R_ESP] = frame_addr;
-- 
2.31.1

[PULL 17/26] linux-user/openrisc: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the rt signal trampoline.

Reviewed-by: Stafford Horne 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-18-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/openrisc/signal.c| 22 ++
 linux-user/openrisc/target_signal.h |  2 ++
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/linux-user/openrisc/signal.c b/linux-user/openrisc/signal.c
index ca2532bf500f..be8b68784a20 100644
--- a/linux-user/openrisc/signal.c
+++ b/linux-user/openrisc/signal.c
@@ -38,7 +38,6 @@ typedef struct target_ucontext {
 typedef struct target_rt_sigframe {
 struct target_siginfo info;
 target_ucontext uc;
-uint32_t retcode[4];  /* trampoline code */
 } target_rt_sigframe;
 
 static void restore_sigcontext(CPUOpenRISCState *env, target_sigcontext *sc)
@@ -116,14 +115,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 __put_user(set->sig[i], &frame->uc.tuc_sigmask.sig[i]);
 }
 
-/* This is l.ori r11,r0,__NR_sigreturn; l.sys 1; l.nop; l.nop */
-__put_user(0xa960 | TARGET_NR_rt_sigreturn, frame->retcode + 0);
-__put_user(0x2001, frame->retcode + 1);
-__put_user(0x1500, frame->retcode + 2);
-__put_user(0x1500, frame->retcode + 3);
-
 /* Set up registers for signal handler */
-cpu_set_gpr(env, 9, frame_addr + offsetof(target_rt_sigframe, retcode));
+cpu_set_gpr(env, 9, default_rt_sigreturn);
 cpu_set_gpr(env, 3, sig);
 cpu_set_gpr(env, 4, frame_addr + offsetof(target_rt_sigframe, info));
 cpu_set_gpr(env, 5, frame_addr + offsetof(target_rt_sigframe, uc));
@@ -169,3 +162,16 @@ long do_rt_sigreturn(CPUOpenRISCState *env)
 force_sig(TARGET_SIGSEGV);
 return 0;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 8, 0);
+assert(tramp != NULL);
+
+/* This is l.ori r11,r0,__NR_sigreturn; l.sys 1 */
+__put_user(0xa960 | TARGET_NR_rt_sigreturn, tramp + 0);
+__put_user(0x2001, tramp + 1);
+
+default_rt_sigreturn = sigtramp_page;
+unlock_user(tramp, sigtramp_page, 8);
+}
diff --git a/linux-user/openrisc/target_signal.h 
b/linux-user/openrisc/target_signal.h
index 8283eaf54419..077ec3d5e8d7 100644
--- a/linux-user/openrisc/target_signal.h
+++ b/linux-user/openrisc/target_signal.h
@@ -26,4 +26,6 @@ typedef struct target_sigaltstack {
 
 #include "../generic/signal.h"
 
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* OPENRISC_TARGET_SIGNAL_H */
-- 
2.31.1

[PULL 14/26] linux-user/mips: Tidy install_sigtramp

[Laurent Vivier]

From: Richard Henderson 

The return value is constant 0, and unused as well -- change to void.
Drop inline marker.  Change tramp type to uint32_t* for clarity.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-15-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/mips/signal.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/linux-user/mips/signal.c b/linux-user/mips/signal.c
index d174b3453cc0..64072779b94a 100644
--- a/linux-user/mips/signal.c
+++ b/linux-user/mips/signal.c
@@ -87,10 +87,8 @@ struct target_rt_sigframe {
 };
 
 /* Install trampoline to jump back from signal handler */
-static inline int install_sigtramp(unsigned int *tramp,   unsigned int syscall)
+static void install_sigtramp(uint32_t *tramp, unsigned int syscall)
 {
-int err = 0;
-
 /*
  * Set up the return code ...
  *
@@ -100,7 +98,6 @@ static inline int install_sigtramp(unsigned int *tramp,   
unsigned int syscall)
 
 __put_user(0x2402 + syscall, tramp + 0);
 __put_user(0x000c  , tramp + 1);
-return err;
 }
 
 static inline void setup_sigcontext(CPUMIPSState *regs,
-- 
2.31.1

[PULL 26/26] tests/tcg/multiarch: Re-enable signals test for most guests

[Laurent Vivier]

From: Richard Henderson 

With signal trampolines safely off the stack for all
guests besides hppa, we can re-enable this test.

It does show up a problem with sh4 (unrelated?),
so leave that test disabled for now.

Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-27-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 tests/tcg/hppa/Makefile.target  | 7 +++
 tests/tcg/i386/Makefile.target  | 3 ---
 tests/tcg/multiarch/Makefile.target | 8 
 tests/tcg/sh4/Makefile.target   | 7 +++
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/tests/tcg/hppa/Makefile.target b/tests/tcg/hppa/Makefile.target
index 473864d1d468..d0d5e0e25761 100644
--- a/tests/tcg/hppa/Makefile.target
+++ b/tests/tcg/hppa/Makefile.target
@@ -5,3 +5,10 @@
 # On parisc Linux supports 4K/16K/64K (but currently only 4k works)
 EXTRA_RUNS+=run-test-mmap-4096 # run-test-mmap-16384 run-test-mmap-65536
 
+# This triggers failures for hppa-linux about 1% of the time
+# HPPA is the odd target that can't use the sigtramp page;
+# it requires the full vdso with dwarf2 unwind info.
+run-signals: signals
+   $(call skip-test, $<, "BROKEN awaiting vdso support")
+run-plugin-signals-with-%:
+   $(call skip-test, $<, "BROKEN awaiting vdso support")
diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target
index a053ca3f1532..38c10379af0f 100644
--- a/tests/tcg/i386/Makefile.target
+++ b/tests/tcg/i386/Makefile.target
@@ -65,9 +65,6 @@ run-plugin-%-with-libinsn.so:
   -d plugin -D $*-with-libinsn.so.pout $*, \
"$* (inline) on $(TARGET_NAME)")
 
-run-plugin-signals-with-libinsn.so:
-   $(call skip-test, $<, "BROKEN awaiting sigframe clean-ups and vdso 
support")
-
 # Update TESTS
 I386_TESTS:=$(filter-out $(SKIP_I386_TESTS), $(ALL_X86_TESTS))
 TESTS=$(MULTIARCH_TESTS) $(I386_TESTS)
diff --git a/tests/tcg/multiarch/Makefile.target 
b/tests/tcg/multiarch/Makefile.target
index 85a6fb7a2ea0..3f283eabe6da 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -32,14 +32,6 @@ threadcount: LDFLAGS+=-lpthread
 
 signals: LDFLAGS+=-lrt -lpthread
 
-# This triggers failures on s390x hosts about 4% of the time
-# This triggers failures for hppa-linux about 1% of the time
-run-signals: signals
-   $(call skip-test, $<, "BROKEN awaiting sigframe clean-ups and vdso 
support")
-
-run-plugin-signals-with-%:
-   $(call skip-test, $<, "BROKEN awaiting sigframe clean-ups and vdso 
support")
-
 # We define the runner for test-mmap after the individual
 # architectures have defined their supported pages sizes. If no
 # additional page sizes are defined we only run the default test.
diff --git a/tests/tcg/sh4/Makefile.target b/tests/tcg/sh4/Makefile.target
index 9d18d44612e1..47c39a44b690 100644
--- a/tests/tcg/sh4/Makefile.target
+++ b/tests/tcg/sh4/Makefile.target
@@ -5,3 +5,10 @@
 
 # On sh Linux supports 4k, 8k, 16k and 64k pages (but only 4k currently works)
 EXTRA_RUNS+=run-test-mmap-4096 # run-test-mmap-8192 run-test-mmap-16384 
run-test-mmap-65536
+
+# This triggers failures for sh4-linux about 10% of the time.
+# Random SIGSEGV at unpredictable guest address, cause unknown.
+run-signals: signals
+   $(call skip-test, $<, "BROKEN")
+run-plugin-signals-with-%:
+   $(call skip-test, $<, "BROKEN")
-- 
2.31.1

[PULL 19/26] linux-user/ppc: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the two signal trampolines.

Cc: qemu-...@nongnu.org
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-20-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/ppc/signal.c| 34 ++
 linux-user/ppc/target_signal.h |  2 ++
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index 77f37b9f0131..c37744c8fc55 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -203,9 +203,6 @@ struct target_func_ptr {
 
 #endif
 
-/* We use the mc_pad field for the signal return trampoline.  */
-#define tramp mc_pad
-
 /* See arch/powerpc/kernel/signal.c.  */
 static target_ulong get_sigframe(struct target_sigaction *ka,
  CPUPPCState *env,
@@ -436,12 +433,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
 /* Save user regs.  */
 save_user_regs(env, &frame->mctx);
 
-/* Construct the trampoline code on the stack. */
-encode_trampoline(TARGET_NR_sigreturn, (uint32_t *)&frame->mctx.tramp);
-
-/* The kernel checks for the presence of a VDSO here.  We don't
-   emulate a vdso, so use a sigreturn system call.  */
-env->lr = (target_ulong) h2g(frame->mctx.tramp);
+env->lr = default_sigreturn;
 
 /* Turn off all fp exceptions.  */
 env->fpscr = 0;
@@ -477,7 +469,6 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 target_sigset_t *set, CPUPPCState *env)
 {
 struct target_rt_sigframe *rt_sf;
-uint32_t *trampptr = 0;
 struct target_mcontext *mctx = 0;
 target_ulong rt_sf_addr, newsp = 0;
 int i, err = 0;
@@ -507,22 +498,17 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 
 #if defined(TARGET_PPC64)
 mctx = &rt_sf->uc.tuc_sigcontext.mcontext;
-trampptr = &rt_sf->trampoline[0];
 
 sc = &rt_sf->uc.tuc_sigcontext;
 __put_user(h2g(mctx), &sc->regs);
 __put_user(sig, &sc->signal);
 #else
 mctx = &rt_sf->uc.tuc_mcontext;
-trampptr = (uint32_t *)&rt_sf->uc.tuc_mcontext.tramp;
 #endif
 
 save_user_regs(env, mctx);
-encode_trampoline(TARGET_NR_rt_sigreturn, trampptr);
 
-/* The kernel checks for the presence of a VDSO here.  We don't
-   emulate a vdso, so use a sigreturn system call.  */
-env->lr = (target_ulong) h2g(trampptr);
+env->lr = default_rt_sigreturn;
 
 /* Turn off all fp exceptions.  */
 env->fpscr = 0;
@@ -720,3 +706,19 @@ abi_long do_swapcontext(CPUArchState *env, abi_ulong 
uold_ctx,
 
 return 0;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 2 * 8, 0);
+assert(tramp != NULL);
+
+#ifdef TARGET_ARCH_HAS_SETUP_FRAME
+default_sigreturn = sigtramp_page;
+encode_trampoline(TARGET_NR_sigreturn, tramp + 0);
+#endif
+
+default_rt_sigreturn = sigtramp_page + 8;
+encode_trampoline(TARGET_NR_rt_sigreturn, tramp + 2);
+
+unlock_user(tramp, sigtramp_page, 2 * 8);
+}
diff --git a/linux-user/ppc/target_signal.h b/linux-user/ppc/target_signal.h
index 72fcdd9bfa20..82184ab8f2ef 100644
--- a/linux-user/ppc/target_signal.h
+++ b/linux-user/ppc/target_signal.h
@@ -24,4 +24,6 @@ typedef struct target_sigaltstack {
 #if !defined(TARGET_PPC64)
 #define TARGET_ARCH_HAS_SETUP_FRAME
 #endif
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* PPC_TARGET_SIGNAL_H */
-- 
2.31.1

[PULL 04/26] linux-user/arm: Drop "_v2" from symbols in signal.c

[Laurent Vivier]

From: Richard Henderson 

Since we no longer support "v1", there's no need to distinguish "v2".

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-5-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/arm/signal.c | 115 
 1 file changed, 45 insertions(+), 70 deletions(-)

diff --git a/linux-user/arm/signal.c b/linux-user/arm/signal.c
index d0940bab479c..ed7d1d80bb9c 100644
--- a/linux-user/arm/signal.c
+++ b/linux-user/arm/signal.c
@@ -46,7 +46,7 @@ struct target_sigcontext {
 abi_ulong fault_address;
 };
 
-struct target_ucontext_v2 {
+struct target_ucontext {
 abi_ulong tuc_flags;
 abi_ulong tuc_link;
 target_stack_t tuc_stack;
@@ -90,16 +90,16 @@ struct target_iwmmxt_sigframe {
 #define TARGET_VFP_MAGIC 0x56465001
 #define TARGET_IWMMXT_MAGIC 0x12ef842a
 
-struct sigframe_v2
+struct sigframe
 {
-struct target_ucontext_v2 uc;
+struct target_ucontext uc;
 abi_ulong retcode[4];
 };
 
-struct rt_sigframe_v2
+struct rt_sigframe
 {
 struct target_siginfo info;
-struct target_ucontext_v2 uc;
+struct target_ucontext uc;
 abi_ulong retcode[4];
 };
 
@@ -270,7 +270,7 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
 return 0;
 }
 
-static abi_ulong *setup_sigframe_v2_vfp(abi_ulong *regspace, CPUARMState *env)
+static abi_ulong *setup_sigframe_vfp(abi_ulong *regspace, CPUARMState *env)
 {
 int i;
 struct target_vfp_sigframe *vfpframe;
@@ -287,8 +287,7 @@ static abi_ulong *setup_sigframe_v2_vfp(abi_ulong 
*regspace, CPUARMState *env)
 return (abi_ulong*)(vfpframe+1);
 }
 
-static abi_ulong *setup_sigframe_v2_iwmmxt(abi_ulong *regspace,
-   CPUARMState *env)
+static abi_ulong *setup_sigframe_iwmmxt(abi_ulong *regspace, CPUARMState *env)
 {
 int i;
 struct target_iwmmxt_sigframe *iwmmxtframe;
@@ -307,15 +306,15 @@ static abi_ulong *setup_sigframe_v2_iwmmxt(abi_ulong 
*regspace,
 return (abi_ulong*)(iwmmxtframe+1);
 }
 
-static void setup_sigframe_v2(struct target_ucontext_v2 *uc,
-  target_sigset_t *set, CPUARMState *env)
+static void setup_sigframe(struct target_ucontext *uc,
+   target_sigset_t *set, CPUARMState *env)
 {
 struct target_sigaltstack stack;
 int i;
 abi_ulong *regspace;
 
 /* Clear all the bits of the ucontext we don't use.  */
-memset(uc, 0, offsetof(struct target_ucontext_v2, tuc_mcontext));
+memset(uc, 0, offsetof(struct target_ucontext, tuc_mcontext));
 
 memset(&stack, 0, sizeof(stack));
 target_save_altstack(&stack, env);
@@ -325,10 +324,10 @@ static void setup_sigframe_v2(struct target_ucontext_v2 
*uc,
 /* Save coprocessor signal frame.  */
 regspace = uc->tuc_regspace;
 if (cpu_isar_feature(aa32_vfp_simd, env_archcpu(env))) {
-regspace = setup_sigframe_v2_vfp(regspace, env);
+regspace = setup_sigframe_vfp(regspace, env);
 }
 if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
-regspace = setup_sigframe_v2_iwmmxt(regspace, env);
+regspace = setup_sigframe_iwmmxt(regspace, env);
 }
 
 /* Write terminating magic word */
@@ -339,10 +338,10 @@ static void setup_sigframe_v2(struct target_ucontext_v2 
*uc,
 }
 }
 
-static void setup_frame_v2(int usig, struct target_sigaction *ka,
-   target_sigset_t *set, CPUARMState *regs)
+void setup_frame(int usig, struct target_sigaction *ka,
+ target_sigset_t *set, CPUARMState *regs)
 {
-struct sigframe_v2 *frame;
+struct sigframe *frame;
 abi_ulong frame_addr = get_sigframe(ka, regs, sizeof(*frame));
 
 trace_user_setup_frame(regs, frame_addr);
@@ -350,10 +349,10 @@ static void setup_frame_v2(int usig, struct 
target_sigaction *ka,
 goto sigsegv;
 }
 
-setup_sigframe_v2(&frame->uc, set, regs);
+setup_sigframe(&frame->uc, set, regs);
 
 if (setup_return(regs, ka, frame->retcode, frame_addr, usig,
- frame_addr + offsetof(struct sigframe_v2, retcode))) {
+ frame_addr + offsetof(struct sigframe, retcode))) {
 goto sigsegv;
 }
 
@@ -364,17 +363,11 @@ sigsegv:
 force_sigsegv(usig);
 }
 
-void setup_frame(int usig, struct target_sigaction *ka,
- target_sigset_t *set, CPUARMState *regs)
-{
-setup_frame_v2(usig, ka, set, regs);
-}
-
-static void setup_rt_frame_v2(int usig, struct target_sigaction *ka,
-  target_siginfo_t *info,
-  target_sigset_t *set, CPUARMState *env)
+void setup_rt_frame(int usig, struct target_sigaction *ka,
+target_siginfo_t *info,
+target_sigset_t *set, CPUARMState *env)
 {
-struct rt_sigframe_v2 *frame;
+struct rt_sigframe *frame;
 abi_ulong frame_addr = get_sigframe(ka, env, sizeof(*frame));
 abi_ulong

[PULL 06/26] linux-user/alpha: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the two signal trampolines.
Use them when the guest does not use ka_restorer.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-7-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/alpha/signal.c| 34 +++-
 linux-user/alpha/target_signal.h |  1 +
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/linux-user/alpha/signal.c b/linux-user/alpha/signal.c
index 3a820f616b3f..bbe3dd175a7c 100644
--- a/linux-user/alpha/signal.c
+++ b/linux-user/alpha/signal.c
@@ -55,13 +55,11 @@ struct target_ucontext {
 
 struct target_sigframe {
 struct target_sigcontext sc;
-unsigned int retcode[3];
 };
 
 struct target_rt_sigframe {
 target_siginfo_t info;
 struct target_ucontext uc;
-unsigned int retcode[3];
 };
 
 #define INSN_MOV_R30_R160x47fe0410
@@ -142,12 +140,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
 if (ka->ka_restorer) {
 r26 = ka->ka_restorer;
 } else {
-__put_user(INSN_MOV_R30_R16, &frame->retcode[0]);
-__put_user(INSN_LDI_R0 + TARGET_NR_sigreturn,
-   &frame->retcode[1]);
-__put_user(INSN_CALLSYS, &frame->retcode[2]);
-/* imb() */
-r26 = frame_addr + offsetof(struct target_sigframe, retcode);
+r26 = default_sigreturn;
 }
 
 unlock_user_struct(frame, frame_addr, 1);
@@ -196,12 +189,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 if (ka->ka_restorer) {
 r26 = ka->ka_restorer;
 } else {
-__put_user(INSN_MOV_R30_R16, &frame->retcode[0]);
-__put_user(INSN_LDI_R0 + TARGET_NR_rt_sigreturn,
-   &frame->retcode[1]);
-__put_user(INSN_CALLSYS, &frame->retcode[2]);
-/* imb(); */
-r26 = frame_addr + offsetof(struct target_rt_sigframe, retcode);
+r26 = default_rt_sigreturn;
 }
 
 if (err) {
@@ -269,3 +257,21 @@ badframe:
 force_sig(TARGET_SIGSEGV);
 return -TARGET_QEMU_ESIGRETURN;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 6 * 4, 0);
+assert(tramp != NULL);
+
+default_sigreturn = sigtramp_page;
+__put_user(INSN_MOV_R30_R16, &tramp[0]);
+__put_user(INSN_LDI_R0 + TARGET_NR_sigreturn, &tramp[1]);
+__put_user(INSN_CALLSYS, &tramp[2]);
+
+default_rt_sigreturn = sigtramp_page + 3 * 4;
+__put_user(INSN_MOV_R30_R16, &tramp[3]);
+__put_user(INSN_LDI_R0 + TARGET_NR_rt_sigreturn, &tramp[4]);
+__put_user(INSN_CALLSYS, &tramp[5]);
+
+unlock_user(tramp, sigtramp_page, 6 * 4);
+}
diff --git a/linux-user/alpha/target_signal.h b/linux-user/alpha/target_signal.h
index 250642913e2a..0b6a39de6576 100644
--- a/linux-user/alpha/target_signal.h
+++ b/linux-user/alpha/target_signal.h
@@ -93,6 +93,7 @@ typedef struct target_sigaltstack {
 
 #define TARGET_ARCH_HAS_SETUP_FRAME
 #define TARGET_ARCH_HAS_KA_RESTORER
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
 
 /* bit-flags */
 #define TARGET_SS_AUTODISARM (1U << 31) /* disable sas during sighandling */
-- 
2.31.1

[PULL 18/26] linux-user/ppc: Simplify encode_trampoline

[Laurent Vivier]

From: Richard Henderson 

The sigret parameter is never 0, and even if it was the encoding
of the LI instruction would still work.

Reported-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-19-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/ppc/signal.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index e4d0dfa3bf75..77f37b9f0131 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -309,10 +309,8 @@ static void save_user_regs(CPUPPCState *env, struct 
target_mcontext *frame)
 static void encode_trampoline(int sigret, uint32_t *tramp)
 {
 /* Set up the sigreturn trampoline: li r0,sigret; sc.  */
-if (sigret) {
-__put_user(0x3800 | sigret, &tramp[0]);
-__put_user(0x4402, &tramp[1]);
-}
+__put_user(0x3800 | sigret, &tramp[0]);
+__put_user(0x4402, &tramp[1]);
 }
 
 static void restore_user_regs(CPUPPCState *env,
-- 
2.31.1

[PULL 07/26] linux-user/cris: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Split out setup_sigreturn so that we can continue to
initialize the words on the stack, as documented.
However, use the off-stack trampoline.

Cc: Edgar E. Iglesias 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-8-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/cris/signal.c| 29 +
 linux-user/cris/target_signal.h |  2 ++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/linux-user/cris/signal.c b/linux-user/cris/signal.c
index 2c39bdf7277f..7f6aca934e10 100644
--- a/linux-user/cris/signal.c
+++ b/linux-user/cris/signal.c
@@ -97,6 +97,14 @@ static abi_ulong get_sigframe(CPUCRISState *env, int 
framesize)
 return sp - framesize;
 }
 
+static void setup_sigreturn(uint16_t *retcode)
+{
+/* This is movu.w __NR_sigreturn, r9; break 13; */
+__put_user(0x9c5f, retcode + 0);
+__put_user(TARGET_NR_sigreturn, retcode + 1);
+__put_user(0xe93d, retcode + 2);
+}
+
 void setup_frame(int sig, struct target_sigaction *ka,
  target_sigset_t *set, CPUCRISState *env)
 {
@@ -112,14 +120,8 @@ void setup_frame(int sig, struct target_sigaction *ka,
 /*
  * The CRIS signal return trampoline. A real linux/CRIS kernel doesn't
  * use this trampoline anymore but it sets it up for GDB.
- * In QEMU, using the trampoline simplifies things a bit so we use it.
- *
- * This is movu.w __NR_sigreturn, r9; break 13;
  */
-__put_user(0x9c5f, frame->retcode+0);
-__put_user(TARGET_NR_sigreturn,
-   frame->retcode + 1);
-__put_user(0xe93d, frame->retcode + 2);
+setup_sigreturn(frame->retcode);
 
 /* Save the mask.  */
 __put_user(set->sig[0], &frame->sc.oldmask);
@@ -135,7 +137,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
 env->regs[10] = sig;
 env->pc = (unsigned long) ka->_sa_handler;
 /* Link SRP so the guest returns through the trampoline.  */
-env->pregs[PR_SRP] = frame_addr + offsetof(typeof(*frame), retcode);
+env->pregs[PR_SRP] = default_sigreturn;
 
 unlock_user_struct(frame, frame_addr, 1);
 return;
@@ -187,3 +189,14 @@ long do_rt_sigreturn(CPUCRISState *env)
 qemu_log_mask(LOG_UNIMP, "do_rt_sigreturn: not implemented\n");
 return -TARGET_ENOSYS;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint16_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 6, 0);
+assert(tramp != NULL);
+
+default_sigreturn = sigtramp_page;
+setup_sigreturn(tramp);
+
+unlock_user(tramp, sigtramp_page, 6);
+}
diff --git a/linux-user/cris/target_signal.h b/linux-user/cris/target_signal.h
index 495a14289681..83a515550745 100644
--- a/linux-user/cris/target_signal.h
+++ b/linux-user/cris/target_signal.h
@@ -22,4 +22,6 @@ typedef struct target_sigaltstack {
 #include "../generic/signal.h"
 
 #define TARGET_ARCH_HAS_SETUP_FRAME
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* CRIS_TARGET_SIGNAL_H */
-- 
2.31.1

Re: [RFC PATCH] hw/arm: fix the position of vcram for raspi

[Philippe Mathieu-Daudé]

On 10/1/21 19:42, Alex Bennée wrote:
> The previous calculation fell over when I tried to create a 8gb Pi 4
> because the values where only 32 bit. However the quirk of the Pi

Typo 'where' -> 'were'?

> hardware is the vcram can only appear in the first 1gb of address
> space. This also limits where the initial kernel and DTB can be
> loaded (notice the DTS for the 8gb Pi4 still only uses 32 bit sizes).
> Fix this cleaning up setup_boot to directly use vcram_base and
> documenting what is going on.
> 
> NB: the aliases are confusing.
> 
> Signed-off-by: Alex Bennée 
> Cc: Michael Bishop 
> Cc: Philippe Mathieu-Daudé 
> ---
>  hw/arm/bcm2835_peripherals.c | 14 +++---
>  hw/arm/bcm2836.c |  2 ++
>  hw/arm/raspi.c   | 19 ---
>  3 files changed, 25 insertions(+), 10 deletions(-)

LGTM:
Reviewed-by: Philippe Mathieu-Daudé 

[PULL 15/26] linux-user/mips: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the two signal trampolines.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-16-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/mips/signal.c  | 34 ++-
 linux-user/mips/target_signal.h   |  1 +
 linux-user/mips64/target_signal.h |  2 ++
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/linux-user/mips/signal.c b/linux-user/mips/signal.c
index 64072779b94a..8f79e405ecb7 100644
--- a/linux-user/mips/signal.c
+++ b/linux-user/mips/signal.c
@@ -209,8 +209,6 @@ void setup_frame(int sig, struct target_sigaction * ka,
 goto give_sigsegv;
 }
 
-install_sigtramp(frame->sf_code, TARGET_NR_sigreturn);
-
 setup_sigcontext(regs, &frame->sf_sc);
 
 for(i = 0; i < TARGET_NSIG_WORDS; i++) {
@@ -231,7 +229,7 @@ void setup_frame(int sig, struct target_sigaction * ka,
 regs->active_tc.gpr[ 5] = 0;
 regs->active_tc.gpr[ 6] = frame_addr + offsetof(struct sigframe, sf_sc);
 regs->active_tc.gpr[29] = frame_addr;
-regs->active_tc.gpr[31] = frame_addr + offsetof(struct sigframe, sf_code);
+regs->active_tc.gpr[31] = default_sigreturn;
 /* The original kernel code sets CP0_EPC to the handler
 * since it returns to userland using eret
 * we cannot do this here, and we must set PC directly */
@@ -305,8 +303,6 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 goto give_sigsegv;
 }
 
-install_sigtramp(frame->rs_code, TARGET_NR_rt_sigreturn);
-
 tswap_siginfo(&frame->rs_info, info);
 
 __put_user(0, &frame->rs_uc.tuc_flags);
@@ -335,11 +331,13 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 env->active_tc.gpr[ 6] = frame_addr
  + offsetof(struct target_rt_sigframe, rs_uc);
 env->active_tc.gpr[29] = frame_addr;
-env->active_tc.gpr[31] = frame_addr
- + offsetof(struct target_rt_sigframe, rs_code);
-/* The original kernel code sets CP0_EPC to the handler
-* since it returns to userland using eret
-* we cannot do this here, and we must set PC directly */
+env->active_tc.gpr[31] = default_rt_sigreturn;
+
+/*
+ * The original kernel code sets CP0_EPC to the handler
+ * since it returns to userland using eret
+ * we cannot do this here, and we must set PC directly
+ */
 env->active_tc.PC = env->active_tc.gpr[25] = ka->_sa_handler;
 mips_set_hflags_isa_mode_from_pc(env);
 unlock_user_struct(frame, frame_addr, 1);
@@ -379,3 +377,19 @@ badframe:
 force_sig(TARGET_SIGSEGV);
 return -TARGET_QEMU_ESIGRETURN;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 2 * 8, 0);
+assert(tramp != NULL);
+
+#ifdef TARGET_ARCH_HAS_SETUP_FRAME
+default_sigreturn = sigtramp_page;
+install_sigtramp(tramp, TARGET_NR_sigreturn);
+#endif
+
+default_rt_sigreturn = sigtramp_page + 8;
+install_sigtramp(tramp + 2, TARGET_NR_rt_sigreturn);
+
+unlock_user(tramp, sigtramp_page, 2 * 8);
+}
diff --git a/linux-user/mips/target_signal.h b/linux-user/mips/target_signal.h
index d521765f6b2c..780a4ddf29de 100644
--- a/linux-user/mips/target_signal.h
+++ b/linux-user/mips/target_signal.h
@@ -73,6 +73,7 @@ typedef struct target_sigaltstack {
 /* compare linux/arch/mips/kernel/signal.c:setup_frame() */
 #define TARGET_ARCH_HAS_SETUP_FRAME
 #endif
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
 
 /* bit-flags */
 #define TARGET_SS_AUTODISARM (1U << 31) /* disable sas during sighandling */
diff --git a/linux-user/mips64/target_signal.h 
b/linux-user/mips64/target_signal.h
index d857c55e4c6c..275e9b7f9a2a 100644
--- a/linux-user/mips64/target_signal.h
+++ b/linux-user/mips64/target_signal.h
@@ -76,4 +76,6 @@ typedef struct target_sigaltstack {
 /* compare linux/arch/mips/kernel/signal.c:setup_frame() */
 #define TARGET_ARCH_HAS_SETUP_FRAME
 #endif
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* MIPS64_TARGET_SIGNAL_H */
-- 
2.31.1

[PULL 10/26] linux-user/i386: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the two signal trampolines.
Use them when the guest does not use SA_RESTORER.
Note that x86_64 does not use this code.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-11-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/i386/signal.c  | 56 +--
 linux-user/i386/target_signal.h   |  2 ++
 linux-user/x86_64/target_signal.h |  3 ++
 3 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/linux-user/i386/signal.c b/linux-user/i386/signal.c
index 3b4b55fc0a24..b38b5f108eaf 100644
--- a/linux-user/i386/signal.c
+++ b/linux-user/i386/signal.c
@@ -310,6 +310,22 @@ get_sigframe(struct target_sigaction *ka, CPUX86State 
*env, size_t frame_size)
 }
 
 #ifndef TARGET_X86_64
+static void install_sigtramp(void *tramp)
+{
+/* This is popl %eax ; movl $syscall,%eax ; int $0x80 */
+__put_user(0xb858, (uint16_t *)(tramp + 0));
+__put_user(TARGET_NR_sigreturn, (int32_t *)(tramp + 2));
+__put_user(0x80cd, (uint16_t *)(tramp + 6));
+}
+
+static void install_rt_sigtramp(void *tramp)
+{
+/* This is movl $syscall,%eax ; int $0x80 */
+__put_user(0xb8, (uint8_t *)(tramp + 0));
+__put_user(TARGET_NR_rt_sigreturn, (int32_t *)(tramp + 1));
+__put_user(0x80cd, (uint16_t *)(tramp + 5));
+}
+
 /* compare linux/arch/i386/kernel/signal.c:setup_frame() */
 void setup_frame(int sig, struct target_sigaction *ka,
  target_sigset_t *set, CPUX86State *env)
@@ -338,16 +354,9 @@ void setup_frame(int sig, struct target_sigaction *ka,
 if (ka->sa_flags & TARGET_SA_RESTORER) {
 __put_user(ka->sa_restorer, &frame->pretcode);
 } else {
-uint16_t val16;
-abi_ulong retcode_addr;
-retcode_addr = frame_addr + offsetof(struct sigframe, retcode);
-__put_user(retcode_addr, &frame->pretcode);
-/* This is popl %eax ; movl $,%eax ; int $0x80 */
-val16 = 0xb858;
-__put_user(val16, (uint16_t *)(frame->retcode+0));
-__put_user(TARGET_NR_sigreturn, (int *)(frame->retcode+2));
-val16 = 0x80cd;
-__put_user(val16, (uint16_t *)(frame->retcode+6));
+/* This is no longer used, but is retained for ABI compatibility. */
+install_sigtramp(frame->retcode);
+__put_user(default_sigreturn, &frame->pretcode);
 }
 
 /* Set up registers for signal handler */
@@ -416,14 +425,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 if (ka->sa_flags & TARGET_SA_RESTORER) {
 __put_user(ka->sa_restorer, &frame->pretcode);
 } else {
-uint16_t val16;
-addr = frame_addr + offsetof(struct rt_sigframe, retcode);
-__put_user(addr, &frame->pretcode);
-/* This is movl $,%eax ; int $0x80 */
-__put_user(0xb8, (char *)(frame->retcode+0));
-__put_user(TARGET_NR_rt_sigreturn, (int *)(frame->retcode+1));
-val16 = 0x80cd;
-__put_user(val16, (uint16_t *)(frame->retcode+5));
+/* This is no longer used, but is retained for ABI compatibility. */
+install_rt_sigtramp(frame->retcode);
+__put_user(default_rt_sigreturn, &frame->pretcode);
 }
 #else
 /* XXX: Would be slightly better to return -EFAULT here if test fails
@@ -592,3 +596,19 @@ badframe:
 force_sig(TARGET_SIGSEGV);
 return -TARGET_QEMU_ESIGRETURN;
 }
+
+#ifndef TARGET_X86_64
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint16_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 2 * 8, 0);
+assert(tramp != NULL);
+
+default_sigreturn = sigtramp_page;
+install_sigtramp(tramp);
+
+default_rt_sigreturn = sigtramp_page + 8;
+install_rt_sigtramp(tramp + 8);
+
+unlock_user(tramp, sigtramp_page, 2 * 8);
+}
+#endif
diff --git a/linux-user/i386/target_signal.h b/linux-user/i386/target_signal.h
index 50361af8746e..64d09f2e75bd 100644
--- a/linux-user/i386/target_signal.h
+++ b/linux-user/i386/target_signal.h
@@ -22,4 +22,6 @@ typedef struct target_sigaltstack {
 #include "../generic/signal.h"
 
 #define TARGET_ARCH_HAS_SETUP_FRAME
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* I386_TARGET_SIGNAL_H */
diff --git a/linux-user/x86_64/target_signal.h 
b/linux-user/x86_64/target_signal.h
index 4ea74f20dd42..4673c5a88691 100644
--- a/linux-user/x86_64/target_signal.h
+++ b/linux-user/x86_64/target_signal.h
@@ -21,4 +21,7 @@ typedef struct target_sigaltstack {
 
 #include "../generic/signal.h"
 
+/* For x86_64, use of SA_RESTORER is mandatory. */
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
+
 #endif /* X86_64_TARGET_SIGNAL_H */
-- 
2.31.1

Re: [PATCH v3 01/22] qapi/misc-target: Wrap long 'SEV Attestation Report' long lines

[Paolo Bonzini]

On 02/10/21 14:52, Philippe Mathieu-Daudé wrote:

Wrap long lines before 70 characters for legibility.

Suggested-by: Markus Armbruster 
Reviewed-by: Markus Armbruster 
Signed-off-by: Philippe Mathieu-Daudé 
---
  qapi/misc-target.json | 17 ++---
  1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 594fbd1577f..ae5577e0390 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -300,8 +300,8 @@
  ##
  # @SevAttestationReport:
  #
-# The struct describes attestation report for a Secure Encrypted Virtualization
-# feature.
+# The struct describes attestation report for a Secure Encrypted
+# Virtualization feature.
  #
  # @data:  guest attestation report (base64 encoded)
  #
@@ -315,10 +315,11 @@
  ##
  # @query-sev-attestation-report:
  #
-# This command is used to get the SEV attestation report, and is supported on 
AMD
-# X86 platforms only.
+# This command is used to get the SEV attestation report, and is
+# supported on AMD X86 platforms only.
  #
-# @mnonce: a random 16 bytes value encoded in base64 (it will be included in 
report)
+# @mnonce: a random 16 bytes value encoded in base64 (it will be
+#  included in report)
  #
  # Returns: SevAttestationReport objects.
  #
@@ -326,11 +327,13 @@
  #
  # Example:
  #
-# -> { "execute" : "query-sev-attestation-report", "arguments": { "mnonce": 
"aaa" } }
+# -> { "execute" : "query-sev-attestation-report",
+#  "arguments": { "mnonce": "aaa" } }
  # <- { "return" : { "data": "bbbd"} }
  #
  ##
-{ 'command': 'query-sev-attestation-report', 'data': { 'mnonce': 'str' },
+{ 'command': 'query-sev-attestation-report',
+  'data': { 'mnonce': 'str' },
'returns': 'SevAttestationReport',
'if': 'TARGET_I386' }
  



Reviewed-by: Paolo Bonzini 

[PULL 09/26] linux-user/hppa: Document non-use of setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

We cannot use a raw sigtramp page for hppa,
but must wait for full vdso support.

Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-10-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/hppa/target_signal.h | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/linux-user/hppa/target_signal.h b/linux-user/hppa/target_signal.h
index 7f525362e91a..d558119ee7bd 100644
--- a/linux-user/hppa/target_signal.h
+++ b/linux-user/hppa/target_signal.h
@@ -71,4 +71,18 @@ typedef struct target_sigaltstack {
 /* mask for all SS_xxx flags */
 #define TARGET_SS_FLAG_BITS  TARGET_SS_AUTODISARM
 
+/*
+ * We cannot use a bare sigtramp page for hppa-linux.
+ *
+ * Unlike other guests where we use the instructions at PC to validate
+ * an offset from SP, the hppa libgcc signal frame fallback unwinding uses
+ * the PC address itself to find the frame.  This is due to the fact that
+ * the hppa grows the stack upward, and the frame is of unknown size.
+ *
+ * TODO: We should be able to use a VDSO to address this, by providing
+ * proper unwind info for the sigtramp code, at which point the fallback
+ * unwinder will not be used.
+ */
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
+
 #endif /* HPPA_TARGET_SIGNAL_H */
-- 
2.31.1

[PULL 20/26] linux-user/riscv: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the rt signal trampoline.

This fixes a bug wrt libgcc fallback unwinding.  It expects
the stack pointer to point to the siginfo_t, whereas we had
inexplicably placed our private signal trampoline at the start
of the signal frame instead of the end.  Now moot because we
have removed it from the stack frame entirely.

Reviewed-by: Alistair Francis 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-21-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/riscv/signal.c| 22 +-
 linux-user/riscv/target_signal.h |  2 ++
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
index f7f33bc90aed..a0f9542ce39a 100644
--- a/linux-user/riscv/signal.c
+++ b/linux-user/riscv/signal.c
@@ -47,7 +47,6 @@ struct target_ucontext {
 };
 
 struct target_rt_sigframe {
-uint32_t tramp[2]; /* not in kernel, which uses VDSO instead */
 struct target_siginfo info;
 struct target_ucontext uc;
 };
@@ -105,12 +104,6 @@ static void setup_ucontext(struct target_ucontext *uc,
 setup_sigcontext(&uc->uc_mcontext, env);
 }
 
-static inline void install_sigtramp(uint32_t *tramp)
-{
-__put_user(0x08b00893, tramp + 0);  /* li a7, 139 = __NR_rt_sigreturn */
-__put_user(0x0073, tramp + 1);  /* ecall */
-}
-
 void setup_rt_frame(int sig, struct target_sigaction *ka,
 target_siginfo_t *info,
 target_sigset_t *set, CPURISCVState *env)
@@ -127,14 +120,13 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 
 setup_ucontext(&frame->uc, env, set);
 tswap_siginfo(&frame->info, info);
-install_sigtramp(frame->tramp);
 
 env->pc = ka->_sa_handler;
 env->gpr[xSP] = frame_addr;
 env->gpr[xA0] = sig;
 env->gpr[xA1] = frame_addr + offsetof(struct target_rt_sigframe, info);
 env->gpr[xA2] = frame_addr + offsetof(struct target_rt_sigframe, uc);
-env->gpr[xRA] = frame_addr + offsetof(struct target_rt_sigframe, tramp);
+env->gpr[xRA] = default_rt_sigreturn;
 
 return;
 
@@ -203,3 +195,15 @@ badframe:
 force_sig(TARGET_SIGSEGV);
 return 0;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 8, 0);
+assert(tramp != NULL);
+
+__put_user(0x08b00893, tramp + 0);  /* li a7, 139 = __NR_rt_sigreturn */
+__put_user(0x0073, tramp + 1);  /* ecall */
+
+default_rt_sigreturn = sigtramp_page;
+unlock_user(tramp, sigtramp_page, 8);
+}
diff --git a/linux-user/riscv/target_signal.h b/linux-user/riscv/target_signal.h
index f113ba9a55f6..3e36fddc9dbb 100644
--- a/linux-user/riscv/target_signal.h
+++ b/linux-user/riscv/target_signal.h
@@ -15,4 +15,6 @@ typedef struct target_sigaltstack {
 
 #include "../generic/signal.h"
 
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* RISCV_TARGET_SIGNAL_H */
-- 
2.31.1

[PULL 21/26] linux-user/s390x: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the two signal trampolines.
Use them when the guest does not use SA_RESTORER.

Cc: qemu-s3...@nongnu.org
Tested-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-22-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/s390x/signal.c| 24 
 linux-user/s390x/target_signal.h |  2 ++
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 80f34086d7b5..676b94814769 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -68,7 +68,6 @@ typedef struct {
 target_sigregs sregs;
 int signo;
 target_sigregs_ext sregs_ext;
-uint16_t retcode;
 } sigframe;
 
 #define TARGET_UC_VXRS 2
@@ -85,7 +84,6 @@ struct target_ucontext {
 
 typedef struct {
 uint8_t callee_used_stack[__SIGNAL_FRAMESIZE];
-uint16_t retcode;
 struct target_siginfo info;
 struct target_ucontext uc;
 } rt_sigframe;
@@ -209,9 +207,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
 if (ka->sa_flags & TARGET_SA_RESTORER) {
 restorer = ka->sa_restorer;
 } else {
-restorer = frame_addr + offsetof(sigframe, retcode);
-__put_user(S390_SYSCALL_OPCODE | TARGET_NR_sigreturn,
-   &frame->retcode);
+restorer = default_sigreturn;
 }
 
 /* Set up registers for signal handler */
@@ -262,9 +258,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 if (ka->sa_flags & TARGET_SA_RESTORER) {
 restorer = ka->sa_restorer;
 } else {
-restorer = frame_addr + offsetof(typeof(*frame), retcode);
-__put_user(S390_SYSCALL_OPCODE | TARGET_NR_rt_sigreturn,
-   &frame->retcode);
+restorer = default_rt_sigreturn;
 }
 
 /* Create siginfo on the signal stack. */
@@ -405,3 +399,17 @@ long do_rt_sigreturn(CPUS390XState *env)
 unlock_user_struct(frame, frame_addr, 0);
 return -TARGET_QEMU_ESIGRETURN;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint16_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 2 + 2, 0);
+assert(tramp != NULL);
+
+default_sigreturn = sigtramp_page;
+__put_user(S390_SYSCALL_OPCODE | TARGET_NR_sigreturn, &tramp[0]);
+
+default_rt_sigreturn = sigtramp_page + 2;
+__put_user(S390_SYSCALL_OPCODE | TARGET_NR_rt_sigreturn, &tramp[1]);
+
+unlock_user(tramp, sigtramp_page, 2 + 2);
+}
diff --git a/linux-user/s390x/target_signal.h b/linux-user/s390x/target_signal.h
index bbfc464d4417..64f5f422010f 100644
--- a/linux-user/s390x/target_signal.h
+++ b/linux-user/s390x/target_signal.h
@@ -19,4 +19,6 @@ typedef struct target_sigaltstack {
 #include "../generic/signal.h"
 
 #define TARGET_ARCH_HAS_SETUP_FRAME
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* S390X_TARGET_SIGNAL_H */
-- 
2.31.1

Re: [PATCH v3 11/16] iotests/297: return error code from run_linters()

[Hanna Reitz]

On 22.09.21 22:18, John Snow wrote:



On Fri, Sep 17, 2021 at 7:00 AM Hanna Reitz > wrote:


[...]



As you say, run_linters() to me seems very iotests-specific still: It
emits a specific output that is compared against a reference output.
Fine for 297, but not fine for a function provided by a
linters.py, I’d say.

I’d prefer run_linters() to return something like a Map[str,
Optional[str]], that would map a tool to its output in case of error,
i.e. ideally:

`{'pylint': None, 'mypy': None}`


Splitting the test apart into two sub-tests is completely reasonable. 
Python CI right now has individual tests for pylint, mypy, etc.


297 could format it something like

```
for tool, output in run_linters().items():
 print(f'=== {tool} ===')
 if output is not None:
 print(output)
```

Or something.

To check for error, you could put a Python script in python/tests
that
checks `any(output is not None for output in
run_linters().values())` or
something (and on error print the output).


Pulling out run_linters() into an external file and having it print
something to stdout just seems too iotests-centric to me.  I
suppose as
long as the return code is right (which this patch is for) it should
work for Avocado’s simple tests, too (which I don’t like very much
either, by the way, because they too seem archaic to me), but,
well.  It
almost seems like the Avocado test should just run ./check then.


Yeh. Ideally, we'd just have a mypy.ini and a pylintrc that configures 
the tests adequately, and then 297 (or whomever else) would just call 
the linters which would in turn read the same configuration. This 
series is somewhat of a stop-gap to measure the temperature of the 
room to see how important it was to leave 297 inside of iotests. So, I 
did the conservative thing that's faster to review even if it now 
looks *slightly* fishy.


As for things being archaic or not ... possibly, but why involve extra 
complexity if it isn't warranted?


My opinion is that I find an interface of “prints something to stdout 
and returns an integer status code” to be non-intuitive and thus rather 
complex actually.  That’s why I’d prefer different complexity, namely 
one which has a more intuitive interface.


I’m also not sure where the extra complexity would be for a 
`run_linters() -> Map[str, Optional[str]]`, because 297 just needs the 
loop suggested above over `run_linters().items()`, and as for the 
Avocado test...



a simple pass/fail works perfectly well.


I don’t find `any(error_msg for error_msg in run_linters().values())` 
much more complex than pass/fail.


(Note: Above, I called it `output`.  Probably should have called it 
`error_msg` like I did now to clarify that it’s `None` in case of 
success and a string otherwise.)


(And the human can read the output to understand WHY it failed.) If 
you want more rigorous analytics for some reason, we can discuss the 
use cases and figure out how to represent that information, but that's 
definitely a bit beyond scope here.


[...]


Like, can’t we have a Python script in python/tests that imports
linters.py, invokes run_linters() and sensibly checks the output? Or,
you know, at the very least not have run_linters() print anything to
stdout and not have it return an integer code. linters.py:main()
can do
that conversion.


Well, I certainly don't want to bother parsing output from python 
tools and trying to make sure that it works sensibly cross-version and 
cross-distro. The return code being 0/non-zero is vastly simpler. Let 
the human read the output log on failure cases to get a sense of what 
exactly went wrong. Is there some reason why parsing the output would 
be beneficial to anyone?


Perhaps there was a misunderstanding here, because there’s no need to 
parse the output in my suggestion.  `run_linters() -> Map[str, 
Optional[str]]` would map a tool name to its potential error output; if 
the tool exited successfully (exit code 0), then that `Optional[str]` 
error output would be `None`.  It would only be something if there was 
an error.


(The Python GitLab CI hooks don't even bother printing output to the 
console unless it returns non-zero, and then it will just print 
whatever it saw. Seems to work well in practice.)



Or, something completely different, perhaps my problem is that you
put
linters.py as a fully standalone test into the iotests directory,
without it being an iotest.  So, I think I could also agree on
putting
linters.py into python/tests, and then having 297 execute that. 
Or you
know, we just drop 297 altogether, as you suggest in patch 13 – if
that’s what it takes, then so be it.


I can definitely drop 297 if you'd like, and work on making the linter 
configuration more declarative. I erred on the side of less movement 
instead o

[PULL 13/26] linux-user/microblaze: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the rt signal trampoline.

Cc: Edgar E. Iglesias 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-14-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/microblaze/signal.c| 24 +---
 linux-user/microblaze/target_signal.h |  2 ++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/linux-user/microblaze/signal.c b/linux-user/microblaze/signal.c
index b822679d1805..8ebb6a1b7dfe 100644
--- a/linux-user/microblaze/signal.c
+++ b/linux-user/microblaze/signal.c
@@ -161,17 +161,11 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 
 /* Kernel does not use SA_RESTORER. */
 
-/* addi r12, r0, __NR_sigreturn */
-__put_user(0x3180U | TARGET_NR_rt_sigreturn, frame->tramp + 0);
-/* brki r14, 0x8 */
-__put_user(0xb9cc0008U, frame->tramp + 1);
-
 /*
  * Return from sighandler will jump to the tramp.
  * Negative 8 offset because return is rtsd r15, 8
  */
-env->regs[15] =
-frame_addr + offsetof(struct target_rt_sigframe, tramp) - 8;
+env->regs[15] = default_rt_sigreturn - 8;
 
 /* Set up registers for signal handler */
 env->regs[1] = frame_addr;
@@ -220,3 +214,19 @@ long do_rt_sigreturn(CPUMBState *env)
 force_sig(TARGET_SIGSEGV);
 return -TARGET_QEMU_ESIGRETURN;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 8, 0);
+assert(tramp != NULL);
+
+/*
+ * addi r12, r0, __NR_rt_sigreturn
+ * brki r14, 0x8
+ */
+__put_user(0x3180U | TARGET_NR_rt_sigreturn, tramp);
+__put_user(0xb9cc0008U, tramp + 1);
+
+default_rt_sigreturn = sigtramp_page;
+unlock_user(tramp, sigtramp_page, 8);
+}
diff --git a/linux-user/microblaze/target_signal.h 
b/linux-user/microblaze/target_signal.h
index 1c326296de42..e8b510f6b182 100644
--- a/linux-user/microblaze/target_signal.h
+++ b/linux-user/microblaze/target_signal.h
@@ -21,4 +21,6 @@ typedef struct target_sigaltstack {
 
 #include "../generic/signal.h"
 
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* MICROBLAZE_TARGET_SIGNAL_H */
-- 
2.31.1

Re: [PATCH v3 02/22] qapi/misc-target: Group SEV QAPI definitions

[Paolo Bonzini]

On 02/10/21 14:52, Philippe Mathieu-Daudé wrote:

There is already a section with various SEV commands / types,
so move the SEV guest attestation together.

Signed-off-by: Philippe Mathieu-Daudé 
---
  qapi/misc-target.json | 80 +--
  1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index ae5577e0390..5aa2b95b7d4 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -229,6 +229,46 @@
'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
'if': 'TARGET_I386' }
  
+##

+# @SevAttestationReport:
+#
+# The struct describes attestation report for a Secure Encrypted
+# Virtualization feature.
+#
+# @data:  guest attestation report (base64 encoded)
+#
+#
+# Since: 6.1
+##
+{ 'struct': 'SevAttestationReport',
+  'data': { 'data': 'str'},
+  'if': 'TARGET_I386' }
+
+##
+# @query-sev-attestation-report:
+#
+# This command is used to get the SEV attestation report, and is
+# supported on AMD X86 platforms only.
+#
+# @mnonce: a random 16 bytes value encoded in base64 (it will be
+#  included in report)
+#
+# Returns: SevAttestationReport objects.
+#
+# Since: 6.1
+#
+# Example:
+#
+# -> { "execute" : "query-sev-attestation-report",
+#  "arguments": { "mnonce": "aaa" } }
+# <- { "return" : { "data": "bbbd"} }
+#
+##
+{ 'command': 'query-sev-attestation-report',
+  'data': { 'mnonce': 'str' },
+  'returns': 'SevAttestationReport',
+  'if': 'TARGET_I386' }
+
  ##
  # @dump-skeys:
  #
@@ -297,46 +337,6 @@
'if': 'TARGET_ARM' }
  
  
-##

-# @SevAttestationReport:
-#
-# The struct describes attestation report for a Secure Encrypted
-# Virtualization feature.
-#
-# @data:  guest attestation report (base64 encoded)
-#
-#
-# Since: 6.1
-##
-{ 'struct': 'SevAttestationReport',
-  'data': { 'data': 'str'},
-  'if': 'TARGET_I386' }
-
-##
-# @query-sev-attestation-report:
-#
-# This command is used to get the SEV attestation report, and is
-# supported on AMD X86 platforms only.
-#
-# @mnonce: a random 16 bytes value encoded in base64 (it will be
-#  included in report)
-#
-# Returns: SevAttestationReport objects.
-#
-# Since: 6.1
-#
-# Example:
-#
-# -> { "execute" : "query-sev-attestation-report",
-#  "arguments": { "mnonce": "aaa" } }
-# <- { "return" : { "data": "bbbd"} }
-#
-##
-{ 'command': 'query-sev-attestation-report',
-  'data': { 'mnonce': 'str' },
-  'returns': 'SevAttestationReport',
-  'if': 'TARGET_I386' }
-
  ##
  # @SGXInfo:
  #



Reviewed-by: Paolo Bonzini 

[PULL 12/26] linux-user/m68k: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the two signal trampolines.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-13-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/m68k/signal.c| 47 +++--
 linux-user/m68k/target_signal.h |  2 ++
 2 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/linux-user/m68k/signal.c b/linux-user/m68k/signal.c
index 4f8eb6f727e4..ec33482e1403 100644
--- a/linux-user/m68k/signal.c
+++ b/linux-user/m68k/signal.c
@@ -39,7 +39,6 @@ struct target_sigframe
 int sig;
 int code;
 abi_ulong psc;
-char retcode[8];
 abi_ulong extramask[TARGET_NSIG_WORDS-1];
 struct target_sigcontext sc;
 };
@@ -76,7 +75,6 @@ struct target_rt_sigframe
 int sig;
 abi_ulong pinfo;
 abi_ulong puc;
-char retcode[8];
 struct target_siginfo info;
 struct target_ucontext uc;
 };
@@ -130,7 +128,6 @@ void setup_frame(int sig, struct target_sigaction *ka,
 {
 struct target_sigframe *frame;
 abi_ulong frame_addr;
-abi_ulong retcode_addr;
 abi_ulong sc_addr;
 int i;
 
@@ -152,16 +149,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
 }
 
 /* Set up to return from userspace.  */
-
-retcode_addr = frame_addr + offsetof(struct target_sigframe, retcode);
-__put_user(retcode_addr, &frame->pretcode);
-
-/* moveq #,d0; trap #0 */
-
-__put_user(0x70004e40 + (TARGET_NR_sigreturn << 16),
-   (uint32_t *)(frame->retcode));
-
-/* Set up to return from userspace */
+__put_user(default_sigreturn, &frame->pretcode);
 
 env->aregs[7] = frame_addr;
 env->pc = ka->_sa_handler;
@@ -288,7 +276,6 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 {
 struct target_rt_sigframe *frame;
 abi_ulong frame_addr;
-abi_ulong retcode_addr;
 abi_ulong info_addr;
 abi_ulong uc_addr;
 int err = 0;
@@ -325,17 +312,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 }
 
 /* Set up to return from userspace.  */
-
-retcode_addr = frame_addr + offsetof(struct target_sigframe, retcode);
-__put_user(retcode_addr, &frame->pretcode);
-
-/* moveq #,d0; notb d0; trap #0 */
-
-__put_user(0x70004600 + ((TARGET_NR_rt_sigreturn ^ 0xff) << 16),
-   (uint32_t *)(frame->retcode + 0));
-__put_user(0x4e40, (uint16_t *)(frame->retcode + 4));
-
-/* Set up to return from userspace */
+__put_user(default_rt_sigreturn, &frame->pretcode);
 
 env->aregs[7] = frame_addr;
 env->pc = ka->_sa_handler;
@@ -411,3 +388,23 @@ badframe:
 force_sig(TARGET_SIGSEGV);
 return -TARGET_QEMU_ESIGRETURN;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+void *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 4 + 6, 0);
+assert(tramp != NULL);
+
+default_sigreturn = sigtramp_page;
+
+/* moveq #,d0; trap #0 */
+__put_user(0x70004e40 + (TARGET_NR_sigreturn << 16), (uint32_t *)tramp);
+
+default_rt_sigreturn = sigtramp_page + 4;
+
+/* moveq #,d0; notb d0; trap #0 */
+__put_user(0x70004600 + ((TARGET_NR_rt_sigreturn ^ 0xff) << 16),
+   (uint32_t *)(tramp + 4));
+__put_user(0x4e40, (uint16_t *)(tramp + 8));
+
+unlock_user(tramp, sigtramp_page, 4 + 6);
+}
diff --git a/linux-user/m68k/target_signal.h b/linux-user/m68k/target_signal.h
index d096544ef842..94157bf1f48d 100644
--- a/linux-user/m68k/target_signal.h
+++ b/linux-user/m68k/target_signal.h
@@ -22,4 +22,6 @@ typedef struct target_sigaltstack {
 #include "../generic/signal.h"
 
 #define TARGET_ARCH_HAS_SETUP_FRAME
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* M68K_TARGET_SIGNAL_H */
-- 
2.31.1

[PULL 24/26] linux-user/xtensa: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the rt signal trampoline.
Use it when the guest does not use SA_RESTORER.

Reviewed-by: Max Filippov 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-25-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/xtensa/signal.c| 56 ---
 linux-user/xtensa/target_signal.h |  2 ++
 2 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/linux-user/xtensa/signal.c b/linux-user/xtensa/signal.c
index 7a3bfb92cadc..81572a5fc7b3 100644
--- a/linux-user/xtensa/signal.c
+++ b/linux-user/xtensa/signal.c
@@ -128,6 +128,29 @@ static int setup_sigcontext(struct target_rt_sigframe 
*frame,
 return 1;
 }
 
+static void install_sigtramp(uint8_t *tramp)
+{
+#ifdef TARGET_WORDS_BIGENDIAN
+/* Generate instruction:  MOVI a2, __NR_rt_sigreturn */
+__put_user(0x22, &tramp[0]);
+__put_user(0x0a, &tramp[1]);
+__put_user(TARGET_NR_rt_sigreturn, &tramp[2]);
+/* Generate instruction:  SYSCALL */
+__put_user(0x00, &tramp[3]);
+__put_user(0x05, &tramp[4]);
+__put_user(0x00, &tramp[5]);
+#else
+/* Generate instruction:  MOVI a2, __NR_rt_sigreturn */
+__put_user(0x22, &tramp[0]);
+__put_user(0xa0, &tramp[1]);
+__put_user(TARGET_NR_rt_sigreturn, &tramp[2]);
+/* Generate instruction:  SYSCALL */
+__put_user(0x00, &tramp[3]);
+__put_user(0x50, &tramp[4]);
+__put_user(0x00, &tramp[5]);
+#endif
+}
+
 void setup_rt_frame(int sig, struct target_sigaction *ka,
 target_siginfo_t *info,
 target_sigset_t *set, CPUXtensaState *env)
@@ -164,26 +187,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 if (ka->sa_flags & TARGET_SA_RESTORER) {
 ra = ka->sa_restorer;
 } else {
-ra = frame_addr + offsetof(struct target_rt_sigframe, retcode);
-#ifdef TARGET_WORDS_BIGENDIAN
-/* Generate instruction:  MOVI a2, __NR_rt_sigreturn */
-__put_user(0x22, &frame->retcode[0]);
-__put_user(0x0a, &frame->retcode[1]);
-__put_user(TARGET_NR_rt_sigreturn, &frame->retcode[2]);
-/* Generate instruction:  SYSCALL */
-__put_user(0x00, &frame->retcode[3]);
-__put_user(0x05, &frame->retcode[4]);
-__put_user(0x00, &frame->retcode[5]);
-#else
-/* Generate instruction:  MOVI a2, __NR_rt_sigreturn */
-__put_user(0x22, &frame->retcode[0]);
-__put_user(0xa0, &frame->retcode[1]);
-__put_user(TARGET_NR_rt_sigreturn, &frame->retcode[2]);
-/* Generate instruction:  SYSCALL */
-__put_user(0x00, &frame->retcode[3]);
-__put_user(0x50, &frame->retcode[4]);
-__put_user(0x00, &frame->retcode[5]);
-#endif
+/* Not used, but retain for ABI compatibility. */
+install_sigtramp(frame->retcode);
+ra = default_rt_sigreturn;
 }
 memset(env->regs, 0, sizeof(env->regs));
 env->pc = ka->_sa_handler;
@@ -264,3 +270,13 @@ badframe:
 force_sig(TARGET_SIGSEGV);
 return -TARGET_QEMU_ESIGRETURN;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint8_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 6, 0);
+assert(tramp != NULL);
+
+default_rt_sigreturn = sigtramp_page;
+install_sigtramp(tramp);
+unlock_user(tramp, sigtramp_page, 6);
+}
diff --git a/linux-user/xtensa/target_signal.h 
b/linux-user/xtensa/target_signal.h
index c60bf656f6b6..1c7ee73154ac 100644
--- a/linux-user/xtensa/target_signal.h
+++ b/linux-user/xtensa/target_signal.h
@@ -20,4 +20,6 @@ typedef struct target_sigaltstack {
 
 #include "../generic/signal.h"
 
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif
-- 
2.31.1

Re: [PATCH v3 10/22] target/i386/sev: sev_get_attestation_report use g_autofree

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

From: "Dr. David Alan Gilbert" 

Removes a whole bunch of g_free's and a goto.

Signed-off-by: Dr. David Alan Gilbert 
Reviewed-by: Connor Kuehl 
Reviewed-by: Brijesh Singh 
Message-Id: <20210603113017.34922-1-dgilb...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/sev.c | 11 +++
  1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index c88cd808410..aefbef4bb63 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -493,8 +493,8 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
  struct kvm_sev_attestation_report input = {};
  SevAttestationReport *report = NULL;
  SevGuestState *sev = sev_guest;
-guchar *data;
-guchar *buf;
+g_autofree guchar *data = NULL;
+g_autofree guchar *buf = NULL;
  gsize len;
  int err = 0, ret;
  
@@ -514,7 +514,6 @@ sev_get_attestation_report(const char *mnonce, Error **errp)

  if (len != sizeof(input.mnonce)) {
  error_setg(errp, "SEV: mnonce must be %zu bytes (got %" G_GSIZE_FORMAT 
")",
  sizeof(input.mnonce), len);
-g_free(buf);
  return NULL;
  }
  
@@ -525,7 +524,6 @@ sev_get_attestation_report(const char *mnonce, Error **errp)

  if (err != SEV_RET_INVALID_LEN) {
  error_setg(errp, "failed to query the attestation report length "
  "ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
-g_free(buf);
  return NULL;
  }
  }
@@ -540,7 +538,7 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
  if (ret) {
  error_setg_errno(errp, errno, "Failed to get attestation report"
  " ret=%d fw_err=%d (%s)", ret, err, fw_error_to_str(err));
-goto e_free_data;
+return NULL;
  }
  
  report = g_new0(SevAttestationReport, 1);

@@ -548,9 +546,6 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
  
  trace_kvm_sev_attestation_report(mnonce, report->data);
  
-e_free_data:

-g_free(data);
-g_free(buf);
  return report;
  }
  



Reviewed-by: Paolo Bonzini 

[PULL 08/26] linux-user/hexagon: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Continue to initialize the words on the stack, as documented.
However, use the off-stack trampoline.

Reviewed-by: Taylor Simpson 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-9-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/hexagon/signal.c| 19 +--
 linux-user/hexagon/target_signal.h |  2 ++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/linux-user/hexagon/signal.c b/linux-user/hexagon/signal.c
index c7f0bf6b9283..74e61739a0ab 100644
--- a/linux-user/hexagon/signal.c
+++ b/linux-user/hexagon/signal.c
@@ -162,6 +162,11 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 
 setup_ucontext(&frame->uc, env, set);
 tswap_siginfo(&frame->info, info);
+/*
+ * The on-stack signal trampoline is no longer executed;
+ * however, the libgcc signal frame unwinding code checks
+ * for the presence of these two numeric magic values.
+ */
 install_sigtramp(frame->tramp);
 
 env->gpr[HEX_REG_PC] = ka->_sa_handler;
@@ -171,8 +176,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 frame_addr + offsetof(struct target_rt_sigframe, info);
 env->gpr[HEX_REG_R02] =
 frame_addr + offsetof(struct target_rt_sigframe, uc);
-env->gpr[HEX_REG_LR] =
-frame_addr + offsetof(struct target_rt_sigframe, tramp);
+env->gpr[HEX_REG_LR] = default_rt_sigreturn;
 
 return;
 
@@ -271,3 +275,14 @@ badframe:
 force_sig(TARGET_SIGSEGV);
 return 0;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 4 * 2, 0);
+assert(tramp != NULL);
+
+default_rt_sigreturn = sigtramp_page;
+install_sigtramp(tramp);
+
+unlock_user(tramp, sigtramp_page, 4 * 2);
+}
diff --git a/linux-user/hexagon/target_signal.h 
b/linux-user/hexagon/target_signal.h
index 345cf1cbb843..9e0223d32225 100644
--- a/linux-user/hexagon/target_signal.h
+++ b/linux-user/hexagon/target_signal.h
@@ -31,4 +31,6 @@ typedef struct target_sigaltstack {
 
 #include "../generic/signal.h"
 
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* TARGET_SIGNAL_H */
-- 
2.31.1

[PULL 25/26] linux-user: Remove default for TARGET_ARCH_HAS_SIGTRAMP_PAGE

[Laurent Vivier]

From: Richard Henderson 

All targets now define TARGET_ARCH_HAS_SIGTRAMP_PAGE.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-26-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/elfload.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 459a26ef1d93..2404d482bafa 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -30,10 +30,6 @@
 #undef ELF_ARCH
 #endif
 
-#ifndef TARGET_ARCH_HAS_SIGTRAMP_PAGE
-#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
-#endif
-
 #define ELF_OSABI   ELFOSABI_SYSV
 
 /* from personality.h */
-- 
2.31.1

Re: [PATCH v3 11/22] target/i386/sev: Restrict SEV to system emulation

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

SEV is irrelevant on user emulation, so restrict it to sysemu.
Some stubs are still required because used in cpu.c by
x86_register_cpudef_types(), so move the sysemu specific stubs
to sev-sysemu-stub.c instead. This will allow us to simplify
monitor.c (which is not available in user emulation) in the
next commit.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/sev-stub.c| 43 -
  target/i386/sev-sysemu-stub.c | 60 +++
  target/i386/meson.build   |  4 ++-
  3 files changed, 63 insertions(+), 44 deletions(-)
  create mode 100644 target/i386/sev-sysemu-stub.c

diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 4668365fd3e..8eae5d2fa8d 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -15,11 +15,6 @@
  #include "qapi/error.h"
  #include "sev_i386.h"
  
-SevInfo *sev_get_info(void)

-{
-return NULL;
-}
-
  bool sev_enabled(void)
  {
  return false;
@@ -35,45 +30,7 @@ uint32_t sev_get_reduced_phys_bits(void)
  return 0;
  }
  
-char *sev_get_launch_measurement(void)

-{
-return NULL;
-}
-
-SevCapability *sev_get_capabilities(Error **errp)
-{
-error_setg(errp, "SEV is not available in this QEMU");
-return NULL;
-}
-
-int sev_inject_launch_secret(const char *hdr, const char *secret,
- uint64_t gpa, Error **errp)
-{
-return 1;
-}
-
-int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
-{
-g_assert_not_reached();
-}
-
  bool sev_es_enabled(void)
  {
  return false;
  }
-
-void sev_es_set_reset_vector(CPUState *cpu)
-{
-}
-
-int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
-{
-g_assert_not_reached();
-}
-
-SevAttestationReport *
-sev_get_attestation_report(const char *mnonce, Error **errp)
-{
-error_setg(errp, "SEV is not available in this QEMU");
-return NULL;
-}
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
new file mode 100644
index 000..d556b4f091f
--- /dev/null
+++ b/target/i386/sev-sysemu-stub.c
@@ -0,0 +1,60 @@
+/*
+ * QEMU SEV system stub
+ *
+ * Copyright Advanced Micro Devices 2018
+ *
+ * Authors:
+ *  Brijesh Singh 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qapi-commands-misc-target.h"
+#include "qapi/error.h"
+#include "sev_i386.h"
+
+SevInfo *sev_get_info(void)
+{
+return NULL;
+}
+
+char *sev_get_launch_measurement(void)
+{
+return NULL;
+}
+
+SevCapability *sev_get_capabilities(Error **errp)
+{
+error_setg(errp, "SEV is not available in this QEMU");
+return NULL;
+}
+
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa, Error **errp)
+{
+return 1;
+}
+
+int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
+{
+g_assert_not_reached();
+}
+
+void sev_es_set_reset_vector(CPUState *cpu)
+{
+}
+
+int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
+{
+g_assert_not_reached();
+}
+
+SevAttestationReport *sev_get_attestation_report(const char *mnonce,
+ Error **errp)
+{
+error_setg(errp, "SEV is not available in this QEMU");
+return NULL;
+}
diff --git a/target/i386/meson.build b/target/i386/meson.build
index dac19ec00d4..a4f45c3ec1d 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -6,7 +6,7 @@
'xsave_helper.c',
'cpu-dump.c',
  ))
-i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c', 'sev.c'), 
if_false: files('sev-stub.c'))
+i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: 
files('sev-stub.c'))
  
  # x86 cpu type

  i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
@@ -20,6 +20,8 @@
'monitor.c',
'cpu-sysemu.c',
  ))
+i386_softmmu_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: 
files('sev-sysemu-stub.c'))
+
  i386_user_ss = ss.source_set()
  
  subdir('kvm')




Reviewed-by: Paolo Bonzini 

Re: [PATCH v3 03/22] target/i386/kvm: Introduce i386_softmmu_kvm Meson source set

[Paolo Bonzini]

On 02/10/21 14:52, Philippe Mathieu-Daudé wrote:

Introduce the i386_softmmu_kvm Meson source set to be able to
add features dependent on CONFIG_KVM.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/kvm/meson.build | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/i386/kvm/meson.build b/target/i386/kvm/meson.build
index 0a533411cab..b1c76957c76 100644
--- a/target/i386/kvm/meson.build
+++ b/target/i386/kvm/meson.build
@@ -1,8 +1,12 @@
  i386_ss.add(when: 'CONFIG_KVM', if_false: files('kvm-stub.c'))
  
-i386_softmmu_ss.add(when: 'CONFIG_KVM', if_true: files(

+i386_softmmu_kvm_ss = ss.source_set()
+
+i386_softmmu_kvm_ss.add(files(
'kvm.c',
'kvm-cpu.c',
  ))
  
  i386_softmmu_ss.add(when: 'CONFIG_HYPERV', if_true: files('hyperv.c'), if_false: files('hyperv-stub.c'))

+
+i386_softmmu_ss.add_all(when: 'CONFIG_KVM', if_true: i386_softmmu_kvm_ss)



Reviewed-by: Paolo Bonzini 

[PULL 23/26] linux-user/sparc: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the two signal trampolines.
Use them when the guest does not use SA_RESTORER.

Acked-by: Mark Cave-Ayland 
Reviewed-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-24-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/sparc/signal.c| 40 +---
 linux-user/sparc/target_signal.h |  4 
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c
index 3bc023d281a7..23e1e761de42 100644
--- a/linux-user/sparc/signal.c
+++ b/linux-user/sparc/signal.c
@@ -242,6 +242,12 @@ static void restore_fpu(struct target_siginfo_fpu *fpu, 
CPUSPARCState *env)
 }
 
 #ifdef TARGET_ARCH_HAS_SETUP_FRAME
+static void install_sigtramp(uint32_t *tramp, int syscall)
+{
+__put_user(0x82102000u + syscall, &tramp[0]); /* mov syscall, %g1 */
+__put_user(0x91d02010u, &tramp[1]);   /* t 0x10 */
+}
+
 void setup_frame(int sig, struct target_sigaction *ka,
  target_sigset_t *set, CPUSPARCState *env)
 {
@@ -291,13 +297,9 @@ void setup_frame(int sig, struct target_sigaction *ka,
 if (ka->ka_restorer) {
 env->regwptr[WREG_O7] = ka->ka_restorer;
 } else {
-env->regwptr[WREG_O7] = sf_addr +
-offsetof(struct target_signal_frame, insns) - 2 * 4;
-
-/* mov __NR_sigreturn, %g1 */
-__put_user(0x821020d8u, &sf->insns[0]);
-/* t 0x10 */
-__put_user(0x91d02010u, &sf->insns[1]);
+/* Not used, but retain for ABI compatibility. */
+install_sigtramp(sf->insns, TARGET_NR_sigreturn);
+env->regwptr[WREG_O7] = default_sigreturn;
 }
 unlock_user(sf, sf_addr, sf_size);
 }
@@ -358,13 +360,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 if (ka->ka_restorer) {
 env->regwptr[WREG_O7] = ka->ka_restorer;
 } else {
-env->regwptr[WREG_O7] =
-sf_addr + offsetof(struct target_rt_signal_frame, insns) - 2 * 4;
-
-/* mov __NR_rt_sigreturn, %g1 */
-__put_user(0x82102065u, &sf->insns[0]);
-/* t 0x10 */
-__put_user(0x91d02010u, &sf->insns[1]);
+/* Not used, but retain for ABI compatibility. */
+install_sigtramp(sf->insns, TARGET_NR_rt_sigreturn);
+env->regwptr[WREG_O7] = default_rt_sigreturn;
 }
 #else
 env->regwptr[WREG_O7] = ka->ka_restorer;
@@ -775,4 +773,18 @@ do_sigsegv:
 unlock_user_struct(ucp, ucp_addr, 1);
 force_sig(TARGET_SIGSEGV);
 }
+#else
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 2 * 8, 0);
+assert(tramp != NULL);
+
+default_sigreturn = sigtramp_page;
+install_sigtramp(tramp, TARGET_NR_sigreturn);
+
+default_rt_sigreturn = sigtramp_page + 8;
+install_sigtramp(tramp + 2, TARGET_NR_rt_sigreturn);
+
+unlock_user(tramp, sigtramp_page, 2 * 8);
+}
 #endif
diff --git a/linux-user/sparc/target_signal.h b/linux-user/sparc/target_signal.h
index 34f9a1251909..e661ddd6ab3c 100644
--- a/linux-user/sparc/target_signal.h
+++ b/linux-user/sparc/target_signal.h
@@ -69,6 +69,10 @@ typedef struct target_sigaltstack {
 
 #ifdef TARGET_ABI32
 #define TARGET_ARCH_HAS_SETUP_FRAME
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+#else
+/* For sparc64, use of KA_RESTORER is mandatory. */
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
 #endif
 
 /* bit-flags */
-- 
2.31.1

[PULL 22/26] linux-user/sh4: Implement setup_sigtramp

[Laurent Vivier]

From: Richard Henderson 

Create and record the two signal trampolines.
Use them when the guest does not use SA_RESTORER.

Cc: Yoshinori Sato 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Message-Id: <20210929130553.121567-23-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 linux-user/sh4/signal.c| 40 +++---
 linux-user/sh4/target_signal.h |  2 ++
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/linux-user/sh4/signal.c b/linux-user/sh4/signal.c
index d70d744befc2..faa869fb1958 100644
--- a/linux-user/sh4/signal.c
+++ b/linux-user/sh4/signal.c
@@ -52,7 +52,6 @@ struct target_sigframe
 {
 struct target_sigcontext sc;
 target_ulong extramask[TARGET_NSIG_WORDS-1];
-uint16_t retcode[3];
 };
 
 
@@ -68,7 +67,6 @@ struct target_rt_sigframe
 {
 struct target_siginfo info;
 struct target_ucontext uc;
-uint16_t retcode[3];
 };
 
 
@@ -190,15 +188,9 @@ void setup_frame(int sig, struct target_sigaction *ka,
 /* Set up to return from userspace.  If provided, use a stub
already in userspace.  */
 if (ka->sa_flags & TARGET_SA_RESTORER) {
-regs->pr = (unsigned long) ka->sa_restorer;
+regs->pr = ka->sa_restorer;
 } else {
-/* Generate return code (system call to sigreturn) */
-abi_ulong retcode_addr = frame_addr +
- offsetof(struct target_sigframe, retcode);
-__put_user(MOVW(2), &frame->retcode[0]);
-__put_user(TRAP_NOARG, &frame->retcode[1]);
-__put_user((TARGET_NR_sigreturn), &frame->retcode[2]);
-regs->pr = (unsigned long) retcode_addr;
+regs->pr = default_sigreturn;
 }
 
 /* Set up registers for signal handler */
@@ -248,15 +240,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
 /* Set up to return from userspace.  If provided, use a stub
already in userspace.  */
 if (ka->sa_flags & TARGET_SA_RESTORER) {
-regs->pr = (unsigned long) ka->sa_restorer;
+regs->pr = ka->sa_restorer;
 } else {
-/* Generate return code (system call to sigreturn) */
-abi_ulong retcode_addr = frame_addr +
- offsetof(struct target_rt_sigframe, retcode);
-__put_user(MOVW(2), &frame->retcode[0]);
-__put_user(TRAP_NOARG, &frame->retcode[1]);
-__put_user((TARGET_NR_rt_sigreturn), &frame->retcode[2]);
-regs->pr = (unsigned long) retcode_addr;
+regs->pr = default_rt_sigreturn;
 }
 
 /* Set up registers for signal handler */
@@ -334,3 +320,21 @@ badframe:
 force_sig(TARGET_SIGSEGV);
 return -TARGET_QEMU_ESIGRETURN;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+uint16_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 2 * 6, 0);
+assert(tramp != NULL);
+
+default_sigreturn = sigtramp_page;
+__put_user(MOVW(2), &tramp[0]);
+__put_user(TRAP_NOARG, &tramp[1]);
+__put_user(TARGET_NR_sigreturn, &tramp[2]);
+
+default_rt_sigreturn = sigtramp_page + 6;
+__put_user(MOVW(2), &tramp[3]);
+__put_user(TRAP_NOARG, &tramp[4]);
+__put_user(TARGET_NR_rt_sigreturn, &tramp[5]);
+
+unlock_user(tramp, sigtramp_page, 2 * 6);
+}
diff --git a/linux-user/sh4/target_signal.h b/linux-user/sh4/target_signal.h
index d7309b7136d7..04069cba6641 100644
--- a/linux-user/sh4/target_signal.h
+++ b/linux-user/sh4/target_signal.h
@@ -22,4 +22,6 @@ typedef struct target_sigaltstack {
 #include "../generic/signal.h"
 
 #define TARGET_ARCH_HAS_SETUP_FRAME
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* SH4_TARGET_SIGNAL_H */
-- 
2.31.1

Re: [PATCH v3 12/22] target/i386/sev: Declare system-specific functions in 'sev_i386.h'

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

While prefixed with sysemu, 'sysemu/sev.h' contains the architecture
specific declarations. The system specific parts are declared in
'sev_i386.h'.


While outside target/i386, 'sysemu/sev.h' contains some architecture 
specific declarations. Move them to 'sev_i386.h'.


Otherwise,

Reviewed-by: Paolo Bonzini 

Paolo


Signed-off-by: Philippe Mathieu-Daudé 
---
  include/sysemu/sev.h   | 6 --
  target/i386/sev_i386.h | 7 +++
  hw/i386/pc_sysfw.c | 2 +-
  3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index 94d821d737c..a329ed75c1c 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -18,11 +18,5 @@
  
  bool sev_enabled(void);

  int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
-int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
-int sev_inject_launch_secret(const char *hdr, const char *secret,
- uint64_t gpa, Error **errp);
-
-int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size);
-void sev_es_set_reset_vector(CPUState *cpu);
  
  #endif

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index afa19a0a161..0798ab3519a 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -33,4 +33,11 @@ extern SevCapability *sev_get_capabilities(Error **errp);
  extern SevAttestationReport *
  sev_get_attestation_report(const char *mnonce, Error **errp);
  
+int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);

+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa, Error **errp);
+
+int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size);
+void sev_es_set_reset_vector(CPUState *cpu);
+
  #endif
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 68d6b1f783e..0b202138b66 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -37,7 +37,7 @@
  #include "hw/qdev-properties.h"
  #include "hw/block/flash.h"
  #include "sysemu/kvm.h"
-#include "sysemu/sev.h"
+#include "sev_i386.h"
  
  #define FLASH_SECTOR_SIZE 4096
  

Re: [PATCH v4 0/2] x86/sev: Measured Linux SEV guest with kernel/initrd/cmdline

[Paolo Bonzini]

Queued, thanks.  However, it would be nice to have a documentation of
all our SEV firmware interfaces somewhere in docs/specs/.

Paolo

On Thu, Sep 30, 2021 at 7:49 AM Dov Murik  wrote:
>
> Currently booting with -kernel/-initrd/-append is not supported in SEV
> confidential guests, because the content of these blobs is not measured
> and therefore not trusted by the SEV guest.
>
> However, in some cases the kernel, initrd, and cmdline are not secret
> but should not be modified by the host.  In such a case, we want to
> verify inside the trusted VM that the kernel, initrd, and cmdline are
> indeed the ones expected by the Guest Owner, and only if that is the
> case go on and boot them up (removing the need for grub inside OVMF in
> that mode).
>
> To support that, OVMF adds a special area for hashes of
> kernel/initrd/cmdline; that area is expected to be filled by QEMU and
> encrypted as part of the initial SEV guest launch.  This in turn makes
> the hashes part of the AMD PSP measured content, and OVMF can trust
> these inputs if they match the hashes.
>
> This series adds an SEV function to generate the table of hashes for
> OVMF and encrypt it (patch 1/2), and calls this function if SEV is
> enabled when the kernel/initrd/cmdline are prepared (patch 2/2).
>
> Corresponding OVMF support [1] is already available in edk2 (patch series
> "Measured SEV boot with kernel/initrd/cmdline").
>
> [1] https://edk2.groups.io/g/devel/message/78250
>
> ---
>
> v4 changes:
>  - struct and variable renames (KernelLoaderContext -> SevKernelLoaderContext,
>kernel_loader_context -> sev_load_ctx).
>
> v3 resend: 
> https://lore.kernel.org/qemu-devel/20210825073538.959525-1-dovmu...@linux.ibm.com/
> v3: 
> https://lore.kernel.org/qemu-devel/20210624102040.2015280-1-dovmu...@linux.ibm.com/
> v3 changes:
>  - initrd hash is now mandatory; if no -initrd is passed, calculate the
>hash of the empty buffer.  This is now aligned with the OVMF
>behaviour which verifies the empty initrd (correctly).
>  - make SevHashTable entries fixed: 3 entries for cmdline, initrd, and kernel.
>  - in sev_add_kernel_loader_hashes: first calculate all the hashes, only then
>fill-in the hashes table in the guest's memory.
>  - Use g_assert_not_reached in sev-stub.c.
>  - Use QEMU_PACKED attribute for structs.
>  - Use QemuUUID type for guids.
>  - in sev_add_kernel_loader_hashes: use ARRAY_SIZE(iov) instead of literal 2.
>
> v2: 
> https://lore.kernel.org/qemu-devel/20210621190553.1763020-1-dovmu...@linux.ibm.com/
> v2 changes:
>  - Extract main functionality to sev.c (with empty stub in sev-stub.c)
>  - Use sev_enabled() instead of machine->cgs->ready to detect SEV guest
>  - Coding style changes
>
> v1: 
> https://lore.kernel.org/qemu-devel/20210525065931.1628554-1-dovmu...@linux.ibm.com/
>
>
> Dov Murik (2):
>   sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux
> boot
>   x86/sev: generate SEV kernel loader hashes in x86_load_linux
>
>  target/i386/sev_i386.h |  12 
>  hw/i386/x86.c  |  25 +++-
>  target/i386/sev-stub.c |   5 ++
>  target/i386/sev.c  | 137 +
>  4 files changed, 178 insertions(+), 1 deletion(-)
>
> --
> 2.25.1
>

Re: [PATCH v3 04/22] target/i386/kvm: Restrict SEV stubs to x86 architecture

[Paolo Bonzini]

On 02/10/21 14:52, Philippe Mathieu-Daudé wrote:

SEV is x86-specific, no need to add its stub to other
architectures. Move the stub file to target/i386/kvm/.

Signed-off-by: Philippe Mathieu-Daudé 
---
  {accel => target/i386}/kvm/sev-stub.c | 0
  accel/kvm/meson.build | 1 -
  target/i386/kvm/meson.build   | 2 ++
  3 files changed, 2 insertions(+), 1 deletion(-)
  rename {accel => target/i386}/kvm/sev-stub.c (100%)

diff --git a/accel/kvm/sev-stub.c b/target/i386/kvm/sev-stub.c
similarity index 100%
rename from accel/kvm/sev-stub.c
rename to target/i386/kvm/sev-stub.c
diff --git a/accel/kvm/meson.build b/accel/kvm/meson.build
index 8d219bea507..397a1fe1fd1 100644
--- a/accel/kvm/meson.build
+++ b/accel/kvm/meson.build
@@ -3,6 +3,5 @@
'kvm-all.c',
'kvm-accel-ops.c',
  ))
-kvm_ss.add(when: 'CONFIG_SEV', if_false: files('sev-stub.c'))
  
  specific_ss.add_all(when: 'CONFIG_KVM', if_true: kvm_ss)

diff --git a/target/i386/kvm/meson.build b/target/i386/kvm/meson.build
index b1c76957c76..736df8b72e3 100644
--- a/target/i386/kvm/meson.build
+++ b/target/i386/kvm/meson.build
@@ -7,6 +7,8 @@
'kvm-cpu.c',
  ))
  
+i386_softmmu_kvm_ss.add(when: 'CONFIG_SEV', if_false: files('sev-stub.c'))

+
  i386_softmmu_ss.add(when: 'CONFIG_HYPERV', if_true: files('hyperv.c'), 
if_false: files('hyperv-stub.c'))
  
  i386_softmmu_ss.add_all(when: 'CONFIG_KVM', if_true: i386_softmmu_kvm_ss)




Reviewed-by: Paolo Bonzini 

Re: [PATCH v3 07/16] iotests/297: Don't rely on distro-specific linter binaries

[Hanna Reitz]

On 22.09.21 21:53, John Snow wrote:
(This email just explains python packaging stuff. No action items in 
here. Skim away.)


On Fri, Sep 17, 2021 at 5:43 AM Hanna Reitz > wrote:


On 16.09.21 06:09, John Snow wrote:
> 'pylint-3' is another Fedora-ism. Use "python3 -m pylint" or
"python3 -m
> mypy" to access these scripts instead. This style of invocation will
> prefer the "correct" tool when run in a virtual environment.
>
> Note that we still check for "pylint-3" before the test begins
-- this
> check is now "overly strict", but shouldn't cause anything that was
> already running correctly to start failing.
>
> Signed-off-by: John Snow mailto:js...@redhat.com>>
> Reviewed-by: Vladimir Sementsov-Ogievskiy
mailto:vsement...@virtuozzo.com>>
> Reviewed-by: Philippe Mathieu-Daudé mailto:phi...@redhat.com>>
> ---
>   tests/qemu-iotests/297 | 45
--
>   1 file changed, 26 insertions(+), 19 deletions(-)

I know it sounds silly, but to be honest I have no idea if replacing
`mypy` by `python3 -m mypy` is correct, because no mypy documentation
seems to suggest it.


Right, I don't think it's necessarily documented that you can do this. 
It just happens to be a very convenient way to invoke the same script 
without needing to know *where* mypy is. You let python figure out 
where it's going to import mypy from, and it handles the rest.


(This also makes it easier to use things like mypy, pylint etc with an 
explicitly specified PYTHON interpreter. I don't happen to do that in 
this patch, but ... we could.)


 From what I understand, that’s generally how Python “binaries” work,
though (i.e., installed as a module invokable with `python -m`,
and then
providing some stub binary that, well, effectively does this, but
kind
of in a weird way, I just don’t understand it), and none of the
parameters seem to be hurt in this conversion, so:


Right. Technically, any python package can ask for any number of 
executables to be installed, but the setuptools packaging ecosystem 
provides a way to "generate" these based on package configuration. I 
use a few in our own Python packages. If you look in python/setup.cfg, 
you'll see stuff like this:


[options.entry_points]
console_scripts =
    qom = qemu.qmp.qom:main
    qom-set = qemu.qmp.qom:QOMSet.entry_point
    qom-get = qemu.qmp.qom:QOMGet.entry_point
    qom-list = qemu.qmp.qom:QOMList.entry_point
    qom-tree = qemu.qmp.qom:QOMTree.entry_point
    qom-fuse = qemu.qmp.qom_fuse:QOMFuse.entry_point [fuse]
    qemu-ga-client = qemu.qmp.qemu_ga_client:main
    qmp-shell = qemu.qmp.qmp_shell:main

These entries cause those weird little binary wrapper scripts to be 
generated for you when the package is *installed*. So our packages 
will put 'qmp-shell' and 'qom-tree' into your $PATH*.The stuff to the 
right of the equals sign is just a pointer to a function that can be 
executed that implements the CLI command. qemu.qmp.qmp_shell points to 
the module to import, and ':main' points to the function to run.


The last bit of this is that many, though not all (and there's zero 
requirement this has to be true), python packages that implement CLI 
commands will often have a stanza in their __init__.py module that 
says something like this:


if __name__ == '__main__':
    do_the_command_line_stuff()

Alternatively, a package can include a literal __main__.py file that 
python knows to check for, and this module is the one that gets 
executed for `python3 -m mypackage` invocations. This is what mypy does.


Those are the magical blurbs that allow you to execute a module as if 
it were a script by running "python3 -m mymodule" -- that hooks 
directly into that little execution stanza. For python code 
distributed as packages, that's the real reason to have that little 
magic stanza -- it provides a convenient way to run stuff without 
needing to write the incredibly more tedious:


python3 -c "from mypy.__main__ import console_entry; console_entry();"

... which is quite a bit more porcelain too, depending on how they 
re/factor the code inside of the package.


Seeing as how mypy explicitly includes a __main__.py file: 
https://github.com/python/mypy/blob/master/mypy/__main__.py 
, I am 
taking it as a given that they are fully aware of invoking mypy in 
this fashion, and believe it safe to rely on.


Wow, thanks a lot for this detailed explanation!

There will be a quiz later.
(There will not be a quiz.)


I’m ready to fail any test on Python so one day I can get a “Officially 
knows nothing about Python” badge.


Hanna

Re: [PATCH v3 05/22] target/i386/monitor: Return QMP error when SEV is disabled in build

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

If the management layer tries to inject a secret, it gets an empty
response in case the binary built without SEV:

   { "execute": "sev-inject-launch-secret",
 "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 
4294959104 }
   }
   {
   "return": {
   }
   }

Make it clearer by returning an error, mentioning the feature is
disabled:

   { "execute": "sev-inject-launch-secret",
 "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 
4294959104 }
   }
   {
   "error": {
   "class": "GenericError",
   "desc": "this feature or command is not currently supported"
   }
   }

Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Connor Kuehl 
Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/monitor.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 196c1c9e77f..a9f85acd473 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -28,6 +28,7 @@
  #include "monitor/hmp-target.h"
  #include "monitor/hmp.h"
  #include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qerror.h"
  #include "sysemu/kvm.h"
  #include "sysemu/sev.h"
  #include "qapi/error.h"
@@ -743,6 +744,10 @@ void qmp_sev_inject_launch_secret(const char *packet_hdr,
bool has_gpa, uint64_t gpa,
Error **errp)
  {
+if (!sev_enabled()) {
+error_setg(errp, QERR_UNSUPPORTED);
+return;
+}
  if (!has_gpa) {
  uint8_t *data;
  struct sev_secret_area *area;



This should be done in the sev_inject_launch_secret stub instead, I 
think.  Or if you do it here, you can remove the "if (!sev_guest)" 
conditional in the non-stub version.


Paolo

Re: [PATCH v2 1/2] hw/adc: Add basic Aspeed ADC model

[Cédric Le Goater]

On 10/3/21 21:18, p...@fb.com wrote:

From: Andrew Jeffery 

This model implements enough behaviour to do basic functionality tests
such as device initialisation and read out of dummy sample values. The
sample value generation strategy is similar to the STM ADC already in
the tree.

Signed-off-by: Andrew Jeffery 
[clg : support for multiple engines (AST2600) ]
Signed-off-by: Cédric Le Goater 
[pdel : refactored engine register struct fields to regs[] array field]
[pdel : added guest-error checking for upper-8 channel regs in AST2600]
Signed-off-by: Peter Delevoryas 


Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/adc/aspeed_adc.c | 422 
  hw/adc/meson.build  |   1 +
  hw/adc/trace-events |   3 +
  include/hw/adc/aspeed_adc.h |  55 +
  4 files changed, 481 insertions(+)
  create mode 100644 hw/adc/aspeed_adc.c
  create mode 100644 include/hw/adc/aspeed_adc.h

diff --git a/hw/adc/aspeed_adc.c b/hw/adc/aspeed_adc.c
new file mode 100644
index 00..fcd93d6853
--- /dev/null
+++ b/hw/adc/aspeed_adc.c
@@ -0,0 +1,422 @@
+/*
+ * Aspeed ADC
+ *
+ * Copyright 2017-2021 IBM Corp.
+ *
+ * Andrew Jeffery 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "hw/adc/aspeed_adc.h"
+#include "trace.h"
+
+#define ASPEED_ADC_MEMORY_REGION_SIZE   0x1000
+#define ASPEED_ADC_ENGINE_MEMORY_REGION_SIZE0x100
+#define  ASPEED_ADC_ENGINE_CH_EN_MASK   0x
+#define   ASPEED_ADC_ENGINE_CH_EN(x)((BIT(x)) << 16)
+#define  ASPEED_ADC_ENGINE_INIT BIT(8)
+#define  ASPEED_ADC_ENGINE_AUTO_COMPBIT(5)
+#define  ASPEED_ADC_ENGINE_COMP BIT(4)
+#define  ASPEED_ADC_ENGINE_MODE_MASK0x000e
+#define   ASPEED_ADC_ENGINE_MODE_OFF(0b000 << 1)
+#define   ASPEED_ADC_ENGINE_MODE_STANDBY(0b001 << 1)
+#define   ASPEED_ADC_ENGINE_MODE_NORMAL (0b111 << 1)
+#define  ASPEED_ADC_ENGINE_EN   BIT(0)
+#define ASPEED_ADC_HYST_EN  BIT(31)
+
+#define ASPEED_ADC_L_MASK   ((1 << 10) - 1)
+#define ASPEED_ADC_L(x) ((x) & ASPEED_ADC_L_MASK)
+#define ASPEED_ADC_H(x) (((x) >> 16) & ASPEED_ADC_L_MASK)
+#define ASPEED_ADC_LH_MASK  (ASPEED_ADC_L_MASK << 16 | ASPEED_ADC_L_MASK)
+#define LOWER_CHANNEL_MASK  ((1 << 10) - 1)
+#define LOWER_CHANNEL_DATA(x)   ((x) & LOWER_CHANNEL_MASK)
+#define UPPER_CHANNEL_DATA(x)   (((x) >> 16) & LOWER_CHANNEL_MASK)
+
+#define TO_REG(addr) (addr >> 2)
+
+#define ENGINE_CONTROL  TO_REG(0x00)
+#define INTERRUPT_CONTROL   TO_REG(0x04)
+#define VGA_DETECT_CONTROL  TO_REG(0x08)
+#define CLOCK_CONTROL   TO_REG(0x0C)
+#define DATA_CHANNEL_1_AND_0TO_REG(0x10)
+#define DATA_CHANNEL_7_AND_6TO_REG(0x1C)
+#define DATA_CHANNEL_9_AND_8TO_REG(0x20)
+#define DATA_CHANNEL_15_AND_14  TO_REG(0x2C)
+#define BOUNDS_CHANNEL_0TO_REG(0x30)
+#define BOUNDS_CHANNEL_7TO_REG(0x4C)
+#define BOUNDS_CHANNEL_8TO_REG(0x50)
+#define BOUNDS_CHANNEL_15   TO_REG(0x6C)
+#define HYSTERESIS_CHANNEL_0TO_REG(0x70)
+#define HYSTERESIS_CHANNEL_7TO_REG(0x8C)
+#define HYSTERESIS_CHANNEL_8TO_REG(0x90)
+#define HYSTERESIS_CHANNEL_15   TO_REG(0xAC)
+#define INTERRUPT_SOURCETO_REG(0xC0)
+#define COMPENSATING_AND_TRIMMING   TO_REG(0xC4)
+
+static inline uint32_t update_channels(uint32_t current)
+{
+return current >> 16) & ASPEED_ADC_L_MASK) + 7) << 16) |
+((current + 5) & ASPEED_ADC_L_MASK);
+}
+
+static bool breaks_threshold(AspeedADCEngineState *s, int reg)
+{
+assert(reg >= DATA_CHANNEL_1_AND_0 &&
+   reg < DATA_CHANNEL_1_AND_0 + s->nr_channels / 2);
+
+int a_bounds_reg = BOUNDS_CHANNEL_0 + (reg - DATA_CHANNEL_1_AND_0) * 2;
+int b_bounds_reg = a_bounds_reg + 1;
+uint32_t a_and_b = s->regs[reg];
+uint32_t a_bounds = s->regs[a_bounds_reg];
+uint32_t b_bounds = s->regs[b_bounds_reg];
+uint32_t a = ASPEED_ADC_L(a_and_b);
+uint32_t b = ASPEED_ADC_H(a_and_b);
+uint32_t a_lower = ASPEED_ADC_L(a_bounds);
+uint32_t a_upper = ASPEED_ADC_H(a_bounds);
+uint32_t b_lower = ASPEED_ADC_L(b_bounds);
+uint32_t b_upper = ASPEED_ADC_H(b_bounds);
+
+return (a < a_lower || a > a_upper) ||
+   (b < b_lower || b > b_upper);
+}
+
+static uint32_t read_channel_sample(AspeedADCEngineState *s, int reg)
+{
+assert(reg >= DATA_CHANNEL_1_AND_0 &&
+   reg < DATA_CHANNEL_1_AND_0 + s->nr_channels / 2);
+
+/* Poor man's sampling */
+uint32_t value = s->regs[reg];
+s->regs[reg] = update_channels(s->regs[reg]);
+
+if (breaks_threshold(s, reg)) {
+s->regs[INTERRUPT_CONTROL] |= BIT(reg - DATA_CHANNEL_1_AND_0);
+qemu_irq_ra

Re: [PATCH v3 13/22] target/i386/sev: Remove stubs by using code elision

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Only declare sev_enabled() and sev_es_enabled() when CONFIG_SEV is
set, to allow the compiler to elide unused code. Remove unnecessary
stubs.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/sysemu/sev.h| 14 +-
  target/i386/sev_i386.h  |  3 ---
  target/i386/cpu.c   | 16 +---
  target/i386/sev-stub.c  | 36 
  target/i386/meson.build |  2 +-
  5 files changed, 23 insertions(+), 48 deletions(-)
  delete mode 100644 target/i386/sev-stub.c

diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index a329ed75c1c..f5c625bb3b3 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -14,9 +14,21 @@
  #ifndef QEMU_SEV_H
  #define QEMU_SEV_H
  
-#include "sysemu/kvm.h"

+#ifndef CONFIG_USER_ONLY
+#include CONFIG_DEVICES /* CONFIG_SEV */
+#endif
  
+#ifdef CONFIG_SEV

  bool sev_enabled(void);
+bool sev_es_enabled(void);
+#else
+#define sev_enabled() 0
+#define sev_es_enabled() 0
+#endif


This means that sev.h can only be included from target-specific files.

An alternative could be:

#ifdef NEED_CPU_H
# include CONFIG_DEVICES
#endif

#if defined NEED_CPU_H && !defined CONFIG_SEV
# define sev_enabled() 0
# define sev_es_enabled() 0
#else
bool sev_enabled(void);
bool sev_es_enabled(void);
#endif

... but in fact sysemu/sev.h _is_ only used from x86-specific files. So 
should it be moved to include/hw/i386, and even merged with 
target/i386/sev_i386.h?  Do we need two files?


Thanks,

Paolo


+uint32_t sev_get_cbit_position(void);
+uint32_t sev_get_reduced_phys_bits(void);
+
  int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
  
  #endif

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 0798ab3519a..2d9a1a0112e 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -24,10 +24,7 @@
  #define SEV_POLICY_DOMAIN   0x10
  #define SEV_POLICY_SEV  0x20
  
-extern bool sev_es_enabled(void);

  extern SevInfo *sev_get_info(void);
-extern uint32_t sev_get_cbit_position(void);
-extern uint32_t sev_get_reduced_phys_bits(void);
  extern char *sev_get_launch_measurement(void);
  extern SevCapability *sev_get_capabilities(Error **errp);
  extern SevAttestationReport *
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index e169a01713d..27992bdc9f8 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -25,8 +25,8 @@
  #include "tcg/helper-tcg.h"
  #include "sysemu/reset.h"
  #include "sysemu/hvf.h"
+#include "sysemu/sev.h"
  #include "kvm/kvm_i386.h"
-#include "sev_i386.h"
  #include "qapi/error.h"
  #include "qapi/qapi-visit-machine.h"
  #include "qapi/qmp/qerror.h"
@@ -38,6 +38,7 @@
  #include "exec/address-spaces.h"
  #include "hw/boards.h"
  #include "hw/i386/sgx-epc.h"
+#include "sev_i386.h"
  #endif
  
  #include "disas/capstone.h"

@@ -5764,12 +5765,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
  *edx = 0;
  break;
  case 0x801F:
-*eax = sev_enabled() ? 0x2 : 0;
-*eax |= sev_es_enabled() ? 0x8 : 0;
-*ebx = sev_get_cbit_position();
-*ebx |= sev_get_reduced_phys_bits() << 6;
-*ecx = 0;
-*edx = 0;
+*eax = *ebx = *ecx = *edx = 0;
+if (sev_enabled()) {
+*eax = 0x2;
+*eax |= sev_es_enabled() ? 0x8 : 0;
+*ebx = sev_get_cbit_position();
+*ebx |= sev_get_reduced_phys_bits() << 6;
+}
  break;
  default:
  /* reserved values: zero */
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
deleted file mode 100644
index 8eae5d2fa8d..000
--- a/target/i386/sev-stub.c
+++ /dev/null
@@ -1,36 +0,0 @@
-/*
- * QEMU SEV stub
- *
- * Copyright Advanced Micro Devices 2018
- *
- * Authors:
- *  Brijesh Singh 
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#include "qemu/osdep.h"
-#include "qapi/error.h"
-#include "sev_i386.h"
-
-bool sev_enabled(void)
-{
-return false;
-}
-
-uint32_t sev_get_cbit_position(void)
-{
-return 0;
-}
-
-uint32_t sev_get_reduced_phys_bits(void)
-{
-return 0;
-}
-
-bool sev_es_enabled(void)
-{
-return false;
-}
diff --git a/target/i386/meson.build b/target/i386/meson.build
index a4f45c3ec1d..ae38dc95635 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -6,7 +6,7 @@
'xsave_helper.c',
'cpu-dump.c',
  ))
-i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'), if_false: 
files('sev-stub.c'))
+i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c'))
  
  # x86 cpu type

  i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))

Re: [PATCH v3 14/22] target/i386/sev: Move qmp_query_sev_attestation_report() to sev.c

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Move qmp_query_sev_attestation_report() from monitor.c to sev.c
and make sev_get_attestation_report() static. We don't need the
stub anymore, remove it.

Signed-off-by: Philippe Mathieu-Daudé 


This was done on purpose, but I have no objection to changing it this 
way.  We might in fact remove the indirection for SGX as well, and/or 
even move the implementation of the monitor commands from target/i386 to 
hw/i386 (the monitor is sysemu-specific).


Reviewed-by: Paolo Bonzini 

Thanks,

Paolo


  target/i386/sev_i386.h|  2 --
  target/i386/monitor.c |  6 --
  target/i386/sev-sysemu-stub.c |  7 ---
  target/i386/sev.c | 12 ++--
  4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 2d9a1a0112e..5f367f78eb7 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -27,8 +27,6 @@
  extern SevInfo *sev_get_info(void);
  extern char *sev_get_launch_measurement(void);
  extern SevCapability *sev_get_capabilities(Error **errp);
-extern SevAttestationReport *
-sev_get_attestation_report(const char *mnonce, Error **errp);
  
  int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);

  int sev_inject_launch_secret(const char *hdr, const char *secret,
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index a9f85acd473..c05d70252a2 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -764,12 +764,6 @@ void qmp_sev_inject_launch_secret(const char *packet_hdr,
  sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
  }
  
-SevAttestationReport *

-qmp_query_sev_attestation_report(const char *mnonce, Error **errp)
-{
-return sev_get_attestation_report(mnonce, errp);
-}
-
  SGXInfo *qmp_query_sgx(Error **errp)
  {
  return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index d556b4f091f..813b9a6a03b 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -13,6 +13,7 @@
  
  #include "qemu/osdep.h"

  #include "qapi/qapi-commands-misc-target.h"
+#include "qapi/qmp/qerror.h"
  #include "qapi/error.h"
  #include "sev_i386.h"
  
@@ -52,9 +53,9 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)

  g_assert_not_reached();
  }
  
-SevAttestationReport *sev_get_attestation_report(const char *mnonce,

- Error **errp)
+SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
+   Error **errp)
  {
-error_setg(errp, "SEV is not available in this QEMU");
+error_setg(errp, QERR_UNSUPPORTED);
  return NULL;
  }
diff --git a/target/i386/sev.c b/target/i386/sev.c
index aefbef4bb63..91a217bbb85 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -31,6 +31,8 @@
  #include "migration/blocker.h"
  #include "qom/object.h"
  #include "monitor/monitor.h"
+#include "qapi/qapi-commands-misc-target.h"
+#include "qapi/qmp/qerror.h"
  #include "exec/confidential-guest-support.h"
  #include "hw/i386/pc.h"
  
@@ -487,8 +489,8 @@ out:

  return cap;
  }
  
-SevAttestationReport *

-sev_get_attestation_report(const char *mnonce, Error **errp)
+static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
+Error **errp)
  {
  struct kvm_sev_attestation_report input = {};
  SevAttestationReport *report = NULL;
@@ -549,6 +551,12 @@ sev_get_attestation_report(const char *mnonce, Error 
**errp)
  return report;
  }
  
+SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,

+   Error **errp)
+{
+return sev_get_attestation_report(mnonce, errp);
+}
+
  static int
  sev_read_file_base64(const char *filename, guchar **data, gsize *len)
  {

Re: [PATCH v3 07/22] target/i386/sev_i386.h: Remove unused headers

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Declarations don't require these headers, remove them.

Reviewed-by: Connor Kuehl 
Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/sev_i386.h | 4 
  target/i386/sev-stub.c | 1 +
  2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index ae6d8404787..f4223f1febf 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -14,11 +14,7 @@
  #ifndef QEMU_SEV_I386_H
  #define QEMU_SEV_I386_H
  
-#include "qom/object.h"

-#include "qapi/error.h"
-#include "sysemu/kvm.h"
  #include "sysemu/sev.h"
-#include "qemu/error-report.h"
  #include "qapi/qapi-types-misc-target.h"
  
  #define SEV_POLICY_NODBG0x1

diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 0227cb51778..d91c2ece784 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -12,6 +12,7 @@
   */
  
  #include "qemu/osdep.h"

+#include "qapi/error.h"
  #include "sev_i386.h"
  
  SevInfo *sev_get_info(void)




Reviewed-by: Paolo Bonzini 

Re: [PATCH v3 15/22] target/i386/sev: Move qmp_sev_inject_launch_secret() to sev.c

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Move qmp_sev_inject_launch_secret() from monitor.c to sev.c
and make sev_inject_launch_secret() static. We don't need the
stub anymore, remove it.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/monitor.c | 31 ---
  target/i386/sev-sysemu-stub.c |  6 +++---
  target/i386/sev.c | 31 +++
  3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index c05d70252a2..188203da6f2 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -733,37 +733,6 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
  return sev_get_capabilities(errp);
  }
  
-#define SEV_SECRET_GUID "4c2eb361-7d9b-4cc3-8081-127c90d3d294"

-struct sev_secret_area {
-uint32_t base;
-uint32_t size;
-};
-
-void qmp_sev_inject_launch_secret(const char *packet_hdr,
-  const char *secret,
-  bool has_gpa, uint64_t gpa,
-  Error **errp)
-{
-if (!sev_enabled()) {
-error_setg(errp, QERR_UNSUPPORTED);
-return;
-}
-if (!has_gpa) {
-uint8_t *data;
-struct sev_secret_area *area;
-
-if (!pc_system_ovmf_table_find(SEV_SECRET_GUID, &data, NULL)) {
-error_setg(errp, "SEV: no secret area found in OVMF,"
-   " gpa must be specified.");
-return;
-}
-area = (struct sev_secret_area *)data;
-gpa = area->base;
-}
-
-sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
-}
-
  SGXInfo *qmp_query_sgx(Error **errp)
  {
  return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 813b9a6a03b..66b69540aa5 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -33,10 +33,10 @@ SevCapability *sev_get_capabilities(Error **errp)
  return NULL;
  }
  
-int sev_inject_launch_secret(const char *hdr, const char *secret,

- uint64_t gpa, Error **errp)
+void qmp_sev_inject_launch_secret(const char *packet_header, const char 
*secret,
+  bool has_gpa, uint64_t gpa, Error **errp)
  {
-return 1;
+error_setg(errp, QERR_UNSUPPORTED);
  }
  
  int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 91a217bbb85..2198d550be2 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -949,6 +949,37 @@ int sev_inject_launch_secret(const char *packet_hdr, const 
char *secret,
  return 0;
  }
  
+#define SEV_SECRET_GUID "4c2eb361-7d9b-4cc3-8081-127c90d3d294"

+struct sev_secret_area {
+uint32_t base;
+uint32_t size;
+};
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+  const char *secret,
+  bool has_gpa, uint64_t gpa,
+  Error **errp)
+{
+if (!sev_enabled()) {
+error_setg(errp, QERR_UNSUPPORTED);
+return;
+}
+if (!has_gpa) {
+uint8_t *data;
+struct sev_secret_area *area;
+
+if (!pc_system_ovmf_table_find(SEV_SECRET_GUID, &data, NULL)) {
+error_setg(errp, "SEV: no secret area found in OVMF,"
+   " gpa must be specified.");
+return;
+}
+area = (struct sev_secret_area *)data;
+gpa = area->base;
+}
+
+sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
+}
+
  static int
  sev_es_parse_reset_block(SevInfoBlock *info, uint32_t *addr)
  {



Ok, this indirectly addresses my comment on patch 5.

Reviewed-by: Paolo Bonzini 

Re: [PATCH v3 22/22] MAINTAINERS: Cover AMD SEV files

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Add an entry to list SEV-related files.

Signed-off-by: Philippe Mathieu-Daudé 
---
  MAINTAINERS | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 50435b8d2f5..733a5201e76 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3038,6 +3038,13 @@ F: hw/core/clock-vmstate.c
  F: hw/core/qdev-clock.c
  F: docs/devel/clocks.rst
  
+AMD Secure Encrypted Virtualization (SEV)

+S: Orphan
+F: docs/amd-memory-encryption.txt
+F: target/i386/sev*
+F: target/i386/kvm/sev-stub.c
+F: include/sysemu/sev.h


I don't think it qualifies as orphan; it's covered by x86 maintainers.

Paolo

Re: [PATCH v3 06/22] target/i386/cpu: Add missing 'qapi/error.h' header

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Commit 00b81053244 ("target-i386: Remove assert_no_error usage")
forgot to add the "qapi/error.h" for &error_abort, add it now.

Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Connor Kuehl 
Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/cpu.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index cacec605bf1..e169a01713d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -27,6 +27,7 @@
  #include "sysemu/hvf.h"
  #include "kvm/kvm_i386.h"
  #include "sev_i386.h"
+#include "qapi/error.h"
  #include "qapi/qapi-visit-machine.h"
  #include "qapi/qmp/qerror.h"
  #include "qapi/qapi-commands-machine-target.h"



Reviewed-by: Paolo Bonzini 

Re: [PATCH v3 08/22] target/i386/sev: Remove sev_get_me_mask()

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Unused dead code makes review harder, so remove it.

Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Connor Kuehl 
Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/sev_i386.h | 1 -
  target/i386/sev-stub.c | 5 -
  target/i386/sev.c  | 9 -
  3 files changed, 15 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index f4223f1febf..afa19a0a161 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -25,7 +25,6 @@
  #define SEV_POLICY_SEV  0x20
  
  extern bool sev_es_enabled(void);

-extern uint64_t sev_get_me_mask(void);
  extern SevInfo *sev_get_info(void);
  extern uint32_t sev_get_cbit_position(void);
  extern uint32_t sev_get_reduced_phys_bits(void);
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index d91c2ece784..eb0c89bf2be 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -25,11 +25,6 @@ bool sev_enabled(void)
  return false;
  }
  
-uint64_t sev_get_me_mask(void)

-{
-return ~0;
-}
-
  uint32_t sev_get_cbit_position(void)
  {
  return 0;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index fa7210473a6..c88cd808410 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -64,7 +64,6 @@ struct SevGuestState {
  uint8_t api_major;
  uint8_t api_minor;
  uint8_t build_id;
-uint64_t me_mask;
  int sev_fd;
  SevState state;
  gchar *measurement;
@@ -362,12 +361,6 @@ sev_es_enabled(void)
  return sev_enabled() && (sev_guest->policy & SEV_POLICY_ES);
  }
  
-uint64_t

-sev_get_me_mask(void)
-{
-return sev_guest ? sev_guest->me_mask : ~0;
-}
-
  uint32_t
  sev_get_cbit_position(void)
  {
@@ -804,8 +797,6 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error 
**errp)
  goto err;
  }
  
-sev->me_mask = ~(1UL << sev->cbitpos);

-
  devname = object_property_get_str(OBJECT(sev), "sev-device", NULL);
  sev->sev_fd = open(devname, O_RDWR);
  if (sev->sev_fd < 0) {



RB

Re: [PATCH v3 18/22] target/i386/sev: Move qmp_query_sev() & hmp_info_sev() to sev.c

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Move qmp_query_sev() & hmp_info_sev()() from monitor.c to sev.c
and make sev_get_info() static. We don't need the stub anymore,
remove it. Add a stub for hmp_info_sev().

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/sev_i386.h|  3 ---
  target/i386/monitor.c | 38 +-
  target/i386/sev-sysemu-stub.c | 10 -
  target/i386/sev.c | 39 +--
  4 files changed, 47 insertions(+), 43 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 1699376ad87..15a959d6174 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -15,7 +15,6 @@
  #define QEMU_SEV_I386_H
  
  #include "sysemu/sev.h"

-#include "qapi/qapi-types-misc-target.h"
  
  #define SEV_POLICY_NODBG0x1

  #define SEV_POLICY_NOKS 0x2
@@ -24,8 +23,6 @@
  #define SEV_POLICY_DOMAIN   0x10
  #define SEV_POLICY_SEV  0x20
  
-extern SevInfo *sev_get_info(void);

-
  int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
  int sev_inject_launch_secret(const char *hdr, const char *secret,
   uint64_t gpa, Error **errp);
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 0b38e970c73..890870b252d 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -28,11 +28,9 @@
  #include "monitor/hmp-target.h"
  #include "monitor/hmp.h"
  #include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qerror.h"
+//#include "qapi/qmp/qerror.h"
  #include "sysemu/kvm.h"
-#include "sysemu/sev.h"
  #include "qapi/error.h"
-#include "sev_i386.h"
  #include "qapi/qapi-commands-misc-target.h"
  #include "qapi/qapi-commands-misc.h"
  #include "hw/i386/pc.h"
@@ -677,40 +675,6 @@ void hmp_info_io_apic(Monitor *mon, const QDict *qdict)
 "removed soon. Please use 'info pic' instead.\n");
  }
  
-SevInfo *qmp_query_sev(Error **errp)

-{
-SevInfo *info;
-
-info = sev_get_info();
-if (!info) {
-error_setg(errp, "SEV feature is not available");
-return NULL;
-}
-
-return info;
-}
-
-void hmp_info_sev(Monitor *mon, const QDict *qdict)
-{
-SevInfo *info = sev_get_info();
-
-if (info && info->enabled) {
-monitor_printf(mon, "handle: %d\n", info->handle);
-monitor_printf(mon, "state: %s\n", SevState_str(info->state));
-monitor_printf(mon, "build: %d\n", info->build_id);
-monitor_printf(mon, "api version: %d.%d\n",
-   info->api_major, info->api_minor);
-monitor_printf(mon, "debug: %s\n",
-   info->policy & SEV_POLICY_NODBG ? "off" : "on");
-monitor_printf(mon, "key-sharing: %s\n",
-   info->policy & SEV_POLICY_NOKS ? "off" : "on");
-} else {
-monitor_printf(mon, "SEV is not enabled\n");
-}
-
-qapi_free_SevInfo(info);
-}
-
  SGXInfo *qmp_query_sgx(Error **errp)
  {
  return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 355391c16c4..1836b32e4fc 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -12,13 +12,16 @@
   */
  
  #include "qemu/osdep.h"

+#include "monitor/monitor.h"
+#include "monitor/hmp.h"
  #include "qapi/qapi-commands-misc-target.h"
  #include "qapi/qmp/qerror.h"
  #include "qapi/error.h"
  #include "sev_i386.h"
  
-SevInfo *sev_get_info(void)

+SevInfo *qmp_query_sev(Error **errp)
  {
+error_setg(errp, QERR_UNSUPPORTED);
  return NULL;
  }
  
@@ -60,3 +63,8 @@ SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,

  error_setg(errp, QERR_UNSUPPORTED);
  return NULL;
  }
+
+void hmp_info_sev(Monitor *mon, const QDict *qdict)
+{
+monitor_printf(mon, "SEV is not available in this QEMU\n");
+}
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 8e9cce62196..7caaa117ff7 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -27,10 +27,12 @@
  #include "sev_i386.h"
  #include "sysemu/sysemu.h"
  #include "sysemu/runstate.h"
+#include "sysemu/sev.h"
  #include "trace.h"
  #include "migration/blocker.h"
  #include "qom/object.h"
  #include "monitor/monitor.h"
+#include "monitor/hmp.h"
  #include "qapi/qapi-commands-misc-target.h"
  #include "qapi/qmp/qerror.h"
  #include "exec/confidential-guest-support.h"
@@ -375,8 +377,7 @@ sev_get_reduced_phys_bits(void)
  return sev_guest ? sev_guest->reduced_phys_bits : 0;
  }
  
-SevInfo *

-sev_get_info(void)
+static SevInfo *sev_get_info(void)
  {
  SevInfo *info;
  
@@ -395,6 +396,40 @@ sev_get_info(void)

  return info;
  }
  
+SevInfo *qmp_query_sev(Error **errp)

+{
+SevInfo *info;
+
+info = sev_get_info();
+if (!info) {
+error_setg(errp, "SEV feature is not available");
+return NULL;
+}
+
+return info;
+}
+
+void hmp_info_sev(Monitor *mon, const QDict *qdict)
+{
+SevInfo *info = sev_get_info();
+
+if

Re: [PATCH v3 08/22] target/i386/sev: Remove sev_get_me_mask()

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Unused dead code makes review harder, so remove it.

Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Connor Kuehl 
Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/sev_i386.h | 1 -
  target/i386/sev-stub.c | 5 -
  target/i386/sev.c  | 9 -
  3 files changed, 15 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index f4223f1febf..afa19a0a161 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -25,7 +25,6 @@
  #define SEV_POLICY_SEV  0x20
  
  extern bool sev_es_enabled(void);

-extern uint64_t sev_get_me_mask(void);
  extern SevInfo *sev_get_info(void);
  extern uint32_t sev_get_cbit_position(void);
  extern uint32_t sev_get_reduced_phys_bits(void);
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index d91c2ece784..eb0c89bf2be 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -25,11 +25,6 @@ bool sev_enabled(void)
  return false;
  }
  
-uint64_t sev_get_me_mask(void)

-{
-return ~0;
-}
-
  uint32_t sev_get_cbit_position(void)
  {
  return 0;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index fa7210473a6..c88cd808410 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -64,7 +64,6 @@ struct SevGuestState {
  uint8_t api_major;
  uint8_t api_minor;
  uint8_t build_id;
-uint64_t me_mask;
  int sev_fd;
  SevState state;
  gchar *measurement;
@@ -362,12 +361,6 @@ sev_es_enabled(void)
  return sev_enabled() && (sev_guest->policy & SEV_POLICY_ES);
  }
  
-uint64_t

-sev_get_me_mask(void)
-{
-return sev_guest ? sev_guest->me_mask : ~0;
-}
-
  uint32_t
  sev_get_cbit_position(void)
  {
@@ -804,8 +797,6 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error 
**errp)
  goto err;
  }
  
-sev->me_mask = ~(1UL << sev->cbitpos);

-
  devname = object_property_get_str(OBJECT(sev), "sev-device", NULL);
  sev->sev_fd = open(devname, O_RDWR);
  if (sev->sev_fd < 0) {



Reviewed-by: Paolo Bonzini 

[PULL 3/5] qemu-options: Add missing "sockets=2, maxcpus=2" to CLI "-smp 2"

[Laurent Vivier]

From: Yanan Wang 

There is one numa config example in qemu-options.hx currently
using "-smp 2" and assuming that there will be 2 sockets and
2 cpus totally. However now the actual calculation logic of
missing sockets and cores is not immutable and is considered
liable to change. Although we will get maxcpus=2 finally based
on current parser, it's always stable to specify it explicitly.

So "-smp 2,sockets=2,maxcpus=2" will be optimal when we expect
multiple sockets and 2 cpus totally.

Signed-off-by: Yanan Wang 
Reviewed-by: Philippe Mathieu-Daude 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Andrew Jones 
Message-Id: <20210928121134.21064-3-wangyana...@huawei.com>
Signed-off-by: Laurent Vivier 
---
 qemu-options.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index bba1ef973fec..5f375bbfa666 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -401,7 +401,7 @@ SRST
 -m 2G \
 -object memory-backend-ram,size=1G,id=m0 \
 -object memory-backend-ram,size=1G,id=m1 \
--smp 2 \
+-smp 2,sockets=2,maxcpus=2 \
 -numa node,nodeid=0,memdev=m0 \
 -numa node,nodeid=1,memdev=m1,initiator=0 \
 -numa cpu,node-id=0,socket-id=0 \
-- 
2.31.1

Re: [PATCH v3 09/22] target/i386/sev: Mark unreachable code with g_assert_not_reached()

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

The unique sev_encrypt_flash() invocation (in pc_system_flash_map)
is protected by the "if (sev_enabled())" check, so is not
reacheable.
Replace the abort() call in sev_es_save_reset_vector() by
g_assert_not_reached() which meaning is clearer.

Reviewed-by: Connor Kuehl 
Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/sev-stub.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index eb0c89bf2be..4668365fd3e 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -54,7 +54,7 @@ int sev_inject_launch_secret(const char *hdr, const char 
*secret,
  
  int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)

  {
-return 0;
+g_assert_not_reached();
  }
  
  bool sev_es_enabled(void)

@@ -68,7 +68,7 @@ void sev_es_set_reset_vector(CPUState *cpu)
  
  int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)

  {
-abort();
+g_assert_not_reached();
  }
  
  SevAttestationReport *




Reviewed-by: Paolo Bonzini 

[PULL 5/5] hw/remote/proxy: Categorize Wireless devices as 'Network' ones

[Laurent Vivier]

From: Philippe Mathieu-Daudé 

QEMU doesn't distinct network devices per link layer (Ethernet,
Wi-Fi, CAN, ...). Categorize PCI Wireless cards as Network
devices.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Jagannathan Raman 
Message-Id: <20210926201926.1690896-1-f4...@amsat.org>
Signed-off-by: Laurent Vivier 
---
 hw/remote/proxy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
index 499f540c947d..bad164299dd4 100644
--- a/hw/remote/proxy.c
+++ b/hw/remote/proxy.c
@@ -324,6 +324,7 @@ static void probe_pci_info(PCIDevice *dev, Error **errp)
 set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 break;
 case PCI_BASE_CLASS_NETWORK:
+case PCI_BASE_CLASS_WIRELESS:
 set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
 break;
 case PCI_BASE_CLASS_INPUT:
-- 
2.31.1

Re: [PATCH v3 16/22] target/i386/sev: Move qmp_query_sev_capabilities() to sev.c

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Move qmp_query_sev_capabilities() from monitor.c to sev.c
and make sev_get_capabilities() static. We don't need the
stub anymore, remove it.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/sev_i386.h| 1 -
  target/i386/monitor.c | 5 -
  target/i386/sev-sysemu-stub.c | 4 ++--
  target/i386/sev.c | 8 ++--
  4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 5f367f78eb7..8d9388d8c5c 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -26,7 +26,6 @@
  
  extern SevInfo *sev_get_info(void);

  extern char *sev_get_launch_measurement(void);
-extern SevCapability *sev_get_capabilities(Error **errp);
  
  int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);

  int sev_inject_launch_secret(const char *hdr, const char *secret,
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 188203da6f2..da36522fa15 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -728,11 +728,6 @@ SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error 
**errp)
  return info;
  }
  
-SevCapability *qmp_query_sev_capabilities(Error **errp)

-{
-return sev_get_capabilities(errp);
-}
-
  SGXInfo *qmp_query_sgx(Error **errp)
  {
  return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 66b69540aa5..cc486a1afbe 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -27,9 +27,9 @@ char *sev_get_launch_measurement(void)
  return NULL;
  }
  
-SevCapability *sev_get_capabilities(Error **errp)

+SevCapability *qmp_query_sev_capabilities(Error **errp)
  {
-error_setg(errp, "SEV is not available in this QEMU");
+error_setg(errp, QERR_UNSUPPORTED);
  return NULL;
  }
  
diff --git a/target/i386/sev.c b/target/i386/sev.c

index 2198d550be2..fce007d6749 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -438,8 +438,7 @@ e_free:
  return 1;
  }
  
-SevCapability *

-sev_get_capabilities(Error **errp)
+static SevCapability *sev_get_capabilities(Error **errp)
  {
  SevCapability *cap = NULL;
  guchar *pdh_data = NULL;
@@ -489,6 +488,11 @@ out:
  return cap;
  }
  
+SevCapability *qmp_query_sev_capabilities(Error **errp)

+{
+return sev_get_capabilities(errp);
+}
+
  static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
  Error **errp)
  {



Reviewed-by: Paolo Bonzini 

Re: [PATCH v3 14/16] iotests/linters: Add workaround for mypy bug #9852

[Hanna Reitz]

On 22.09.21 22:38, John Snow wrote:



On Wed, Sep 22, 2021 at 4:37 PM John Snow > wrote:



On Fri, Sep 17, 2021 at 7:16 AM Hanna Reitz mailto:hre...@redhat.com>> wrote:


Question is, when “can we use” mypy >= 0.920? Should we check the
version string and append this switch as required?


The answer to that question depends on how the block maintainers
feel about what environments they expect this test to be runnable
under. I lightly teased kwolf once about an "ancient" version of
pylint they were running, but felt kind of bad about it in
retrospect: the tests I write should "just work" for everyone
without them needing to really know anything about python or
setting up or managing their dependencies, environments, etc.

(1) We can use it the very moment it is released if you're OK with
running 'make check-dev' from the python/ directory. That script
sets up its own virtual environment and manages its own
dependencies. If I set a new minimum version, it will always use
it. (Same story for 'make check-tox', or 'make check-pipenv'. The
differences between the tests are primarily on what other
dependencies they have on your environment -- the details are
boring, see python/Makefile for further reading if desired.)

(2) Otherwise, it depends on how people feel about being able to
run this test directly from iotests and what versions of
mypy/pylint they are using. Fedora 33 for instance has
0.782-2.fc33 still, so I can't really "expect" people to have a
bleeding-edge version of mypy unless they went out of their way to
install one themselves. (pip3 install --user --upgrade mypy, by
the way.) Since people are used to running these python scripts
*outside* of a managed environment (using their OS packages
directly), I have largely made every effort to support versions as
old as I reasonably can -- to avoid disruption whenever I possibly
can.

So, basically, it kind of depends on if you want to keep 297 or
not. Keeping it implies some additional cost for the sake of
maximizing compatibility. If we ditch it, you can let the scripts
in ./python do their thing and set up their own environments to
run tests that should probably "just work" for everyone.297 could
even just be updated to a bash script that just hopped over to
./python and ran a special avocado command that ran /only/ the
iotest linters, if you wanted. I just felt that step #1 was to
change as little as possible, prove the new approach worked, and
then when folks were comfortable with it drop the old approach.


Oh, uh, and to answer your more concrete question: Nah, we don't need 
to conditionally append this workaround. The speed lost from making 
the check incremental is made up for by not invoking the tool 20 
times, so it's OK to just unconditionally add it for now.


Well, yes, but it could be even faster!

And also it would save us from the pain of waiting and judging when it’s 
reasonable to drop `--no-incremental`, and/or just forgetting that this 
workaround even exists.  If we checked the version string, it would be 
OK to just forget about it, I think.


Hanna

[PULL 2/5] qemu-options: Tweak [, maxcpus=cpus] to [, maxcpus=maxcpus]

[Laurent Vivier]

From: Yanan Wang 

In qemu-option.hx, there is "-smp [[cpus=]n][,maxcpus=cpus]..." in the
DEF part, and "-smp [[cpus=]n][,maxcpus=maxcpus]..." in the RST part.
Obviously the later is right, let's fix the previous one.

Signed-off-by: Yanan Wang 
Reviewed-by: Damien Hedde 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Andrew Jones 
Message-Id: <20210928121134.21064-2-wangyana...@huawei.com>
Signed-off-by: Laurent Vivier 
---
 qemu-options.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 4f2dc91e0b27..bba1ef973fec 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -206,7 +206,7 @@ SRST
 ERST
 
 DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-"-smp 
[[cpus=]n][,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
+"-smp 
[[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
 "set the number of CPUs to 'n' [default=1]\n"
 "maxcpus= maximum number of total CPUs, including\n"
 "offline CPUs for hotplug, etc\n"
-- 
2.31.1

Re: [PATCH] hw/misc: applesmc: use host osk as default on macs

[Paolo Bonzini]

On 02/10/21 22:24, Pedro Tôrres wrote:

#define APPLESMC_DEFAULT_IOBASE0x300
@@ -332,7 +497,27 @@ static void applesmc_isa_realize(DeviceState *dev, Error 
**errp)
 s->iobase + APPLESMC_ERR_PORT);

 if (!s->osk || (strlen(s->osk) != 64)) {
+#if defined(__APPLE__) && defined(__MACH__)
+IOReturn ret;
+IOByteCount size = 32;
+
+ret = smc_read_key('OSK0', (uint8_t *) default_osk, &size);
+if (ret != kIOReturnSuccess) {
+goto failure;
+}
+
+ret = smc_read_key('OSK1', (uint8_t *) default_osk + size, &size);
+if (ret != kIOReturnSuccess) {
+goto failure;
+}
+
+warn_report("Using AppleSMC with host key");
+goto success;
+#endif
+failure:
 warn_report("Using AppleSMC with invalid key");
+
+success:
 s->osk = default_osk;
 }

--


I think it is incorrect to use host key if strlen(s->osk) != 64.  So I
would change this code to something like this:

@@ -315,6 +480,7 @@ static const MemoryRegionOps applesmc_err_io_ops = {
 static void applesmc_isa_realize(DeviceState *dev, Error **errp)
 {
 AppleSMCState *s = APPLE_SMC(dev);
+bool valid_key = false;
 
 memory_region_init_io(&s->io_data, OBJECT(s), &applesmc_data_io_ops, s,

   "applesmc-data", 1);
@@ -331,7 +497,31 @@ static void applesmc_isa_realize(DeviceState *dev, Error 
**errp)
 isa_register_ioport(&s->parent_obj, &s->io_err,
 s->iobase + APPLESMC_ERR_PORT);
 
-if (!s->osk || (strlen(s->osk) != 64)) {

+if (s->osk) {
+valid_key = strlen(s->osk) == 64;
+} else {
+#if defined(__APPLE__) && defined(__MACH__)
+IOReturn ret;
+IOByteCount size = 32;
+
+ret = smc_read_key('OSK0', (uint8_t *) default_osk, &size);
+if (ret != kIOReturnSuccess) {
+goto failure;
+}
+
+ret = smc_read_key('OSK1', (uint8_t *) default_osk + size, &size);
+if (ret != kIOReturnSuccess) {
+goto failure;
+}
+
+warn_report("Using AppleSMC with host key");
+valid_key = true;
+s->osk = default_osk;
+failure:
+#endif
+}
+
+if (!valid_key) {
 warn_report("Using AppleSMC with invalid key");
 s->osk = default_osk;
 }

Otherwise looks good, so I queued it (haven't yet compile-tested it, but I
will before sending out my pull request).

Thanks,

Paolo

Re: [PATCH v3 17/22] target/i386/sev: Move qmp_query_sev_launch_measure() to sev.c

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Move qmp_query_sev_launch_measure() from monitor.c to sev.c
and make sev_get_launch_measurement() static. We don't need the
stub anymore, remove it.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/sev_i386.h|  1 -
  target/i386/monitor.c | 17 -
  target/i386/sev-sysemu-stub.c |  3 ++-
  target/i386/sev.c | 20 ++--
  4 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 8d9388d8c5c..1699376ad87 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -25,7 +25,6 @@
  #define SEV_POLICY_SEV  0x20
  
  extern SevInfo *sev_get_info(void);

-extern char *sev_get_launch_measurement(void);
  
  int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);

  int sev_inject_launch_secret(const char *hdr, const char *secret,
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index da36522fa15..0b38e970c73 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -711,23 +711,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict)
  qapi_free_SevInfo(info);
  }
  
-SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)

-{
-char *data;
-SevLaunchMeasureInfo *info;
-
-data = sev_get_launch_measurement();
-if (!data) {
-error_setg(errp, "Measurement is not available");
-return NULL;
-}
-
-info = g_malloc0(sizeof(*info));
-info->data = data;
-
-return info;
-}
-
  SGXInfo *qmp_query_sgx(Error **errp)
  {
  return sgx_get_info(errp);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index cc486a1afbe..355391c16c4 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -22,8 +22,9 @@ SevInfo *sev_get_info(void)
  return NULL;
  }
  
-char *sev_get_launch_measurement(void)

+SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
  {
+error_setg(errp, QERR_UNSUPPORTED);
  return NULL;
  }
  
diff --git a/target/i386/sev.c b/target/i386/sev.c

index fce007d6749..8e9cce62196 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -718,8 +718,7 @@ free_measurement:
  g_free(measurement);
  }
  
-char *

-sev_get_launch_measurement(void)
+static char *sev_get_launch_measurement(void)
  {
  if (sev_guest &&
  sev_guest->state >= SEV_STATE_LAUNCH_SECRET) {
@@ -729,6 +728,23 @@ sev_get_launch_measurement(void)
  return NULL;
  }
  
+SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)

+{
+char *data;
+SevLaunchMeasureInfo *info;
+
+data = sev_get_launch_measurement();
+if (!data) {
+error_setg(errp, "Measurement is not available");
+return NULL;
+}
+
+info = g_malloc0(sizeof(*info));
+info->data = data;
+
+return info;
+}
+
  static Notifier sev_machine_done_notify = {
  .notify = sev_launch_get_measure,
  };



Reviewed-by: Paolo Bonzini 

[PULL 1/5] qemu-options: -chardev reconnect=seconds duplicated in help, tidy up

[Laurent Vivier]

From: Markus Armbruster 

Fixes: 5dd1f02b4bc2f2c2ef3a2adfd8a412c8c8769085
Signed-off-by: Markus Armbruster 
Reviewed-by: Daniel P. Berrangé 
Message-Id: <20210928071449.1416022-1-arm...@redhat.com>
Signed-off-by: Laurent Vivier 
---
 qemu-options.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 8ef178180db6..4f2dc91e0b27 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3202,7 +3202,7 @@ DEFHEADING(Character device options:)
 DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
 "-chardev help\n"
 "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
-"-chardev 
socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off][,reconnect=seconds]\n"
+"-chardev 
socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off]\n"
 " 
[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,mux=on|off]\n"
 " [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID] 
(tcp)\n"
 "-chardev 
socket,id=id,path=path[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds]\n"
-- 
2.31.1

Re: [PATCH 01/12] macfb: handle errors that occur during realize

[Laurent Vivier]

Le 02/10/2021 à 12:59, Mark Cave-Ayland a écrit :
> Make sure any errors that occur within the macfb realize chain are detected
> and handled correctly to prevent crashes and to ensure that error messages are
> reported back to the user.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/display/macfb.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index 76808b69cc..2b747a8de8 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -379,6 +379,10 @@ static void macfb_sysbus_realize(DeviceState *dev, Error 
> **errp)
>  MacfbState *ms = &s->macfb;
>  
>  macfb_common_realize(dev, ms, errp);
> +if (*errp) {
> +return;
> +}
> +
>  sysbus_init_mmio(SYS_BUS_DEVICE(s), &ms->mem_ctrl);
>  sysbus_init_mmio(SYS_BUS_DEVICE(s), &ms->mem_vram);
>  }
> @@ -391,8 +395,15 @@ static void macfb_nubus_realize(DeviceState *dev, Error 
> **errp)
>  MacfbState *ms = &s->macfb;
>  
>  ndc->parent_realize(dev, errp);
> +if (*errp) {
> +return;
> +}
>  
>  macfb_common_realize(dev, ms, errp);
> +if (*errp) {
> +return;
> +}
> +
>  memory_region_add_subregion(&nd->slot_mem, DAFB_BASE, &ms->mem_ctrl);
>  memory_region_add_subregion(&nd->slot_mem, VIDEO_BASE, &ms->mem_vram);
>  }
> 

Perhaps as suggested by Philippe you change macfb_common_realize() to return a 
value and an error?

Otherwise:

Reviewed-by: Laurent Vivier 

Re: [PATCH v3 19/22] monitor: Restrict 'info sev' to x86 targets

[Paolo Bonzini]

On 02/10/21 14:53, Philippe Mathieu-Daudé wrote:

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/monitor/hmp-target.h  | 1 +
  include/monitor/hmp.h | 1 -
  target/i386/sev-sysemu-stub.c | 2 +-
  target/i386/sev.c | 2 +-
  4 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
index dc53add7eef..96956d0fc41 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -49,6 +49,7 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict);
  void hmp_mce(Monitor *mon, const QDict *qdict);
  void hmp_info_local_apic(Monitor *mon, const QDict *qdict);
  void hmp_info_io_apic(Monitor *mon, const QDict *qdict);
+void hmp_info_sev(Monitor *mon, const QDict *qdict);
  void hmp_info_sgx(Monitor *mon, const QDict *qdict);
  
  #endif /* MONITOR_HMP_TARGET_H */

diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 3baa1058e2c..6bc27639e01 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -124,7 +124,6 @@ void hmp_info_ramblock(Monitor *mon, const QDict *qdict);
  void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
  void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
  void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
-void hmp_info_sev(Monitor *mon, const QDict *qdict);
  void hmp_info_replay(Monitor *mon, const QDict *qdict);
  void hmp_replay_break(Monitor *mon, const QDict *qdict);
  void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
index 1836b32e4fc..b2a4033a030 100644
--- a/target/i386/sev-sysemu-stub.c
+++ b/target/i386/sev-sysemu-stub.c
@@ -13,7 +13,7 @@
  
  #include "qemu/osdep.h"

  #include "monitor/monitor.h"
-#include "monitor/hmp.h"
+#include "monitor/hmp-target.h"
  #include "qapi/qapi-commands-misc-target.h"
  #include "qapi/qmp/qerror.h"
  #include "qapi/error.h"
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 7caaa117ff7..c6d8fc52eb2 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -32,7 +32,7 @@
  #include "migration/blocker.h"
  #include "qom/object.h"
  #include "monitor/monitor.h"
-#include "monitor/hmp.h"
+#include "monitor/hmp-target.h"
  #include "qapi/qapi-commands-misc-target.h"
  #include "qapi/qmp/qerror.h"
  #include "exec/confidential-guest-support.h"




This is only a cleanup, isn't it?  The #ifdef is already in 
hmp-commands-info.hx.


Anyway,

Reviewed-by: Paolo Bonzini 

but please adjust the commit message in v4.

Paolo

Re: [PATCH v3 14/16] iotests/linters: Add workaround for mypy bug #9852

[Hanna Reitz]

On 22.09.21 22:37, John Snow wrote:



On Fri, Sep 17, 2021 at 7:16 AM Hanna Reitz > wrote:


On 16.09.21 06:09, John Snow wrote:
> This one is insidious: if you use the invocation
> "from {namespace} import {subpackage}" as mirror-top-perms does,
> mypy will fail on every-other invocation *if* the package being
> imported is a package.
>
> Now, I could just edit mirror-top-perms to avoid this
invocation, but
> since I tripped on a landmine, I might as well head it off at
the pass
> and make sure nobody else trips on the same landmine.
>
> It seems to have something to do with the order in which files are
> checked as well, meaning the random order in which set(os.listdir())
> produces the list of files to test will cause problems
intermittently.
>
> mypy >= 0.920 isn't released yet, so add this workaround for now.
>
> See also:
> https://github.com/python/mypy/issues/11010

> https://github.com/python/mypy/issues/9852

>
> Signed-off-by: John Snow mailto:js...@redhat.com>>
> ---
>   tests/qemu-iotests/linters.py | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/tests/qemu-iotests/linters.py
b/tests/qemu-iotests/linters.py
> index 4df062a973..9c97324e87 100755
> --- a/tests/qemu-iotests/linters.py
> +++ b/tests/qemu-iotests/linters.py
> @@ -100,6 +100,9 @@ def run_linters(
>                   '--warn-unused-ignores',
>                   '--no-implicit-reexport',
>                   '--namespace-packages',
> +                # Until we can use mypy >= 0.920, see
> +                # https://github.com/python/mypy/
issues
/9852

> +                '--no-incremental',
>                   filename,
>               ),

I’m afraid I still don’t really understand this, but I’m happy
with this
given as the reported workaround and you saying it works.

Reviewed-by: Hanna Reitz mailto:hre...@redhat.com>>

Question is, when “can we use” mypy >= 0.920?  Should we check the
version string and append this switch as required?


The answer to that question depends on how the block maintainers feel 
about what environments they expect this test to be runnable under. I 
lightly teased kwolf once about an "ancient" version of pylint they 
were running, but felt kind of bad about it in retrospect: the tests I 
write should "just work" for everyone without them needing to really 
know anything about python or setting up or managing their 
dependencies, environments, etc.


(1) We can use it the very moment it is released if you're OK with 
running 'make check-dev' from the python/ directory. That script sets 
up its own virtual environment and manages its own dependencies. If I 
set a new minimum version, it will always use it. (Same story for 
'make check-tox', or 'make check-pipenv'. The differences between the 
tests are primarily on what other dependencies they have on your 
environment -- the details are boring, see python/Makefile for further 
reading if desired.)


(2) Otherwise, it depends on how people feel about being able to run 
this test directly from iotests and what versions of mypy/pylint they 
are using. Fedora 33 for instance has 0.782-2.fc33 still, so I can't 
really "expect" people to have a bleeding-edge version of mypy unless 
they went out of their way to install one themselves. (pip3 install 
--user --upgrade mypy, by the way.) Since people are used to running 
these python scripts *outside* of a managed environment (using their 
OS packages directly), I have largely made every effort to support 
versions as old as I reasonably can -- to avoid disruption whenever I 
possibly can.


So, basically, it kind of depends on if you want to keep 297 or not. 
Keeping it implies some additional cost for the sake of maximizing 
compatibility. If we ditch it, you can let the scripts in ./python do 
their thing and set up their own environments to run tests that should 
probably "just work" for everyone.297 could even just be updated to a 
bash script that just hopped over to ./python and ran a special 
avocado command that ran /only/ the iotest linters, if you wanted. I 
just felt that step #1 was to change as little as possible, prove the 
new approach worked, and then when folks were comfortable with it drop 
the old approach.


Hm.  So the gist is, since we apparently want to keep 297, we have to 
wait for some indefinite time until in some years or so someone 
remembers this workaround and removes it?


Doesn’t sound quite ideal, but well...

Hanna

Re: [PATCH 04/12] macfb: use memory_region_init_ram() in macfb_common_realize() for the framebuffer

[Laurent Vivier]

Le 02/10/2021 à 12:59, Mark Cave-Ayland a écrit :
> Currently macfb_common_realize() defines the framebuffer RAM memory region as
> being non-migrateable but then immediately registers it for migration. Replace
> memory_region_init_ram_nomigrate() with memory_region_init_ram() which is 
> clearer
> and does exactly the same thing.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/display/macfb.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index f4e789d0d7..e86f64 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -368,11 +368,10 @@ static void macfb_common_realize(DeviceState *dev, 
> MacfbState *s, Error **errp)
>  memory_region_init_io(&s->mem_ctrl, OBJECT(dev), &macfb_ctrl_ops, s,
>"macfb-ctrl", 0x1000);
>  
> -memory_region_init_ram_nomigrate(&s->mem_vram, OBJECT(dev), "macfb-vram",
> - MACFB_VRAM_SIZE, errp);
> +memory_region_init_ram(&s->mem_vram, OBJECT(dev), "macfb-vram",
> +   MACFB_VRAM_SIZE, errp);
>  s->vram = memory_region_get_ram_ptr(&s->mem_vram);
>  s->vram_bit_mask = MACFB_VRAM_SIZE - 1;
> -vmstate_register_ram(&s->mem_vram, dev);
>  memory_region_set_coalescing(&s->mem_vram);
>  }
>  
> 

Reviewed-by: Laurent Vivier 

Re: [PATCH v3 00/22] target/i386/sev: Housekeeping SEV + measured Linux SEV guest

[Paolo Bonzini]

On 02/10/21 14:52, Philippe Mathieu-Daudé wrote:

Hi,

While testing James & Dov patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg810571.html
I wasted some time trying to figure out how OVMF was supposed to
behave until realizing the binary I was using was built without SEV
support... Then wrote this series to help other developers to not
hit the same problem.

Since v2:
- Rebased on top of SGX
- Addressed review comments from Markus / David
- Included/rebased 'Measured Linux SEV guest' from Dov [1]
- Added orphean MAINTAINERS section


I have queued Dov's patches already, but apart from that the changes 
from v3 to v4 should be minimal.


Thanks for this work!

Paolo


[1] 
https://lore.kernel.org/qemu-devel/20210825073538.959525-1-dovmu...@linux.ibm.com/

Supersedes: <20210616204328.2611406-1-phi...@redhat.com>

Dov Murik (2):
   sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux
 boot
   x86/sev: generate SEV kernel loader hashes in x86_load_linux

Dr. David Alan Gilbert (1):
   target/i386/sev: sev_get_attestation_report use g_autofree

Philippe Mathieu-Daudé (19):
   qapi/misc-target: Wrap long 'SEV Attestation Report' long lines
   qapi/misc-target: Group SEV QAPI definitions
   target/i386/kvm: Introduce i386_softmmu_kvm Meson source set
   target/i386/kvm: Restrict SEV stubs to x86 architecture
   target/i386/monitor: Return QMP error when SEV is disabled in build
   target/i386/cpu: Add missing 'qapi/error.h' header
   target/i386/sev_i386.h: Remove unused headers
   target/i386/sev: Remove sev_get_me_mask()
   target/i386/sev: Mark unreachable code with g_assert_not_reached()
   target/i386/sev: Restrict SEV to system emulation
   target/i386/sev: Declare system-specific functions in 'sev_i386.h'
   target/i386/sev: Remove stubs by using code elision
   target/i386/sev: Move qmp_query_sev_attestation_report() to sev.c
   target/i386/sev: Move qmp_sev_inject_launch_secret() to sev.c
   target/i386/sev: Move qmp_query_sev_capabilities() to sev.c
   target/i386/sev: Move qmp_query_sev_launch_measure() to sev.c
   target/i386/sev: Move qmp_query_sev() & hmp_info_sev() to sev.c
   monitor: Restrict 'info sev' to x86 targets
   MAINTAINERS: Cover AMD SEV files

  qapi/misc-target.json |  77 
  include/monitor/hmp-target.h  |   1 +
  include/monitor/hmp.h |   1 -
  include/sysemu/sev.h  |  20 +-
  target/i386/sev_i386.h|  32 +--
  hw/i386/pc_sysfw.c|   2 +-
  hw/i386/x86.c |  25 ++-
  target/i386/cpu.c |  17 +-
  {accel => target/i386}/kvm/sev-stub.c |   0
  target/i386/monitor.c |  92 +
  target/i386/sev-stub.c|  83 
  target/i386/sev-sysemu-stub.c |  70 +++
  target/i386/sev.c | 268 +++---
  MAINTAINERS   |   7 +
  accel/kvm/meson.build |   1 -
  target/i386/kvm/meson.build   |   8 +-
  target/i386/meson.build   |   4 +-
  17 files changed, 438 insertions(+), 270 deletions(-)
  rename {accel => target/i386}/kvm/sev-stub.c (100%)
  delete mode 100644 target/i386/sev-stub.c
  create mode 100644 target/i386/sev-sysemu-stub.c

Re: [RFC PATCH 1/1] virtio: write back features before verify

[Michael S. Tsirkin]

On Mon, Oct 04, 2021 at 04:23:23AM +0200, Halil Pasic wrote:
> On Sat, 2 Oct 2021 14:13:37 -0400
> "Michael S. Tsirkin"  wrote:
> 
> > > Anyone else have an idea? This is a nasty regression; we could revert the
> > > patch, which would remove the symptoms and give us some time, but that
> > > doesn't really feel right, I'd do that only as a last resort.  
> > 
> > Well we have Halil's hack (except I would limit it
> > to only apply to BE, only do devices with validate,
> > and only in modern mode), and we will fix QEMU to be spec compliant.
> > Between these why do we need any conditional compiles?
> 
> We don't. As I stated before, this hack is flawed because it
> effectively breaks fencing features by the driver with QEMU. Some
> features can not be unset after once set, because we tend to try to
> enable the corresponding functionality whenever we see a write
> features operation with the feature bit set, and we don't disable, if a
> subsequent features write operation stores the feature bit as not set.

Something to fix in QEMU too, I think.

> But it looks like VIRTIO_1 is fine to get cleared afterwards.

We'd never clear it though - why would we?

> So my hack
> should actually look like posted below, modulo conditions.


Looking at it some more, I see that vhost-user actually
does not send features to the backend until FEATURES_OK.
However, the code in contrib for vhost-user-blk at least seems
broken wrt endian-ness ATM. What about other backends though?
Hard to be sure right?
Cc Raphael and Stefan so they can take a look.
And I guess it's time we CC'd qemu-devel too.

For now I am beginning to think we should either revert or just limit
validation to LE and think about all this some more. And I am inclining
to do a revert. These are all hypervisors that shipped for a long time.
Do we need a flag for early config space access then?



> 
> Regarding the conditions I guess checking that driver_features has
> F_VERSION_1 already satisfies "only modern mode", or?

Right.

> For now
> I've deliberately omitted the has verify and the is big endian
> conditions so we have a better chance to see if something breaks
> (i.e. the approach does not work). I can add in those extra conditions
> later.

Or maybe if we will go down that road just the verify check (for
performance). I'm a bit unhappy we have the extra exit but consistency
seems more important.

> 
> --8<-
> 
> From: Halil Pasic 
> Date: Thu, 30 Sep 2021 02:38:47 +0200
> Subject: [PATCH] virtio: write back feature VERSION_1 before verify
> 
> This patch fixes a regression introduced by commit 82e89ea077b9
> ("virtio-blk: Add validation for block size in config space") and
> enables similar checks in verify() on big endian platforms.
> 
> The problem with checking multi-byte config fields in the verify
> callback, on big endian platforms, and with a possibly transitional
> device is the following. The verify() callback is called between
> config->get_features() and virtio_finalize_features(). That we have a
> device that offered F_VERSION_1 then we have the following options
> either the device is transitional, and then it has to present the legacy
> interface, i.e. a big endian config space until F_VERSION_1 is
> negotiated, or we have a non-transitional device, which makes
> F_VERSION_1 mandatory, and only implements the non-legacy interface and
> thus presents a little endian config space. Because at this point we
> can't know if the device is transitional or non-transitional, we can't
> know do we need to byte swap or not.

Well we established that we can know. Here's an alternative explanation:

The virtio specification virtio-v1.1-cs01 states:

Transitional devices MUST detect Legacy drivers by detecting that
VIRTIO_F_VERSION_1 has not been acknowledged by the driver.
This is exactly what QEMU as of 6.1 has done relying solely
on VIRTIO_F_VERSION_1 for detecting that.

However, the specification also says:
driver MAY read (but MUST NOT write) the device-specific
configuration fields to check that it can support the device before
accepting it.

In that case, any device relying solely on VIRTIO_F_VERSION_1
for detecting legacy drivers will return data in legacy format.
In particular, this implies that it is in big endian format
for big endian guests. This naturally confuses the driver
which expects little endian in the modern mode.

It is probably a good idea to amend the spec to clarify that
VIRTIO_F_VERSION_1 can only be relied on after the feature negotiation
is complete. However, we already have regression so let's
try to address it.


> 
> The virtio spec explicitly states that the driver MAY read config
> between reading and writing the features so saying that first accessing
> the config before feature negotiation is done is not an option. The
> specifica

Re: [PATCH 05/12] macfb: add trace events for reading and writing the control registers

[Laurent Vivier]

Le 02/10/2021 à 13:00, Mark Cave-Ayland a écrit :
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/display/macfb.c  | 8 +++-
>  hw/display/trace-events | 4 
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index e86f64..62c2727a5b 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -20,6 +20,7 @@
>  #include "qapi/error.h"
>  #include "hw/qdev-properties.h"
>  #include "migration/vmstate.h"
> +#include "trace.h"
>  
>  #define VIDEO_BASE 0x1000
>  #define DAFB_BASE  0x0080
> @@ -289,7 +290,10 @@ static uint64_t macfb_ctrl_read(void *opaque,
>  hwaddr addr,
>  unsigned int size)
>  {
> -return 0;
> +uint64_t val = 0;
> +
> +trace_macfb_ctrl_read(addr, val, size);
> +return val;
>  }
>  
>  static void macfb_ctrl_write(void *opaque,
> @@ -312,6 +316,8 @@ static void macfb_ctrl_write(void *opaque,
>  }
>  break;
>  }
> +
> +trace_macfb_ctrl_write(addr, val, size);
>  }
>  
>  static const MemoryRegionOps macfb_ctrl_ops = {
> diff --git a/hw/display/trace-events b/hw/display/trace-events
> index f03f6655bc..be1353e8e7 100644
> --- a/hw/display/trace-events
> +++ b/hw/display/trace-events
> @@ -167,3 +167,7 @@ sm501_disp_ctrl_read(uint32_t addr, uint32_t val) 
> "addr=0x%x, val=0x%x"
>  sm501_disp_ctrl_write(uint32_t addr, uint32_t val) "addr=0x%x, val=0x%x"
>  sm501_2d_engine_read(uint32_t addr, uint32_t val) "addr=0x%x, val=0x%x"
>  sm501_2d_engine_write(uint32_t addr, uint32_t val) "addr=0x%x, val=0x%x"
> +
> +# macfb.c
> +macfb_ctrl_read(uint64_t addr, uint64_t value, int size) "addr 0x%"PRIx64 " 
> value 0x%"PRIx64 " size %d"
> +macfb_ctrl_write(uint64_t addr, uint64_t value, int size) "addr 0x%"PRIx64 " 
> value 0x%"PRIx64 " size %d"
> 

As suggested by Philippe: size is unsigned

Reviewed-by: Laurent Vivier 

[PULL 0/5] Trivial branch for 6.2 patches

[Laurent Vivier]

The following changes since commit 30bd1db58b09c12b68c35f041f919014b885482d:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2021-10-03 08:45:19 -0400)

are available in the Git repository at:

  git://github.com/vivier/qemu.git tags/trivial-branch-for-6.2-pull-request

for you to fetch changes up to daf0db06308b55c518312abc39a4bf74413ac007:

  hw/remote/proxy: Categorize Wireless devices as 'Network' ones (2021-10-04 
09:47:26 +0200)


Pull request trivial-patches 2021104



Markus Armbruster (1):
  qemu-options: -chardev reconnect=seconds duplicated in help, tidy up

Philippe Mathieu-Daudé (1):
  hw/remote/proxy: Categorize Wireless devices as 'Network' ones

Richard Henderson (1):
  target/sh4: Use lookup_symbol in sh4_tr_disas_log

Yanan Wang (2):
  qemu-options: Tweak [, maxcpus=cpus] to [, maxcpus=maxcpus]
  qemu-options: Add missing "sockets=2, maxcpus=2" to CLI "-smp 2"

 hw/remote/proxy.c  | 1 +
 qemu-options.hx| 6 +++---
 target/sh4/translate.c | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

-- 
2.31.1

Re: [PATCH 1/1] hw: aspeed_gpio: Fix GPIO array indexing

[Cédric Le Goater]

On 9/28/21 05:43, p...@fb.com wrote:

From: Peter Delevoryas 

The gpio array is declared as a dense array:

   qemu_irq gpios[ASPEED_GPIO_NR_PINS];

(AST2500 has 228, AST2400 has 216, AST2600 has 208)

However, this array is used like a matrix of GPIO sets
(e.g. gpio[NR_SETS][NR_PINS_PER_SET] = gpio[8][32])

   size_t offset = set * GPIOS_PER_SET + gpio;
   qemu_set_irq(s->gpios[offset], !!(new & mask));

This can result in an out-of-bounds access to "s->gpios" because the
gpio sets do _not_ have the same length. Some of the groups (e.g.
GPIOAB) only have 4 pins. 228 != 8 * 32 == 256.

To fix this, I converted the gpio array from dense to sparse, to match
both the hardware layout and this existing indexing code.

Fixes: 4b7f956862dc2db4c5c ("hw/gpio: Add basic Aspeed GPIO model for AST2400 and 
AST2500")
Signed-off-by: Peter Delevoryas 



This patch is raising an error in qtest-arm/qom-test when compiled
with clang :

Running test qtest-arm/qom-test
Unexpected error in object_property_try_add() at ../qom/object.c:1224:
qemu-system-arm: attempt to add duplicate property 'gpio0' to object (type 
'aspeed.gpio-ast2600-1_8v')
Broken pipe
ERROR qtest-arm/qom-test - too few tests run (expected 78, got 0)
make: *** [Makefile.mtest:24: run-test-1] Error 1

Thanks,

C.



---
  hw/gpio/aspeed_gpio.c | 72 ++-
  include/hw/gpio/aspeed_gpio.h |  5 +--
  2 files changed, 31 insertions(+), 46 deletions(-)

diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index dfa6d6cb40..f04d4a454c 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -16,11 +16,7 @@
  #include "hw/irq.h"
  #include "migration/vmstate.h"
  
-#define GPIOS_PER_REG 32

-#define GPIOS_PER_SET GPIOS_PER_REG
-#define GPIO_PIN_GAP_SIZE 4
  #define GPIOS_PER_GROUP 8
-#define GPIO_GROUP_SHIFT 3
  
  /* GPIO Source Types */

  #define ASPEED_CMD_SRC_MASK 0x01010101
@@ -259,7 +255,7 @@ static void aspeed_gpio_update(AspeedGPIOState *s, GPIOSets 
*regs,
  
  diff = old ^ new;

  if (diff) {
-for (gpio = 0; gpio < GPIOS_PER_REG; gpio++) {
+for (gpio = 0; gpio < ASPEED_GPIOS_PER_SET; gpio++) {
  uint32_t mask = 1 << gpio;
  
  /* If the gpio needs to be updated... */

@@ -283,8 +279,7 @@ static void aspeed_gpio_update(AspeedGPIOState *s, GPIOSets 
*regs,
  if (direction & mask) {
  /* ...trigger the line-state IRQ */
  ptrdiff_t set = aspeed_gpio_set_idx(s, regs);
-size_t offset = set * GPIOS_PER_SET + gpio;
-qemu_set_irq(s->gpios[offset], !!(new & mask));
+qemu_set_irq(s->gpios[set][gpio], !!(new & mask));
  } else {
  /* ...otherwise if we meet the line's current IRQ policy... */
  if (aspeed_evaluate_irq(regs, old & mask, gpio)) {
@@ -297,21 +292,6 @@ static void aspeed_gpio_update(AspeedGPIOState *s, 
GPIOSets *regs,
  qemu_set_irq(s->irq, !!(s->pending));
  }
  
-static uint32_t aspeed_adjust_pin(AspeedGPIOState *s, uint32_t pin)

-{
-AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
-/*
- * The 2500 has a 4 pin gap in group AB and the 2400 has a 4 pin
- * gap in group Y (and only four pins in AB but this is the last group so
- * it doesn't matter).
- */
-if (agc->gap && pin >= agc->gap) {
-pin += GPIO_PIN_GAP_SIZE;
-}
-
-return pin;
-}
-
  static bool aspeed_gpio_get_pin_level(AspeedGPIOState *s, uint32_t set_idx,
uint32_t pin)
  {
@@ -367,7 +347,7 @@ static uint32_t update_value_control_source(GPIOSets *regs, 
uint32_t old_value,
  uint32_t new_value = 0;
  
  /* for each group in set */

-for (i = 0; i < GPIOS_PER_REG; i += GPIOS_PER_GROUP) {
+for (i = 0; i < ASPEED_GPIOS_PER_SET; i += GPIOS_PER_GROUP) {
  cmd_source = extract32(regs->cmd_source_0, i, 1)
  | (extract32(regs->cmd_source_1, i, 1) << 1);
  
@@ -637,7 +617,7 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data,

   *   bidirectional  |   1   |   1|  data
   *   input only |   1   |   0|   0
   *   output only|   0   |   1|   1
- *   no pin / gap   |   0   |   0|   0
+ *   no pin |   0   |   0|   0
   *
   *  which is captured by:
   *  data = ( data | ~input) & output;
@@ -836,14 +816,20 @@ static void aspeed_gpio_realize(DeviceState *dev, Error 
**errp)
  AspeedGPIOState *s = ASPEED_GPIO(dev);
  SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
  AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
-int pin;
  
  /* Interrupt parent line */

  sysbus_init_irq(sbd, &s->irq);
  
  /* Individual GPIOs */

-for (pin = 0; pin < agc->nr_gpio_pins; pin++) {
-sysbus_init_irq(sbd, &s->gpios[pin]);
+for (int i = 0; i < ASPEED_GPIO_MAX_NR

[PULL 4/5] target/sh4: Use lookup_symbol in sh4_tr_disas_log

[Laurent Vivier]

From: Richard Henderson 

The correct thing to do has been present but commented
out since the initial commit of the sh4 translator.

Fixes: fdf9b3e831e
Signed-off-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210929130316.121330-1-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier 
---
 target/sh4/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index cf5fe9243d6c..d36305027240 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -2344,7 +2344,7 @@ static void sh4_tr_tb_stop(DisasContextBase *dcbase, 
CPUState *cs)
 
 static void sh4_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
 {
-qemu_log("IN:\n");  /* , lookup_symbol(dcbase->pc_first)); */
+qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
 log_target_disas(cs, dcbase->pc_first, dcbase->tb->size);
 }
 
-- 
2.31.1

[PATCH] MAINTAINERS: Split HPPA TCG vs HPPA machines/hardware

[Philippe Mathieu-Daudé]

Hardware emulated models don't belong to the TCG MAINTAINERS
section. Move them to the 'HP-PARISC Machines' section.

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 50435b8d2f5..002620c6cad 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -205,10 +205,7 @@ HPPA (PA-RISC) TCG CPUs
 M: Richard Henderson 
 S: Maintained
 F: target/hppa/
-F: hw/hppa/
 F: disas/hppa.c
-F: hw/net/*i82596*
-F: include/hw/net/lasi_82596.h
 
 M68K TCG CPUs
 M: Laurent Vivier 
@@ -1098,6 +1095,8 @@ R: Helge Deller 
 S: Odd Fixes
 F: configs/devices/hppa-softmmu/default.mak
 F: hw/hppa/
+F: hw/net/*i82596*
+F: include/hw/net/lasi_82596.h
 F: pc-bios/hppa-firmware.img
 
 M68K Machines
-- 
2.31.1

Re: Moving QEMU downloads to GitLab Releases?

[Stefan Hajnoczi]

On Fri, Oct 01, 2021 at 09:39:13AM +0200, Philippe Mathieu-Daudé wrote:
> On 9/30/21 15:40, Stefan Hajnoczi wrote:
> > Hi Mike,
> > QEMU downloads are currently hosted on qemu.org's Apache web server.
> > Paolo and I were discussing ways to reduce qemu.org network traffic to
> > save money and eventually turn off the qemu.org server since there is no
> > full-time sysadmin for it. I'd like to discuss moving QEMU downloads to
> > GitLab Releases.
> > 
> > Since you create and sign QEMU releases I wanted to see what you think
> > about the idea. GitLab Releases has two ways of creating release assets:
> > archiving a git tree and attaching arbitrary binaries. The
> > scripts/make-release script fetches submodules and generates version
> > files, so it may be necessary to treat QEMU tarballs as arbitrary
> > binaries instead of simply letting GitLab create git tree archives:
> > https://docs.gitlab.com/ee/user/project/releases/#use-a-generic-package-for-attaching-binaries
> > 
> > Releases can be uploaded via the GitLab API from your local machine or
> > deployed as a GitLab CI job. Uploading from your local machine would be
> > the closest to the current workflow.
> > 
> > In the long term we could have a CI job that automatically publishes
> > QEMU releases when a new qemu.git tag is pushed. The release process
> > could be fully automated so that manual steps are no longer necessary,
> > although we'd have to trust GitLab with QEMU GPG signing keys.
> 
> Before having to trust a SaaS for GPG signing, could this work?
> 
> - make-release script should produce a reproducible tarball in a
>   gitlab job, along with a file containing the tarball hash.
> 
> - Mike (or whoever is responsible of releases) keeps doing a local
>   manual build
> 
> - Mike checks the local hash matches the Gitlab artifact hash
> 
> - Mike signs its local build/hash and uses the GitLab API to upload
>   that .sig to job artifacts
> 
> - we can have an extra manual job that checks the tarball.sig
>   (hash and pubkey) and on success deploys updating the download
>   page, adding the new release

I wonder what Mike sees as the way forward.

Stefan


signature.asc
Description: PGP signature

Re: [PATCH 03/12] macfb: fix overflow of color_palette array

[Laurent Vivier]

Le 02/10/2021 à 12:59, Mark Cave-Ayland a écrit :
> The palette_current index counter has a maximum size of 256 * 3 to cover a 
> full
> color palette of 256 RGB entries. Linux assumes that the palette_current index
> wraps back around to zero after writing 256 RGB entries so ensure that
> palette_current is reset at this point to prevent data corruption within
> MacfbState.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/display/macfb.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index 815870f2fe..f4e789d0d7 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -307,6 +307,9 @@ static void macfb_ctrl_write(void *opaque,
>  if (s->palette_current % 3) {
>  macfb_invalidate_display(s);
>  }
> +if (s->palette_current >= sizeof(s->color_palette)) {
> +s->palette_current = 0;
> +}
>  break;
>  }
>  }
> 

What about "s->palette_current = (s->palette_current + 1) % 
sizeof(s->color_palette)"?

Thanks,
Laurent

Re: [PATCH 02/12] macfb: fix invalid object reference in macfb_common_realize()

[Laurent Vivier]

Le 02/10/2021 à 12:59, Mark Cave-Ayland a écrit :
> During realize memory_region_init_ram_nomigrate() is used to initialise the 
> RAM
> memory region used for the framebuffer but the owner object reference is
> incorrect since MacFbState is a typedef and not a QOM type.
> 
> Change the memory region owner to be the corresponding DeviceState to fix the
> issue and prevent random crashes during macfb_common_realize().
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/display/macfb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index 2b747a8de8..815870f2fe 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -365,7 +365,7 @@ static void macfb_common_realize(DeviceState *dev, 
> MacfbState *s, Error **errp)
>  memory_region_init_io(&s->mem_ctrl, OBJECT(dev), &macfb_ctrl_ops, s,
>"macfb-ctrl", 0x1000);
>  
> -memory_region_init_ram_nomigrate(&s->mem_vram, OBJECT(s), "macfb-vram",
> +memory_region_init_ram_nomigrate(&s->mem_vram, OBJECT(dev), "macfb-vram",
>   MACFB_VRAM_SIZE, errp);
>  s->vram = memory_region_get_ram_ptr(&s->mem_vram);
>  s->vram_bit_mask = MACFB_VRAM_SIZE - 1;
> 

Reviewed-by: Laurent Vivier 

Re: [PATCH 06/12] macfb: implement mode sense to allow display type to be detected

[Laurent Vivier]

Le 02/10/2021 à 13:00, Mark Cave-Ayland a écrit :
> The MacOS toolbox ROM uses the monitor sense to detect the display type and 
> then
> offer a fixed set of resolutions and colour depths accordingly. Implement the
> monitor sense using information found in Apple Technical Note HW26: "Macintosh
> Quadra Built-In Video" along with some local experiments.
> 
> Since the default configuration is 640 x 480 with 8-bit colour then hardcode
> the sense register to return MACFB_DISPLAY_VGA for now.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/display/macfb.c | 117 -
>  hw/display/trace-events|   2 +
>  include/hw/display/macfb.h |  20 +++
>  3 files changed, 137 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index 62c2727a5b..5c95aa4a11 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -28,8 +28,66 @@
>  #define MACFB_PAGE_SIZE 4096
>  #define MACFB_VRAM_SIZE (4 * MiB)
>  
> -#define DAFB_RESET  0x200
> -#define DAFB_LUT0x213
> +#define DAFB_MODE_SENSE 0x1c
> +#define DAFB_RESET  0x200
> +#define DAFB_LUT0x213
> +
> +
> +/*
> + * Quadra sense codes taken from Apple Technical Note HW26:
> + * "Macintosh Quadra Built-In Video". The sense codes and

https://developer.apple.com/library/archive/technotes/hw/hw_26.html

> + * extended sense codes have different meanings:
> + *
> + * Sense:
> + *bit 2: SENSE2 (pin 10)
> + *bit 1: SENSE1 (pin 7)
> + *bit 0: SENSE0 (pin 4)
> + *
> + * 0 = pin tied to ground
> + * 1 = pin unconnected
> + *
> + * Extended Sense:
> + *bit 2: pins 4-10
> + *bit 1: pins 10-7
> + *bit 0: pins 7-4
> + *
> + * 0 = pins tied together
> + * 1 = pins unconnected
> + *
> + * Reads from the sense register appear to be active low, i.e. a 1 indicates
> + * that the pin is tied to ground, a 0 indicates the pin is disconnected.
> + *
> + * Writes to the sense register appear to activate pulldowns i.e. a 1 enables
> + * a pulldown on a particular pin.
> + *
> + * The MacOS toolbox appears to use a series of reads and writes to first
> + * determine if extended sense is to be used, and then check which pins are
> + * tied together in order to determine the display type.
> + */
> +
> +typedef struct MacFbSense {
> +uint8_t type;
> +uint8_t sense;
> +uint8_t ext_sense;
> +} MacFbSense;
> +
> +static MacFbSense macfb_sense_table[] = {
> +{ MACFB_DISPLAY_APPLE_21_COLOR, 0x0, 0 },
> +{ MACFB_DISPLAY_APPLE_PORTRAIT, 0x1, 0 },
> +{ MACFB_DISPLAY_APPLE_12_RGB, 0x2, 0 },
> +{ MACFB_DISPLAY_APPLE_2PAGE_MONO, 0x3, 0 },
> +{ MACFB_DISPLAY_NTSC_UNDERSCAN, 0x4, 0 },
> +{ MACFB_DISPLAY_NTSC_OVERSCAN, 0x4, 0 },
> +{ MACFB_DISPLAY_APPLE_12_MONO, 0x6, 0 },
> +{ MACFB_DISPLAY_APPLE_13_RGB, 0x6, 0 },
> +{ MACFB_DISPLAY_16_COLOR, 0x7, 0x3 },
> +{ MACFB_DISPLAY_PAL1_UNDERSCAN, 0x7, 0x0 },
> +{ MACFB_DISPLAY_PAL1_OVERSCAN, 0x7, 0x0 },
> +{ MACFB_DISPLAY_PAL2_UNDERSCAN, 0x7, 0x6 },
> +{ MACFB_DISPLAY_PAL2_OVERSCAN, 0x7, 0x6 },
> +{ MACFB_DISPLAY_VGA, 0x7, 0x5 },
> +{ MACFB_DISPLAY_SVGA, 0x7, 0x5 },

Perhaps it could be interesting to also have the "no external monitor" entry?
But generally not to have monitor prevents the boot of MacOS.

...

Reviewed-by: Laurent Vivier 

Re: Moving QEMU downloads to GitLab Releases?

[Stefan Hajnoczi]

On Fri, Oct 01, 2021 at 09:52:20AM +0100, Daniel P. Berrangé wrote:
> On Fri, Oct 01, 2021 at 08:11:35AM +0100, Stefan Hajnoczi wrote:
> > We need to keep the security of QEMU releases in mind. Mike Roth
> > signs and publishes releases. Whoever facilitates or hosts the files
> > should not be able to modify the files after Mike has blessed them. One
> > way to do this is to keep hosting the .sig files on download.qemu.org
> > and to redirect the actual tarballs to a file hosting provider. A way to
> > securely publish files without hosting anything on qemu.org would be
> > even better though (maybe it's enough to publish signatures on the
> > static GitLab Pages website).
> 
> If someone modifies the download files, then when you verify the sig
> it will be detected. It doesn't matter whether the sig is on the same
> host or not, because if someone modifies the sig too, then it will
> still fail validation. The important thing is that the user has got
> the right public key to verify with.
> 
> IOW, hosting the .sig separately is not required. We need to ensure
> that our public key, however, is published & discoverable in a
> trustworthy place that is separate from the download server. We fail
> at that today because www.qemu.org and download.qemu.org are the
> same server.
> 
> So it will be beneficial if the download site is split off from
> the public website, compared to our current setup.

You're right. Thanks for pointing this out. I was thinking of the .sig
as a checksum but it's a signature :-).

Stefan


signature.asc
Description: PGP signature

Re: [PATCH v3 2/3] hw/virtio: Acquire RCU read lock in virtqueue_packed_drop_all()

[Stefan Hajnoczi]

On Mon, Sep 06, 2021 at 12:43:17PM +0200, Philippe Mathieu-Daudé wrote:
> vring_get_region_caches() must be called with the RCU read lock
> acquired. virtqueue_packed_drop_all() does not, and uses the
> 'caches' pointer. Fix that by using the RCU_READ_LOCK_GUARD()
> macro.

Is this a bug that has been encountered, is it a latent bug, a code
cleanup, etc? The impact of this isn't clear but it sounds a little
scary so I wanted to check.

> 
> Reported-by: Stefano Garzarella 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/virtio/virtio.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 97f60017466..7d3bf9091ee 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1704,6 +1704,8 @@ static unsigned int virtqueue_packed_drop_all(VirtQueue 
> *vq)
>  VirtIODevice *vdev = vq->vdev;
>  VRingPackedDesc desc;
>  
> +RCU_READ_LOCK_GUARD();
> +
>  caches = vring_get_region_caches(vq);
>  if (!caches) {
>  return 0;
> -- 
> 2.31.1
> 


signature.asc
Description: PGP signature

Re: [PATCH 03/15] python/aqmp: Return cleared events from EventListener.clear()

[Hanna Reitz]

On 17.09.21 19:19, John Snow wrote:



On Fri, Sep 17, 2021 at 8:36 AM Hanna Reitz > wrote:


On 17.09.21 07:40, John Snow wrote:
> This serves two purposes:
>
> (1) It is now possible to discern whether or not clear() removed any
> event(s) from the queue with absolute certainty, and
>
> (2) It is now very easy to get a List of all pending events in one
> chunk, which is useful for the sync bridge.
>
> Signed-off-by: John Snow mailto:js...@redhat.com>>
> ---
>   python/qemu/aqmp/events.py | 9 +++--
>   1 file changed, 7 insertions(+), 2 deletions(-)

Not sure if `clear` is an ideal name then, but `drain` sounds like
something that would block, and `drop` is really much different from
`clear`.  Also, doesn’t matter, having Collection.delete return the
deleted element is a common thing in any language’s standard
library, so
why not have `clear` do the same.


It isn't too late to change the name, but it sounds like you don't 
necessarily prefer any of those others over what's there now.


Oh, no, I was just thinking aloud.

Hanna

Re: [PATCH v3 3/3] hw/virtio: Have virtqueue_get_avail_bytes() pass caches arg to callees

[Stefan Hajnoczi]

On Mon, Sep 06, 2021 at 12:43:18PM +0200, Philippe Mathieu-Daudé wrote:
> Both virtqueue_packed_get_avail_bytes() and
> virtqueue_split_get_avail_bytes() access the region cache, but
> their caller also does. Simplify by having virtqueue_get_avail_bytes
> calling both with RCU lock held, and passing the caches as argument.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/virtio/virtio.c | 29 -
>  1 file changed, 12 insertions(+), 17 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

[PATCH 3/5] hexagon: use env keyword argument to pass PYTHONPATH

[Paolo Bonzini]

This feature is new in meson 0.57 and allows getting rid of the "env" wrapper.

Signed-off-by: Paolo Bonzini 
---
 target/hexagon/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
index 6fd9360b74..c6d858ffb2 100644
--- a/target/hexagon/meson.build
+++ b/target/hexagon/meson.build
@@ -156,7 +156,8 @@ dectree_generated = custom_target(
 'dectree_generated.h.inc',
 output: 'dectree_generated.h.inc',
 depends: [iset_py],
-command: ['env', 'PYTHONPATH=' + meson.current_build_dir(), 
files('dectree.py'), '@OUTPUT@'],
+env: {'PYTHONPATH': meson.current_build_dir()},
+command: [python, files('dectree.py'), '@OUTPUT@'],
 )
 hexagon_ss.add(dectree_generated)
 
-- 
2.31.1

Re: [PATCH v3 1/3] hw/virtio: Comment virtqueue_flush() must be called with RCU read lock

[Stefan Hajnoczi]

On Mon, Sep 27, 2021 at 01:29:46PM +0200, Cornelia Huck wrote:
> On Mon, Sep 27 2021, Philippe Mathieu-Daudé  wrote:
> 
> > On 9/27/21 12:18, Cornelia Huck wrote:
> >> On Mon, Sep 06 2021, Philippe Mathieu-Daudé  wrote:
> >> 
> >>> Reported-by: Stefano Garzarella 
> >>> Suggested-by: Stefan Hajnoczi 
> >>> Signed-off-by: Philippe Mathieu-Daudé 
> >>> ---
> >>>  include/hw/virtio/virtio.h | 7 +++
> >>>  hw/virtio/virtio.c | 1 +
> >>>  2 files changed, 8 insertions(+)
> >>>
> >>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> >>> index 8bab9cfb750..c1c5f6e53c8 100644
> >>> --- a/include/hw/virtio/virtio.h
> >>> +++ b/include/hw/virtio/virtio.h
> >>> @@ -186,6 +186,13 @@ void virtio_delete_queue(VirtQueue *vq);
> >>>  
> >>>  void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
> >>>  unsigned int len);
> >>> +/**
> >>> + * virtqueue_flush:
> >>> + * @vq: The #VirtQueue
> >>> + * @count: Number of elements to flush
> >>> + *
> >>> + * Must be called within RCU critical section.
> >>> + */
> >> 
> >> Hm... do these doc comments belong into .h files, or rather into .c files?
> >
> > Maybe we should restart this old thread, vote(?) and settle on a style?
> >
> > https://lore.kernel.org/qemu-devel/349cd87b-0526-30b8-d9cd-0eee537ab...@redhat.com/
> 
> My vote would still go to putting this into .c files :) Not sure if we
> want to go through the hassle of a wholesale cleanup; but if others
> agree, we could at least start with putting new doc comments next to the
> implementation.

In the virtio.c/h case doc comments (and especially the RCU-related
ones) are in the .c file. The exception is that constants and structs
are documented in the header file.

I would follow that and avoid duplicating doc comments into the .h file.

Stefan


signature.asc
Description: PGP signature

Re: [PATCH 07/12] macfb: add qdev property to specify display type

[Laurent Vivier]

Le 02/10/2021 à 13:00, Mark Cave-Ayland a écrit :
> Since the available resolutions and colour depths are determined by the 
> attached
> display type, add a qdev property to allow the display type to be specified.
> 
> The main resolutions of interest are high resolution 1152x870 with 8-bit 
> colour
> and SVGA resolution up to 800x600 with 32-bit colour so update the q800 
> machine
> to allow high resolution mode if specified and otherwise fall back to SVGA.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/display/macfb.c | 6 +-
>  hw/m68k/q800.c | 5 +
>  include/hw/display/macfb.h | 1 +
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index 5c95aa4a11..023d1f0cd1 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -316,7 +316,7 @@ static uint32_t macfb_sense_read(MacfbState *s)
>  MacFbSense *macfb_sense;
>  uint8_t sense;
>  
> -macfb_sense = &macfb_sense_table[MACFB_DISPLAY_VGA];
> +macfb_sense = &macfb_sense_table[s->type];
>  if (macfb_sense->sense == 0x7) {
>  /* Extended sense */
>  sense = 0;
> @@ -545,6 +545,8 @@ static Property macfb_sysbus_properties[] = {
>  DEFINE_PROP_UINT32("width", MacfbSysBusState, macfb.width, 640),
>  DEFINE_PROP_UINT32("height", MacfbSysBusState, macfb.height, 480),
>  DEFINE_PROP_UINT8("depth", MacfbSysBusState, macfb.depth, 8),
> +DEFINE_PROP_UINT8("display", MacfbSysBusState, macfb.type,
> +  MACFB_DISPLAY_VGA),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -552,6 +554,8 @@ static Property macfb_nubus_properties[] = {
>  DEFINE_PROP_UINT32("width", MacfbNubusState, macfb.width, 640),
>  DEFINE_PROP_UINT32("height", MacfbNubusState, macfb.height, 480),
>  DEFINE_PROP_UINT8("depth", MacfbNubusState, macfb.depth, 8),
> +DEFINE_PROP_UINT8("display", MacfbNubusState, macfb.type,
> +  MACFB_DISPLAY_VGA),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 09b3366024..5223b880bc 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -421,6 +421,11 @@ static void q800_init(MachineState *machine)
>  qdev_prop_set_uint32(dev, "width", graphic_width);
>  qdev_prop_set_uint32(dev, "height", graphic_height);
>  qdev_prop_set_uint8(dev, "depth", graphic_depth);
> +if (graphic_width == 1152 && graphic_height == 870 && graphic_depth == 
> 8) {
> +qdev_prop_set_uint8(dev, "display", MACFB_DISPLAY_APPLE_21_COLOR);
> +} else {
> +qdev_prop_set_uint8(dev, "display", MACFB_DISPLAY_VGA);
> +}
>  qdev_realize_and_unref(dev, BUS(nubus), &error_fatal);
>  
>  cs = CPU(cpu);
> diff --git a/include/hw/display/macfb.h b/include/hw/display/macfb.h
> index febf4ce0e8..e95a97ebdc 100644
> --- a/include/hw/display/macfb.h
> +++ b/include/hw/display/macfb.h
> @@ -46,6 +46,7 @@ typedef struct MacfbState {
>  uint8_t color_palette[256 * 3];
>  uint32_t width, height; /* in pixels */
>  uint8_t depth;
> +uint8_t type;
>  
>  uint32_t sense;
>  } MacfbState;
> 

I think the display modes should be documentend somewhere to be directly usable 
by the user and get
ride of the graphic_XXX variables (and -g).

Perhaps it could also be merged with the previous one.

Thanks,
Laurent

[PATCH 4/4] MAINTAINERS: Agree to maintain nanoMIPS TCG frontend

[Philippe Mathieu-Daudé]

As of this commit, the nanoMIPS toolchains haven't been merged
in mainstream projects. However MediaTek provides a toolchain:
https://github.com/MediaTek-Labs/nanomips-gnu-toolchain/releases/tag/nanoMIPS-2021.02-01

Since I now have spent more time with the ISA, I agree to maintain
it along with the other MIPS ISA.

For historical notes, see commit a60442eb8 ("Deprecate nanoMIPS ISA").

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f1d7279a0f2..8ce47417eff 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -237,14 +237,10 @@ R: Aleksandar Rikalo 
 S: Odd Fixes
 F: target/mips/
 F: disas/mips.c
+X: disas/nanomips.*
 F: docs/system/cpu-models-mips.rst.inc
 F: tests/tcg/mips/
 
-MIPS TCG CPUs (nanoMIPS ISA)
-S: Orphan
-F: disas/nanomips.*
-F: target/mips/tcg/*nanomips*
-
 NiosII TCG CPUs
 M: Chris Wulff 
 M: Marek Vasut 
-- 
2.31.1

[PATCH 0/4] MAINTAINERS: Sanitize 'MIPS TCG CPUs' section

[Philippe Mathieu-Daudé]

Move various files unrelated to MIPS TCG frontend into
new sections.

Philippe Mathieu-Daudé (4):
  MAINTAINERS: Add MIPS general architecture support entry
  MAINTAINERS: Add entries to cover MIPS CPS / GIC hardware
  MAINTAINERS: Split MIPS TCG frontend vs MIPS machines/hardware
  MAINTAINERS: Agree to maintain nanoMIPS TCG frontend

 MAINTAINERS | 45 ++---
 1 file changed, 30 insertions(+), 15 deletions(-)

-- 
2.31.1

Re: [PATCH v3 2/3] hw/virtio: Acquire RCU read lock in virtqueue_packed_drop_all()

[Philippe Mathieu-Daudé]

On 10/4/21 11:23, Stefan Hajnoczi wrote:
> On Mon, Sep 06, 2021 at 12:43:17PM +0200, Philippe Mathieu-Daudé wrote:
>> vring_get_region_caches() must be called with the RCU read lock
>> acquired. virtqueue_packed_drop_all() does not, and uses the
>> 'caches' pointer. Fix that by using the RCU_READ_LOCK_GUARD()
>> macro.
> 
> Is this a bug that has been encountered, is it a latent bug, a code
> cleanup, etc? The impact of this isn't clear but it sounds a little
> scary so I wanted to check.

I'll defer to Stefano, but IIUC it is a latent bug discovered
during code audit.

> 
>>
>> Reported-by: Stefano Garzarella 
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  hw/virtio/virtio.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 97f60017466..7d3bf9091ee 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -1704,6 +1704,8 @@ static unsigned int 
>> virtqueue_packed_drop_all(VirtQueue *vq)
>>  VirtIODevice *vdev = vq->vdev;
>>  VRingPackedDesc desc;
>>  
>> +RCU_READ_LOCK_GUARD();
>> +
>>  caches = vring_get_region_caches(vq);
>>  if (!caches) {
>>  return 0;
>> -- 
>> 2.31.1
>>

[PATCH 1/4] MAINTAINERS: Add MIPS general architecture support entry

[Philippe Mathieu-Daudé]

The architecture is covered in TCG (frontend and backend)
and hardware models. Add a generic section matching the
'mips' word in patch subjects.

Cc: Jiaxun Yang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 50435b8d2f5..cfee52a3046 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -109,6 +109,12 @@ K: ^Subject:.*(?i)s390x?
 T: git https://gitlab.com/cohuck/qemu.git s390-next
 L: qemu-s3...@nongnu.org
 
+MIPS general architecture support
+M: Philippe Mathieu-Daudé 
+R: Jiaxun Yang 
+S: Odd Fixes
+K: ^Subject:.*(?i)mips
+
 Guest CPU cores (TCG)
 -
 Overall TCG CPUs
@@ -242,7 +248,6 @@ F: include/hw/mips/
 F: include/hw/misc/mips_*
 F: include/hw/timer/mips_gictimer.h
 F: tests/tcg/mips/
-K: ^Subject:.*(?i)mips
 
 MIPS TCG CPUs (nanoMIPS ISA)
 S: Orphan
-- 
2.31.1

[PATCH 2/4] MAINTAINERS: Add entries to cover MIPS CPS / GIC hardware

[Philippe Mathieu-Daudé]

MIPS CPS and GIC models are unrelated to the TCG frontend.
Move them as new sections under the 'Devices' group.

Cc: Paul Burton 
Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index cfee52a3046..a5268ad0651 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -239,14 +239,8 @@ F: target/mips/
 F: configs/devices/mips*/*
 F: disas/mips.c
 F: docs/system/cpu-models-mips.rst.inc
-F: hw/intc/mips_gic.c
 F: hw/mips/
-F: hw/misc/mips_*
-F: hw/timer/mips_gictimer.c
-F: include/hw/intc/mips_gic.h
 F: include/hw/mips/
-F: include/hw/misc/mips_*
-F: include/hw/timer/mips_gictimer.h
 F: tests/tcg/mips/
 
 MIPS TCG CPUs (nanoMIPS ISA)
@@ -2271,6 +2265,22 @@ S: Odd Fixes
 F: hw/intc/openpic.c
 F: include/hw/ppc/openpic.h
 
+MIPS CPS
+M: Paul Burton 
+M: Philippe Mathieu-Daudé 
+S: Odd Fixes
+F: hw/misc/mips_*
+F: include/hw/misc/mips_*
+
+MIPS GIC
+M: Paul Burton 
+M: Philippe Mathieu-Daudé 
+S: Odd Fixes
+F: hw/intc/mips_gic.c
+F: hw/timer/mips_gictimer.c
+F: include/hw/intc/mips_gic.h
+F: include/hw/timer/mips_gictimer.h
+
 Subsystems
 --
 Overall Audio backends
-- 
2.31.1

[PATCH 5/5] meson: show library versions in the summary

[Paolo Bonzini]

Meson 0.57 allows passing external programs and dependency objects
to summary().  Use this to show library versions and paths in the
summary.

Signed-off-by: Paolo Bonzini 
---
 meson.build | 112 +---
 1 file changed, 54 insertions(+), 58 deletions(-)

diff --git a/meson.build b/meson.build
index 17e77fe4ef..7b596fdcd9 100644
--- a/meson.build
+++ b/meson.build
@@ -2859,13 +2859,13 @@ summary_info = {}
 summary_info += {'git':   config_host['GIT']}
 summary_info += {'make':  config_host['MAKE']}
 summary_info += {'python':'@0@ (version: 
@1@)'.format(python.full_path(), python.language_version())}
-summary_info += {'sphinx-build':  sphinx_build.found()}
+summary_info += {'sphinx-build':  sphinx_build}
 if config_host.has_key('HAVE_GDB_BIN')
   summary_info += {'gdb': config_host['HAVE_GDB_BIN']}
 endif
 summary_info += {'genisoimage':   config_host['GENISOIMAGE']}
 if targetos == 'windows' and config_host.has_key('CONFIG_GUEST_AGENT')
-  summary_info += {'wixl':wixl.found() ? wixl.full_path() : false}
+  summary_info += {'wixl':wixl}
 endif
 if slirp_opt != 'disabled' and 'CONFIG_SLIRP_SMBD' in config_host
   summary_info += {'smbd':config_host['CONFIG_SMBD_COMMAND']}
@@ -2956,7 +2956,7 @@ if get_option('cfi')
   summary_info += {'CFI debug support': get_option('cfi_debug')}
 endif
 summary_info += {'strip binaries':get_option('strip')}
-summary_info += {'sparse':sparse.found() ? sparse.full_path() : 
false}
+summary_info += {'sparse':sparse}
 summary_info += {'mingw32 support':   targetos == 'windows'}
 
 # snarf the cross-compilation information for tests
@@ -3028,19 +3028,19 @@ if have_block
   summary_info += {'vvfat support': config_host.has_key('CONFIG_VVFAT')}
   summary_info += {'qed support':   config_host.has_key('CONFIG_QED')}
   summary_info += {'parallels support': 
config_host.has_key('CONFIG_PARALLELS')}
-  summary_info += {'FUSE exports':  fuse.found()}
+  summary_info += {'FUSE exports':  fuse}
 endif
 summary(summary_info, bool_yn: true, section: 'Block layer support')
 
 # Crypto
 summary_info = {}
 summary_info += {'TLS priority':  config_host['CONFIG_TLS_PRIORITY']}
-summary_info += {'GNUTLS support':gnutls.found()}
-summary_info += {'GNUTLS crypto': gnutls_crypto.found()}
-# TODO: add back version
-summary_info += {'libgcrypt': gcrypt.found()}
-# TODO: add back version
-summary_info += {'nettle':nettle.found()}
+summary_info += {'GNUTLS support':gnutls}
+if gnutls.found()
+  summary_info += {'  GNUTLS crypto':   gnutls_crypto.found()}
+endif
+summary_info += {'libgcrypt': gcrypt}
+summary_info += {'nettle':nettle}
 if nettle.found()
summary_info += {'  XTS': xts != 'private'}
 endif
@@ -3052,76 +3052,72 @@ summary(summary_info, bool_yn: true, section: 'Crypto')
 # Libraries
 summary_info = {}
 if targetos == 'darwin'
-  summary_info += {'Cocoa support':   cocoa.found()}
+  summary_info += {'Cocoa support':   cocoa}
 endif
-# TODO: add back version
-summary_info += {'SDL support':   sdl.found()}
-summary_info += {'SDL image support': sdl_image.found()}
-# TODO: add back version
-summary_info += {'GTK support':   gtk.found()}
-summary_info += {'pixman':pixman.found()}
-# TODO: add back version
-summary_info += {'VTE support':   vte.found()}
-# TODO: add back version
-summary_info += {'slirp support': slirp_opt == 'disabled' ? false : 
slirp_opt}
-summary_info += {'libtasn1':  tasn1.found()}
-summary_info += {'PAM':   pam.found()}
-summary_info += {'iconv support': iconv.found()}
-summary_info += {'curses support':curses.found()}
-# TODO: add back version
-summary_info += {'virgl support': virgl.found()}
-summary_info += {'curl support':  curl.found()}
-summary_info += {'Multipath support': mpathpersist.found()}
-summary_info += {'VNC support':   vnc.found()}
+summary_info += {'SDL support':   sdl}
+summary_info += {'SDL image support': sdl_image}
+summary_info += {'GTK support':   gtk}
+summary_info += {'pixman':pixman}
+summary_info += {'VTE support':   vte}
+summary_info += {'slirp support': slirp_opt == 'internal' ? slirp_opt : 
slirp}
+summary_info += {'libtasn1':  tasn1}
+summary_info += {'PAM':   pam}
+summary_info += {'iconv support': iconv}
+summary_info += {'curses support':curses}
+summary_info += {'virgl support': virgl}
+summary_info += {'curl support':  curl}
+summary_info += {'Multipath support': mpathpersist}
+summary_info += {'VNC support':   vnc}
 if vnc.found()
-  summary_info += {'VNC SASL support':  sasl.found()}
-  summary_info += {'VNC JPEG support':  jpeg.found()}
-  summary_info += {'VNC PNG support':   png.found()}
+  summary_info += {'VNC SASL support':  sasl}
+  summary_i

[PATCH 0/5] Upgrade minimum Meson version to 0.58.2, recommended to 0.59.2

[Paolo Bonzini]

Now that 0.59.2 is out, we can show library versions in the summary.
Since that feature is supported in 0.58.2 but broken in 0.59.{0,1},
allow --meson= to use 0.58.2 but default to the bundled submodule
if the host has anything less than 0.59.2.

Paolo

Paolo Bonzini (5):
  meson: bump submodule to 0.59.2
  meson: switch minimum meson version to 0.58.2, minimum recommended to
0.59.2
  hexagon: use env keyword argument to pass PYTHONPATH
  target/xtensa: list cores in a text file
  meson: show library versions in the summary

 configure |   8 +-
 docs/meson.build  |  14 +--
 meson |   2 +-
 meson.build   | 166 --
 plugins/meson.build   |   4 +-
 scripts/mtest2make.py |   7 +-
 target/hexagon/meson.build|   3 +-
 target/xtensa/cores.list  |   9 ++
 target/xtensa/import_core.sh  |   3 +
 target/xtensa/meson.build |   4 +-
 tests/qapi-schema/meson.build |   4 +-
 tests/qtest/meson.build   |   2 +-
 tests/unit/meson.build|   2 +-
 trace/meson.build |   4 +-
 14 files changed, 115 insertions(+), 117 deletions(-)
 create mode 100644 target/xtensa/cores.list

-- 
2.31.1

[PATCH 3/4] MAINTAINERS: Split MIPS TCG frontend vs MIPS machines/hardware

[Philippe Mathieu-Daudé]

Hardware emulated models don't belong to the TCG MAINTAINERS
section. Move them to a new 'Overall MIPS Machines' section
in the 'MIPS Machines' group.

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index a5268ad0651..f1d7279a0f2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -236,11 +236,8 @@ R: Jiaxun Yang 
 R: Aleksandar Rikalo 
 S: Odd Fixes
 F: target/mips/
-F: configs/devices/mips*/*
 F: disas/mips.c
 F: docs/system/cpu-models-mips.rst.inc
-F: hw/mips/
-F: include/hw/mips/
 F: tests/tcg/mips/
 
 MIPS TCG CPUs (nanoMIPS ISA)
@@ -1168,6 +1165,13 @@ F: hw/microblaze/petalogix_ml605_mmu.c
 
 MIPS Machines
 -
+Overall MIPS Machines
+M: Philippe Mathieu-Daudé 
+S: Odd Fixes
+F: configs/devices/mips*/*
+F: hw/mips/
+F: include/hw/mips/
+
 Jazz
 M: Hervé Poussineau 
 R: Aleksandar Rikalo 
-- 
2.31.1

[PATCH 1/5] meson: bump submodule to 0.59.2

[Paolo Bonzini]

The update to 0.57 has been delayed due to it causing warnings for
some actual issues, but it brings in important bugfixes and new
features.  0.58 also brings in a bugfix that is useful for modinfo.

Important bugfixes:

- 0.57: https://github.com/mesonbuild/meson/pull/7760, build: use PIE
objects for non-PIC static libraries if b_pie=true

- 0.57: https://github.com/mesonbuild/meson/pull/7900, thus avoiding
unnecessary rebuilds after running meson.

- 0.58.2: https://github.com/mesonbuild/meson/pull/8900, fixes for
passing extract_objects() to custom_target (useful for modinfo)

Features:

- 0.57: the keyval module has now been stabilized

- 0.57: env argument to custom_target (useful for hexagon)

- 0.57: Feature parity between "meson test" and QEMU's TAP driver

- 0.57: https://github.com/mesonbuild/meson/pull/8231, allows bringing
back version numbers in the configuration summary

- 0.59: Utility methods for feature objects

Signed-off-by: Paolo Bonzini 
---
 meson | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson b/meson
index 776acd2a80..b25d94e7c7 16
--- a/meson
+++ b/meson
@@ -1 +1 @@
-Subproject commit 776acd2a805c9b42b4f0375150977df42130317f
+Subproject commit b25d94e7c77fda05a7fdfe8afe562cf9760d69da
-- 
2.31.1

Re: [PATCH 12/12] q800: wire macfb IRQ to separate video interrupt on VIA2

[Laurent Vivier]

Le 02/10/2021 à 13:00, Mark Cave-Ayland a écrit :
> Whilst the in-built Quadra 800 framebuffer exists within the Nubus address
> space for slot 9, it has its own dedicated interrupt on VIA2. Force the
> macfb device to occupy slot 9 in the q800 machine and wire its IRQ to the
> separate video interrupt since this is what is expected by the MacOS
> interrupt handler.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/m68k/q800.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index df3fd3711e..fd4855047e 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -407,8 +407,10 @@ static void q800_init(MachineState *machine)
>  MAC_NUBUS_FIRST_SLOT * NUBUS_SUPER_SLOT_SIZE);
>  sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, NUBUS_SLOT_BASE +
>  MAC_NUBUS_FIRST_SLOT * NUBUS_SLOT_SIZE);
> -
> -for (i = 0; i < VIA2_NUBUS_IRQ_NB; i++) {
> +qdev_connect_gpio_out(dev, 9,
> +  qdev_get_gpio_in_named(via2_dev, "nubus-irq",
> +  VIA2_NUBUS_IRQ_INTVIDEO));
> +for (i = 1; i < VIA2_NUBUS_IRQ_NB; i++) {
>  qdev_connect_gpio_out(dev, 9 + i,
>qdev_get_gpio_in_named(via2_dev, "nubus-irq",
>   VIA2_NUBUS_IRQ_9 + i));
> @@ -419,6 +421,7 @@ static void q800_init(MachineState *machine)
>  /* framebuffer in nubus slot #9 */
>  
>  dev = qdev_new(TYPE_NUBUS_MACFB);
> +qdev_prop_set_uint32(dev, "slot", 9);
>  qdev_prop_set_uint32(dev, "width", graphic_width);
>  qdev_prop_set_uint32(dev, "height", graphic_height);
>  qdev_prop_set_uint8(dev, "depth", graphic_depth);
> 

Reviewed-by: Laurent Vivier 

Re: [PATCH 10/15] python/machine: Add support for AQMP backend

[Hanna Reitz]

On 18.09.21 01:48, John Snow wrote:



On Fri, Sep 17, 2021 at 10:16 AM Hanna Reitz > wrote:


On 17.09.21 07:40, John Snow wrote:
> To use the AQMP backend, Machine just needs to be a little more
diligent
> about what happens when closing a QMP connection. The operation
is no
> longer a freebie in the async world.
>
> Because async QMP continues to check for messages
asynchronously, it's
> almost certainly likely that the loop will have exited due to
EOF after
> issuing the last 'quit' command. That error will ultimately be
bubbled
> up when attempting to close the QMP connection. The manager
class here
> then is free to discard it if it was expected.
>
> Signed-off-by: John Snow mailto:js...@redhat.com>>
>
> ---
>
> Yes, I regret that this class has become quite a dumping ground for
> complexity around the exit path. It's in need of a refactor to help
> separate out the exception handling and cleanup mechanisms from the
> VM-related stuff, but it's not a priority to do that just yet -- but
> it's on the list.
>
> ---
>   python/qemu/machine/machine.py | 51
++
>   1 file changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/python/qemu/machine/machine.py
b/python/qemu/machine/machine.py
> index 6e58d2f951..8f5a6649e5 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -342,9 +342,15 @@ def _post_shutdown(self) -> None:
>           # Comprehensive reset for the failed launch case:
>           self._early_cleanup()
>
> -        if self._qmp_connection:
> -            self._qmp.close()
> -            self._qmp_connection = None
> +        try:
> +            self._close_qmp_connection()
> +        except Exception as err:  # pylint: disable=broad-except
> +            LOG.warning(
> +                "Exception closing QMP connection: %s",
> +                str(err) if str(err) else type(err).__name__
> +            )
> +        finally:
> +            assert self._qmp_connection is None
>
>           self._close_qemu_log_file()
>
> @@ -420,6 +426,31 @@ def _launch(self) -> None:
> close_fds=False)
>           self._post_launch()
>
> +    def _close_qmp_connection(self) -> None:
> +        """
> +        Close the underlying QMP connection, if any.
> +
> +        Dutifully report errors that occurred while closing,
but assume
> +        that any error encountered indicates an abnormal
termination
> +        process and not a failure to close.
> +        """
> +        if self._qmp_connection is None:
> +            return
> +
> +        try:
> +            self._qmp.close()
> +        except EOFError:
> +            # EOF can occur as an Exception here when using the
Async
> +            # QMP backend. It indicates that the server closed the
> +            # stream. If we successfully issued 'quit' at any
point,
> +            # then this was expected. If the remote went away
without
> +            # our permission, it's worth reporting that as an
abnormal
> +            # shutdown case.
> +            if not self._has_quit:
> +                raise
> +        finally:
> +            self._qmp_connection = None
> +
>       def _early_cleanup(self) -> None:
>           """
>           Perform any cleanup that needs to happen before the VM
exits.
> @@ -461,8 +492,18 @@ def _soft_shutdown(self, timeout:
Optional[int]) -> None:
>
>           if self._qmp_connection:
>               if not self._has_quit:
> -                # Might raise ConnectionReset
> -                self.qmp('quit')
> +                try:
> +                    # May raise ExecInterruptedError or
StateError if the
> +                    # connection dies or has already died.
> +                    self.qmp('quit')
> +                finally:
> +                    # If 'quit' fails, we'll still want to call
close(),
> +                    # which will raise any final errors that
may have
> +                    # occurred while trying to send 'quit'.
> +                    self._close_qmp_connection()
> +            else:
> +                # Regardless, we want to tidy up the socket.
> +                self._close_qmp_connection()

Why can’t we wait for _post_shutdown to do that?  Has it something
to do
with this operation being “no longer a freeby” (I’m not quite sure
what
this refers to, execution time, or the set of possible exceptions, or
perhaps something else entirely), and so we want to do it in the
already-not-instantaneous _soft_shutdown?

[PATCH 2/5] meson: switch minimum meson version to 0.58.2, minimum recommended to 0.59.2

[Paolo Bonzini]

Meson 0.58.2 does not need b_staticpic=$pie anymore, and has
stabilized the keyval module.  Remove the workaround and use a few
replacements for features deprecated in the 0.57.0 release cycle.

One feature that we would like to use is passing dependencies to
summary.  However, that was broken in 0.59.0 and 0.59.1.  Therefore,
use the embedded Meson if the host has anything older than 0.59.2,
but allow --meson= to use 0.58.2.

Signed-off-by: Paolo Bonzini 
---
 configure |  8 ++
 docs/meson.build  | 14 -
 meson.build   | 54 ---
 plugins/meson.build   |  4 +--
 scripts/mtest2make.py |  7 ++---
 tests/qapi-schema/meson.build |  4 +--
 tests/qtest/meson.build   |  2 +-
 tests/unit/meson.build|  2 +-
 trace/meson.build |  4 +--
 9 files changed, 44 insertions(+), 55 deletions(-)

diff --git a/configure b/configure
index 1d3f099498..877bf3d76a 100755
--- a/configure
+++ b/configure
@@ -1994,7 +1994,7 @@ python_version=$($python -c 'import sys; print("%d.%d.%d" 
% (sys.version_info[0]
 python="$python -B"
 
 if test -z "$meson"; then
-if test "$explicit_python" = no && has meson && version_ge "$(meson 
--version)" 0.55.3; then
+if test "$explicit_python" = no && has meson && version_ge "$(meson 
--version)" 0.59.2; then
 meson=meson
 elif test $git_submodules_action != 'ignore' ; then
 meson=git
@@ -5163,10 +5163,6 @@ if test "$skip_meson" = no; then
   mv $cross config-meson.cross
 
   rm -rf meson-private meson-info meson-logs
-  unset staticpic
-  if ! version_ge "$($meson --version)" 0.56.0; then
-staticpic=$(if test "$pie" = yes; then echo true; else echo false; fi)
-  fi
   NINJA=$ninja $meson setup \
 --prefix "$prefix" \
 --libdir "$libdir" \
@@ -5186,7 +5182,6 @@ if test "$skip_meson" = no; then
 -Dwerror=$(if test "$werror" = yes; then echo true; else echo false; 
fi) \
 -Dstrip=$(if test "$strip_opt" = yes; then echo true; else echo false; 
fi) \
 -Db_pie=$(if test "$pie" = yes; then echo true; else echo false; fi) \
-${staticpic:+-Db_staticpic=$staticpic} \
 -Db_coverage=$(if test "$gcov" = yes; then echo true; else echo false; 
fi) \
 -Db_lto=$lto -Dcfi=$cfi -Dcfi_debug=$cfi_debug \
 -Dmalloc=$malloc -Dmalloc_trim=$malloc_trim -Dsparse=$sparse \
@@ -5222,6 +5217,7 @@ else
 perl -i -ne '
   s/^gettext = true$/gettext = auto/;
   s/^gettext = false$/gettext = disabled/;
+  /^b_staticpic/ && next;
   print;' meson-private/cmd_line.txt
   fi
 fi
diff --git a/docs/meson.build b/docs/meson.build
index cffe1ecf1d..be4dc30f39 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -37,14 +37,14 @@ endif
 if build_docs
   SPHINX_ARGS += ['-Dversion=' + meson.project_version(), '-Drelease=' + 
config_host['PKGVERSION']]
 
-  sphinx_extn_depends = [ meson.source_root() / 'docs/sphinx/depfile.py',
-  meson.source_root() / 'docs/sphinx/hxtool.py',
-  meson.source_root() / 'docs/sphinx/kerneldoc.py',
-  meson.source_root() / 'docs/sphinx/kernellog.py',
-  meson.source_root() / 'docs/sphinx/qapidoc.py',
-  meson.source_root() / 'docs/sphinx/qmp_lexer.py',
+  sphinx_extn_depends = [ meson.current_source_dir() / 'sphinx/depfile.py',
+  meson.current_source_dir() / 'sphinx/hxtool.py',
+  meson.current_source_dir() / 'sphinx/kerneldoc.py',
+  meson.current_source_dir() / 'sphinx/kernellog.py',
+  meson.current_source_dir() / 'sphinx/qapidoc.py',
+  meson.current_source_dir() / 'sphinx/qmp_lexer.py',
   qapi_gen_depends ]
-  sphinx_template_files = [ meson.source_root() / 
'docs/_templates/footer.html' ]
+  sphinx_template_files = [ meson.project_source_root() / 
'docs/_templates/footer.html' ]
 
   have_ga = have_tools and config_host.has_key('CONFIG_GUEST_AGENT')
 
diff --git a/meson.build b/meson.build
index 60f4f45165..17e77fe4ef 100644
--- a/meson.build
+++ b/meson.build
@@ -1,14 +1,10 @@
-project('qemu', ['c'], meson_version: '>=0.55.0',
-default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++11', 
'b_colorout=auto'] +
- (meson.version().version_compare('>=0.56.0') ? [ 
'b_staticpic=false' ] : []),
-version: run_command('head', meson.source_root() / 
'VERSION').stdout().strip())
+project('qemu', ['c'], meson_version: '>=0.58.2',
+default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++11', 
'b_colorout=auto',
+  'b_staticpic=false'],
+version: files('VERSION'))
 
 not_found = dependency('', required: false)
-if meson.version().version_compare('>=0.56.0')
-  keyval = import('keyval')
-else
-  keyval = imp

Re: [PATCH v3 14/22] target/i386/sev: Move qmp_query_sev_attestation_report() to sev.c

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (phi...@redhat.com) wrote:
> Move qmp_query_sev_attestation_report() from monitor.c to sev.c
> and make sev_get_attestation_report() static. We don't need the
> stub anymore, remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  target/i386/sev_i386.h|  2 --
>  target/i386/monitor.c |  6 --
>  target/i386/sev-sysemu-stub.c |  7 ---
>  target/i386/sev.c | 12 ++--
>  4 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
> index 2d9a1a0112e..5f367f78eb7 100644
> --- a/target/i386/sev_i386.h
> +++ b/target/i386/sev_i386.h
> @@ -27,8 +27,6 @@
>  extern SevInfo *sev_get_info(void);
>  extern char *sev_get_launch_measurement(void);
>  extern SevCapability *sev_get_capabilities(Error **errp);
> -extern SevAttestationReport *
> -sev_get_attestation_report(const char *mnonce, Error **errp);
>  
>  int sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp);
>  int sev_inject_launch_secret(const char *hdr, const char *secret,
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index a9f85acd473..c05d70252a2 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -764,12 +764,6 @@ void qmp_sev_inject_launch_secret(const char *packet_hdr,
>  sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
>  }
>  
> -SevAttestationReport *
> -qmp_query_sev_attestation_report(const char *mnonce, Error **errp)
> -{
> -return sev_get_attestation_report(mnonce, errp);
> -}
> -
>  SGXInfo *qmp_query_sgx(Error **errp)
>  {
>  return sgx_get_info(errp);
> diff --git a/target/i386/sev-sysemu-stub.c b/target/i386/sev-sysemu-stub.c
> index d556b4f091f..813b9a6a03b 100644
> --- a/target/i386/sev-sysemu-stub.c
> +++ b/target/i386/sev-sysemu-stub.c
> @@ -13,6 +13,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qapi/qapi-commands-misc-target.h"
> +#include "qapi/qmp/qerror.h"
>  #include "qapi/error.h"
>  #include "sev_i386.h"
>  
> @@ -52,9 +53,9 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t 
> flash_size)
>  g_assert_not_reached();
>  }
>  
> -SevAttestationReport *sev_get_attestation_report(const char *mnonce,
> - Error **errp)
> +SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
> +   Error **errp)
>  {
> -error_setg(errp, "SEV is not available in this QEMU");
> +error_setg(errp, QERR_UNSUPPORTED);

I did like that message making it clear the reason it was unsupported
was this build, rather than lack of host support or not enabling it.

Dave

>  return NULL;
>  }
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index aefbef4bb63..91a217bbb85 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -31,6 +31,8 @@
>  #include "migration/blocker.h"
>  #include "qom/object.h"
>  #include "monitor/monitor.h"
> +#include "qapi/qapi-commands-misc-target.h"
> +#include "qapi/qmp/qerror.h"
>  #include "exec/confidential-guest-support.h"
>  #include "hw/i386/pc.h"
>  
> @@ -487,8 +489,8 @@ out:
>  return cap;
>  }
>  
> -SevAttestationReport *
> -sev_get_attestation_report(const char *mnonce, Error **errp)
> +static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
> +Error **errp)
>  {
>  struct kvm_sev_attestation_report input = {};
>  SevAttestationReport *report = NULL;
> @@ -549,6 +551,12 @@ sev_get_attestation_report(const char *mnonce, Error 
> **errp)
>  return report;
>  }
>  
> +SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
> +   Error **errp)
> +{
> +return sev_get_attestation_report(mnonce, errp);
> +}
> +
>  static int
>  sev_read_file_base64(const char *filename, guchar **data, gsize *len)
>  {
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH 12/15] iotests: Disable AQMP logging under non-debug modes

[Hanna Reitz]

On 18.09.21 02:58, John Snow wrote:



On Fri, Sep 17, 2021 at 10:30 AM Hanna Reitz > wrote:


On 17.09.21 07:40, John Snow wrote:
> Disable the aqmp logger, which likes to (at the moment) print out
> intermediate warnings and errors that cause session termination;
disable
> them so they don't interfere with the job output.
>
> Leave any "CRITICAL" warnings enabled though, those are ones that we
> should never see, no matter what.

I mean, looks OK to me, but from what I understand (i.e. little),
qmp_client doesn’t log CRITICAL messages, at least I can’t see
any. Only
ERRORs.


There's *one* critical message in protocol.py, used for a circumstance 
that I *think* should be impossible. I do not think I currently use 
any WARNING level statements.


I guess I’m missing some CRITICAL messages in external functions
called
from qmp_client.py, but shouldn’t we still keep ERRORs?


...Mayybe?

The errors logged by AQMP are *almost always* raised as Exceptions 
somewhere else, eventually. Sometimes when we encounter them in one 
context, we need to save them and then re-raise them in a different 
execution context. There's one good exception to this: My pal, EOFError.


If the reader context encounters EOF, it raises EOFError and this 
causes a disconnect to be scheduled asynchronously. *Any* Exception 
that causes a disconnect to be scheduled asynchronously is dutifully 
logged as an ERROR. At this point in the code, we don't really know if 
the user of the library considers this an "error" yet or not. I've 
waffled a lot on how exactly to treat this circumstance. ...Hm, I 
guess that's really the only case where I have an error that really 
ought to be suppressed. I suppose what I will do here is: if the 
exception happens to be an EOFError I will drop the severity of the 
log message down to INFO. I don't know why it takes being challenged 
on this stuff to start thinking clearly about it, but here we are. 
Thank you for your feedback :~)


Errr... You’re welcome!! *cough*

Hanna