Cross compiling 6.2.0 aarch64-softmmu for aarch64 host with musl, struct redefiniton errors

[Adam Baxter]

Hi,

I'm trying to compile qemu 6.2.0 using musl-cross.

I am getting the following errors on make:

/output/aarch64-linux-musl/include/asm/sigcontext.h:83:8: error: redefinition 
of 'struct esr_context'
/output/aarch64-linux-musl/include/asm/sigcontext.h:116:8: error: redefinition 
of 'struct extra_context'
/output/aarch64-linux-musl/include/asm/sigcontext.h:125:8: error: redefinition 
of 'struct sve_context'

I have made a few hacks to make it build statically, I do not believe this is 
causing this particular issue.

Attached are the logs and Dockerfile I'm using.

Is this a musl issue or a qemu issue?

Thanks,
Adam

Dockerfile
Description: Binary data
QEMU_CFLAGS  : -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 
-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -W
old-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels 
-Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs 
-Wno-shift-negative-value -Wno-psabi -fstack-protector-strong
QEMU_LDFLAGS : -Wl,--warn-common -Wl,-z,relro -Wl,-z,now  
-fstack-protector-strong
profiler : NO
link-time optimization (LTO) : NO
PIE  : NO
static build : NO
malloc trim support  : NO
membarrier   : NO
debug stack usage: NO
mutex debugging  : NO
memory allocator : system
avx2 optimization: NO
avx512f optimization : NO
gprof enabled: NO
gcov : NO
thread sanitizer : NO
CFI support  : NO
strip binaries   : YES
sparse   : NO
mingw32 support  : NO
aarch64 tests: aarch64-linux-musl-gcc

  Targets and accelerators
KVM support  : YES
HAX support  : NO
HVF support  : NO
WHPX support : NO
NVMM support : NO
Xen support  : NO
TCG support  : YES
TCG backend  : native (aarch64)
TCG plugins  : YES
TCG debug enabled: NO
target list  : aarch64-softmmu
default devices  : YES
out of process emulation : YES

  Block layer support
coroutine backend: sigaltstack
coroutine pool   : YES
Block whitelist (rw) :
Block whitelist (ro) :
Use block whitelist in tools : NO
VirtFS support   : NO
build virtiofs daemon: NO
Live block migration : YES
replication support  : YES
bochs support: YES
cloop support: YES
dmg support  : YES
qcow v1 support  : YES
vdi support  : YES
vvfat support: YES
qed support  : YES
parallels support: YES
FUSE exports : NO

  Crypto
TLS priority : "NORMAL"
GNUTLS support   : NO
libgcrypt: NO
nettle   : NO
crypto afalg : NO
rng-none : NO
Linux keyring: YES

  Dependencies
SDL support  : NO
SDL image support: NO
GTK support  : NO
pixman   : YES 0.40.0
VTE support  : NO
slirp support: internal
libtasn1 : NO
PAM  : NO
iconv support: YES
curses support   : NO
virgl support: NO
curl support : NO
Multipath support: NO
VNC support  : YES
VNC SASL support : NO
VNC JPEG support : NO
VNC PNG support  : NO
OSS support  : YES
ALSA support : NO
PulseAudio support   : NO
JACK support : NO
brlapi support   : NO
vde support  : NO
netmap support   : NO
l2tpv3 support   : YES
Linux AIO support: NO
Linux io_uring support   : NO
ATTR/XATTR support   : YES
RDMA support : NO
PVRDMA support   : NO
fdt support  : internal
libcap-ng support: NO
bpf support  : NO
spice protocol support   : NO
rbd support   

Re: [RFC PATCH 2/2] hw/i386/sgx: Attach SGX-EPC to its memory backend

[Yang Zhong]

On Mon, Jan 17, 2022 at 12:48:10PM +0100, Paolo Bonzini wrote:
> On 1/17/22 00:53, Philippe Mathieu-Daudé via wrote:
> >We have one SGX-EPC address/size/node per memory backend,
> >make it child of the backend in the QOM composition tree.
> >
> >Cc: Yang Zhong 
> >Signed-off-by: Philippe Mathieu-Daudé 
> >---
> >  hw/i386/sgx.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> >diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
> >index 5de5dd08936..6362e5e9d02 100644
> >--- a/hw/i386/sgx.c
> >+++ b/hw/i386/sgx.c
> >@@ -300,6 +300,9 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
> >  /* set the memdev link with memory backend */
> >  object_property_parse(obj, SGX_EPC_MEMDEV_PROP, 
> > list->value->memdev,
> >&error_fatal);
> >+object_property_add_child(OBJECT(list->value->memdev), "sgx-epc",
> >+  OBJECT(obj));
> >+
> >  /* set the numa node property for sgx epc object */
> >  object_property_set_uint(obj, SGX_EPC_NUMA_NODE_PROP, 
> > list->value->node,
> >   &error_fatal);
> 
> I don't think this is a good idea; only list->value->memdev should
> add something below itself in the tree.
> 
> However, I think obj can be added under the machine itself as
> /machine/sgx-epc-device[*].
> 

  Philippe, Sorry I can't receive all Qemu mails from my mutt tool.

  https://lists.nongnu.org/archive/html/qemu-devel/2022-01/msg03535.html
  I verified this patch, and the issue was reported as below:

  Unexpected error in object_property_try_add() at ../qom/object.c:1224:
  qemu-system-x86_64: attempt to add duplicate property 'sgx-epc' to object 
