Re: [RFC PATCH 04/18] qemu: Introduce 'qemu/legacy_binary_info.h'

2025-03-22 Thread Markus Armbruster
Philippe Mathieu-Daudé  writes:

> Cc'ing Markus.
>
> On 6/3/25 02:56, Richard Henderson wrote:
>> On 3/5/25 07:39, Philippe Mathieu-Daudé wrote:
>>> +void legacy_binary_info_init(const char *argv0)
>>> +{
>
>
>>> +    for (size_t i = 0; i < ARRAY_SIZE(legacy_binary_infos); i++) {
>>> +    if (!strcmp(legacy_binary_infos[i].binary_name, binary_name)) {
>>> +    current_index = i;
>>> +    return;
>>> +    }
>>> +    }
>
>
>> When testing for errors before and after a patch, I often rename
>> the binary, e.g. qemu-system-aarch64-good / qemu-system-aarch64-bad.
>
> I'd not qemu-system-microblazeel-good to match qemu-system-microblaze.
>
>> Leaving it in the same build directory is required in order to let
>> it find the uninstalled rom images.
>> Is there a way we can preserve something akin to this?
>> Do we need to add the -target command-line option that Pierrick mooted?

Having behavior depend on the binary name is problematic.  When users
run it with some other name (renamed binary, link to binary), behavior
changes, which is generally not desired and may be quite confusing.

I guess you want to do it here to replace multiple binaries by a single
one with several names.  Correct?

The stupid solution is to configure the single binary's behavior the
non-clever way with command line options such as -target, then provide
compatibility wrappers that run the single binary with suitable options.
Drawback: wrappers are slow, ugly, and can also be confusing.  Say when
you rename just the wrapper to -good and -bad.

If we want to go with behavior depending on the binary name, we could
try to reduce confusion by making unorthodox names fail cleanly.  Say
make -target optional only when the binary name matches exactly.

> Not that easy, CLI is evaluated *after* QOM types are registered.
> IIUC we'd need to add this as a -preconfig option, Markus is that right?

Ah, the startup mess.  I don't remember a thing.  Except for the need to
have QMP up and running before any non-trivial startup.  To get that,
the command line needs to be processed this early, too.

-preconfig is a disgusting hack to delay parts of startup until it's
explicitly triggered in the monitor.  Not a general solution for "need
to configurate more before startup", and not sure it helps here.




Re: [PATCH 00/10] gdbstub: conversion to runtime endianess helpers

2025-03-22 Thread Pierrick Bouvier

On 3/20/25 12:52, Pierrick Bouvier wrote:

On 3/19/25 11:22, Alex Bennée wrote:

The aim of this work is to get rid of the endian aware helpers in
gdbstub/helpers.h which due to their use of tswap() mean target
gdbstubs need to be built multiple times. While this series doesn't
actually build each stub once it introduces a new helper -
gdb_get_register_value() which takes a MemOp which can describe the
current endian state of the system. This will be a lot easier to
dynamically feed from a helper function.

The most complex example is PPC which has a helper called
ppc_maybe_bswap_register() which was doing this.

This is still an RFC so I'm interested in feedback:

- is the API sane
- can we avoid lots of (uint8_t *) casting?


Even though the series has a good intent, the fact we make everything
"generic" makes that we lose all guarantees we could get by relying on
static typing, and that we had possibility of mistakes when passing size
(which happened in patch 4 if I'm correct). And explicit casting comes
as a *strong* warning about that.

By patch 7, I was really feeling it's not a win vs explicit functions
per size.

If the goal of the series is to get rid of endian aware helpers, well,
this can be fixed in the helpers themselves, without needing to
introduce a "generic" size helper. Maybe we are trying to solve two
different problems here?


- should we have a reverse helper for setting registers

If this seems like the right approach I can have a go at more of the
frontends later.



Looking at include/gdbstub/helpers.h, gdb_get_reg128 can be solved by 
using target_words_bigendian() instead of TARGET_BIG_ENDIAN, which is 
already what tswap primitives are doing.


For gdb_get_regl, I would get rid of it completely, by editing all 
targets gdbstub.c, and replacing with gdb_get_reg32 or gdb_get_reg64 
explicit calls.
ppc is a very naughty boy, because registers are defined as 
target_ulong, while other arch use fixed types. The solution might be as 
simple as changing ppc registers definition to uint64_t.
If it's too complicated, you can postpone the problem by leaving 
gdb_get_regl defined only in ppc gdbstub, and clean up all other arch.
Thanks to static typing, it will be easy to spot a wrong gdb_get_regl 
conversion, so it's a no-risk operaton.


For ldtul_p, ldtul_le_p, and ldtul_be_p, it's a similar game. It's 
harder because only return type will differ, and you might miss occurences.
A safe way could be to replace ldtul_p by call to a function taking 
return value through pointer in parameter. This way, you can replace 
easily with l and q variants, without any risk off implicit conversion.
And for the one left depending on target_ulong/target_long, leave that 
for now, and move ldtul*_p to a target-helper.h included only for archs 
needing this.



There are a few other misc clean-ups I did on the way which might be
worth cherry picking for 10.0 but I'll leave that up to maintainers.

Alex.

Alex Bennée (10):
include/gdbstub: fix include guard in commands.h
gdbstub: introduce target independent gdb register helper
target/arm: convert 32 bit gdbstub to new helper
target/arm: convert 64 bit gdbstub to new helper
target/ppc: expand comment on FP/VMX/VSX access functions
target/ppc: make ppc_maybe_bswap_register static
target/ppc: convert gdbstub to new helper (!hacky)
gdbstub: assert earlier in handle_read_all_regs
include/exec: fix assert in size_memop
target/microblaze: convert gdbstub to new helper

   include/exec/memop.h|   4 +-
   include/gdbstub/commands.h  |   2 +-
   include/gdbstub/registers.h |  30 ++
   target/ppc/cpu.h|   8 +-
   gdbstub/gdbstub.c   |  24 -
   target/arm/gdbstub.c|  57 +++
   target/arm/gdbstub64.c  |  53 ++
   target/microblaze/gdbstub.c |  44 
   target/ppc/gdbstub.c| 194 
   9 files changed, 257 insertions(+), 159 deletions(-)
   create mode 100644 include/gdbstub/registers.h







Re: [PATCH v3] QIOChannelSocket: Flush zerocopy socket error queue on sendmsg failure due to ENOBUF

2025-03-22 Thread Daniel P . Berrangé
On Sun, Mar 16, 2025 at 09:52:31PM -0400, Manish Mishra wrote:
> We allocate extra metadata SKBs in case of a zerocopy send. This metadata
> memory is accounted for in the OPTMEM limit. If there is any error while
> sending zerocopy packets or if zerocopy is skipped, these metadata SKBs are
> queued in the socket error queue. This error queue is freed when userspace
> reads it.
> 
> Usually, if there are continuous failures, we merge the metadata into a single
> SKB and free another one. As a result, it never exceeds the OPTMEM limit.
> However, if there is any out-of-order processing or intermittent zerocopy
> failures, this error chain can grow significantly, exhausting the OPTMEM 
> limit.
> As a result, all new sendmsg requests fail to allocate any new SKB, leading to
> an ENOBUF error. Depending on the amount of data queued before the flush
> (i.e., large live migration iterations), even large OPTMEM limits are prone to
> failure.
> 
> To work around this, if we encounter an ENOBUF error with a zerocopy sendmsg,
> we flush the error queue and retry once more.
> 
> V2:
>   1. Removed the dirty_sync_missed_zero_copy migration stat.
>   2. Made the call to qio_channel_socket_flush_internal() from
>  qio_channel_socket_writev() non-blocking.
> 
> V3:
>   1. Add the dirty_sync_missed_zero_copy migration stat again.

These notes about changes each version should always be below
the '---' line, because they shouldn't remain in the commit message
when merged.

> 
> Signed-off-by: Manish Mishra 
> ---
>  include/io/channel-socket.h |  5 +++
>  io/channel-socket.c | 74 ++---
>  2 files changed, 65 insertions(+), 14 deletions(-)
> 

> diff --git a/io/channel-socket.c b/io/channel-socket.c
> index 608bcf066e..604ca9890d 100644
> --- a/io/channel-socket.c
> +++ b/io/channel-socket.c
> @@ -37,6 +37,12 @@
>  
>  #define SOCKET_MAX_FDS 16
>  
> +#ifdef QEMU_MSG_ZEROCOPY
> +static int qio_channel_socket_flush_internal(QIOChannel *ioc,
> + bool block,
> + Error **errp);
> +#endif
> +
>  SocketAddress *
>  qio_channel_socket_get_local_address(QIOChannelSocket *ioc,
>   Error **errp)
> @@ -65,6 +71,7 @@ qio_channel_socket_new(void)
>  sioc->fd = -1;
>  sioc->zero_copy_queued = 0;
>  sioc->zero_copy_sent = 0;
> +sioc->new_zero_copy_sent_success = FALSE;
>  
>  ioc = QIO_CHANNEL(sioc);
>  qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_SHUTDOWN);
> @@ -566,6 +573,7 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc,
>  size_t fdsize = sizeof(int) * nfds;
>  struct cmsghdr *cmsg;
>  int sflags = 0;
> +bool zero_copy_flush_pending = TRUE;
>  
>  memset(control, 0, CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS));
>  
> @@ -612,9 +620,25 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc,
>  goto retry;
>  case ENOBUFS:
>  if (flags & QIO_CHANNEL_WRITE_FLAG_ZERO_COPY) {
> -error_setg_errno(errp, errno,
> - "Process can't lock enough memory for using 
> MSG_ZEROCOPY");
> -return -1;
> +/**
> + * Socket error queueing may exhaust the OPTMEM limit. Try
> + * flushing the error queue once.
> + */
> +if (zero_copy_flush_pending) {
> +ret = qio_channel_socket_flush_internal(ioc, false, 
> errp);

Hardcoding block==false isn't right.  The socket may be in either
blocking or non-blocking mode, and that needs to be taken into
account, otherwise we'll get spurious failures in blocking mode
if "flush" would otherwise want to block.

> +if (ret < 0) {
> +error_setg_errno(errp, errno,
> + "Zerocopy flush failed");
> +return -1;
> +}
> +zero_copy_flush_pending = FALSE;
> +goto retry;
> +} else {
> +error_setg_errno(errp, errno,
> + "Process can't lock enough memory for "
> + "using MSG_ZEROCOPY");
> +return -1;
> +}
>  }
>  break;
>  }
> @@ -725,8 +749,9 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc,
>  
>  
>  #ifdef QEMU_MSG_ZEROCOPY
> -static int qio_channel_socket_flush(QIOChannel *ioc,
> -Error **errp)
> +static int qio_channel_socket_flush_internal(QIOChannel *ioc,
> + bool block,
> + Error **errp)
>  {
>  QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc);
>  struct msghdr msg = {};
> @@ -734,7 +759,6 @@ static int qio_channel_socket_flush(QI

Re: [PATCH v2] hw/rtc: Add RTC PCF8563 module

2025-03-22 Thread Bernhard Beschow



Am 10. März 2025 11:36:34 UTC schrieb Ilya Chichkov :
>Add PCF8563 a real-time clock with calendar and I2C interface.
>This commit adds support for interfacing with it and implements
>functionality of setting timer, alarm, reading and writing time.
>
>Signed-off-by: Ilya Chichkov 
>---
>v1->v2
>Phil:
>- Add hot reset
>- Fix trace message
>- Add testing coverage with qtest
>
>Bernhard:
>- Move datasheet link to source code top comment section
>- Fix typos
>- Update licence identifier to SPDX
>- Remove unused import libraries
>- Change OBJECT_CHECK to OBJECT_DECLARE_SIMPLE_TYPE
>- Remove outdated comment
>- Rename i2c to parent_obj
>- Moved get_time inside capture_time function that is
>  called only when I2C request starts
>- Add fields inside VMStateDescription
>- Removed pcf8563_realize
>- Change type_init to DEFINE_TYPES
>---
> hw/rtc/Kconfig   |   5 +
> hw/rtc/meson.build   |   1 +
> hw/rtc/pcf8563_rtc.c | 740 +++
> hw/rtc/trace-events  |  12 +
> 4 files changed, 758 insertions(+)
> create mode 100644 hw/rtc/pcf8563_rtc.c
>
>diff --git a/hw/rtc/Kconfig b/hw/rtc/Kconfig
>index b90c2e510a..4e7a1f75ef 100644
>--- a/hw/rtc/Kconfig
>+++ b/hw/rtc/Kconfig
>@@ -27,5 +27,10 @@ config GOLDFISH_RTC
> config LS7A_RTC
> bool
> 
>+config PCF8563_RTC
>+bool
>+depends on I2C
>+default y if I2C_DEVICES
>+
> config STM32_RTC
> bool
>\ No newline at end of file
>diff --git a/hw/rtc/meson.build b/hw/rtc/meson.build
>index b6bb7436c7..6180ffc6d9 100644
>--- a/hw/rtc/meson.build
>+++ b/hw/rtc/meson.build
>@@ -13,4 +13,5 @@ system_ss.add(when: 'CONFIG_GOLDFISH_RTC', if_true: 
>files('goldfish_rtc.c'))
> system_ss.add(when: 'CONFIG_LS7A_RTC', if_true: files('ls7a_rtc.c'))
> system_ss.add(when: 'CONFIG_ALLWINNER_H3', if_true: files('allwinner-rtc.c'))
> system_ss.add(when: 'CONFIG_MC146818RTC', if_true: files('mc146818rtc.c'))
>+system_ss.add(when: 'CONFIG_PCF8563_RTC', if_true: files('pcf8563_rtc.c'))
> system_ss.add(when: 'CONFIG_STM32_RTC', if_true: files('stm32-rtc.c'))
>diff --git a/hw/rtc/pcf8563_rtc.c b/hw/rtc/pcf8563_rtc.c
>new file mode 100644
>index 00..195b697598
>--- /dev/null
>+++ b/hw/rtc/pcf8563_rtc.c
>@@ -0,0 +1,740 @@
>+// SPDX-License-Identifier: GPL-2.0-or-later
>+/*
>+ * Real-time clock/calendar PCF8563 with I2C interface.
>+ *
>+ * Datasheet: https://www.nxp.com/docs/en/data-sheet/PCF8563.pdf
>+ *
>+ * Author (c) 2024 Ilya Chichkov 
>+ */
>+
>+#include "qemu/osdep.h"
>+#include "hw/sysbus.h"
>+#include "hw/register.h"
>+#include "hw/registerfields.h"
>+#include "hw/irq.h"
>+#include "qemu/bitops.h"
>+#include "hw/qdev-properties.h"
>+#include "qemu/timer.h"
>+#include "hw/i2c/i2c.h"
>+#include "qemu/bcd.h"
>+#include "qom/object.h"
>+#include "sysemu/sysemu.h"
>+#include "sysemu/rtc.h"
>+#include "migration/vmstate.h"
>+#include "qapi/visitor.h"
>+
>+#include "trace.h"
>+
>+#define TYPE_PCF8563 "pcf8563"
>+OBJECT_DECLARE_SIMPLE_TYPE(Pcf8563State, PCF8563)
>+
>+#define  PCF8563_CS10x00
>+#define  PCF8563_CS20x01
>+#define  PCF8563_VLS0x02
>+#define  PCF8563_MINUTES0x03
>+#define  PCF8563_HOURS  0x04
>+#define  PCF8563_DAYS   0x05
>+#define  PCF8563_WEEKDAYS   0x06
>+#define  PCF8563_CENTURY_MONTHS 0x07
>+#define  PCF8563_YEARS  0x08
>+#define  PCF8563_MINUTE_A   0x09
>+#define  PCF8563_HOUR_A 0x0A
>+#define  PCF8563_DAY_A  0x0B
>+#define  PCF8563_WEEKDAY_A  0x0C
>+#define  PCF8563_CLKOUT_CTL 0x0D
>+#define  PCF8563_TIMER_CTL  0x0E
>+#define  PCF8563_TIMER  0x0F
>+
>+#define MINUTES_IN_HOUR 60
>+#define HOURS_IN_DAY 24
>+#define DAYS_IN_MONTH 31
>+#define DAYS_IN_WEEK 7
>+
>+REG8(PCF8563_CS1, 0x00)
>+FIELD(PCF8563_CS1, RSVD0,  0,  3)
>+FIELD(PCF8563_CS1, TESTC,  3,  1)
>+FIELD(PCF8563_CS1, RSVD1,  4,  1)
>+FIELD(PCF8563_CS1, STOP,   5,  1)
>+FIELD(PCF8563_CS1, RSVD2,  6,  1)
>+FIELD(PCF8563_CS1, TEST1,  7,  1)
>+
>+REG8(PCF8563_CS2, 0x01)
>+FIELD(PCF8563_CS2, TIE,   0,  1)
>+FIELD(PCF8563_CS2, AIE,   1,  1)
>+FIELD(PCF8563_CS2, TF,2,  1)
>+FIELD(PCF8563_CS2, AF,3,  1)
>+FIELD(PCF8563_CS2, TI_TP, 4,  1)
>+FIELD(PCF8563_CS2, RSVD,  5,  3)
>+
>+REG8(PCF8563_VLS, 0x02)
>+FIELD(PCF8563_VLS, SECONDS,  0,  7)
>+FIELD(PCF8563_VLS, VL,   7,  1)
>+
>+REG8(PCF8563_MINUTES, 0x03)
>+FIELD(PCF8563_MINUTES, MINUTES, 0,  7)
>+FIELD(PCF8563_MINUTES, RSVD,7,  1)
>+
>+REG8(PCF8563_HOURS, 0x04)
>+FIELD(PCF8563_HOURS, HOURS, 0,  6)
>+FIELD(PCF8563_HOURS, RSVD,  6,  2)
>+
>+REG8(PCF8563_DAYS, 0x05)
>+FIELD(PCF8563_DAYS, DAYS, 0,  6)
>+FIELD(PCF8563_DAYS, RSVD, 6,  2)
>+
>+REG8(PCF8563_WEEKDAYS, 0x06)
>+FIELD(PCF8563_WEEKDAYS, WEEKDAYS, 0,  3)
>+FIELD(PCF8563_WEEKDAYS, RSVD, 3,  5)
>+
>+REG8(PCF8563_CENTURY_MONTHS, 0x07)
>+FIELD(PCF8563_CENTURY_MONTHS, MONTHS,  0,  5)
>+FIELD(PCF8563_CENTURY_MONTHS, RSVD,5,  2)
>+FIELD(PCF8563

[PATCH 09/21] hw/misc: Add dummy ZYNQ DDR controller

2025-03-22 Thread Corvin Köhne
From: YannickV 

A dummy DDR controller for ZYNQ has been added. While all registers are present,
not all are functional. Read and write access is validated, and the user mode
can be set. This provides a basic DDR controller initialization, preventing
system hangs due to endless polling or similar issues.

Signed-off-by: Yannick Voßen 
---
 hw/misc/Kconfig |   3 +
 hw/misc/meson.build |   1 +
 hw/misc/zynq_ddr-ctrl.c | 331 
 3 files changed, 335 insertions(+)
 create mode 100644 hw/misc/zynq_ddr-ctrl.c

diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index ec0fa5aa9f..1bc4228572 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -222,4 +222,7 @@ config IOSB
 config XLNX_VERSAL_TRNG
 bool
 
+config DDR_CTRLR
+bool
+
 source macio/Kconfig
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 6d47de482c..8d4c4279c4 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -91,6 +91,7 @@ system_ss.add(when: 'CONFIG_RASPI', if_true: files(
 ))
 system_ss.add(when: 'CONFIG_SLAVIO', if_true: files('slavio_misc.c'))
 system_ss.add(when: 'CONFIG_ZYNQ', if_true: files('zynq_slcr.c'))
+system_ss.add(when: 'CONFIG_ZYNQ', if_true: files('zynq_ddr-ctrl.c'))
 system_ss.add(when: 'CONFIG_XLNX_ZYNQMP_ARM', if_true: 
files('xlnx-zynqmp-crf.c'))
 system_ss.add(when: 'CONFIG_XLNX_ZYNQMP_ARM', if_true: 
files('xlnx-zynqmp-apu-ctrl.c'))
 system_ss.add(when: 'CONFIG_XLNX_VERSAL', if_true: files(
diff --git a/hw/misc/zynq_ddr-ctrl.c b/hw/misc/zynq_ddr-ctrl.c
new file mode 100644
index 00..8cdf8be743
--- /dev/null
+++ b/hw/misc/zynq_ddr-ctrl.c
@@ -0,0 +1,331 @@
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/register.h"
+#include "qemu/bitops.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "hw/registerfields.h"
+#include "system/block-backend.h"
+#include "exec/address-spaces.h"
+#include "exec/memory.h"
+#include "system/dma.h"
+
+#ifndef DDRCTRL_ERR_DEBUG
+#define DDRCTRL_ERR_DEBUG 0
+#endif
+
+#define DB_PRINT_L(level, ...) do { \
+if (DDRCTRL_ERR_DEBUG > (level)) { \
+fprintf(stderr,  ": %s: ", __func__); \
+fprintf(stderr, ## __VA_ARGS__); \
+} \
+} while (0)
+
+#define DB_PRINT(...) DB_PRINT_L(0, ## __VA_ARGS__)
+
+REG32(DDRC_CTRL, 0x00)
+REG32(TWO_RANK_CFG, 0x04)
+REG32(HPR_REG, 0x08)
+REG32(LPR_REG, 0x0C)
+REG32(WR_REG, 0x10)
+REG32(DRAM_PARAM_REG0, 0x14)
+REG32(DRAM_PARAM_REG1, 0x18)
+REG32(DRAM_PARAM_REG2, 0x1C)
+REG32(DRAM_PARAM_REG3, 0x20)
+REG32(DRAM_PARAM_REG4, 0x24)
+REG32(DRAM_INIT_PARAM, 0x28)
+REG32(DRAM_EMR_REG, 0x2C)
+REG32(DRAM_EMR_MR_REG, 0x30)
+REG32(DRAM_BURST8_RDWR, 0x34)
+REG32(DRAM_DISABLE_DQ, 0x38)
+REG32(DRAM_ADDR_MAP_BANK, 0x3C)
+REG32(DRAM_ADDR_MAP_COL, 0x40)
+REG32(DRAM_ADDR_MAP_ROW, 0x44)
+REG32(DRAM_ODT_REG, 0x48)
+REG32(PHY_DBG_REG, 0x4C)
+REG32(PHY_CMD_TIMEOUT_RDDA, 0x50)
+REG32(TA_CPT, 0x50)
+REG32(MODE_STS_REG, 0x54)
+FIELD(MODE_STS_REG, DDR_REG_DBG_STALL, 3, 3)
+FIELD(MODE_STS_REG, DDR_REG_OPERATING_MODE, 0, 2)
+REG32(DLL_CALIB, 0x58)
+REG32(ODT_DELAY_HOLD, 0x5C)
+REG32(CTRL_REG1, 0x60)
+REG32(CTRL_REG2, 0x64)
+REG32(CTRL_REG3, 0x68)
+REG32(CTRL_REG4, 0x6C)
+REG32(CTRL_REG5, 0x78)
+REG32(CTRL_REG6, 0x7C)
+REG32(CHE_REFRESH_TIMER0, 0xA0)
+REG32(CHE_T_ZQ, 0xA4)
+REG32(CHE_T_ZQ_SHORT_INTERVAL_REG, 0xA8)
+REG32(DEEP_PWRDWN_REG, 0xAC)
+REG32(REG_2C, 0xB0)
+REG32(REG_2D, 0xB4)
+REG32(DFI_TIMING, 0xB8)
+REG32(CHE_ECC_CONTROL_REG_OFFSET, 0xC4)
+REG32(CHE_CORR_ECC_LOG_REG_OFFSET, 0xC8)
+REG32(CHE_CORR_ECC_ADDR_REG_OFFSET, 0xCC)
+REG32(CHE_CORR_ECC_DATA_31_0_REG_OFFSET, 0xD0)
+REG32(CHE_CORR_ECC_DATA_63_32_REG_OFFSET, 0xD4)
+REG32(CHE_CORR_ECC_DATA_71_64_REG_OFFSET, 0xD8)
+REG32(CHE_UNCORR_ECC_LOG_REG_OFFSET, 0xDC)
+REG32(CHE_UNCORR_ECC_ADDR_REG_OFFSET, 0xE0)
+REG32(CHE_UNCORR_ECC_DATA_31_0_REG_OFFSET, 0xE4)
+REG32(CHE_UNCORR_ECC_DATA_63_32_REG_OFFSET, 0xE8)
+REG32(CHE_UNCORR_ECC_DATA_71_64_REG_OFFSET, 0xEC)
+REG32(CHE_ECC_STATS_REG_OFFSET, 0xF0)
+REG32(ECC_SCRUB, 0xF4)
+REG32(CHE_ECC_CORR_BIT_MASK_31_0_REG_OFFSET, 0xF8)
+REG32(CHE_ECC_CORR_BIT_MASK_63_32_REG_OFFSET, 0xFC)
+REG32(PHY_RCVER_ENABLE, 0x114)
+REG32(PHY_CONFIG0, 0x118)
+REG32(PHY_CONFIG1, 0x11C)
+REG32(PHY_CONFIG2, 0x120)
+REG32(PHY_CONFIG3, 0x124)
+REG32(PHY_INIT_RATIO0, 0x12C)
+REG32(PHY_INIT_RATIO1, 0x130)
+REG32(PHY_INIT_RATIO2, 0x134)
+REG32(PHY_INIT_RATIO3, 0x138)
+REG32(PHY_RD_DQS_CFG0, 0x140)
+REG32(PHY_RD_DQS_CFG1, 0x144)
+REG32(PHY_RD_DQS_CFG2, 0x148)
+REG32(PHY_RD_DQS_CFG3, 0x14C)
+REG32(PHY_WR_DQS_CFG0, 0x154)
+REG32(PHY_WR_DQS_CFG1, 0x158)
+REG32(PHY_WR_DQS_CFG2, 0x15C)
+REG32(PHY_WR_DQS_CFG3, 0x160)
+REG32(PHY_WE_CFG0, 0x168)
+REG32(PHY_WE_CFG1, 0x16C)
+REG32(PHY_WE_CFG2, 0x170)
+REG32(PHY_WE_CFG3, 0x174)
+REG32(WR_DATA_SLV0, 0x17C)
+REG32(WR_DATA_SLV1, 0x180)
+REG32(WR_DATA_SLV2, 0x184)
+REG32(WR_DATA_SLV3, 0x188)
+REG32(REG_64, 0x190)
+REG32(REG_65, 0x194)
+REG32(REG69_6A0, 0x1A4)
+REG32(REG69_6A1, 0x1A8)
+REG32(REG6C_6D2, 0x1B0)
+REG32(REG6C_6D3, 0x1B4)
+REG32(REG6E_710, 0x1B8)
+REG32(

Re: [PATCH 1/3] rust: assertions: add static_assert

2025-03-22 Thread Philippe Mathieu-Daudé

On 20/3/25 14:32, Peter Maydell wrote:

From: Paolo Bonzini 

Add a new assertion that is similar to "const { assert!(...) }" but can be used
outside functions and with older versions of Rust.  A similar macro is found in
Linux, whereas the "static_assertions" crate has a const_assert macro that
produces worse error messages.

Suggested-by: Peter Maydell 
Supersedes: <20250320113356.799412-1-pbonz...@redhat.com>


^ extraneous tag


Signed-off-by: Paolo Bonzini 
Signed-off-by: Peter Maydell 
---
  rust/qemu-api/src/assertions.rs | 22 ++
  1 file changed, 22 insertions(+)





[PATCH v3] docs/specs/riscv-iommu: Fixed broken link to external risv iommu document

2025-03-22 Thread hemanshu.khilari.foss
The links to riscv iommu specification document are incorrect. This patch
updates all the said link to point to correct location.

Cc: qemu-ri...@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2808
Signed-off-by: hemanshu.khilari.foss 
---
 docs/specs/riscv-iommu.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/specs/riscv-iommu.rst b/docs/specs/riscv-iommu.rst
index 000c7e1f57..991d376fdc 100644
--- a/docs/specs/riscv-iommu.rst
+++ b/docs/specs/riscv-iommu.rst
@@ -4,7 +4,7 @@ RISC-V IOMMU support for RISC-V machines
 
 
 QEMU implements a RISC-V IOMMU emulation based on the RISC-V IOMMU spec
-version 1.0 `iommu1.0`_.
+version 1.0 `iommu1.0.0`_.
 
 The emulation includes a PCI reference device (riscv-iommu-pci) and a platform
 bus device (riscv-iommu-sys) that QEMU RISC-V boards can use.  The 'virt'
@@ -14,7 +14,7 @@ riscv-iommu-pci reference device
 
 
 This device implements the RISC-V IOMMU emulation as recommended by the section
-"Integrating an IOMMU as a PCIe device" of `iommu1.0`_: a PCI device with base
+"Integrating an IOMMU as a PCIe device" of `iommu1.0.0`_: a PCI device with 
base
 class 08h, sub-class 06h and programming interface 00h.
 
 As a reference device it doesn't implement anything outside of the 
specification,
@@ -109,7 +109,7 @@ riscv-iommu options:
 - "s-stage": enabled
 - "g-stage": enabled
 
-.. _iommu1.0: 
https://github.com/riscv-non-isa/riscv-iommu/releases/download/v1.0/riscv-iommu.pdf
+.. _iommu1.0.0: 
https://github.com/riscv-non-isa/riscv-iommu/releases/download/v1.0.0/riscv-iommu.pdf
 
 .. _linux-v8: 
https://lore.kernel.org/linux-riscv/cover.1718388908.git.tjezn...@rivosinc.com/
 
-- 
2.42.0




[PATCH v1 11/22] hw/misc/aspeed_hace: Add trace-events for better debugging

2025-03-22 Thread Jamin Lin via
Introduced "trace_aspeed_hace_addr", "trace_aspeed_hace_sg",
"trace_aspeed_hace_read", and "trace_aspeed_hace_write" trace events.

Signed-off-by: Jamin Lin 
---
 hw/misc/aspeed_hace.c | 8 
 hw/misc/trace-events  | 6 ++
 2 files changed, 14 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 53b3b390e3..b8e473ee3f 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -18,6 +18,7 @@
 #include "crypto/hash.h"
 #include "hw/qdev-properties.h"
 #include "hw/irq.h"
+#include "trace.h"
 
 #define R_CRYPT_CMD (0x10 / 4)
 
@@ -186,6 +187,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, 
bool sg_mode,
 if (ahc->has_dma64) {
 src = deposit64(src, 32, 32, s->regs[R_HASH_SRC_HI]);
 }
+trace_aspeed_hace_addr("src", src);
 src += i * SG_LIST_ENTRY_SIZE;
 
 len = address_space_ldl_le(&s->dram_as, src,
@@ -194,6 +196,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, 
bool sg_mode,
 sg_addr = address_space_ldl_le(&s->dram_as, src + SG_LIST_LEN_SIZE,
MEMTXATTRS_UNSPECIFIED, NULL);
 sg_addr &= SG_LIST_ADDR_MASK;
+trace_aspeed_hace_sg(i, sg_addr, len);
 /*
  * Ideally, sg_addr should be 64-bit for the AST2700, using the
  * following program to obtain the 64-bit sg_addr and convert it
@@ -237,6 +240,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, 
bool sg_mode,
 } else {
 plen = s->regs[R_HASH_SRC_LEN];
 src = deposit64(src, 0, 32, s->regs[R_HASH_SRC]);
+trace_aspeed_hace_addr("src", src);
 if (ahc->has_dma64) {
 src = deposit64(src, 32, 32, s->regs[R_HASH_SRC_HI]);
 }
@@ -299,6 +303,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, 
bool sg_mode,
 if (ahc->has_dma64) {
 digest_addr = deposit64(digest_addr, 32, 32, s->regs[R_HASH_DEST_HI]);
 }
+trace_aspeed_hace_addr("digest", digest_addr);
 if (address_space_write(&s->dram_as, digest_addr,
 MEMTXATTRS_UNSPECIFIED,
 digest_buf, digest_len)) {
@@ -326,6 +331,7 @@ static uint64_t aspeed_hace_read(void *opaque, hwaddr addr, 
unsigned int size)
 return 0;
 }
 
+trace_aspeed_hace_read(addr << 2, s->regs[addr]);
 return s->regs[addr];
 }
 
@@ -344,6 +350,8 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, 
uint64_t data,
 return;
 }
 
+trace_aspeed_hace_write(addr << 2, data);
+
 switch (addr) {
 case R_STATUS:
 if (data & HASH_IRQ) {
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 4383808d7a..cf96e68cfa 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -302,6 +302,12 @@ aspeed_peci_read(uint64_t offset, uint64_t data) "offset 
0x%" PRIx64 " data 0x%"
 aspeed_peci_write(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 
0x%" PRIx64
 aspeed_peci_raise_interrupt(uint32_t ctrl, uint32_t status) "ctrl 0x%" PRIx32 
" status 0x%" PRIx32
 
+# aspeed_hace.c
+aspeed_hace_read(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 
0x%" PRIx64
+aspeed_hace_write(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 
0x%" PRIx64
+aspeed_hace_sg(int index, uint64_t addr, uint32_t len) "%d: addr 0x%" PRIx64 " 
len 0x%" PRIx32
+aspeed_hace_addr(const char *s, uint64_t addr) "%s: 0x%" PRIx64
+
 # bcm2835_property.c
 bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox 
property tag:0x%08x in_sz:%u out_sz:%zu"
 
-- 
2.43.0




[PATCH v2] virtio-net: Fix the interpretation of max_tx_vq

2025-03-22 Thread Akihiko Odaki
virtio-net uses the max_tx_vq field of struct virtio_net_rss_config to
determine the number of queue pairs and emits an error message saying
"Can't get queue_pairs". However, the field tells only about tx.

Examine unclassified_queue and indirection_table to determine the number
of queues required for rx, and correct the name of field in the error
message, clarifying its correct semantics.

Fixes: 590790297c0d ("virtio-net: implement RSS configuration command")
Signed-off-by: Akihiko Odaki 
---
Changes in v2:
- Handled unclassified_queue too.
- Added a Fixes: tag.
- Link to v1: 
https://lore.kernel.org/qemu-devel/20250321-vq-v1-1-6d6d285e5...@daynix.com
---
 hw/net/virtio-net.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index de87cfadffe1..afc6b82f13c9 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1450,23 +1450,29 @@ static uint16_t virtio_net_handle_rss(VirtIONet *n,
 err_value = (uint32_t)s;
 goto error;
 }
-for (i = 0; i < n->rss_data.indirections_len; ++i) {
-uint16_t val = n->rss_data.indirections_table[i];
-n->rss_data.indirections_table[i] = virtio_lduw_p(vdev, &val);
-}
 offset += size_get;
 size_get = sizeof(temp);
 s = iov_to_buf(iov, iov_cnt, offset, &temp, size_get);
 if (s != size_get) {
-err_msg = "Can't get queue_pairs";
+err_msg = "Can't get max_tx_vq";
 err_value = (uint32_t)s;
 goto error;
 }
-queue_pairs = do_rss ? virtio_lduw_p(vdev, &temp.us) : n->curr_queue_pairs;
-if (queue_pairs == 0 || queue_pairs > n->max_queue_pairs) {
-err_msg = "Invalid number of queue_pairs";
-err_value = queue_pairs;
-goto error;
+if (do_rss) {
+queue_pairs = MAX(virtio_lduw_p(vdev, &temp.us),
+  n->rss_data.default_queue);
+for (i = 0; i < n->rss_data.indirections_len; ++i) {
+uint16_t val = n->rss_data.indirections_table[i];
+n->rss_data.indirections_table[i] = virtio_lduw_p(vdev, &val);
+queue_pairs = MAX(queue_pairs, n->rss_data.indirections_table[i]);
+}
+if (queue_pairs == 0 || queue_pairs > n->max_queue_pairs) {
+err_msg = "Invalid number of queue_pairs";
+err_value = queue_pairs;
+goto error;
+}
+} else {
+queue_pairs = n->curr_queue_pairs;
 }
 if (temp.b > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
 err_msg = "Invalid key size";

---
base-commit: 825b96dbcee23d134b691fc75618b59c5f53da32
change-id: 20250321-vq-87aff4f531bf

Best regards,
-- 
Akihiko Odaki 




Re: [PATCH v6 3/6] hw/loongarch/virt: Fix error handling in cpu unplug

2025-03-22 Thread bibo mao




On 2025/3/21 下午4:08, Markus Armbruster wrote:

bibo mao  writes:


On 2025/3/21 下午3:21, Markus Armbruster wrote:

bibo mao  writes:


+Igor


On 2025/3/21 下午2:47, Markus Armbruster wrote:

Bibo Mao  writes:


In function virt_cpu_unplug(), it will send cpu unplug message to
interrupt controller extioi and ipi irqchip. If there is problem in
this function, system should continue to run and keep state the same
before cpu is removed.

If error happends in cpu unplug stage, send cpu plug message to extioi
and ipi irqchip to restore to previous stage, and then return immediately.

Fixes: 2cd6857f6f5b (hw/loongarch/virt: Implement cpu unplug interface)
Signed-off-by: Bibo Mao 
---
hw/loongarch/virt.c | 6 ++
1 file changed, 6 insertions(+)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 8563967c8b..503362a69e 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -958,6 +958,8 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev,
hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->extioi), dev, &err);
if (err) {
error_propagate(errp, err);
+hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), dev,
+ &error_abort);
return;
}

@@ -965,6 +967,10 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev,

hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &err);
if (err) {
error_propagate(errp, err);
+hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), dev,
+ &error_abort);
+hotplug_handler_plug(HOTPLUG_HANDLER(lvms->extioi), dev,
+ &error_abort);
return;
}


virt_cpu_unplug() calls hotplug_handler_unplug() three times to notify
ipi, extioi, and acpi_get.  If any notification fails, virt_cpu_unplug()
calls hotplug_handler_plug() to "un-notify" the preceeding ones, if any.
This must not fail.

virt_cpu_plug() does it the other way round (see previous patch).

So, hotplug_handler_plug() must not fail in virt_cpu_unplug(), yet we
check for it to fail in virt_cpu_plug().

Can it really fail in virt_cpu_plug()?

If yes, why can't it fail in virt_cpu_unplug()?

you can check function acpi_cpu_plug_cb()/loongarch_ipi_cpu_plug(), that
is cpuplug callback for acpi_ged and ipi. it will not fail.

If *virt_cpu_pre_plug()* pass, it will succeed.

Regards
Bibo Mao



Same questions for hotplug_handler_unplug().


Let me restate my argument.

We call hotplug_handler_plug() on the happy path, and on error recovery
paths.  Four cases:

1. Can fail on the happy path

 Error recovery is required.

1.1 Can fail on the error recovery path

  Error recovery is required, but broken.

1.2 Can't fail on the error recovery path

  Error recovery is required and works, but why it works is not
  obvious.  Deserves a comment explaining why hotplug_handler_plug()
  can't fail here even though it can fail on the happy path next door.

2. Can't fail on the happy path

 Error recovery is unreachable.

2.1 Can fail on the error recovery path

  Error recovery is unreachable and broken.  Possibly a time bomb, and
  possibly misleading readers.

2.2 Can't fail on the error recovery path

  Error recovery is unreachable and would work, but why it would work
  is again a not obvious.

Which of the four cases is it?

By my understanding, it is "2. Can't fail on the happy path",  and Error
recovery is unreachable.


Got it.


I have said that it is impossible and recovery is only for future use.

do you mean recovery should be removed? And directly &error_abort is
used in virt_cpu_plug() such as:
static void virt_cpu_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
if (lvms->ipi) {
  hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), dev, &error_abort);


Yes, I prefer this.  Here's why.
This is ok, however I do not think there is obvious advantage. V6 is ok 
also, there is recovery path only that recovery has problem, system will 
crash, through it is impossible now.


Maybe someone jumps out and say that there is error handling in cpu 
hotplug, however no such in pc_memory_plug(), why does not LoongArch in 
such way. If so, there will endless discussion.


void x86_cpu_plug(HotplugHandler *hotplug_dev,
  DeviceState *dev, Error **errp)
{
CPUArchId *found_cpu;
Error *local_err = NULL;
X86CPU *cpu = X86_CPU(dev);
X86MachineState *x86ms = X86_MACHINE(hotplug_dev);

if (x86ms->acpi_dev) {
hotplug_handler_plug(x86ms->acpi_dev, dev, &local_err);

static void pc_memory_plug(HotplugHandler *hotplug_dev,
   DeviceState *dev, Error **errp)
{
PCMachineState *pcms = PC_MACHINE(hotplug_dev);
X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
MachineState *ms = MACHINE(hotplug_dev);
bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);

   

Re: [PATCH-for-10.1 4/4] tcg: Convert TARGET_SUPPORTS_MTTCG to TCGCPUOps::mttcg_supported field

2025-03-22 Thread Pierrick Bouvier

On 3/21/25 08:59, Philippe Mathieu-Daudé wrote:

Instead of having a compile-time TARGET_SUPPORTS_MTTCG definition,
have each target set the 'mttcg_supported' field in the TCGCPUOps
structure.

Since so far we only emulate one target architecture at a time,
tcg_init_machine() gets whether MTTCG is supported via the
&first_cpu global.

Signed-off-by: Philippe Mathieu-Daudé 
---
  docs/devel/multi-thread-tcg.rst  | 2 +-
  configs/targets/aarch64-softmmu.mak  | 1 -
  configs/targets/alpha-softmmu.mak| 1 -
  configs/targets/arm-softmmu.mak  | 1 -
  configs/targets/hppa-softmmu.mak | 1 -
  configs/targets/i386-softmmu.mak | 1 -
  configs/targets/loongarch64-softmmu.mak  | 1 -
  configs/targets/microblaze-softmmu.mak   | 1 -
  configs/targets/microblazeel-softmmu.mak | 1 -
  configs/targets/mips-softmmu.mak | 1 -
  configs/targets/mipsel-softmmu.mak   | 1 -
  configs/targets/or1k-softmmu.mak | 1 -
  configs/targets/ppc64-softmmu.mak| 1 -
  configs/targets/riscv32-softmmu.mak  | 1 -
  configs/targets/riscv64-softmmu.mak  | 1 -
  configs/targets/s390x-softmmu.mak| 1 -
  configs/targets/sparc-softmmu.mak| 1 -
  configs/targets/sparc64-softmmu.mak  | 1 -
  configs/targets/x86_64-softmmu.mak   | 1 -
  configs/targets/xtensa-softmmu.mak   | 1 -
  configs/targets/xtensaeb-softmmu.mak | 1 -
  include/accel/tcg/cpu-ops.h  | 8 
  include/exec/poison.h| 1 -
  accel/tcg/tcg-all.c  | 7 ++-
  target/alpha/cpu.c   | 1 +
  target/arm/cpu.c | 1 +
  target/arm/tcg/cpu-v7m.c | 1 +
  target/avr/cpu.c | 1 +
  target/hexagon/cpu.c | 1 +
  target/hppa/cpu.c| 1 +
  target/i386/tcg/tcg-cpu.c| 1 +
  target/loongarch/cpu.c   | 1 +
  target/m68k/cpu.c| 1 +
  target/microblaze/cpu.c  | 1 +
  target/mips/cpu.c| 1 +
  target/openrisc/cpu.c| 1 +
  target/ppc/cpu_init.c| 1 +
  target/riscv/tcg/tcg-cpu.c   | 1 +
  target/rx/cpu.c  | 1 +
  target/s390x/cpu.c   | 1 +
  target/sh4/cpu.c | 1 +
  target/sparc/cpu.c   | 1 +
  target/tricore/cpu.c | 1 +
  target/xtensa/cpu.c  | 1 +
  44 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/docs/devel/multi-thread-tcg.rst b/docs/devel/multi-thread-tcg.rst
index 14a2a9dc7b5..da9a1530c9f 100644
--- a/docs/devel/multi-thread-tcg.rst
+++ b/docs/devel/multi-thread-tcg.rst
@@ -30,7 +30,7 @@ user-space thread. This is enabled by default for all FE/BE
  combinations where the host memory model is able to accommodate the
  guest (TCGCPUOps::guest_default_memory_order & ~TCG_TARGET_DEFAULT_MO is zero)
  and the guest has had the required work done to support this safely
-(TARGET_SUPPORTS_MTTCG).
+(TCGCPUOps::mttcg_supported).
  
  System emulation will fall back to the original round robin approach

  if:
diff --git a/configs/targets/aarch64-softmmu.mak 
b/configs/targets/aarch64-softmmu.mak
index 82cb72cb83d..5dfeb35af90 100644
--- a/configs/targets/aarch64-softmmu.mak
+++ b/configs/targets/aarch64-softmmu.mak
@@ -1,6 +1,5 @@
  TARGET_ARCH=aarch64
  TARGET_BASE_ARCH=arm
-TARGET_SUPPORTS_MTTCG=y
  TARGET_KVM_HAVE_GUEST_DEBUG=y
  TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml 
gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml 
gdb-xml/arm-vfp-sysregs.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml 
gdb-xml/arm-m-profile-mve.xml gdb-xml/aarch64-pauth.xml
  # needed by boot.c
diff --git a/configs/targets/alpha-softmmu.mak 
b/configs/targets/alpha-softmmu.mak
index 89f3517aca0..5275076e50d 100644
--- a/configs/targets/alpha-softmmu.mak
+++ b/configs/targets/alpha-softmmu.mak
@@ -1,3 +1,2 @@
  TARGET_ARCH=alpha
-TARGET_SUPPORTS_MTTCG=y
  TARGET_LONG_BITS=64
diff --git a/configs/targets/arm-softmmu.mak b/configs/targets/arm-softmmu.mak
index afc64f5927b..6a5a8eda949 100644
--- a/configs/targets/arm-softmmu.mak
+++ b/configs/targets/arm-softmmu.mak
@@ -1,5 +1,4 @@
  TARGET_ARCH=arm
-TARGET_SUPPORTS_MTTCG=y
  TARGET_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml 
gdb-xml/arm-vfp3.xml gdb-xml/arm-vfp-sysregs.xml gdb-xml/arm-neon.xml 
gdb-xml/arm-m-profile.xml gdb-xml/arm-m-profile-mve.xml
  # needed by boot.c
  TARGET_NEED_FDT=y
diff --git a/configs/targets/hppa-softmmu.mak b/configs/targets/hppa-softmmu.mak
index 63ca74ed5e6..ea331107a08 100644
--- a/configs/targets/hppa-softmmu.mak
+++ b/configs/targets/hppa-softmmu.mak
@@ -1,4 +1,3 @@
  TARGET_ARCH=hppa
  TARGET_BIG_ENDIAN=y
-TARGET_SUPPORTS_MTTCG=y
  TARGET_LONG_BITS=64
diff --git a/configs/targets/i386-softmmu.mak b/configs/targets/i386-softmmu.mak

[PATCH v1 02/22] hw/misc/aspeed_hace: Fix buffer overflow in has_padding function

2025-03-22 Thread Jamin Lin via
The maximum padding size is either 64 or 128 bytes and should always be smaller
than "req_len". If "padding_size" exceeds "req_len", then
"req_len - padding_size" underflows due to "uint32_t" data type, leading to a
large incorrect value (e.g., `0xFFXX`). This causes an out-of-bounds memory
access, potentially leading to a buffer overflow.

Added a check to ensure "padding_size" does not exceed "req_len" before
computing "pad_offset". This prevents "req_len - padding_size" from underflowing
and avoids accessing invalid memory.

Signed-off-by: Jamin Lin 
---
 hw/misc/aspeed_hace.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 8e7e8113a5..d8b5f048bb 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -128,6 +128,11 @@ static bool has_padding(AspeedHACEState *s, struct iovec 
*iov,
 if (*total_msg_len <= s->total_req_len) {
 uint32_t padding_size = s->total_req_len - *total_msg_len;
 uint8_t *padding = iov->iov_base;
+
+if (padding_size > req_len) {
+return false;
+}
+
 *pad_offset = req_len - padding_size;
 if (padding[*pad_offset] == 0x80) {
 return true;
-- 
2.43.0




Can I contribute Vitastor block driver? Or maybe introduce a QAPI plugin system?

2025-03-22 Thread vitalif
Hi!

I'm the author of Vitastor SDS (https://vitastor.io/). My project is an 
opensource SDS with an architecture similar to Ceph, but simpler and faster - 
in the terms of latency, it's ~10x faster, it easily reaches 0.1ms T1Q1 latency 
with NVMe disks.

I have a custom block driver for qemu (block/vitastor.c), currently I package 
it manually and provide my own QEMU packages.

I wanted to ask if I can submit this driver to you to package it upstream? It 
requires libvitastor_client library to build which is also currently available 
either in source form or from my repositories, is it fine?

And actually, if that's a problem, another option for me would be to use a 
hypothetical QAPI plugin system if QEMU had one - because, in fact, the only 
thing which is forcing me to rebuild QEMU is qapi/block-core.json. I have to 
patch it because, otherwise, the JSON options of my block driver aren't 
accepted by QEMU. So if there was a way to dynamically load these qapi 
definitions it would allow me to make compatible *.so block driver builds 
easily, even though there's no "stable" API as I understand. I'd just need a 
separate build for every qemu version, but that's not a problem for me :-). Or 
maybe such thing already exists and I just miss it?

What do you think?

-- 
Thanks in advance,
  Vitaliy Filippov



Re: [PATCH 00/10] gdbstub: conversion to runtime endianess helpers

2025-03-22 Thread Philippe Mathieu-Daudé

On 20/3/25 21:16, Pierrick Bouvier wrote:

On 3/20/25 12:52, Pierrick Bouvier wrote:

On 3/19/25 11:22, Alex Bennée wrote:

The aim of this work is to get rid of the endian aware helpers in
gdbstub/helpers.h which due to their use of tswap() mean target
gdbstubs need to be built multiple times. While this series doesn't
actually build each stub once it introduces a new helper -
gdb_get_register_value() which takes a MemOp which can describe the
current endian state of the system. This will be a lot easier to
dynamically feed from a helper function.

The most complex example is PPC which has a helper called
ppc_maybe_bswap_register() which was doing this.

This is still an RFC so I'm interested in feedback:

    - is the API sane
    - can we avoid lots of (uint8_t *) casting?


Even though the series has a good intent, the fact we make everything
"generic" makes that we lose all guarantees we could get by relying on
static typing, and that we had possibility of mistakes when passing size
(which happened in patch 4 if I'm correct). And explicit casting comes
as a *strong* warning about that.

By patch 7, I was really feeling it's not a win vs explicit functions
per size.

If the goal of the series is to get rid of endian aware helpers, well,
this can be fixed in the helpers themselves, without needing to
introduce a "generic" size helper. Maybe we are trying to solve two
different problems here?


    - should we have a reverse helper for setting registers

If this seems like the right approach I can have a go at more of the
frontends later.



Looking at include/gdbstub/helpers.h, gdb_get_reg128 can be solved by 
using target_words_bigendian() instead of TARGET_BIG_ENDIAN, which is 
already what tswap primitives are doing.


We'll need to eventually remove target_words_bigendian(), so that'd just
be postponing that.




Re: [PATCH v2 17/30] exec/target_page: runtime defintion for TARGET_PAGE_BITS_MIN

2025-03-22 Thread Richard Henderson

On 3/21/25 17:20, Pierrick Bouvier wrote:

On 3/21/25 17:01, Pierrick Bouvier wrote:

On 3/21/25 15:19, Richard Henderson wrote:

On 3/21/25 13:11, Pierrick Bouvier wrote:

On 3/21/25 12:27, Richard Henderson wrote:

On 3/21/25 11:09, Pierrick Bouvier wrote:

Mmm, ok I guess.  Yesterday I would have suggested merging this with 
page-vary.h, but
today I'm actively working on making TARGET_PAGE_BITS_MIN a global constant.



When you mention this, do you mean "constant accross all architectures", or a 
global
(const) variable vs having a function call?

The first -- constant across all architectures.



That's great.
Does choosing the min(set_of(TARGET_PAGE_BITS_MIN)) is what we want there, or 
is the
answer more subtle than that?


It will be, yes.

This isn't as hard as it seems, because there are exactly two targets with
TARGET_PAGE_BITS < 12: arm and avr.

Because we still support armv4, TARGET_PAGE_BITS_MIN must be <= 10.

AVR currently has TARGET_PAGE_BITS == 8, which is a bit of a problem.
My first task is to allow avr to choose TARGET_PAGE_BITS_MIN >= 10.

Which will leave us with TARGET_PAGE_BITS_MIN == 10.



Ok.

  From what I understand, we make sure tlb flags are stored in an
immutable position, within virtual addresses related to guest, by using
lower bits belonging to address range inside a given page, since page
addresses are aligned on page size, leaving those bits free.

bits [0..2) are bswap, watchpoint and check_aligned.
bits [TARGET_PAGE_BITS_MIN - 5..TARGET_PAGE_BITS_MIN) are slow,
discard_write, mmio, notdirty, and invalid mask.
And the compile time check we have is to make sure we don't overlap
those sets (would happen in TARGET_PAGE_BITS_MIN <= 7).

I wonder why we can't use bits [3..8) everywhere, like it's done for
AVR, even for bigger page sizes. I noticed the comment about "address
alignment bits", but I'm confused why bits [0..2) can be used, and not
upper ones.

Are we storing something else in the middle on other archs, or did I
miss some piece of the puzzle?



After looking better, TLB_SLOW_FLAGS are not part of address, so we don't use 
bits [0..2).

For a given TARGET_PAGE_SIZE, how do we define alignment bits?


Alignment bits are the least significant bits that must be 0 in order to enforce a 
particular alignment.  The specific alignment is requested via MO_ALIGN et al as part of 
the guest memory reference.


I think the piece you're missing is the softmmu fast path test in the generated 
code.

We begin by indexing the tlb to find an entry.  At that index, the entry may or may not 
match because (1) we have never looked up the page so the entry is empty, (2) we have 
looked up a different page that aliases, or (3) the page is present and (3a) correct, or 
(3b) invalidated, or (3c) some other condition that forces the slow path.


The target address and the comparator have several fields:

  page address   [63 ... TARGET_PAGE_BITS]
  page flags [TARGET_PAGE_BITS - 1 ... TARGET_PAGE_BITS - 5]
  unused [TARGET_PAGE_BITS - 6 ... align_bits], or empty.
  alignment  [align_bits - 1 ... 0], or empty

In the comparator, the unused and alignment bits are always zero; the page flags may be 
non-zero in order to force the comparison to fail.


In the target address, we mask the page flags and unused bits; if the alignment bits of 
the address are set, then the address is of course unaligned and so the comparison fails.


In order for all this work, the alignment field cannot overlap the page flags.

The maximum alignment currently used by any guest is 5 bits, for Arm Neon,
which means the minimum value for TARGET_PAGE_BITS_MIN is 10.


r~



Re: [PATCH-for-10.0 1/1] goldfish_rtc: keep time offset when resetting

2025-03-22 Thread Philippe Mathieu-Daudé

On 21/3/25 17:10, Heinrich Schuchardt wrote:

On 21.03.25 17:08, Philippe Mathieu-Daudé wrote:

Hi Heinrich,

On 21/3/25 09:12, Heinrich Schuchardt wrote:

Currently resetting leads to resynchronizing the Goldfish RTC with the
system clock of the host. In real hardware an RTC reset would not change
the wall time. Other RTCs like pl031 do not show this behavior.

Move the synchronization of the RTC with the system clock to the 
instance

realization.



Cc: qemu-sta...@nongnu.org
Fixes: 9a5b40b8427 ("hw: rtc: Add Goldfish RTC device")


Reported-by: Frederik Du Toit Lotter 
Signed-off-by: Heinrich Schuchardt 
---
  hw/rtc/goldfish_rtc.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
index 0f1b53e0e4..203a343511 100644
--- a/hw/rtc/goldfish_rtc.c
+++ b/hw/rtc/goldfish_rtc.c
@@ -239,15 +239,8 @@ static const VMStateDescription 
goldfish_rtc_vmstate = {

  static void goldfish_rtc_reset(DeviceState *dev)
  {
  GoldfishRTCState *s = GOLDFISH_RTC(dev);
-    struct tm tm;
  timer_del(s->timer);
-
-    qemu_get_timedate(&tm, 0);
-    s->tick_offset = mktimegm(&tm);
-    s->tick_offset *= NANOSECONDS_PER_SECOND;
-    s->tick_offset -= qemu_clock_get_ns(rtc_clock);
-    s->tick_offset_vmstate = 0;
  s->alarm_next = 0;
  s->alarm_running = 0;
  s->irq_pending = 0;
@@ -258,6 +251,7 @@ static void goldfish_rtc_realize(DeviceState *d, 
Error **errp)

  {
  SysBusDevice *dev = SYS_BUS_DEVICE(d);
  GoldfishRTCState *s = GOLDFISH_RTC(d);
+    struct tm tm;
  memory_region_init_io(&s->iomem, OBJECT(s),
    &goldfish_rtc_ops[s->big_endian], s,
@@ -267,6 +261,12 @@ static void goldfish_rtc_realize(DeviceState *d, 
Error **errp)

  sysbus_init_irq(dev, &s->irq);
  s->timer = timer_new_ns(rtc_clock, goldfish_rtc_interrupt, s);
+
+    qemu_get_timedate(&tm, 0);
+    s->tick_offset = mktimegm(&tm);
+    s->tick_offset *= NANOSECONDS_PER_SECOND;
+    s->tick_offset -= qemu_clock_get_ns(rtc_clock);


OK


+    s->tick_offset_vmstate = 0;


This last line is pointless. Otherwise:
Reviewed-by: Philippe Mathieu-Daudé 


Thanks for reviewing. Is the DeviceState structure fill with 0x00 when 
allocated?


The QOM hierarchy of this device is:

GOLDFISH_RTC -> SYS_BUS_DEVICE -> DEVICE -> OBJECT

When objects are created, object_new() ends up calling
object_initialize_with_type() which calls memset(0).

Objects initialized "in place" via object_initialize_child()
also end calling object_initialize() -> object_initialize_with_type()
then memset(0).

So yes, QOM-based objects have their state zero-initialized.

Regards,

Phil.




Re: [PATCH v6 3/6] hw/loongarch/virt: Fix error handling in cpu unplug

2025-03-22 Thread Markus Armbruster
bibo mao  writes:

> +Igor
>
>
> On 2025/3/21 下午2:47, Markus Armbruster wrote:
>> Bibo Mao  writes:
>> 
>>> In function virt_cpu_unplug(), it will send cpu unplug message to
>>> interrupt controller extioi and ipi irqchip. If there is problem in
>>> this function, system should continue to run and keep state the same
>>> before cpu is removed.
>>>
>>> If error happends in cpu unplug stage, send cpu plug message to extioi
>>> and ipi irqchip to restore to previous stage, and then return immediately.
>>>
>>> Fixes: 2cd6857f6f5b (hw/loongarch/virt: Implement cpu unplug interface)
>>> Signed-off-by: Bibo Mao 
>>> ---
>>>   hw/loongarch/virt.c | 6 ++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
>>> index 8563967c8b..503362a69e 100644
>>> --- a/hw/loongarch/virt.c
>>> +++ b/hw/loongarch/virt.c
>>> @@ -958,6 +958,8 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev,
>>>   hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->extioi), dev, &err);
>>>   if (err) {
>>>   error_propagate(errp, err);
>>> +hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), dev,
>>> + &error_abort);
>>>   return;
>>>   }
>>>   
>>> @@ -965,6 +967,10 @@ static void virt_cpu_unplug(HotplugHandler 
>>> *hotplug_dev,
>>>   hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &err);
>>>   if (err) {
>>>   error_propagate(errp, err);
>>> +hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), dev,
>>> + &error_abort);
>>> +hotplug_handler_plug(HOTPLUG_HANDLER(lvms->extioi), dev,
>>> + &error_abort);
>>>   return;
>>>   }
>> 
>> virt_cpu_unplug() calls hotplug_handler_unplug() three times to notify
>> ipi, extioi, and acpi_get.  If any notification fails, virt_cpu_unplug()
>> calls hotplug_handler_plug() to "un-notify" the preceeding ones, if any.
>> This must not fail.
>> 
>> virt_cpu_plug() does it the other way round (see previous patch).
>> 
>> So, hotplug_handler_plug() must not fail in virt_cpu_unplug(), yet we
>> check for it to fail in virt_cpu_plug().
>> 
>> Can it really fail in virt_cpu_plug()?
>> 
>> If yes, why can't it fail in virt_cpu_unplug()?
> you can check function acpi_cpu_plug_cb()/loongarch_ipi_cpu_plug(), that 
> is cpuplug callback for acpi_ged and ipi. it will not fail.
>
> If *virt_cpu_pre_plug()* pass, it will succeed.
>
> Regards
> Bibo Mao
>
>> 
>> Same questions for hotplug_handler_unplug().

Let me restate my argument.

We call hotplug_handler_plug() on the happy path, and on error recovery
paths.  Four cases:

1. Can fail on the happy path

   Error recovery is required.

1.1 Can fail on the error recovery path

Error recovery is required, but broken.

1.2 Can't fail on the error recovery path

Error recovery is required and works, but why it works is not
obvious.  Deserves a comment explaining why hotplug_handler_plug()
can't fail here even though it can fail on the happy path next door.

2. Can't fail on the happy path

   Error recovery is unreachable.

2.1 Can fail on the error recovery path

Error recovery is unreachable and broken.  Possibly a time bomb, and
possibly misleading readers.

2.2 Can't fail on the error recovery path

Error recovery is unreachable and would work, but why it would work
is again a not obvious.

Which of the four cases is it?




Re: [PATCH v1 02/22] hw/misc/aspeed_hace: Fix buffer overflow in has_padding function

2025-03-22 Thread Cédric Le Goater

On 3/21/25 10:47, Jamin Lin wrote:

Hi Cedric,


Subject: [PATCH v1 02/22] hw/misc/aspeed_hace: Fix buffer overflow in
has_padding function

The maximum padding size is either 64 or 128 bytes and should always be
smaller than "req_len". If "padding_size" exceeds "req_len", then "req_len -
padding_size" underflows due to "uint32_t" data type, leading to a large
incorrect value (e.g., `0xFFXX`). This causes an out-of-bounds memory
access, potentially leading to a buffer overflow.

Added a check to ensure "padding_size" does not exceed "req_len" before
computing "pad_offset". This prevents "req_len - padding_size" from
underflowing and avoids accessing invalid memory.

Signed-off-by: Jamin Lin 
---
  hw/misc/aspeed_hace.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index
8e7e8113a5..d8b5f048bb 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -128,6 +128,11 @@ static bool has_padding(AspeedHACEState *s, struct
iovec *iov,
  if (*total_msg_len <= s->total_req_len) {
  uint32_t padding_size = s->total_req_len - *total_msg_len;
  uint8_t *padding = iov->iov_base;
+
+if (padding_size > req_len) {
+return false;
+}
+
  *pad_offset = req_len - padding_size;
  if (padding[*pad_offset] == 0x80) {
  return true;
--
2.43.0


Fixes: 5cd7d8564a8b563da724b9e6264c967f0a091afa ("aspeed/hace: Support AST2600 HACE 
")




Reviewed-by: Cédric Le Goater 

Thanks,

C.





Re: [PATCH for-10.1 24/32] vfio: Introduce new files for dirty tracking definitions and declarations

2025-03-22 Thread Cédric Le Goater

On 3/20/25 10:52, Duan, Zhenzhong wrote:




-Original Message-
From: Cédric Le Goater 
Subject: [PATCH for-10.1 24/32] vfio: Introduce new files for dirty tracking
definitions and declarations

File "common.c" has been emptied of most of its definitions by the
previous changes and the only definitions left are related to dirty
tracking. Rename it to "dirty-tracking.c" and introduce its associated
"dirty-tracking.h" header file for the declarations.

Cleanup a little the includes while at it.

Signed-off-by: Cédric Le Goater 
---
hw/vfio/dirty-tracking.h   | 22 ++
include/hw/vfio/vfio-common.h  | 10 --
hw/vfio/container.c|  1 +
hw/vfio/{common.c => dirty-tracking.c} |  5 +
hw/vfio/iommufd.c  |  1 +
hw/vfio/meson.build|  2 +-
hw/vfio/trace-events   |  2 +-
7 files changed, 27 insertions(+), 16 deletions(-)
create mode 100644 hw/vfio/dirty-tracking.h
rename hw/vfio/{common.c => dirty-tracking.c} (99%)

diff --git a/hw/vfio/dirty-tracking.h b/hw/vfio/dirty-tracking.h
new file mode 100644
index
..4b83dc54ab50dabfff040d7cc3
db27b80bfe2d3a
--- /dev/null
+++ b/hw/vfio/dirty-tracking.h
@@ -0,0 +1,22 @@
+/*
+ * VFIO dirty page tracking routines
+ *
+ * Copyright Red Hat, Inc. 2025
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_VFIO_DIRTY_TRACKING_H
+#define HW_VFIO_DIRTY_TRACKING_H
+
+extern const MemoryListener vfio_memory_listener;
+
+bool vfio_devices_all_dirty_tracking_started(const VFIOContainerBase
*bcontainer);
+bool vfio_devices_all_device_dirty_tracking(const VFIOContainerBase
*bcontainer);
+int vfio_devices_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
+VFIOBitmap *vbmap, hwaddr iova, hwaddr 
size,
+Error **errp);
+int vfio_get_dirty_bitmap(const VFIOContainerBase *bcontainer, uint64_t iova,
+  uint64_t size, ram_addr_t ram_addr, Error **errp);
+
+#endif /* HW_VFIO_DIRTY_TRACKING_H */
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index
184a422916f62259158e8759efc473a5efb2b2f7..cc20110d9de8ac173b67e6e878
d4d61818497426 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -130,7 +130,6 @@ VFIODevice *vfio_get_vfio_device(Object *obj);

typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
extern VFIODeviceList vfio_device_list;
-extern const MemoryListener vfio_memory_listener;

#ifdef CONFIG_LINUX
int vfio_get_region_info(VFIODevice *vbasedev, int index,
@@ -140,15 +139,6 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev,
uint32_t type,
bool vfio_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_type);
#endif

-bool vfio_devices_all_dirty_tracking_started(
-const VFIOContainerBase *bcontainer);
-bool
-vfio_devices_all_device_dirty_tracking(const VFIOContainerBase *bcontainer);
-int vfio_devices_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
-VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
-int vfio_get_dirty_bitmap(const VFIOContainerBase *bcontainer, uint64_t iova,
-  uint64_t size, ram_addr_t ram_addr, Error **errp);
-
/* Returns 0 on success, or a negative errno. */
bool vfio_device_get_name(VFIODevice *vbasedev, Error **errp);
void vfio_device_set_fd(VFIODevice *vbasedev, const char *str, Error **errp);
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index
4e41a7476549a0c5e464e499d059db5aca6e3470..e88dfe12edd6dee469c06ee2e
46ab9c8b5019ae7 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -35,6 +35,7 @@
#include "hw/vfio/vfio-container.h"
#include "helpers.h"
#include "cpr.h"
+#include "dirty-tracking.h"

#define TYPE_HOST_IOMMU_DEVICE_LEGACY_VFIO
TYPE_HOST_IOMMU_DEVICE "-legacy-vfio"

diff --git a/hw/vfio/common.c b/hw/vfio/dirty-tracking.c
similarity index 99%
rename from hw/vfio/common.c
rename to hw/vfio/dirty-tracking.c
index
ed2f2ed8839caaf40fabb0cbbcaa1df2c5b70d67..441f9d9a08c06a88dda44ef143d
cee5f0a89a900 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/dirty-tracking.c
@@ -20,14 +20,10 @@

#include "qemu/osdep.h"
#include 
-#ifdef CONFIG_KVM
-#include 
-#endif


It looks this change unrelated to this patch?


#include 

#include "hw/vfio/vfio-common.h"
#include "hw/vfio/pci.h"
-#include "exec/address-spaces.h"


Same here.


yes and no. Commit log says :

  Cleanup a little the includes while at it.

but if you prefer we can address the include proliferation in a patch
of its own ?


Thanks,

C.





[PATCH v1 19/22] test/qtest/hace: Support 64-bit source and digest addresses for AST2700

2025-03-22 Thread Jamin Lin via
Added "HACE_HASH_SRC_HI" and "HACE_HASH_DIGEST_HI", "HACE_HASH_KEY_BUFF_HI"
registers to store upper 32 bits.
Updated "write_regs" to handle 64-bit source and digest addresses.

Signed-off-by: Jamin Lin 
---
 tests/qtest/aspeed-hace-utils.h | 3 +++
 tests/qtest/aspeed-hace-utils.c | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/tests/qtest/aspeed-hace-utils.h b/tests/qtest/aspeed-hace-utils.h
index 0382570fa2..d8684d3f83 100644
--- a/tests/qtest/aspeed-hace-utils.h
+++ b/tests/qtest/aspeed-hace-utils.h
@@ -36,6 +36,9 @@
 #define HACE_HASH_KEY_BUFF   0x28
 #define HACE_HASH_DATA_LEN   0x2c
 #define HACE_HASH_CMD0x30
+#define HACE_HASH_SRC_HI 0x90
+#define HACE_HASH_DIGEST_HI  0x94
+#define HACE_HASH_KEY_BUFF_HI0x98
 
 /* Scatter-Gather Hash */
 #define SG_LIST_LEN_LAST BIT(31)
diff --git a/tests/qtest/aspeed-hace-utils.c b/tests/qtest/aspeed-hace-utils.c
index f39bb8ea48..8d9c464f72 100644
--- a/tests/qtest/aspeed-hace-utils.c
+++ b/tests/qtest/aspeed-hace-utils.c
@@ -157,7 +157,9 @@ static void write_regs(QTestState *s, uint32_t base, 
uint64_t src,
uint32_t length, uint64_t out, uint32_t method)
 {
 qtest_writel(s, base + HACE_HASH_SRC, extract64(src, 0, 32));
+qtest_writel(s, base + HACE_HASH_SRC_HI, extract64(src, 32, 32));
 qtest_writel(s, base + HACE_HASH_DIGEST, extract64(out, 0, 32));
+qtest_writel(s, base + HACE_HASH_DIGEST_HI, extract64(out, 32, 32));
 qtest_writel(s, base + HACE_HASH_DATA_LEN, length);
 qtest_writel(s, base + HACE_HASH_CMD, HACE_SHA_BE_EN | method);
 }
-- 
2.43.0




Re: [PATCH 1/1] goldfish_rtc: keep time offset when resetting

2025-03-22 Thread Anup Patel
On Fri, Mar 21, 2025 at 1:43 PM Heinrich Schuchardt
 wrote:
>
> Currently resetting leads to resynchronizing the Goldfish RTC with the
> system clock of the host. In real hardware an RTC reset would not change
> the wall time. Other RTCs like pl031 do not show this behavior.
>
> Move the synchronization of the RTC with the system clock to the instance
> realization.
>
> Reported-by: Frederik Du Toit Lotter 
> Signed-off-by: Heinrich Schuchardt 

LGTM.

Reviewed-by: Anup Patel 

Regards,
Anup

> ---
>  hw/rtc/goldfish_rtc.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
> index 0f1b53e0e4..203a343511 100644
> --- a/hw/rtc/goldfish_rtc.c
> +++ b/hw/rtc/goldfish_rtc.c
> @@ -239,15 +239,8 @@ static const VMStateDescription goldfish_rtc_vmstate = {
>  static void goldfish_rtc_reset(DeviceState *dev)
>  {
>  GoldfishRTCState *s = GOLDFISH_RTC(dev);
> -struct tm tm;
>
>  timer_del(s->timer);
> -
> -qemu_get_timedate(&tm, 0);
> -s->tick_offset = mktimegm(&tm);
> -s->tick_offset *= NANOSECONDS_PER_SECOND;
> -s->tick_offset -= qemu_clock_get_ns(rtc_clock);
> -s->tick_offset_vmstate = 0;
>  s->alarm_next = 0;
>  s->alarm_running = 0;
>  s->irq_pending = 0;
> @@ -258,6 +251,7 @@ static void goldfish_rtc_realize(DeviceState *d, Error 
> **errp)
>  {
>  SysBusDevice *dev = SYS_BUS_DEVICE(d);
>  GoldfishRTCState *s = GOLDFISH_RTC(d);
> +struct tm tm;
>
>  memory_region_init_io(&s->iomem, OBJECT(s),
>&goldfish_rtc_ops[s->big_endian], s,
> @@ -267,6 +261,12 @@ static void goldfish_rtc_realize(DeviceState *d, Error 
> **errp)
>  sysbus_init_irq(dev, &s->irq);
>
>  s->timer = timer_new_ns(rtc_clock, goldfish_rtc_interrupt, s);
> +
> +qemu_get_timedate(&tm, 0);
> +s->tick_offset = mktimegm(&tm);
> +s->tick_offset *= NANOSECONDS_PER_SECOND;
> +s->tick_offset -= qemu_clock_get_ns(rtc_clock);
> +s->tick_offset_vmstate = 0;
>  }
>
>  static const Property goldfish_rtc_properties[] = {
> --
> 2.48.1
>
>



[PULL 1/6] hw/uefi: flush variable store to disk in post load

2025-03-22 Thread Gerd Hoffmann
Make live migration more robust.  Commit 4c0cfc72b31a ("pflash_cfi01:
write flash contents to bdrv on incoming migration") elaborates in
detail on the motivation.

Cc: Peter Krempa 
Reviewed-by: Peter Krempa 
Signed-off-by: Gerd Hoffmann 
Message-ID: <20250319141159.1461621-2-kra...@redhat.com>
---
 hw/uefi/var-service-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/uefi/var-service-core.c b/hw/uefi/var-service-core.c
index 8ed8378ab991..4836a0cb8116 100644
--- a/hw/uefi/var-service-core.c
+++ b/hw/uefi/var-service-core.c
@@ -29,6 +29,7 @@ static int uefi_vars_post_load(void *opaque, int version_id)
 uefi_vars_state *uv = opaque;
 
 uefi_vars_update_storage(uv);
+uefi_vars_json_save(uv);
 uv->buffer = g_malloc(uv->buf_size);
 return 0;
 }
-- 
2.49.0