(type 'pc-q35-7.0-machine')
  Aborted (core dumped)

  Even I changed it to another name, which still reported same kind of issue.

  I tried below patch as my previous patch, and it can work
  diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
  index d60485fc422..66444745b47 100644
  --- a/hw/i386/sgx.c
  +++ b/hw/i386/sgx.c
  @@ -281,6 +281,7 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
   SGXEPCState *sgx_epc = &pcms->sgx_epc;
   X86MachineState *x86ms = X86_MACHINE(pcms);
   SgxEPCList *list = NULL;
  +int sgx_count = 0;
   Object *obj;

   memset(sgx_epc, 0, sizeof(SGXEPCState));
  @@ -297,7 +298,9 @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
   for (list = x86ms->sgx_epc_list; list; list = list->next) {
   obj = object_new("sgx-epc");

  -object_property_add_child(OBJECT(pcms), "sgx-epc", OBJECT(obj));
  +gchar *name = g_strdup_printf("device[%d]", sgx_count++);
  +object_property_add_child(container_get(qdev_get_machine(), 
"/sgx-epc-device"),
  +  name, obj);
  
   /* set the memdev link with memory backend */
   object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev,


  From the monitor, 
  (qemu) qom-list /machine/sgx-epc-device
  type (string)
  device[0] (child)
  device[1] (child)
  
  This can normally show two sgx epc section devices.
  
  If you have new patch, I can help verify, thanks!

  Yang

> Paolo

Re: Cross compiling 6.2.0 aarch64-softmmu for aarch64 host with musl, struct redefiniton errors

[Peter Maydell]

On Sun, 23 Jan 2022 at 08:58, Adam Baxter  wrote:
> I'm trying to compile qemu 6.2.0 using musl-cross.
>
> I am getting the following errors on make:
>
> /output/aarch64-linux-musl/include/asm/sigcontext.h:83:8: error: redefinition 
> of 'struct esr_context'
> /output/aarch64-linux-musl/include/asm/sigcontext.h:116:8: error: 
> redefinition of 'struct extra_context'
> /output/aarch64-linux-musl/include/asm/sigcontext.h:125:8: error: 
> redefinition of 'struct sve_context'

I think this must be a musl problem. Quoting the full error
message:

In file included from /output/aarch64-linux-musl/include/asm/ptrace.h:26,
 from linux-headers/asm/kvm.h:37,
 from /src/qemu/linux-headers/linux/kvm.h:15,
 from /src/qemu/include/sysemu/kvm.h:25,
 from ../hw/arm/boot.c:18:
/output/aarch64-linux-musl/include/asm/sigcontext.h:28:8: error:
redefinition of 'struct sigcontext'
   28 | struct sigcontext {
  |^~
In file included from /output/aarch64-linux-musl/include/signal.h:48,
 from /src/qemu/include/qemu/osdep.h:105,
 from ../hw/arm/boot.c:10:
/output/aarch64-linux-musl/include/bits/signal.h:18:16: note:
originally defined here
   18 | typedef struct sigcontext {
  |^~
In file included from /output/aarch64-linux-musl/include/asm/ptrace.h:26,
 from linux-headers/asm/kvm.h:37,
 from /src/qemu/linux-headers/linux/kvm.h:15,
 from /src/qemu/include/sysemu/kvm.h:25,
 from ../hw/arm/boot.c:18:
/output/aarch64-linux-musl/include/asm/sigcontext.h:66:8: error:
redefinition of 'struct _aarch64_ctx'
   66 | struct _aarch64_ctx {
  |^~~~
In file included from /output/aarch64-linux-musl/include/signal.h:48,
 from /src/qemu/include/qemu/osdep.h:105,
 from ../hw/arm/boot.c:10:
/output/aarch64-linux-musl/include/bits/signal.h:29:8: note:
originally defined here
   29 | struct _aarch64_ctx {
  |^~~~


The compiler is complaining because the system headers (provided by
musl) are defining these structs both in asm/sigcontext.h and
in signal.h, which means those headers can't both be included
at once. This isn't QEMU related: musl's headers just seem to
be broken here. glibc doesn't have this bug.

-- PMM

[PATCH] linux-user/syscall: Do not ignore info.si_pid == 0 in waitid()

[Serge Belyshev]

When called with WNOHANG and no child has exited, waitid returns with
info.si_pid set to zero and thus check for info.si_pid != 0 will cause
target siginfo structure to be uninitialized.  Fixed by removing the check.

Signed-off-by: Serge Belyshev 
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/817
---
 linux-user/syscall.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5950222a77..b80531ac4c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8724,9 +8724,8 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 case TARGET_NR_waitid:
 {
 siginfo_t info;
-info.si_pid = 0;
 ret = get_errno(safe_waitid(arg1, arg2, &info, arg4, NULL));
-if (!is_error(ret) && arg3 && info.si_pid != 0) {
+if (!is_error(ret) && arg3) {
 if (!(p = lock_user(VERIFY_WRITE, arg3, 
sizeof(target_siginfo_t), 0)))
 return -TARGET_EFAULT;
 host_to_target_siginfo(p, &info);
-- 
2.34.1

[PATCH] linux-user/alpha: Fix target rlimits for alpha and rearrange for clarity

[Serge Belyshev]

Alpha uses different values of some TARGET_RLIMIT_* constants, which were
missing and caused bugs like #577, fixed thus.  Also rearranged all three
(alpha, mips and sparc) that differ from everyone else for clarity.

Signed-off-by: Serge Belyshev 
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/577
---
 linux-user/syscall_defs.h | 31 ++-
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index f23f0a2178..3fcabaeae3 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -730,44 +730,41 @@ struct target_rlimit {
 #define TARGET_RLIM_INFINITY   ((abi_ulong)-1)
 #endif
 
-#if defined(TARGET_MIPS)
 #define TARGET_RLIMIT_CPU  0
 #define TARGET_RLIMIT_FSIZE1
 #define TARGET_RLIMIT_DATA 2
 #define TARGET_RLIMIT_STACK3
 #define TARGET_RLIMIT_CORE 4
+#if defined(TARGET_MIPS)
+#define TARGET_RLIMIT_NOFILE   5
+#define TARGET_RLIMIT_AS   6
 #define TARGET_RLIMIT_RSS  7
 #define TARGET_RLIMIT_NPROC8
-#define TARGET_RLIMIT_NOFILE   5
 #define TARGET_RLIMIT_MEMLOCK  9
-#define TARGET_RLIMIT_AS   6
-#define TARGET_RLIMIT_LOCKS10
-#define TARGET_RLIMIT_SIGPENDING   11
-#define TARGET_RLIMIT_MSGQUEUE 12
-#define TARGET_RLIMIT_NICE 13
-#define TARGET_RLIMIT_RTPRIO   14
-#else
-#define TARGET_RLIMIT_CPU  0
-#define TARGET_RLIMIT_FSIZE1
-#define TARGET_RLIMIT_DATA 2
-#define TARGET_RLIMIT_STACK3
-#define TARGET_RLIMIT_CORE 4
+#elif defined(TARGET_ALPHA)
+#define TARGET_RLIMIT_RSS  5
+#define TARGET_RLIMIT_NOFILE   6
+#define TARGET_RLIMIT_AS   7
+#define TARGET_RLIMIT_NPROC8
+#define TARGET_RLIMIT_MEMLOCK  9
+#elif defined(TARGET_SPARC)
 #define TARGET_RLIMIT_RSS  5
-#if defined(TARGET_SPARC)
 #define TARGET_RLIMIT_NOFILE   6
 #define TARGET_RLIMIT_NPROC7
+#define TARGET_RLIMIT_MEMLOCK  8
+#define TARGET_RLIMIT_AS   9
 #else
+#define TARGET_RLIMIT_RSS  5
 #define TARGET_RLIMIT_NPROC6
 #define TARGET_RLIMIT_NOFILE   7
-#endif
 #define TARGET_RLIMIT_MEMLOCK  8
 #define TARGET_RLIMIT_AS   9
+#endif
 #define TARGET_RLIMIT_LOCKS10
 #define TARGET_RLIMIT_SIGPENDING   11
 #define TARGET_RLIMIT_MSGQUEUE 12
 #define TARGET_RLIMIT_NICE 13
 #define TARGET_RLIMIT_RTPRIO   14
-#endif
 
 struct target_pollfd {
 int fd;   /* file descriptor */
-- 
2.34.1

[CXL HDM DECODER PROGRAMMING] - Question: Does Qemu program HDM decoder register of the CXL endpoint?

[Samarth Saxena]

Hi All,

I had a question about the CXL HDM Decoder register programming.
Is there any part of Qemu, that automatically programs the enable bit of the 
HDM decoder register in the Component registers of the CXL endpoint?
The CDR (component registers) are hosted inside the memory of the CXL endpoint.

Regards,
[CadenceLogoRed185Regcopy1583174817new51584636989.png]
Samarth Saxena
Sr Principal Software Engineer
T: 911204308300
[UIcorrectsize1583179003.png]
[16066EmailSignatureFortune100Best2021White92x1271617625037.png]

Re: [PATCH v2 01/15] audio: replace open-coded buffer arithmetic

[Christian Schoenebeck]

On Samstag, 22. Januar 2022 13:57:31 CET Volker Rümelin wrote:
> Replace open-coded buffer arithmetic with the new function
> audio_ring_posb(). That's the position in backward direction
> of a given point at a given distance.
> 
> Signed-off-by: Volker Rümelin 
> ---

First of all, getting rid of all those redundant, open coded ringbuffer
traversal code places highly makes sense!

>  audio/audio.c | 25 +++--
>  audio/audio_int.h |  6 ++
>  audio/coreaudio.c | 10 --
>  audio/sdlaudio.c  | 11 +--
>  4 files changed, 22 insertions(+), 30 deletions(-)
> 
> diff --git a/audio/audio.c b/audio/audio.c
> index dc28685d22..e7a139e289 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -574,19 +574,13 @@ static size_t audio_pcm_sw_get_rpos_in(SWVoiceIn *sw)
>  {
>  HWVoiceIn *hw = sw->hw;
>  ssize_t live = hw->total_samples_captured - 
> sw->total_hw_samples_acquired;
> -ssize_t rpos;
> 
>  if (audio_bug(__func__, live < 0 || live > hw->conv_buf->size)) {
>  dolog("live=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size);
>  return 0;
>  }
> 
> -rpos = hw->conv_buf->pos - live;
> -if (rpos >= 0) {
> -return rpos;
> -} else {
> -return hw->conv_buf->size + rpos;
> -}
> +return audio_ring_posb(hw->conv_buf->pos, live, hw->conv_buf->size);
>  }
> 
>  static size_t audio_pcm_sw_read(SWVoiceIn *sw, void *buf, size_t size)
> @@ -1394,12 +1388,10 @@ void audio_generic_run_buffer_in(HWVoiceIn *hw)
> 
>  void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size)
>  {
> -ssize_t start = (ssize_t)hw->pos_emul - hw->pending_emul;
> +size_t start;
> 
> -if (start < 0) {
> -start += hw->size_emul;
> -}
> -assert(start >= 0 && start < hw->size_emul);
> +start = audio_ring_posb(hw->pos_emul, hw->pending_emul, hw->size_emul);
> +assert(start < hw->size_emul);
> 
>  *size = MIN(*size, hw->pending_emul);
>  *size = MIN(*size, hw->size_emul - start);
> @@ -1415,13 +1407,10 @@ void audio_generic_put_buffer_in(HWVoiceIn *hw, void
> *buf, size_t size) void audio_generic_run_buffer_out(HWVoiceOut *hw)
>  {
>  while (hw->pending_emul) {
> -size_t write_len, written;
> -ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul;
> +size_t write_len, written, start;
> 
> -if (start < 0) {
> -start += hw->size_emul;
> -}
> -assert(start >= 0 && start < hw->size_emul);
> +start = audio_ring_posb(hw->pos_emul, hw->pending_emul, 
> hw->size_emul);
> +assert(start < hw->size_emul);
> 
>  write_len = MIN(hw->pending_emul, hw->size_emul - start);

Just refactoring so far, which looks good so far.

> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index 428a091d05..2fb459f34e 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -266,6 +266,12 @@ static inline size_t audio_ring_dist(size_t dst, size_t 
> src, size_t len)
>  return (dst >= src) ? (dst - src) : (len - src + dst);
>  }

You haven't touched this function, but while I am looking at it, all function
arguments are unsigned. So probably modulo operator might be used to get rid
of a branch here.

> 
> +/* return new position in backward direction at given distance */
> +static inline size_t audio_ring_posb(size_t pos, size_t dist, size_t len)
> +{
> +return pos >= dist ? pos - dist : len - dist + pos;
> +}
> +

Which is the exact same code as the already existing audio_ring_dist()
function above, and I see that you actually already used this in v1 before:

#define audio_ring_posb(pos, dist, len) audio_ring_dist(pos, dist, len)

I would definitely not copy-paste the body. Thomas just suggested in v1 to add
a comment, not to duplicate the actual math code:
https://lore.kernel.org/qemu-devel/20220106111718.0ec25...@tuxfamily.org/

Also for consistency, I would have called the function audio_ring_rpos()
and would have commented each argument.

/**
 * Returns new position in ringbuffer in backward direction at given distance.
 * @pos: current position in ringbuffer
 * @dist: distance in ringbuffer to walk in reverse direction
 * @len: size of ringbuffer
 */
static inline size_t audio_ring_rpos(pos, dist, len) {
return audio_ring_dist(pos, dist, len);
}

At least IMO a bit more comments on math code barely hurts.

>  #define dolog(fmt, ...) AUD_log(AUDIO_CAP, fmt, ## __VA_ARGS__)
> 
>  #ifdef DEBUG
> diff --git a/audio/coreaudio.c b/audio/coreaudio.c
> index d8a21d3e50..1fdd1d4b14 100644
> --- a/audio/coreaudio.c
> +++ b/audio/coreaudio.c
> @@ -333,12 +333,10 @@ static OSStatus audioDeviceIOProc(
> 
>  len = frameCount * hw->info.bytes_per_frame;
>  while (len) {
> -size_t write_len;
> -ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul;
> -if (start < 0) {
> -start += hw->size_emul;
> -}
> -assert(start >= 0 && start < hw->size_emul);
> +   

Re: [PATCH v2 06/15] jackaudio: use more jack audio buffers

[Christian Schoenebeck]

On Samstag, 22. Januar 2022 13:57:36 CET Volker Rümelin wrote:
> The next patch reduces the effective qemu playback buffer size
> by timer-period. Increase the number of jack audio buffers by
> one to preserve the total effective buffer size. The size of one
> jack audio buffer is 512 samples. With audio defaults that's
> 512 samples / 44100 samples/s = 11.6 ms and only slightly larger
> than the timer-period of 10 ms.
> 
> The larger jack audio buffer increases audio dropout safety,
> because the high priority jack-audio worker threads can provide
> audio data for a longer period of time as with a smaller buffer
> and more audio data in the mixing engine buffer that they can't
> access.
> 
> Signed-off-by: Volker Rümelin 

Reviewed-by: Christian Schoenebeck 

> ---
>  audio/jackaudio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/audio/jackaudio.c b/audio/jackaudio.c
> index 317009e936..26246c3a8b 100644
> --- a/audio/jackaudio.c
> +++ b/audio/jackaudio.c
> @@ -483,8 +483,8 @@ static int qjack_client_init(QJackClient *c)
>  c->buffersize = 512;
>  }
> 
> -/* create a 2 period buffer */
> -qjack_buffer_create(&c->fifo, c->nchannels, c->buffersize * 2);
> +/* create a 3 period buffer */
> +qjack_buffer_create(&c->fifo, c->nchannels, c->buffersize * 3);
> 
>  qjack_client_connect_ports(c);
>  c->state = QJACK_STATE_RUNNING;

Re: [PATCH v2 2/4] dump/win_dump: add helper macros for Windows dump header access

[Viktor Prutyanov]

On Thu, 13 Jan 2022 03:52:46 +0300
Viktor Prutyanov  wrote:

> Perform read access to Windows dump header fields via helper macros.
> This is preparation for the next 32-bit guest Windows dump support.
> 
> Signed-off-by: Viktor Prutyanov 
> ---
>  dump/win_dump.c | 100
> +++- 1 file changed, 65
> insertions(+), 35 deletions(-)
> 
> diff --git a/dump/win_dump.c b/dump/win_dump.c
> index 29b6e4f670..df3b432ca5 100644
> --- a/dump/win_dump.c
> +++ b/dump/win_dump.c
> @@ -24,11 +24,25 @@
>  #include "hw/misc/vmcoreinfo.h"
>  #include "win_dump.h"
>  
> -static size_t write_run(WinDumpPhyMemRun64 *run, int fd, Error
> **errp) +#define WIN_DUMP_PTR_SIZE sizeof(uint64_t)
> +
> +#define _WIN_DUMP_FIELD(f) (h->f)
> +#define WIN_DUMP_FIELD(field) _WIN_DUMP_FIELD(field)
> +
> +#define _WIN_DUMP_FIELD_PTR(f) ((void *)&h->f)
> +#define WIN_DUMP_FIELD_PTR(field) _WIN_DUMP_FIELD_PTR(field)
> +
> +#define _WIN_DUMP_FIELD_SIZE(f) sizeof(h->f)
> +#define WIN_DUMP_FIELD_SIZE(field) _WIN_DUMP_FIELD_SIZE(field)
> +
> +#define WIN_DUMP_CTX_SIZE sizeof(WinContext64)
> +
> +static size_t write_run(uint64_t base_page, uint64_t page_count,
> +int fd, Error **errp)
>  {
>  void *buf;
> -uint64_t addr = run->BasePage << TARGET_PAGE_BITS;
> -uint64_t size = run->PageCount << TARGET_PAGE_BITS;
> +uint64_t addr = base_page << TARGET_PAGE_BITS;
> +uint64_t size = page_count << TARGET_PAGE_BITS;
>  uint64_t len, l;
>  size_t total = 0;
>  
> @@ -59,13 +73,14 @@ static size_t write_run(WinDumpPhyMemRun64 *run,
> int fd, Error **errp) 
>  static void write_runs(DumpState *s, WinDumpHeader64 *h, Error
> **errp) {
> -WinDumpPhyMemDesc64 *desc = &h->PhysicalMemoryBlock;
> -WinDumpPhyMemRun64 *run = desc->Run;
> +uint64_t BasePage, PageCount;
>  Error *local_err = NULL;
>  int i;
>  
> -for (i = 0; i < desc->NumberOfRuns; i++) {
> -s->written_size += write_run(run + i, s->fd, &local_err);
> +for (i = 0; i <
> WIN_DUMP_FIELD(PhysicalMemoryBlock.NumberOfRuns); i++) {
> +BasePage =
> WIN_DUMP_FIELD(PhysicalMemoryBlock.Run[i].BasePage);
> +PageCount =
> WIN_DUMP_FIELD(PhysicalMemoryBlock.Run[i].PageCount);
> +s->written_size += write_run(BasePage, PageCount, s->fd,
> &local_err); if (local_err) {
>  error_propagate(errp, local_err);
>  return;
> @@ -73,11 +88,24 @@ static void write_runs(DumpState *s,
> WinDumpHeader64 *h, Error **errp) }
>  }
>  
> +static int cpu_read_ptr(CPUState *cpu, uint64_t addr, uint64_t *ptr)
> +{
> +int ret;
> +uint64_t ptr64;
> +
> +ret = cpu_memory_rw_debug(cpu, addr, &ptr64, WIN_DUMP_PTR_SIZE,
> 0); +
> +*ptr = ptr64;
> +
> +return ret;
> +}
> +
>  static void patch_mm_pfn_database(WinDumpHeader64 *h, Error **errp)
>  {
>  if (cpu_memory_rw_debug(first_cpu,
> -h->KdDebuggerDataBlock + KDBG_MM_PFN_DATABASE_OFFSET64,
> -(uint8_t *)&h->PfnDatabase, sizeof(h->PfnDatabase), 0)) {
> +WIN_DUMP_FIELD(KdDebuggerDataBlock) +
> KDBG_MM_PFN_DATABASE_OFFSET64,
> +WIN_DUMP_FIELD_PTR(PfnDatabase),
> +WIN_DUMP_FIELD_SIZE(PfnDatabase), 0)) {
>  error_setg(errp, "win-dump: failed to read MmPfnDatabase");
>  return;
>  }
> @@ -87,16 +115,17 @@ static void patch_bugcheck_data(WinDumpHeader64
> *h, Error **errp) {
>  uint64_t KiBugcheckData;
>  
> -if (cpu_memory_rw_debug(first_cpu,
> -h->KdDebuggerDataBlock + KDBG_KI_BUGCHECK_DATA_OFFSET64,
> -(uint8_t *)&KiBugcheckData, sizeof(KiBugcheckData), 0)) {
> +if (cpu_read_ptr(first_cpu,
> +WIN_DUMP_FIELD(KdDebuggerDataBlock) +
> +KDBG_KI_BUGCHECK_DATA_OFFSET64,
> +&KiBugcheckData)) {
>  error_setg(errp, "win-dump: failed to read KiBugcheckData");
>  return;
>  }
>  
> -if (cpu_memory_rw_debug(first_cpu,
> -KiBugcheckData,
> -h->BugcheckData, sizeof(h->BugcheckData), 0)) {
> +if (cpu_memory_rw_debug(first_cpu, KiBugcheckData,
> +WIN_DUMP_FIELD(BugcheckData),
> +WIN_DUMP_FIELD_SIZE(BugcheckData), 0)) {
>  error_setg(errp, "win-dump: failed to read bugcheck data");
>  return;
>  }
> @@ -105,8 +134,8 @@ static void patch_bugcheck_data(WinDumpHeader64
> *h, Error **errp)
>   * If BugcheckCode wasn't saved, we consider guest OS as alive.
>   */
>  
> -if (!h->BugcheckCode) {
> -h->BugcheckCode = LIVE_SYSTEM_DUMP;
> +if (!WIN_DUMP_FIELD(BugcheckCode)) {
> +*(uint32_t *)WIN_DUMP_FIELD_PTR(BugcheckCode) =
> LIVE_SYSTEM_DUMP; }
>  }
>  
> @@ -155,7 +184,7 @@ static void check_kdbg(WinDumpHeader64 *h, Error
> **errp) {
>  const char OwnerTag[] = "KDBG";
>  char read_OwnerTag[4];
> -uint64_t KdDebuggerDataBlock = h->KdDebuggerDataBlock;
> +uint64_t KdDebuggerDataBlock =
> WIN_DUMP_FIELD(KdDebuggerDataBlock); bool try_fallback 

Re: "Startup" meeting (was Re: Meeting today?)

[Mark Burton]

All, I believe we will have a followup meeting this coming Tuesday 25th 
January, at 15:00 (presumably using the same link: 
https://redhat.bluejeans.com/5402697718).

We (GreenSocs/Xilinx) would like to quickly show what now ‘works’, and to give 
an update on the patches.

Cheers
Mark.




> On 17 Jan 2022, at 18:13, Kevin Wolf  wrote:
> 
> Am 11.01.2022 um 11:22 hat Mark Burton geschrieben:
>> That is my understanding… 
>> See you there!
> 
> Unfortunately, I missed this whole thread until now.
> 
> If the meeting did happen, has anyone taken notes? And if so, where
> could I find them?
> 
> Kevin
> 
>>> On 11 Jan 2022, at 11:20, Philippe Mathieu-Daudé  wrote:
>>> 
>>> Hi,
>>> 
>>> Just checking in, this call is scheduled for today, 3pm CEST, right?
>>> 
>>> Here is the KVM call calendar:
>>> https://www.google.com/calendar/embed?src=dG9iMXRqcXAzN3Y4ZXZwNzRoMHE4a3BqcXNAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ
>>> 
>>> On 1/6/22 12:23, Daniel P. Berrangé wrote:
 No one objected, so I think we can go for the 11th.
 
 On Thu, Jan 06, 2022 at 12:21:56PM +0100, Mark Burton wrote:
> Can we confirm the 11th for this meeting?
> 
> Cheers
> Mark.
> 
> 
>> On 4 Jan 2022, at 10:29, Edgar E. Iglesias  
>> wrote:
>> 
>> 
>> 
>> On Tue, Dec 14, 2021 at 3:49 PM Markus Armbruster > > wrote:
>> Daniel P. Berrangé mailto:berra...@redhat.com>> 
>> writes:
>> 
>>> On Tue, Dec 14, 2021 at 12:37:43PM +0100, Markus Armbruster wrote:
 Mark Burton >>> > writes:
 
> I realise it’s very short notice, but what about having a discussion 
> today at 15:00 ?
 
 I have a conflict today.  I could try to reschedule, but I'd prefer to
 talk next week instead.  Less stress, better prep.
>>> 
>>> I fear we've run out of time for this year if we want all interested
>>> parties to be able to attend.  I'll be off on PTO from end of this
>>> week until the new year, and I know alot of folk are doing similar.
>> 
>> Right.  I'll be off from Dec 23 to Jan 9.  Can we all make Jan 11?
>> 
>> Jan 11th works for me!
>> 
>> Thanks,
>> Edgar
> 
 
 Regards,
 Daniel
>> 
>> 
> 

Re: [PATCH 01/30] bsd-user/arm/target_arch_cpu.h: Move EXCP_ATOMIC to match linux-user

[Richard Henderson]

On 1/10/22 3:18 AM, Warner Losh wrote:

Move the EXCP_ATOMIC case to match linux-user/arm/cpu_loop.c:cpu_loop
ordering.

Signed-off-by: Warner Losh
---
  bsd-user/arm/target_arch_cpu.h | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 02/30] bsd-user/signal.c: implement force_sig_fault

[Richard Henderson]

On 1/10/22 3:18 AM, Warner Losh wrote:

Start to implement the force_sig_fault code. This currently just calls
queue_signal(). The bsd-user fork version of that will handle this the
synchronous nature of this call. Add signal-common.h to hold signal
helper functions like force_sig_fault.

Signed-off-by: Stacey Son
Signed-off-by: Kyle Evans
Signed-off-by: Warner Losh
---
  bsd-user/signal-common.h | 14 ++
  bsd-user/signal.c| 18 ++
  2 files changed, 32 insertions(+)
  create mode 100644 bsd-user/signal-common.h


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 03/30] bsd-user/signal.c: Implement cpu_loop_exit_sigsegv

[Richard Henderson]

On 1/10/22 3:18 AM, Warner Losh wrote:

First attempt at implementing cpu_loop_exit_sigsegv, mostly copied from
linux-user version of this function.

Signed-off-by: Stacey Son
Signed-off-by: Kyle Evans
Signed-off-by: Warner Losh
---
  bsd-user/signal.c | 15 ---
  1 file changed, 12 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 04/30] bsd-user/signal.c: implement cpu_loop_exit_sigbus

[Richard Henderson]

On 1/10/22 3:18 AM, Warner Losh wrote:

First attempt at implementing cpu_loop_exit_sigbus, mostly copied from
linux-user version of this function.

Signed-off-by: Stacey Son
Signed-off-by: Kyle Evans
Signed-off-by: Warner Losh
---
  bsd-user/signal.c | 12 +---
  1 file changed, 9 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 05/30] bsd-user/arm/arget_arch_cpu.h: Move EXCP_DEBUG and EXCP_BKPT together

[Richard Henderson]

On 1/10/22 3:18 AM, Warner Losh wrote:

Implement EXCP_DEBUG and EXCP_BKPT the same, as is done in
linux-user. The prior adjustment of register 15 isn't needed, so remove
that. Remove a redunant comment (that code in FreeBSD never handled
break points).

Signed-off-by: Warner Losh
---
  bsd-user/arm/target_arch_cpu.h | 23 +++
  1 file changed, 3 insertions(+), 20 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 06/30] bsd-user/arm/target_arch_cpu.h: Correct code pointer

[Richard Henderson]

On 1/10/22 3:18 AM, Warner Losh wrote:

The code has moved in FreeBSD since the emulator was started, update the
comment to reflect that change. Remove now-redundant comment saying the
same thing (but incorrectly).

Signed-off-by: Warner Losh
---
  bsd-user/arm/target_arch_cpu.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


With commit message updated, as discussed upthread,
Reviewed-by: Richard Henderson 

r~

Re: [PATCH 01/14] target/arm: Log CPU index in 'Taking exception' log

[Philippe Mathieu-Daudé via]

On 22/1/22 19:24, Peter Maydell wrote:

In an SMP system it can be unclear which CPU is taking an exception;
add the CPU index (which is the same value used in the TCG 'Trace
%d:' logging) to the "Taking exception" log line to clarify it.

Signed-off-by: Peter Maydell 
---
  target/arm/internals.h | 2 +-
  target/arm/helper.c| 9 ++---
  target/arm/m_helper.c  | 2 +-
  3 files changed, 8 insertions(+), 5 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 02/14] hw/intc/arm_gicv3_its: Add tracepoints

[Philippe Mathieu-Daudé via]

On 22/1/22 19:24, Peter Maydell wrote:

The ITS currently has no tracepoints; add a minimal set
that allows basic monitoring of guest register accesses and
reading of commands from the command queue.

Signed-off-by: Peter Maydell 
---
  hw/intc/arm_gicv3_its.c | 11 +++
  hw/intc/trace-events|  8 
  2 files changed, 19 insertions(+)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 07/14] hw/intc/arm_gicv3_its: Sort ITS command list into numeric order

[Philippe Mathieu-Daudé via]

On 22/1/22 19:24, Peter Maydell wrote:

The list of #defines for the ITS command packet numbers is neither
in alphabetical nor numeric order. Sort it into numeric order.

Signed-off-by: Peter Maydell 
---
  hw/intc/gicv3_internal.h | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 10/14] hw/intc/arm_gicv3_its: Provide read accessor for translation_ops

[Philippe Mathieu-Daudé via]

On 22/1/22 19:24, Peter Maydell wrote:

The MemoryRegionOps gicv3_its_translation_ops currently provides only
a .write_with_attrs function, because the only register in this
region is the write-only GITS_TRANSLATER.  However, if you don't
provide a read function and the guest tries reading from this memory
region, QEMU will crash because
memory_region_read_with_attrs_accessor() calls a NULL pointer.

Add a read function which always returns 0, to cover both bogus
attempts to read GITS_TRANSLATER and also reads from the rest of the
region, which is documented to be reserved, RES0.

Signed-off-by: Peter Maydell 
---
  hw/intc/arm_gicv3_its.c | 13 +
  1 file changed, 13 insertions(+)



+static MemTxResult gicv3_its_translation_read(void *opaque, hwaddr offset,
+  uint64_t *data, unsigned size,
+  MemTxAttrs attrs)
+{
+/*
+ * GITS_TRANSLATER is write-only, and all other addresses
+ * in the interrupt translation space frame are RES0.
+ */
+*data = 0;


Maybe log GUEST_ERROR?

Otherwise,
Reviewed-by: Philippe Mathieu-Daudé 


+return MEMTX_OK;
+}

Re: [PATCH 07/30] bsd-user/arm/target_arch_cpu.h: Use force_sig_fault for EXCP_UDEF

[Richard Henderson]

On 1/14/22 4:19 AM, Peter Maydell wrote:

On Sun, 9 Jan 2022 at 16:27, Warner Losh  wrote:


Use force_sig_fault to implement unknown opcode. This just uninlines
that function, so simplify things by using it. Fold in EXCP_NOCP and
EXCP_INVSTATE, as is done in linux-user.

Signed-off-by: Warner Losh 
---
  bsd-user/arm/target_arch_cpu.h | 18 ++
  1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
index 905f13aa1b9..996a361e3fe 100644
--- a/bsd-user/arm/target_arch_cpu.h
+++ b/bsd-user/arm/target_arch_cpu.h
@@ -51,18 +51,12 @@ static inline void target_cpu_loop(CPUARMState *env)
  process_queued_cpu_work(cs);
  switch (trapnr) {
  case EXCP_UDEF:
-{
-/* See arm/arm/undefined.c undefinedinstruction(); */
-info.si_addr = env->regs[15];
-
-/* illegal instruction */
-info.si_signo = TARGET_SIGILL;
-info.si_errno = 0;
-info.si_code = TARGET_ILL_ILLOPC;
-queue_signal(env, info.si_signo, &info);
-
-/* TODO: What about instruction emulation? */
-}
+case EXCP_NOCP:
+case EXCP_INVSTATE:
+/*
+ * See arm/arm/undefined.c undefinedinstruction();
+ */
+force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPC, env->regs[15]);
  break;


Do you want to keep the TODO comment ?

Either way,
Reviewed-by: Peter Maydell 

(Looks like FreeBSD sends SIGILL/ILL_ILLADR for UNDEF where the PC
is misaligned and we're not in Thumb mode, but that's a pretty oddball
corner case so not really worth emulating.)


For qemu, that case will never happen: we'll raise EXCP_PREFETCH_ABORT with fsr=1 
(Alignment).  The freebsd kernel might have this code because the behaviour with real hw 
is CONSTRAINED UNPREDICTABLE (iirc).


Anyway,
Reviewed-by: Richard Henderson 

r~

Re: [PATCH] target/ppc/mmu_common: Fix SRR1/MSR error code on Book-E

[Vitaly Cheptsov]

Hi Cédric,

> and the default ppce500 machine has enough devices for the purpose ?

We cannot test much without a predictable timer emulation on QEMU, sometimes we 
have fairly random freezes, but otherwise the basics work ok. I will let you 
know in case we find something more or less reproducible.

Best,
Vitaly

> On 21 Jan 2022, at 20:33, Cédric Le Goater  wrote:
> 
> Hello Vitaly,
> 
> On 1/21/22 10:33, Vitaly Cheptsov wrote:
>> Hi Cédric,
>>> This looks correct and even fixing an issue that Mario reported
>>> on the TCG e6500 CPU with a kernel + KVM compiled in :
>>> 
>>>  
>>> https://lore.kernel.org/all/R5JFVM$911e343ff81933b99d53fd0992d88...@locati.it/
>>> 
>>> KVM has some issues also with the e6500 but that's another problem
>>> I think.
>> Glad to hear that. Could you schedule the inclusion of the patch in 6.2.1 or 
>> 6.3 please?
> 
> 7.0 it should be.
> 
>>> What is your environment ? Which QEMU machine ? Can you provide a
>>> command line ?
>> We have an in-house RTOS at ISP RAS, which we use to run some environmental 
>> tests on QEMU.
> 
> not a Linux. Diversity is good for the models.
> 
>> The target hardware in this particular example is a QorIQ P3041-based board.
> 
> and the default ppce500 machine has enough devices for the purpose ?
> 
> Thanks,
> 
> C.
> 
>> The command line approximately looks like this:
>> qemu-system-ppc -cpu e500mc -M ppce500 -m 128M -icount 1 -kernel 
>> /path/to/kernel.elf -serial tcp::,server,nodelay
>>> Could you please resend the patch in a non attached way ?  See :
>>> 
>>>  https://www.qemu.org/docs/master/devel/submitting-a-patch.html
>>> 
>>> and copy qemu-devel.
>> Yes, sure. Have just done that.
>> Best regards,
>> Vitaly
>>> On 21 Jan 2022, at 11:17, Cédric Le Goater  wrote:
>>> 
>>> Hello Vitaly
>>> 
>>> On 1/21/22 01:02, Vitaly Cheptsov wrote:
 Hello,
 PowerPC e500mc defines MSR bit 35 differently from most other PowerPC 
 variants. In particular, for e500mc this is GS (Guest Supervisor) bit[1], 
 while for others it is NOEXEC GUARD bit[2].
 QEMU ignores this architectural difference when handling the exceptions of 
 attempting to run not executable code on e500mc, and mistakenly sets the 
 GS bit[3][4].
 Setting this bit eventually leads to crashes, because although QEMU does 
 not support Guest Supervisor mode on e500mc, it still requires it to be 
 disabled[5].
>>> 
>>> This looks correct and even fixing an issue that Mario reported
>>> on the TCG e6500 CPU with a kernel + KVM compiled in :
>>> 
>>>  
>>> https://lore.kernel.org/all/R5JFVM$911e343ff81933b99d53fd0992d88...@locati.it/
>>> 
>>> KVM has some issues also with the e6500 but that's another problem
>>> I think.
>>> 
>>> 
>>> What is your environment ? Which QEMU machine ? Can you provide a
>>> command line ?
>>> 
>>> Could you please resend the patch in a non attached way ?  See :
>>> 
>>>  https://www.qemu.org/docs/master/devel/submitting-a-patch.html
>>> 
>>> and copy qemu-devel.
>>> 
>>> Thanks,
>>> 
>>> C.
>>> 
>>> 
 Best regards,
 Vitaly
 [1] https://www.nxp.com/docs/en/reference-manual/E500MCRM.pdf, 2.7.1 MSR
 [2] https://openpowerfoundation.org/?resource_lib=power-isa-version-3-0, 
 6.5.5 Instruction Storage Interrupt
 [3] https://github.com/qemu/qemu/blob/v6.2.0/target/ppc/mmu_common.c#L1426
 [4] 
 https://github.com/qemu/qemu/blob/v6.2.0/target/ppc/excp_helper.c#L414-L416
 [5] 
 https://github.com/qemu/qemu/blob/v6.2.0/target/ppc/mmu_helper.c#L1078-L1080
>>> 
> 



signature.asc
Description: Message signed with OpenPGP

Re: [PATCH v1] include: hw: remove ibex_plic.h

[Alistair Francis]

On Fri, Jan 21, 2022 at 3:50 PM Alistair Francis
 wrote:
>
> From: Wilfred Mallawa 
>
> This patch removes the left-over/unused `ibex_plic.h` file. Previously
> used by opentitan, which now follows the RISC-V standard and uses the
> SiFivePlicState.
>
> Fixes: 434e7e021 ("hw/intc: Remove the Ibex PLIC")
> Signed-off-by: Wilfred Mallawa 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  include/hw/intc/ibex_plic.h | 67 -
>  1 file changed, 67 deletions(-)
>  delete mode 100644 include/hw/intc/ibex_plic.h
>
> diff --git a/include/hw/intc/ibex_plic.h b/include/hw/intc/ibex_plic.h
> deleted file mode 100644
> index d596436e06..00
> --- a/include/hw/intc/ibex_plic.h
> +++ /dev/null
> @@ -1,67 +0,0 @@
> -/*
> - * QEMU RISC-V lowRISC Ibex PLIC
> - *
> - * Copyright (c) 2020 Western Digital
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms and conditions of the GNU General Public License,
> - * version 2 or later, as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope it will be useful, but WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> - * more details.
> - *
> - * You should have received a copy of the GNU General Public License along 
> with
> - * this program.  If not, see .
> - */
> -
> -#ifndef HW_IBEX_PLIC_H
> -#define HW_IBEX_PLIC_H
> -
> -#include "hw/sysbus.h"
> -#include "qom/object.h"
> -
> -#define TYPE_IBEX_PLIC "ibex-plic"
> -OBJECT_DECLARE_SIMPLE_TYPE(IbexPlicState, IBEX_PLIC)
> -
> -struct IbexPlicState {
> -/*< private >*/
> -SysBusDevice parent_obj;
> -
> -/*< public >*/
> -MemoryRegion mmio;
> -
> -uint32_t *pending;
> -uint32_t *hidden_pending;
> -uint32_t *claimed;
> -uint32_t *source;
> -uint32_t *priority;
> -uint32_t *enable;
> -uint32_t threshold;
> -uint32_t claim;
> -
> -/* config */
> -uint32_t num_cpus;
> -uint32_t num_sources;
> -
> -uint32_t pending_base;
> -uint32_t pending_num;
> -
> -uint32_t source_base;
> -uint32_t source_num;
> -
> -uint32_t priority_base;
> -uint32_t priority_num;
> -
> -uint32_t enable_base;
> -uint32_t enable_num;
> -
> -uint32_t threshold_base;
> -
> -uint32_t claim_base;
> -
> -qemu_irq *external_irqs;
> -};
> -
> -#endif /* HW_IBEX_PLIC_H */
> --
> 2.34.1
>

Re: [PATCH 1/1] Allow setting up to 8 bytes with the generic loader

[Alistair Francis]

On Thu, Jan 20, 2022 at 7:57 PM Petr Tesarik  wrote:
>
> The documentation for the generic loader says that "the maximum size of
> the data is 8 bytes". However, attempts to set data-len=8 trigger the
> following assertion failure:
>
> ../hw/core/generic-loader.c:59: generic_loader_reset: Assertion `s->data_len 
> < sizeof(s->data)' failed.
>
> The type of s->data is uint64_t (i.e. 8 bytes long), so I believe this
> assert should use <= instead of <.
>
> Fixes: e481a1f63c93 ("generic-loader: Add a generic loader")
> Signed-off-by: Petr Tesarik 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  hw/core/generic-loader.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
> index 9a24ffb880..504ed7ca72 100644
> --- a/hw/core/generic-loader.c
> +++ b/hw/core/generic-loader.c
> @@ -56,7 +56,7 @@ static void generic_loader_reset(void *opaque)
>  }
>
>  if (s->data_len) {
> -assert(s->data_len < sizeof(s->data));
> +assert(s->data_len <= sizeof(s->data));
>  dma_memory_write(s->cpu->as, s->addr, &s->data, s->data_len,
>   MEMTXATTRS_UNSPECIFIED);
>  }
> --
> 2.31.1
>
>

[PATCH 0/2] RISC-V: Correctly generate store/amo faults

[Alistair Francis]

From: Alistair Francis 

This series adds a MO_ op to specify that a load instruction should
produce a store fault. This is used on RISC-V to produce a store/amo
fault when an atomic access fails.

This fixes: https://gitlab.com/qemu-project/qemu/-/issues/594

Alistair Francis (2):
  accel: tcg: Allow forcing a store fault on read ops
  targett/riscv: rva: Correctly generate a store/amo fault

 include/exec/memop.h|  2 +
 accel/tcg/cputlb.c  | 11 -
 target/riscv/insn_trans/trans_rva.c.inc | 56 -
 3 files changed, 48 insertions(+), 21 deletions(-)

-- 
2.31.1

[PATCH 2/2] targett/riscv: rva: Correctly generate a store/amo fault

[Alistair Francis]

From: Alistair Francis 

If the atomic operation fails we want to generate a MMU_DATA_STORE
access type so we can produce a RISCV_EXCP_STORE_AMO_ACCESS_FAULT for
the guest.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/594
Signed-off-by: Alistair Francis 
---
 target/riscv/insn_trans/trans_rva.c.inc | 56 -
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rva.c.inc 
b/target/riscv/insn_trans/trans_rva.c.inc
index 45db82c9be..be5c94803b 100644
--- a/target/riscv/insn_trans/trans_rva.c.inc
+++ b/target/riscv/insn_trans/trans_rva.c.inc
@@ -93,7 +93,7 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
 static bool trans_lr_w(DisasContext *ctx, arg_lr_w *a)
 {
 REQUIRE_EXT(ctx, RVA);
-return gen_lr(ctx, a, (MO_ALIGN | MO_TESL));
+return gen_lr(ctx, a, (MO_ALIGN  | MO_TESL));
 }
 
 static bool trans_sc_w(DisasContext *ctx, arg_sc_w *a)
@@ -105,55 +105,64 @@ static bool trans_sc_w(DisasContext *ctx, arg_sc_w *a)
 static bool trans_amoswap_w(DisasContext *ctx, arg_amoswap_w *a)
 {
 REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, (MO_ALIGN | MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
 }
 
 static bool trans_amoadd_w(DisasContext *ctx, arg_amoadd_w *a)
 {
 REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_add_tl, (MO_ALIGN | MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_add_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
 }
 
 static bool trans_amoxor_w(DisasContext *ctx, arg_amoxor_w *a)
 {
 REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_xor_tl, (MO_ALIGN | MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_xor_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
 }
 
 static bool trans_amoand_w(DisasContext *ctx, arg_amoand_w *a)
 {
 REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_and_tl, (MO_ALIGN | MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_and_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
 }
 
 static bool trans_amoor_w(DisasContext *ctx, arg_amoor_w *a)
 {
 REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_or_tl, (MO_ALIGN | MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_or_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
 }
 
 static bool trans_amomin_w(DisasContext *ctx, arg_amomin_w *a)
 {
 REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_smin_tl, (MO_ALIGN | 
MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_smin_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
 }
 
 static bool trans_amomax_w(DisasContext *ctx, arg_amomax_w *a)
 {
 REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_smax_tl, (MO_ALIGN | 
MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_smax_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
 }
 
 static bool trans_amominu_w(DisasContext *ctx, arg_amominu_w *a)
 {
 REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_umin_tl, (MO_ALIGN | 
MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_umin_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
 }
 
 static bool trans_amomaxu_w(DisasContext *ctx, arg_amomaxu_w *a)
 {
 REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_umax_tl, (MO_ALIGN | 
MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_umax_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
 }
 
 static bool trans_lr_d(DisasContext *ctx, arg_lr_d *a)
@@ -171,53 +180,62 @@ static bool trans_sc_d(DisasContext *ctx, arg_sc_d *a)
 static bool trans_amoswap_d(DisasContext *ctx, arg_amoswap_d *a)
 {
 REQUIRE_64BIT(ctx);
-return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, (MO_ALIGN | MO_TEUQ));
+return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TEUQ));
 }
 
 static bool trans_amoadd_d(DisasContext *ctx, arg_amoadd_d *a)
 {
 REQUIRE_64BIT(ctx);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_add_tl, (MO_ALIGN | MO_TEUQ));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_add_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TEUQ));
 }
 
 static bool trans_amoxor_d(DisasContext *ctx, arg_amoxor_d *a)
 {
 REQUIRE_64BIT(ctx);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_xor_tl, (MO_ALIGN | MO_TEUQ));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_xor_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TEUQ));
 }
 
 static bool trans_amoand_d(DisasContext *ctx, arg_amoand_d *a)
 {
 REQUIRE_64BIT(ctx);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_and_tl, (MO_ALIGN | MO_TEUQ));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_and_tl,
+   (MO_ALIGN | MO_WRITE_FA

[PATCH 1/2] accel: tcg: Allow forcing a store fault on read ops

[Alistair Francis]

From: Alistair Francis 

When performing atomic operations TCG will do a read operation then a
write operation. This results in a MMU_DATA_LOAD fault if the address is
invalid.

For some platforms (such as RISC-V) we should produce a store fault if
an atomic operation fails. This patch adds a new MemOp (MO_WRITE_FAULT)
that allows us to indicate that the operation should produce a
MMU_DATA_STORE access type if the operation faults.

Signed-off-by: Alistair Francis 
---
 include/exec/memop.h |  2 ++
 accel/tcg/cputlb.c   | 11 +--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/exec/memop.h b/include/exec/memop.h
index 2a885f3917..93ae1b6a2e 100644
--- a/include/exec/memop.h
+++ b/include/exec/memop.h
@@ -81,6 +81,8 @@ typedef enum MemOp {
 MO_ALIGN_32 = 5 << MO_ASHIFT,
 MO_ALIGN_64 = 6 << MO_ASHIFT,
 
+MO_WRITE_FAULT = 0x100,
+
 /* Combinations of the above, for ease of use.  */
 MO_UB= MO_8,
 MO_UW= MO_16,
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 5e0d0eebc3..320555d5e9 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1362,8 +1362,15 @@ static uint64_t io_readx(CPUArchState *env, 
CPUIOTLBEntry *iotlbentry,
 section->offset_within_address_space -
 section->offset_within_region;
 
-cpu_transaction_failed(cpu, physaddr, addr, memop_size(op), 
access_type,
-   mmu_idx, iotlbentry->attrs, r, retaddr);
+if (op & MO_WRITE_FAULT) {
+cpu_transaction_failed(cpu, physaddr, addr, memop_size(op),
+   MMU_DATA_STORE, mmu_idx, iotlbentry->attrs,
+   r, retaddr);
+} else {
+cpu_transaction_failed(cpu, physaddr, addr, memop_size(op),
+   access_type, mmu_idx, iotlbentry->attrs,
+   r, retaddr);
+}
 }
 if (locked) {
 qemu_mutex_unlock_iothread();
-- 
2.31.1

Re: [PATCH 08/30] bsd-user/arm/target_arch_cpu.h: Implement data faults

[Richard Henderson]

On 1/10/22 3:19 AM, Warner Losh wrote:

Update for the richer set of data faults that are now possible. Copied
largely from linux-user/arm/cpu_loop.c

Signed-off-by: Warner Losh
---
  bsd-user/arm/target_arch_cpu.h | 44 ++
  1 file changed, 34 insertions(+), 10 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 10/30] bsd-user/signal.c: Implement signal_init()

[Richard Henderson]

On 1/10/22 3:19 AM, Warner Losh wrote:

Initialize the signal state for the emulator. Setup a set of sane
default signal handlers, mirroring the host's signals. For fatal signals
(those that exit by default), establish our own set of signal
handlers. Stub out the actual signal handler we use for the moment.

Signed-off-by: Stacey Son 
Signed-off-by: Kyle Evans 
Signed-off-by: Warner Losh 
---
  bsd-user/qemu.h   |  1 +
  bsd-user/signal.c | 68 +++
  2 files changed, 69 insertions(+)

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index 334f8b1d715..0e0b8db708b 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -97,6 +97,7 @@ typedef struct TaskState {
  struct qemu_sigqueue sigqueue_table[MAX_SIGQUEUE_SIZE]; /* siginfo queue 
*/
  struct qemu_sigqueue *first_free; /* first free siginfo queue entry */
  int signal_pending; /* non zero if a signal may be pending */
+sigset_t signal_mask;
  
  uint8_t stack[];

  } __attribute__((aligned(16))) TaskState;
diff --git a/bsd-user/signal.c b/bsd-user/signal.c
index 7ea86149981..b2c91c39379 100644
--- a/bsd-user/signal.c
+++ b/bsd-user/signal.c
@@ -28,6 +28,9 @@
   * fork.
   */
  
+static struct target_sigaction sigact_table[TARGET_NSIG];

+static void host_signal_handler(int host_sig, siginfo_t *info, void *puc);
+
  int host_to_target_signal(int sig)
  {
  return sig;
@@ -47,6 +50,28 @@ void queue_signal(CPUArchState *env, int sig, 
target_siginfo_t *info)
  qemu_log_mask(LOG_UNIMP, "No signal queueing, dropping signal %d\n", sig);
  }
  
+static int fatal_signal(int sig)

+{
+
+switch (sig) {
+case TARGET_SIGCHLD:
+case TARGET_SIGURG:
+case TARGET_SIGWINCH:
+case TARGET_SIGINFO:
+/* Ignored by default. */
+return 0;
+case TARGET_SIGCONT:
+case TARGET_SIGSTOP:
+case TARGET_SIGTSTP:
+case TARGET_SIGTTIN:
+case TARGET_SIGTTOU:
+/* Job control signals.  */
+return 0;
+default:
+return 1;
+}
+}
+
  /*
   * Force a synchronously taken QEMU_SI_FAULT signal. For QEMU the
   * 'force' part is handled in process_pending_signals().
@@ -64,8 +89,51 @@ void force_sig_fault(int sig, int code, abi_ulong addr)
  queue_signal(env, sig, &info);
  }
  
+static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)

+{
+}
+
  void signal_init(void)
  {
+TaskState *ts = (TaskState *)thread_cpu->opaque;
+struct sigaction act;
+struct sigaction oact;
+int i;
+int host_sig;
+
+/* Set the signal mask from the host mask. */
+sigprocmask(0, 0, &ts->signal_mask);
+
+/*
+ * Set all host signal handlers. ALL signals are blocked during the
+ * handlers to serialize them.
+ */
+memset(sigact_table, 0, sizeof(sigact_table));
+
+sigfillset(&act.sa_mask);
+act.sa_sigaction = host_signal_handler;
+act.sa_flags = SA_SIGINFO;
+
+for (i = 1; i <= TARGET_NSIG; i++) {
+host_sig = target_to_host_signal(i);
+sigaction(host_sig, NULL, &oact);


Missing test for CONFIG_GPROF + SIGPROF.  Otherwise,

Reviewed-by: Richard Henderson 

r~

Re: [PATCH 12/30] bsd-user/host/i386/host-signal.h: Implement host_signal_*

[Richard Henderson]

On 1/10/22 3:19 AM, Warner Losh wrote:

Implement host_signal_pc, host_signal_set_pc and host_signal_write for
i386.

Signed-off-by: Kyle Evans
Signed-off-by: Warner Losh
---
  bsd-user/host/i386/host-signal.h | 37 
  1 file changed, 37 insertions(+)
  create mode 100644 bsd-user/host/i386/host-signal.h


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 13/30] bsd-user/host/x86_64/host-signal.h: Implement host_signal_*

[Richard Henderson]

On 1/10/22 3:19 AM, Warner Losh wrote:

Implement host_signal_pc, host_signal_set_pc and host_signal_write for
x86_64.

Signed-off-by: Kyle Evans
Signed-off-by: Warner Losh
---
  bsd-user/host/x86_64/host-signal.h | 37 ++
  1 file changed, 37 insertions(+)
  create mode 100644 bsd-user/host/x86_64/host-signal.h


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 14/30] bsd-user: Add host signals to the build

[Richard Henderson]

On 1/10/22 3:19 AM, Warner Losh wrote:

Start to add the host signal functionality to the build.

Signed-off-by: Warner Losh
---
  bsd-user/meson.build | 1 +
  bsd-user/signal.c| 1 +
  meson.build  | 1 +
  3 files changed, 3 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 15/30] bsd-user: Add trace events for bsd-usr

[Richard Henderson]

On 1/10/22 3:19 AM, Warner Losh wrote:

Add the bsd-user specific events and infrastructure. Only include the
linux-user trace events for linux-user, not bsd-user.

Signed-off-by: Stacey Son
Signed-off-by: Kyle Evans
Signed-off-by: Warner Losh
---
  bsd-user/signal.c |  1 +
  bsd-user/trace-events | 11 +++
  bsd-user/trace.h  |  1 +
  meson.build   |  5 -
  4 files changed, 17 insertions(+), 1 deletion(-)
  create mode 100644 bsd-user/trace-events
  create mode 100644 bsd-user/trace.h


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 16/30] bsd-user/signal.c: host_to_target_siginfo_noswap

[Richard Henderson]

On 1/10/22 3:19 AM, Warner Losh wrote:

+static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
+const siginfo_t *info)
+{
+int sig, code;
+
+sig = host_to_target_signal(info->si_signo);


You now have a target signo, so...


+if (SIGILL == sig || SIGFPE == sig || SIGSEGV == sig || SIGBUS == sig ||
+SIGTRAP == sig) {


... you need TARGET_SIGFOO in the comparision.

Though, really, I think the categorization that Peter suggested is a better way to 
structure this.



r~

Re: [PATCH 17/30] bsd-user/signal.c: Implement rewind_if_in_safe_syscall

[Richard Henderson]

On 1/10/22 3:19 AM, Warner Losh wrote:

Signed-off-by: Kyle Evans
Signed-off-by: Warner Losh
---
  bsd-user/qemu.h   |  2 ++
  bsd-user/signal.c | 12 
  2 files changed, 14 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 19/30] bsd-user/strace.c: print_taken_signal

[Richard Henderson]

On 1/10/22 3:19 AM, Warner Losh wrote:

print_taken_signal() prints signals when we're tracing signals.

Signed-off-by: Stacey Son
Signed-off-by: Kyle Evans
Signed-off-by: Warner Losh
---
  bsd-user/qemu.h   | 10 +
  bsd-user/strace.c | 97 +++
  2 files changed, 107 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 20/30] bsd-user/signal.c: core_dump_signal

[Richard Henderson]

On 1/10/22 3:19 AM, Warner Losh wrote:

Returns 1 for signals that cause core files.

Signed-off-by: Stacey Son
Signed-off-by: Kyle Evans
Signed-off-by: Warner Losh
---
  bsd-user/signal.c | 17 +
  1 file changed, 17 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v11 1/4] migration/dirtyrate: refactor dirty page rate calculation

[Peter Xu]

On Sat, Jan 22, 2022 at 11:22:37AM +0800, Hyman Huang wrote:
> 
> 
> 在 2022/1/17 10:19, Peter Xu 写道:
> > On Wed, Jan 05, 2022 at 01:14:06AM +0800, huang...@chinatelecom.cn wrote:
> > > From: Hyman Huang(黄勇) 
> > > 
> > > +
> > > +static void vcpu_dirty_stat_collect(VcpuStat *stat,
> > > +DirtyPageRecord *records,
> > > +bool start)
> > > +{
> > > +CPUState *cpu;
> > > +
> > > +CPU_FOREACH(cpu) {
> > > +if (!start && cpu->cpu_index >= stat->nvcpu) {
> > > +/*
> > > + * Never go there unless cpu is hot-plugged,
> > > + * just ignore in this case.
> > > + */
> > > +continue;
> > > +}
> > 
> > As commented before, I think the only way to do it right is does not allow 
> > cpu
> > plug/unplug during measurement..
> > 
> > Say, even if index didn't get out of range, an unplug even should generate 
> > very
> > stange output of the unplugged cpu.  Please see more below.
> > 
> > > +record_dirtypages(records, cpu, start);
> > > +}
> > > +}
> > > +
> > > +int64_t vcpu_calculate_dirtyrate(int64_t calc_time_ms,
> > > + int64_t init_time_ms,
> > > + VcpuStat *stat,
> > > + unsigned int flag,
> > > + bool one_shot)
> > > +{
> > > +DirtyPageRecord *records;
> > > +int64_t duration;
> > > +int64_t dirtyrate;
> > > +int i = 0;
> > > +
> > > +cpu_list_lock();
> > > +records = vcpu_dirty_stat_alloc(stat);
> > > +vcpu_dirty_stat_collect(stat, records, true);
> > > +cpu_list_unlock();
> > 
> > Continue with above - then I'm wondering whether we should just keep taking 
> > the
> > lock until vcpu_dirty_stat_collect().
> > 
> > Yes we could be taking the lock for a long time because of the sleep, but 
> > the
> > main thread plug thread will just wait for it to complete and it is at least
> > not a e.g. deadlock.
> > 
> > The other solution is we do cpu_list_unlock() like this, but introduce 
> > another
> > cpu_list_generation_id and boost it after any plug/unplug of cpu, aka, when 
> > cpu
> > list changes.
> > 
> > Then we record cpu generation ID at the entry of this function and retry the
> > whole measurement if at some point we found generation ID changed (we need 
> > to
> > fetch the gen ID after having the lock, of course).  That could avoid us 
> > taking
> > the cpu list lock during dirty_stat_wait(), but it'll start to complicate 
> > cpu
> > list locking rules.
> > 
> > The simpler way is still just to take the lock, imho.
> > 
> Hi, Peter, i'm working on this as you suggetion, and keep taking the
> cpu_list_lock during dirty page rate calculation. I found the deadlock when
> testing hotplug scenario, the logic is as the following:
> 
> calc thread   qemu main thread
> 1. take qemu_cpu_list_lock
>   1. take the BQL
> 2. collect dirty page and wait2. cpu hotplug
>   3. take qemu_cpu_list_lock
> 3. take the BQL
> 
> 4. sync dirty log 
> 
> 5. release the BQL
> 
> I just recall that is one of the reasons why i handle the plug/unplug
> scenario(another is cpu plug may wait a little bit long time when dirtylimit
> in service).

Ah I should have noticed the bql dependency with cpu list lock before..

I think having the cpu plug waiting for one sec is fine, because the mgmt app
should be aware of both so it shouldn't even happen in practise (not good
timing to plug during pre-migration).  However bql is definitely another
story..  which I agree.

> 
> It seems that we have two strategies, one is just keep this logic untouched
> in v12 and add "cpu_list_generation_id" implementaion in TODO list(once this
> patchset been merged, i'll try that out)， another is introducing the
> "cpu_list_generation_id" right now.
> 
> What strategy do you prefer to?

I prefer having the gen_id patch.  The thing is it should be less than 10 lines
and the logic should be fairly straightforward.  While if without it, it seems
always on risk to use this new feature.

I hope I didn't overlook any existing mechanism to block cpu plug/unplug for
some period, though, or we should use it.

> 
> Uh... I think the "unmatched_cnt" also kind of like this too, becauce once
> we remove the "unmatched count" logic, the throttle algo is more likely to
> oscillate and i prefer to add the "unmatched_cnt" in TODO list as above.

Could we tune the differential factor to make it less possible to oscillate?

I still can't say I like "unmatched cnt" idea a lot..  From a PID pov (partial,
integral, differential) you've already got partial + differential, and IMHO
that "unmatched cnt" solution was trying to mimic an "integral" delta.  Instead
of doing an mean value calculation (as in most integral system does) the

Re: [PATCH v11 3/4] softmmu/dirtylimit: implement virtual CPU throttle

[Peter Xu]

On Sat, Jan 22, 2022 at 11:54:07AM +0800, Hyman Huang wrote:
> 
> > > +static void *dirtylimit_thread(void *opaque)
> > > +{
> > > +CPUState *cpu;
> > > +
> > > +rcu_register_thread();
> > > +
> > > +while (!qatomic_read(&dirtylimit_quit)) {
> > > +sleep(DIRTYLIMIT_CALC_TIME_MS / 1000);
> > 
> > Sorry to have not mentioned this: I think we probably don't even need this
> > dirtylimit thread.
> > 
> > It'll be hard to make the "sleep" right here.. you could read two identical
> > values from the dirty calc thread because the 1sec sleep is not accurate, so
> > even after this sleep() the calc thread may not have provided the latest 
> > number
> > yet.
> > 
> > It'll be much cleaner (and most importantly, accurate..) to me if we could 
> > make
> > this a hook function being passed over to the vcpu_dirty_rate_stat_thread()
> > thread, then after each vcpu_dirty_rate_stat_collect() we call the hook

Another cut-off email?  Please try again?.. :)

-- 
Peter Xu

Re: Cross compiling 6.2.0 aarch64-softmmu for aarch64 host with musl, struct redefiniton errors

[Adam Baxter]

Hi Peter
I realise this might be a bit offtopic for this mailing list, but might help 
others searching for the same error message

> I think this must be a musl problem. Quoting the full error
> message:

Turns out this is actually related to the kernel (headers?) version - fixed in 
4.20

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=9966a05c7b80f075f2bc7e48dbb108d3f2927234


Where musl-cross-make is still using 4.19 by default, 
https://github.com/richfelker/musl-cross-make/blob/master/Makefile#L11

Bumping the kernel version allows qemu to build successfully.

Regards,
Adam

Re: [PATCH v11 3/4] softmmu/dirtylimit: implement virtual CPU throttle

[Hyman Huang]

在 2022/1/17 15:32, Peter Xu 写道:

On Wed, Jan 05, 2022 at 01:14:08AM +0800, huang...@chinatelecom.cn wrote:

  ##
+# @DirtyLimitInfo:
+#
+# Dirty page rate limit information of virtual CPU.
+#
+# @cpu-index: index of virtual CPU.
+#
+# @limit-rate: upper limit of dirty page rate for virtual CPU.
+#
+# @current-rate: current dirty page rate for virtual CPU.


Please consider spell out the unit too for all these dirty rate fields (MB/s).


+#
+# Since: 7.0
+#
+##
+{ 'struct': 'DirtyLimitInfo',
+  'data': { 'cpu-index': 'int',
+'limit-rate': 'int64',
+'current-rate': 'int64' } }
+
+##
  # @snapshot-save:
  #
  # Save a VM snapshot
diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
index a10ac6f..c9f5745 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -18,6 +18,26 @@
  #include "sysemu/dirtylimit.h"
  #include "exec/memory.h"
  #include "hw/boards.h"
+#include "sysemu/kvm.h"
+#include "trace.h"
+
+/*
+ * Dirtylimit stop working if dirty page rate error
+ * value less than DIRTYLIMIT_TOLERANCE_RANGE
+ */
+#define DIRTYLIMIT_TOLERANCE_RANGE  25  /* MB/s */
+/*
+ * Plus or minus vcpu sleep time linearly if dirty
+ * page rate error value percentage over
+ * DIRTYLIMIT_LINEAR_ADJUSTMENT_PCT.
+ * Otherwise, plus or minus a fixed vcpu sleep time.
+ */
+#define DIRTYLIMIT_LINEAR_ADJUSTMENT_PCT 50
+/*
+ * Max vcpu sleep time percentage during a cycle
+ * composed of dirty ring full and sleep time.
+ */
+#define DIRTYLIMIT_THROTTLE_PCT_MAX 99


(Thanks for the enriched comments)


+static inline void dirtylimit_vcpu_set_quota(int cpu_index,
+ uint64_t quota,
+ bool on)
+{
+dirtylimit_state->states[cpu_index].quota = quota;


To be clear, we could move this line into the "(on)" if condition, then in !on
case we reset it.


+if (on) {
+if (!dirtylimit_vcpu_get_state(cpu_index)->enabled) {
+dirtylimit_state->limited_nvcpu++;
+}
+} else {
+if (dirtylimit_state->states[cpu_index].enabled) {
+dirtylimit_state->limited_nvcpu--;
+}
+}
+
+dirtylimit_state->states[cpu_index].enabled = on;
+}
+
+static inline int64_t dirtylimit_dirty_ring_full_time(uint64_t dirtyrate)
+{
+static uint64_t max_dirtyrate;
+uint32_t dirty_ring_size = kvm_dirty_ring_size();
+uint64_t dirty_ring_size_meory_MB =
+dirty_ring_size * TARGET_PAGE_SIZE >> 20;
+
+if (max_dirtyrate < dirtyrate) {
+max_dirtyrate = dirtyrate;
+}
+
+return dirty_ring_size_meory_MB * 100 / max_dirtyrate;
+}
+
+static inline bool dirtylimit_done(uint64_t quota,
+   uint64_t current)
+{
+uint64_t min, max;
+
+min = MIN(quota, current);
+max = MAX(quota, current);
+
+return ((max - min) <= DIRTYLIMIT_TOLERANCE_RANGE) ? true : false;
+}
+
+static inline bool
+dirtylimit_need_linear_adjustment(uint64_t quota,
+  uint64_t current)
+{
+uint64_t min, max, pct;
+
+min = MIN(quota, current);
+max = MAX(quota, current);
+
+pct = (max - min) * 100 / max;
+
+return pct > DIRTYLIMIT_LINEAR_ADJUSTMENT_PCT;
+}
+
+static void dirtylimit_set_throttle(CPUState *cpu,
+uint64_t quota,
+uint64_t current)
+{
+int64_t ring_full_time_us = 0;
+uint64_t sleep_pct = 0;
+uint64_t throttle_us = 0;
+
+ring_full_time_us = dirtylimit_dirty_ring_full_time(current);
+
+if (dirtylimit_need_linear_adjustment(quota, current)) {
+if (quota < current) {
+sleep_pct = (current - quota) * 100 / current;
+throttle_us =
+ring_full_time_us * sleep_pct / (double)(100 - sleep_pct);
+cpu->throttle_us_per_full += throttle_us;
+} else {
+sleep_pct = (quota - current) * 100 / quota;
+throttle_us =
+ring_full_time_us * sleep_pct / (double)(100 - sleep_pct);
+cpu->throttle_us_per_full -= throttle_us;
+}
+
+trace_dirtylimit_throttle_pct(cpu->cpu_index,
+  sleep_pct,
+  throttle_us);
+} else {
+if (quota < current) {
+cpu->throttle_us_per_full += ring_full_time_us / 10;
+} else {
+cpu->throttle_us_per_full -= ring_full_time_us / 10;
+}
+}
+
+cpu->throttle_us_per_full = MIN(cpu->throttle_us_per_full,
+ring_full_time_us * DIRTYLIMIT_THROTTLE_PCT_MAX);
+
+cpu->throttle_us_per_full = MAX(cpu->throttle_us_per_full, 0);
+}


This algorithm seems works even worse than the previous version, could you have
a look on what's wrong?

See how it fluctuates when I set a throttle of 300MB/s:

(QMP) set-vcpu-dirty-limit dirty-rate=300

Dirty rate: 17622 (MB/s), duration: 1000 (ms), load: 100.00%
Dirty rate: 17617 (MB/s), duration: 1

Re: [PATCH 21/31] util: Add iova_tree_alloc

[Peter Xu]

On Fri, Jan 21, 2022 at 09:27:23PM +0100, Eugenio Pérez wrote:
> +int iova_tree_alloc(IOVATree *tree, DMAMap *map, hwaddr iova_begin,
> +hwaddr iova_last)
> +{
> +const DMAMapInternal *last, *i;
> +
> +assert(iova_begin < iova_last);
> +
> +/*
> + * Find a valid hole for the mapping
> + *
> + * TODO: Replace all this with g_tree_node_first/next/last when available
> + * (from glib since 2.68). Using a sepparated QTAILQ complicates code.
> + *
> + * Try to allocate first at the end of the list.
> + */
> +last = QTAILQ_LAST(&tree->list);
> +if (iova_tree_alloc_map_in_hole(last, NULL, iova_begin, iova_last,
> +map->size)) {
> +goto alloc;
> +}
> +
> +/* Look for inner hole */
> +last = NULL;
> +for (i = QTAILQ_FIRST(&tree->list); i;
> + last = i, i = QTAILQ_NEXT(i, entry)) {
> +if (iova_tree_alloc_map_in_hole(last, i, iova_begin, iova_last,
> +map->size)) {
> +goto alloc;
> +}
> +}
> +
> +return IOVA_ERR_NOMEM;
> +
> +alloc:
> +map->iova = last ? last->map.iova + last->map.size + 1 : iova_begin;
> +return iova_tree_insert(tree, map);
> +}

Hi, Eugenio,

Have you tried with what Jason suggested previously?

  
https://lore.kernel.org/qemu-devel/cacgkmetzapd9xqtp_r4w296n_qz7vuv1flnb544fevoyo0o...@mail.gmail.com/

That solution still sounds very sensible to me even without the newly
introduced list in previous two patches.

IMHO we could move "DMAMap *previous, *this" into the IOVATreeAllocArgs*
stucture that was passed into the traverse func though, so it'll naturally work
with threading.

Or is there any blocker for it?

Thanks,

-- 
Peter Xu

Re: [PATCH 0/2] RISC-V: Correctly generate store/amo faults

[LIU Zhiwei]

On 2022/1/24 上午8:59, Alistair Francis wrote:

From: Alistair Francis 

This series adds a MO_ op to specify that a load instruction should
produce a store fault. This is used on RISC-V to produce a store/amo
fault when an atomic access fails.


Hi Alistair,

As Richard said,  we  can address this issue in two ways, probe_read(I 
think probe_write is typo) or with another new MO_ op.


In my opinion use MO_op in io_readx may be not right because the issue 
is not only with IO access. And MO_ op in io_readx is too later because 
the exception has been created when tlb_fill.


Currently tlb_fill doesn't receive this parameter. Is it possible to add 
a new Memop parameter to  tlb_fill?


Thanks,
Zhiwei



This fixes: https://gitlab.com/qemu-project/qemu/-/issues/594

Alistair Francis (2):
   accel: tcg: Allow forcing a store fault on read ops
   targett/riscv: rva: Correctly generate a store/amo fault

  include/exec/memop.h|  2 +
  accel/tcg/cputlb.c  | 11 -
  target/riscv/insn_trans/trans_rva.c.inc | 56 -
  3 files changed, 48 insertions(+), 21 deletions(-)

Re: [PATCH 2/2] targett/riscv: rva: Correctly generate a store/amo fault

[LIU Zhiwei]

On 2022/1/24 上午8:59, Alistair Francis wrote:

From: Alistair Francis 

If the atomic operation fails we want to generate a MMU_DATA_STORE
access type so we can produce a RISCV_EXCP_STORE_AMO_ACCESS_FAULT for
the guest.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/594
Signed-off-by: Alistair Francis 
---
  target/riscv/insn_trans/trans_rva.c.inc | 56 -
  1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rva.c.inc 
b/target/riscv/insn_trans/trans_rva.c.inc
index 45db82c9be..be5c94803b 100644
--- a/target/riscv/insn_trans/trans_rva.c.inc
+++ b/target/riscv/insn_trans/trans_rva.c.inc
@@ -93,7 +93,7 @@ static bool gen_amo(DisasContext *ctx, arg_atomic *a,
  static bool trans_lr_w(DisasContext *ctx, arg_lr_w *a)
  {
  REQUIRE_EXT(ctx, RVA);
-return gen_lr(ctx, a, (MO_ALIGN | MO_TESL));
+return gen_lr(ctx, a, (MO_ALIGN  | MO_TESL));


Typo.  This changes nothing.

Besides, I think we can add MO_WRITE_FAULT for SC in this patch or 
another patch.


Otherwise,

Reviewed-by: LIU Zhiwei

Thanks,
Zhiwei


  }
  
  static bool trans_sc_w(DisasContext *ctx, arg_sc_w *a)

@@ -105,55 +105,64 @@ static bool trans_sc_w(DisasContext *ctx, arg_sc_w *a)
  static bool trans_amoswap_w(DisasContext *ctx, arg_amoswap_w *a)
  {
  REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, (MO_ALIGN | MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
  }
  
  static bool trans_amoadd_w(DisasContext *ctx, arg_amoadd_w *a)

  {
  REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_add_tl, (MO_ALIGN | MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_add_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
  }
  
  static bool trans_amoxor_w(DisasContext *ctx, arg_amoxor_w *a)

  {
  REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_xor_tl, (MO_ALIGN | MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_xor_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
  }
  
  static bool trans_amoand_w(DisasContext *ctx, arg_amoand_w *a)

  {
  REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_and_tl, (MO_ALIGN | MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_and_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
  }
  
  static bool trans_amoor_w(DisasContext *ctx, arg_amoor_w *a)

  {
  REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_or_tl, (MO_ALIGN | MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_or_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
  }
  
  static bool trans_amomin_w(DisasContext *ctx, arg_amomin_w *a)

  {
  REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_smin_tl, (MO_ALIGN | 
MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_smin_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
  }
  
  static bool trans_amomax_w(DisasContext *ctx, arg_amomax_w *a)

  {
  REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_smax_tl, (MO_ALIGN | 
MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_smax_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
  }
  
  static bool trans_amominu_w(DisasContext *ctx, arg_amominu_w *a)

  {
  REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_umin_tl, (MO_ALIGN | 
MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_umin_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
  }
  
  static bool trans_amomaxu_w(DisasContext *ctx, arg_amomaxu_w *a)

  {
  REQUIRE_EXT(ctx, RVA);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_umax_tl, (MO_ALIGN | 
MO_TESL));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_umax_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TESL));
  }
  
  static bool trans_lr_d(DisasContext *ctx, arg_lr_d *a)

@@ -171,53 +180,62 @@ static bool trans_sc_d(DisasContext *ctx, arg_sc_d *a)
  static bool trans_amoswap_d(DisasContext *ctx, arg_amoswap_d *a)
  {
  REQUIRE_64BIT(ctx);
-return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl, (MO_ALIGN | MO_TEUQ));
+return gen_amo(ctx, a, &tcg_gen_atomic_xchg_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TEUQ));
  }
  
  static bool trans_amoadd_d(DisasContext *ctx, arg_amoadd_d *a)

  {
  REQUIRE_64BIT(ctx);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_add_tl, (MO_ALIGN | MO_TEUQ));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_add_tl,
+   (MO_ALIGN | MO_WRITE_FAULT | MO_TEUQ));
  }
  
  static bool trans_amoxor_d(DisasContext *ctx, arg_amoxor_d *a)

  {
  REQUIRE_64BIT(ctx);
-return gen_amo(ctx, a, &tcg_gen_atomic_fetch_xor_tl, (MO_ALIGN | MO_TEUQ));
+return gen_amo(ctx, a, &tcg_gen_atomic_fetch_xor_tl,
+   (MO_ALIGN | MO_WRITE

[PATCH] hw/sd: Correct card status clear conditions in SPI-mode

[frank . chang]

From: Frank Chang 

In SPI-mode, unlike SD-mode, card status bits: ILLEGAL_COMMAND and
COM_CRC_ERROR have type C ("cleared by read") clear conditions.
Also, type B ("cleared on valid command") clear condition is not
supported in SPI-mode. As the "In idle state" bit in SPI-mode has type A
("according to current state") clear condition, the CURRENT_STATE bits
in an SPI-mode response should be the SD card's state after the command
is executed, instead of the state when it received the preceding
command.

This patch redefines the card status clear conditions used in SD-mode
and SPI-mode according to SD spec.

Signed-off-by: Frank Chang 
---
 hw/sd/sd.c | 84 ++
 1 file changed, 59 insertions(+), 25 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index cd67a7bac8..4c6c1b9424 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -479,24 +479,50 @@ FIELD(CSR, OUT_OF_RANGE,   31,  1)
 #define CARD_STATUS_A   (R_CSR_READY_FOR_DATA_MASK \
| R_CSR_CARD_ECC_DISABLED_MASK \
| R_CSR_CARD_IS_LOCKED_MASK)
-#define CARD_STATUS_B   (R_CSR_CURRENT_STATE_MASK \
-   | R_CSR_ILLEGAL_COMMAND_MASK \
-   | R_CSR_COM_CRC_ERROR_MASK)
-#define CARD_STATUS_C   (R_CSR_AKE_SEQ_ERROR_MASK \
-   | R_CSR_APP_CMD_MASK \
-   | R_CSR_ERASE_RESET_MASK \
-   | R_CSR_WP_ERASE_SKIP_MASK \
-   | R_CSR_CSD_OVERWRITE_MASK \
-   | R_CSR_ERROR_MASK \
-   | R_CSR_CC_ERROR_MASK \
-   | R_CSR_CARD_ECC_FAILED_MASK \
-   | R_CSR_LOCK_UNLOCK_FAILED_MASK \
-   | R_CSR_WP_VIOLATION_MASK \
-   | R_CSR_ERASE_PARAM_MASK \
-   | R_CSR_ERASE_SEQ_ERROR_MASK \
-   | R_CSR_BLOCK_LEN_ERROR_MASK \
-   | R_CSR_ADDRESS_ERROR_MASK \
-   | R_CSR_OUT_OF_RANGE_MASK)
+
+static uint32_t sd_card_status_b(SDState *sd)
+{
+uint32_t sd_mask = R_CSR_CURRENT_STATE_MASK |
+   R_CSR_ILLEGAL_COMMAND_MASK |
+   R_CSR_COM_CRC_ERROR_MASK;
+
+/* SPI-mode dosen't have type B clear condition. */
+return !sd->spi ? sd_mask : 0;
+}
+
+static uint32_t sd_card_status_c(SDState *sd) {
+uint32_t sd_mask = R_CSR_AKE_SEQ_ERROR_MASK |
+   R_CSR_APP_CMD_MASK |
+   R_CSR_ERASE_RESET_MASK |
+   R_CSR_WP_ERASE_SKIP_MASK |
+   R_CSR_CSD_OVERWRITE_MASK |
+   R_CSR_ERROR_MASK |
+   R_CSR_CC_ERROR_MASK |
+   R_CSR_CARD_ECC_FAILED_MASK |
+   R_CSR_LOCK_UNLOCK_FAILED_MASK |
+   R_CSR_WP_VIOLATION_MASK |
+   R_CSR_ERASE_PARAM_MASK |
+   R_CSR_ERASE_SEQ_ERROR_MASK |
+   R_CSR_BLOCK_LEN_ERROR_MASK |
+   R_CSR_ADDRESS_ERROR_MASK |
+   R_CSR_OUT_OF_RANGE_MASK;
+uint32_t spi_mask = R_CSR_ERASE_RESET_MASK |
+R_CSR_LOCK_UNLOCK_FAILED_MASK |
+R_CSR_WP_ERASE_SKIP_MASK |
+R_CSR_CSD_OVERWRITE_MASK |
+R_CSR_ERROR_MASK |
+R_CSR_CC_ERROR_MASK |
+R_CSR_CARD_ECC_FAILED_MASK |
+R_CSR_ILLEGAL_COMMAND_MASK |
+R_CSR_COM_CRC_ERROR_MASK|
+R_CSR_WP_VIOLATION_MASK |
+R_CSR_ERASE_PARAM_MASK |
+R_CSR_ERASE_SEQ_ERROR_MASK |
+R_CSR_ADDRESS_ERROR_MASK |
+R_CSR_OUT_OF_RANGE_MASK;
+
+return !sd->spi ? sd_mask : spi_mask;
+}
 
 static void sd_set_cardstatus(SDState *sd)
 {
@@ -522,7 +548,7 @@ static void sd_response_r1_make(SDState *sd, uint8_t 
*response)
 stl_be_p(response, sd->card_status);
 
 /* Clear the "clear on read" status bits */
-sd->card_status &= ~CARD_STATUS_C;
+sd->card_status &= ~sd_card_status_c(sd);
 }
 
 static void sd_response_r3_make(SDState *sd, uint8_t *response)
@@ -537,7 +563,7 @@ static void sd_response_r6_make(SDState *sd, uint8_t 
*response)
 status = ((sd->card_status >> 8) & 0xc000) |
  ((sd->card_status >> 6) & 0x2000) |
   (sd->card_status & 0x1fff);
-sd->card_status &= ~(CARD_STATUS_C & 0xc81fff);
+sd->card_status &= ~(sd_card_status_c(sd) & 0xc81fff);
 stw_be_p(response + 0, sd->rca);
 stw_be_p(response + 2, status);
 }
@@ -1757,12 +1783,20 @@ int sd_do_command(SDState *sd, SDRequest *req,
 if (rtype == sd_illegal) {
 

Re: [PATCH] hw/sd: Correct the CURRENT_STATE bits in SPI-mode response

[Frank Chang]

On Tue, Jan 18, 2022 at 10:35 AM  wrote:

> From: Frank Chang 
>
> In SPI-mode, type B ("cleared on valid command") clear condition is not
> supported, and as the "In idle state" bit in SPI-mode has type A
> ("according to current state") clear condition, the CURRENT_STATE bits
> in an SPI-mode response should be the SD card's state after the command
> is executed, instead of the state when it received the preceding
> command.
>
> Also, we don't need to clear the type B ("clear on valid command")
> status bits after the response is updated in SPI-mode.
>
> Signed-off-by: Frank Chang 
> ---
>  hw/sd/sd.c | 26 ++
>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index cd67a7bac8..9736b8912d 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1757,12 +1757,20 @@ int sd_do_command(SDState *sd, SDRequest *req,
>  if (rtype == sd_illegal) {
>  sd->card_status |= ILLEGAL_COMMAND;
>  } else {
> -/* Valid command, we can update the 'state before command' bits.
> - * (Do this now so they appear in r1 responses.)
> - */
>  sd->current_cmd = req->cmd;
>  sd->card_status &= ~CURRENT_STATE;
> -sd->card_status |= (last_state << 9);
> +
> +if (!sd->spi) {
> +/* Valid command, we can update the 'state before command'
> bits.
> + * (Do this now so they appear in r1 responses.)
> + */
> +sd->card_status |= (last_state << 9);
> +} else {
> +/* Type B ("clear on valid command") is not supported
> + * in SPI-mode.
> + */
> +sd->card_status |= (sd->state << 9);
> +}
>  }
>
>  send_response:
> @@ -1808,10 +1816,12 @@ send_response:
>  trace_sdcard_response(sd_response_name(rtype), rsplen);
>
>  if (rtype != sd_illegal) {
> -/* Clear the "clear on valid command" status bits now we've
> - * sent any response
> - */
> -sd->card_status &= ~CARD_STATUS_B;
> +if (!sd->spi) {
> +/* Clear the "clear on valid command" status bits now we've
> + * sent any response
> + */
> +sd->card_status &= ~CARD_STATUS_B;
> +}
>  }
>
>  #ifdef DEBUG_SD
> --
> 2.31.1
>
>
This patch is replaced with more proper SPI clear conditions fix patch:
https://patchew.org/QEMU/20220124060449.22498-1-frank.ch...@sifive.com/
Please ignore this one, sorry for the confusion.

Regards,
Frank Chang

Re: [RFC PATCH v2 20/44] i386/tdx: Parse tdx metadata and store the result into TdxGuestState

[Xiaoyao Li]

On 1/10/2022 7:01 PM, Gerd Hoffmann wrote:

Regarding pflash itself, the read-only KVM memslot is required for it.
Otherwise pflash cannot work as a "ROMD device" (= you can't flip it
back and forth between ROM mode and programming (MMIO) mode).


We don't need Read-only mode for TDVF so far. If for this purpose, is it
acceptable that allowing a pflash without KVM readonly memslot support if
read-only is not required for the specific pflash device?


In case you don't want/need persistent VARS (which strictly speaking is
a UEFI spec violation) you should be able to go for a simple "-bios
OVMF.fd".



Gerd and Laszlo,

First, thank you for your patient explanation of how pflash works in 
QEMU and the clarification of usage of pflash with OVMF.


Below I want to share current situation of loading TDVF.

Restrictions from TDX architecture:
- Current TDX doesn't support read-only memory, i.e., cannot trap write.

- Current TDVF spec states that "In order to simplify the design, TDVF 
does not support non-volatile variables" in chapter 8.3.3



Regarding what interface should be used to load TDVF, there are three 
options:


  1) pflash: the same as how we load OVMF.

  Suppose TDVF support will finally get into OVMF, using this
  interface, it's full compatible with normal VMs. No change required
  to QEMU command line and TDVF binary is mapped to the GPA address
  right below 4G, but they are actually mapped as RAM.

  Of course, we need several hacks (special handling) in QEMU.

  Of course, they don't work as flash, the change made to it doesn't
  persist.

  2) -bios

  exploit "-bios" parameter to load TDVF. But again, read-only is not
  supported. TDVF is mapped as RAM.

  3) generic loader

  Just like what this series does. Implement specific parser in generic
  loader to load and parse TDVF as private RAM.

I'm nor sure if 1) is acceptable from your side. If not, we will go with 
option 3, since 2) doesn't make too much sense.

[PATCH] target/hexagon: remove unused variable

[Zongyuan Li]

When building with clang version 13.0.0 (eg. Fedora 13.0.0-3.fc35),
two unused variables introduced by macro GATHER_FUNCTION and
SCATTER_FUNCTION will cause building process failure due to
[-Werror -Wunused-variable].

Signed-off-by: Zongyuan Li 
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/831
---
 target/hexagon/mmvec/macros.h | 4 
 1 file changed, 4 deletions(-)

diff --git a/target/hexagon/mmvec/macros.h b/target/hexagon/mmvec/macros.h
index 10f4630364..44781cfb4a 100644
--- a/target/hexagon/mmvec/macros.h
+++ b/target/hexagon/mmvec/macros.h
@@ -164,11 +164,9 @@
 target_ulong va = EA; \
 target_ulong va_high = EA + LEN; \
 uintptr_t ra = GETPC(); \
-int log_bank = 0; \
 int log_byte = 0; \
 for (i0 = 0; i0 < ELEMENT_SIZE; i0++) { \
 log_byte = ((va + i0) <= va_high) && QVAL; \
-log_bank |= (log_byte << i0); \
 uint8_t B; \
 B = cpu_ldub_data_ra(env, EA + i0, ra); \
 env->tmp_VRegs[0].ub[ELEMENT_SIZE * IDX + i0] = B; \
@@ -243,11 +241,9 @@
 int i0; \
 target_ulong va = EA; \
 target_ulong va_high = EA + LEN; \
-int log_bank = 0; \
 int log_byte = 0; \
 for (i0 = 0; i0 < ELEM_SIZE; i0++) { \
 log_byte = ((va + i0) <= va_high) && QVAL; \
-log_bank |= (log_byte << i0); \
 LOG_VTCM_BYTE(va + i0, log_byte, IN.ub[ELEM_SIZE * IDX + i0], \
   ELEM_SIZE * IDX + i0); \
 } \
-- 
2.34.1

[PATCH] target/riscv: correct "code should not be reached" for x-rv128

[Frédéric Pétrot]

The addition of uxl support in gdbstub adds a few checks on the maximum
register length, but omitted MXL_RV128, leading to the occurence of
"code should not be reached" in a few places.
This patch makes rv128 react as rv64 for gdb, as previously.

Signed-off-by: Frédéric Pétrot 
---
 target/riscv/gdbstub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index f531a74c2f..9ed049c29e 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -64,6 +64,7 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray 
*mem_buf, int n)
 case MXL_RV32:
 return gdb_get_reg32(mem_buf, tmp);
 case MXL_RV64:
+case MXL_RV128:
 return gdb_get_reg64(mem_buf, tmp);
 default:
 g_assert_not_reached();
@@ -84,6 +85,7 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 length = 4;
 break;
 case MXL_RV64:
+case MXL_RV128:
 if (env->xl < MXL_RV64) {
 tmp = (int32_t)ldq_p(mem_buf);
 } else {
@@ -420,6 +422,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
  1, "riscv-32bit-virtual.xml", 0);
 break;
 case MXL_RV64:
+case MXL_RV128:
 gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
  riscv_gdb_set_virtual,
  1, "riscv-64bit-virtual.xml", 0);
-- 
2.34.1