* [RFC v2 0/2] tests/qtest: Add memory-access attributes (secure/space)
@ 2026-03-11 15:49 Tao Tang
2026-03-11 15:49 ` [RFC v2 1/2] tests/qtest: Add attrs argument support to memory access commands Tao Tang
2026-03-11 15:49 ` [RFC v2 2/2] tests/qtest: Add qtest-attrs-test for memory access with attrs Tao Tang
0 siblings, 2 replies; 7+ messages in thread
From: Tao Tang @ 2026-03-11 15:49 UTC (permalink / raw)
To: Fabiano Rosas, Laurent Vivier, Paolo Bonzini
Cc: qemu-devel, qemu-arm, Peter Maydell, Chen Baozi, Pierrick Bouvier,
Chao Liu, Tao Tang
This is v2 of the patch series, which adds support for memory access attributes
(secure/space) in qtest framework.
Thanks to everyone who reviewed the previous version and provided valuable
feedback.
The major changes from v1 include:
- rework the qtest protocol to use an optional attrs argument and replace with
the related libqtest APIs
- use ARMSS_* and arm_space_is_secure() for ARM security-space handling
- switch AddressSpace lookup to cpu_asidx_from_attrs()
Motivation
----------
IOMMU functional testing is always stack-heavy: for example, even for the
Non-secure Arm SMMU model, end-to-end validation can require a fairly involved
software stack. To keep the testing loop tight and reproducible, I previously
proposed using iommu-testdev + QTest to validate IOMMU/SMMU behaviour without
booting a guest [1].
[1] https://www.qemu.org/docs/master/specs/iommu-testdev.html
However, the current QTest interface cannot tag memory accesses with MemTxAttrs,
so it is hard to write targeted tests for security-sensitive paths. This becomes
a bigger issue for Secure SMMU work, which is RFCing in [2] and for future Arm
RME-DA enablement.
[2] https://lore.kernel.org/qemu-devel/20260221100250.2976287-1-tangtao1634@phytium.com.cn/
This series therefore adds qtest protocol/libqtest helpers to drive memory
transactions and introduces qtest-attrs-test to validate the new APIs on Arm
virt(Secure/NonSecure/Root/Realm spaces) and on x86 pc (SMM AddressSpace).
Compatibility
-------------
This series is strictly additive:
- Existing qtest protocol commands remain unchanged.
- The new *_attrs commands are optional extensions.
- The implementation is confined to qtest.c + libqtest, plus the new test case.
Testing
-------
The new qtest-attrs-test is registered for both x86 and aarch64 qtest lists.
# test command
meson test -C build \
"qtest-aarch64/qtest-attrs-test" \
"qtest-i386/qtest-attrs-test" \
"qtest-x86_64/qtest-attrs-test"
Future work
-----------
Once the Secure SMMU series lands, I plan to build on this qtest API to extend
iommu-testdev-based tests to cover more security contexts (Secure state and,
later, RME-DA-related paths) while still avoiding a full guest software stack.
The branch below [4] demonstrates a modified iommu-testdev using the new qtest
interfaces to exercise Secure SMMU behaviour.
[4] https://gitlab.com/TaoTang/qemu/-/tree/integration/qtest-secure-api-v2
Tao Tang (2):
tests/qtest: Add attrs argument support to memory access commands
tests/qtest: Add qtest-attrs-test for memory access with attrs
system/qtest.c | 266 +++++++++++++++++++++++++++------
tests/qtest/libqtest-single.h | 61 ++++++++
tests/qtest/libqtest.c | 148 ++++++++++++++++++
tests/qtest/libqtest.h | 25 ++++
tests/qtest/meson.build | 4 +-
tests/qtest/qtest-attrs-test.c | 229 ++++++++++++++++++++++++++++
6 files changed, 685 insertions(+), 48 deletions(-)
create mode 100644 tests/qtest/qtest-attrs-test.c
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC v2 1/2] tests/qtest: Add attrs argument support to memory access commands
2026-03-11 15:49 [RFC v2 0/2] tests/qtest: Add memory-access attributes (secure/space) Tao Tang
@ 2026-03-11 15:49 ` Tao Tang
2026-03-12 19:15 ` Peter Maydell
2026-03-11 15:49 ` [RFC v2 2/2] tests/qtest: Add qtest-attrs-test for memory access with attrs Tao Tang
1 sibling, 1 reply; 7+ messages in thread
From: Tao Tang @ 2026-03-11 15:49 UTC (permalink / raw)
To: Fabiano Rosas, Laurent Vivier, Paolo Bonzini
Cc: qemu-devel, qemu-arm, Peter Maydell, Chen Baozi, Pierrick Bouvier,
Chao Liu, Tao Tang
Extend qtest memory access commands to accept an optional attrs argument.
Supported attrs:
- secure (ARM/x86-only, lowercase)
- space=non-secure|secure|root|realm (ARM-only, lowercase)
For memory commands, parse attrs and select AddressSpace via
cpu_asidx_from_attrs(), then issue accesses with the corresponding
MemTxAttrs.
Expose matching libqtest APIs:
- qtest_{read,write}{b,w,l,q}_attrs()
- qtest_mem{read,write,set}_attrs()
Also add libqtest-single wrappers for the *_attrs helpers.
Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
---
system/qtest.c | 266 ++++++++++++++++++++++++++++------
tests/qtest/libqtest-single.h | 61 ++++++++
tests/qtest/libqtest.c | 148 +++++++++++++++++++
tests/qtest/libqtest.h | 25 ++++
4 files changed, 453 insertions(+), 47 deletions(-)
diff --git a/system/qtest.c b/system/qtest.c
index cf90cd53ad..dbf3a63c57 100644
--- a/system/qtest.c
+++ b/system/qtest.c
@@ -22,6 +22,7 @@
#include "hw/core/qdev.h"
#include "hw/core/irq.h"
#include "hw/core/cpu.h"
+#include "hw/arm/arm-security.h"
#include "qemu/accel.h"
#include "system/cpu-timers.h"
#include "qemu/config-file.h"
@@ -218,6 +219,39 @@ static void *qtest_server_send_opaque;
* B64_DATA is an arbitrarily long base64 encoded string.
* If the sizes do not match, the data will be truncated.
*
+ * Memory access with MemTxAttrs:
+ * """"""""""""""""""""""""""""""
+ *
+ * The following commands allow specifying memory transaction attributes,
+ * which is useful for testing devices that behave differently based on
+ * security state (e.g., ARM TrustZone/CCA or System Management Mode in x86).
+ *
+ * The memory access commands above support one optional ATTRS argument:
+ *
+ * .. code-block:: none
+ *
+ * > writeb ADDR VALUE
+ * < OK
+ * > writeb ADDR VALUE secure
+ * < OK
+ * > writeb ADDR VALUE space=realm
+ * < OK
+ * > readb ADDR secure
+ * < OK VALUE
+ * > readb ADDR space=root
+ * < OK VALUE
+ * > read ADDR SIZE space=secure
+ * < OK DATA
+ * > write ADDR SIZE DATA secure
+ * < OK
+ * > memset ADDR SIZE VALUE space=non-secure
+ * < OK
+ *
+ * ``secure`` sets MemTxAttrs.secure=1 (x86/ARM).
+ * ``space=...`` is ARM-specific and accepts:
+ * non-secure, secure, root, realm.
+ * ``space=non-secure`` is equivalent to omitting ATTRS.
+ *
* IRQ management:
* """""""""""""""
*
@@ -353,6 +387,135 @@ static void qtest_install_gpio_out_intercept(DeviceState *dev, const char *name,
*disconnected = qdev_intercept_gpio_out(dev, icpt, name, n);
}
+static bool qtest_parse_mem_attrs(CharFrontend *chr, const char *arg,
+ MemTxAttrs *attrs)
+{
+ if (!arg) {
+ *attrs = MEMTXATTRS_UNSPECIFIED;
+ return true;
+ }
+
+ if (strcmp(arg, "secure") == 0) {
+ *attrs = (MemTxAttrs){ .secure = 1 };
+ return true;
+ }
+
+ if (strncmp(arg, "space=", 6) == 0) {
+ const char *space = arg + 6;
+ ARMSecuritySpace sec_space;
+
+ if (!target_arm() && !target_aarch64()) {
+ qtest_send(chr, "ERR space=<...> is ARM-specific\n");
+ return false;
+ }
+
+ if (strcmp(space, "non-secure") == 0) {
+ *attrs = MEMTXATTRS_UNSPECIFIED;
+ return true;
+ } else if (strcmp(space, "secure") == 0) {
+ sec_space = ARMSS_Secure;
+ } else if (strcmp(space, "root") == 0) {
+ sec_space = ARMSS_Root;
+ } else if (strcmp(space, "realm") == 0) {
+ sec_space = ARMSS_Realm;
+ } else {
+ qtest_send(chr, "ERR invalid space value. Valid space: "
+ "secure/non-secure/root/realm\n");
+ return false;
+ }
+
+ *attrs = (MemTxAttrs){
+ .space = sec_space,
+ .secure = arm_space_is_secure(sec_space),
+ };
+ return true;
+ }
+
+ qtest_send(chr, "ERR invalid attrs argument\n");
+ return false;
+}
+
+static bool qtest_get_mem_as(CharFrontend *chr, MemTxAttrs attrs,
+ AddressSpace **as)
+{
+ int asidx;
+
+ /*
+ * cpu_asidx_from_attrs mainly uses attrs to call ->asidx_from_attrs. We use
+ * first_cpu as it's readily available.
+ */
+
+ asidx = cpu_asidx_from_attrs(first_cpu, attrs);
+ *as = cpu_get_address_space(first_cpu, asidx);
+ if (!*as) {
+ qtest_send(chr, "ERR address space unavailable for attrs\n");
+ return false;
+ }
+
+ return true;
+}
+
+static void qtest_write_sized(AddressSpace *as, uint64_t addr, MemTxAttrs attrs,
+ uint64_t value, char size)
+{
+ switch (size) {
+ case 'b': {
+ uint8_t data = value;
+ address_space_write(as, addr, attrs, &data, 1);
+ break;
+ }
+ case 'w': {
+ uint16_t data = value;
+ tswap16s(&data);
+ address_space_write(as, addr, attrs, &data, 2);
+ break;
+ }
+ case 'l': {
+ uint32_t data = value;
+ tswap32s(&data);
+ address_space_write(as, addr, attrs, &data, 4);
+ break;
+ }
+ case 'q': {
+ uint64_t data = value;
+ tswap64s(&data);
+ address_space_write(as, addr, attrs, &data, 8);
+ break;
+ }
+ default:
+ g_assert_not_reached();
+ }
+}
+
+static uint64_t qtest_read_sized(AddressSpace *as, uint64_t addr,
+ MemTxAttrs attrs, char size)
+{
+ switch (size) {
+ case 'b': {
+ uint8_t data;
+ address_space_read(as, addr, attrs, &data, 1);
+ return data;
+ }
+ case 'w': {
+ uint16_t data;
+ address_space_read(as, addr, attrs, &data, 2);
+ return tswap16(data);
+ }
+ case 'l': {
+ uint32_t data;
+ address_space_read(as, addr, attrs, &data, 4);
+ return tswap32(data);
+ }
+ case 'q': {
+ uint64_t data;
+ address_space_read(as, addr, attrs, &data, 8);
+ return tswap64(data);
+ }
+ default:
+ g_assert_not_reached();
+ }
+}
+
static void qtest_process_command(CharFrontend *chr, gchar **words)
{
const gchar *command;
@@ -510,34 +673,25 @@ static void qtest_process_command(CharFrontend *chr, gchar **words)
strcmp(words[0], "writeq") == 0) {
uint64_t addr;
uint64_t value;
+ MemTxAttrs attrs;
+ AddressSpace *as;
int ret;
g_assert(words[1] && words[2]);
+ if (words[3] && words[4]) {
+ qtest_send(chr, "ERR too many arguments\n");
+ return;
+ }
ret = qemu_strtou64(words[1], NULL, 0, &addr);
g_assert(ret == 0);
ret = qemu_strtou64(words[2], NULL, 0, &value);
g_assert(ret == 0);
-
- if (words[0][5] == 'b') {
- uint8_t data = value;
- address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
- &data, 1);
- } else if (words[0][5] == 'w') {
- uint16_t data = value;
- tswap16s(&data);
- address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
- &data, 2);
- } else if (words[0][5] == 'l') {
- uint32_t data = value;
- tswap32s(&data);
- address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
- &data, 4);
- } else if (words[0][5] == 'q') {
- uint64_t data = value;
- tswap64s(&data);
- address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
- &data, 8);
+ if (!qtest_parse_mem_attrs(chr, words[3], &attrs) ||
+ !qtest_get_mem_as(chr, attrs, &as)) {
+ return;
}
+
+ qtest_write_sized(as, addr, attrs, value, words[0][5]);
qtest_send(chr, "OK\n");
} else if (strcmp(words[0], "readb") == 0 ||
strcmp(words[0], "readw") == 0 ||
@@ -545,50 +699,50 @@ static void qtest_process_command(CharFrontend *chr, gchar **words)
strcmp(words[0], "readq") == 0) {
uint64_t addr;
uint64_t value = UINT64_C(-1);
+ MemTxAttrs attrs;
+ AddressSpace *as;
int ret;
g_assert(words[1]);
+ if (words[2] && words[3]) {
+ qtest_send(chr, "ERR too many arguments\n");
+ return;
+ }
ret = qemu_strtou64(words[1], NULL, 0, &addr);
g_assert(ret == 0);
-
- if (words[0][4] == 'b') {
- uint8_t data;
- address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
- &data, 1);
- value = data;
- } else if (words[0][4] == 'w') {
- uint16_t data;
- address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
- &data, 2);
- value = tswap16(data);
- } else if (words[0][4] == 'l') {
- uint32_t data;
- address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
- &data, 4);
- value = tswap32(data);
- } else if (words[0][4] == 'q') {
- address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
- &value, 8);
- tswap64s(&value);
+ if (!qtest_parse_mem_attrs(chr, words[2], &attrs) ||
+ !qtest_get_mem_as(chr, attrs, &as)) {
+ return;
}
+
+ value = qtest_read_sized(as, addr, attrs, words[0][4]);
qtest_sendf(chr, "OK 0x%016" PRIx64 "\n", value);
} else if (strcmp(words[0], "read") == 0) {
g_autoptr(GString) enc = NULL;
uint64_t addr, len;
uint8_t *data;
+ MemTxAttrs attrs;
+ AddressSpace *as;
int ret;
g_assert(words[1] && words[2]);
+ if (words[3] && words[4]) {
+ qtest_send(chr, "ERR too many arguments\n");
+ return;
+ }
ret = qemu_strtou64(words[1], NULL, 0, &addr);
g_assert(ret == 0);
ret = qemu_strtou64(words[2], NULL, 0, &len);
g_assert(ret == 0);
/* We'd send garbage to libqtest if len is 0 */
g_assert(len);
+ if (!qtest_parse_mem_attrs(chr, words[3], &attrs) ||
+ !qtest_get_mem_as(chr, attrs, &as)) {
+ return;
+ }
data = g_malloc(len);
- address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
- len);
+ address_space_read(as, addr, attrs, data, len);
enc = qemu_hexdump_line(NULL, data, len, 0, 0);
@@ -619,13 +773,23 @@ static void qtest_process_command(CharFrontend *chr, gchar **words)
uint64_t addr, len, i;
uint8_t *data;
size_t data_len;
+ MemTxAttrs attrs;
+ AddressSpace *as;
int ret;
g_assert(words[1] && words[2] && words[3]);
+ if (words[4] && words[5]) {
+ qtest_send(chr, "ERR too many arguments\n");
+ return;
+ }
ret = qemu_strtou64(words[1], NULL, 0, &addr);
g_assert(ret == 0);
ret = qemu_strtou64(words[2], NULL, 0, &len);
g_assert(ret == 0);
+ if (!qtest_parse_mem_attrs(chr, words[4], &attrs) ||
+ !qtest_get_mem_as(chr, attrs, &as)) {
+ return;
+ }
data_len = strlen(words[3]);
if (data_len < 3) {
@@ -642,8 +806,7 @@ static void qtest_process_command(CharFrontend *chr, gchar **words)
data[i] = 0;
}
}
- address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
- len);
+ address_space_write(as, addr, attrs, data, len);
g_free(data);
qtest_send(chr, "OK\n");
@@ -651,26 +814,35 @@ static void qtest_process_command(CharFrontend *chr, gchar **words)
uint64_t addr, len;
uint8_t *data;
unsigned long pattern;
+ MemTxAttrs attrs;
+ AddressSpace *as;
int ret;
g_assert(words[1] && words[2] && words[3]);
+ if (words[4] && words[5]) {
+ qtest_send(chr, "ERR too many arguments\n");
+ return;
+ }
ret = qemu_strtou64(words[1], NULL, 0, &addr);
g_assert(ret == 0);
ret = qemu_strtou64(words[2], NULL, 0, &len);
g_assert(ret == 0);
ret = qemu_strtoul(words[3], NULL, 0, &pattern);
g_assert(ret == 0);
+ if (!qtest_parse_mem_attrs(chr, words[4], &attrs) ||
+ !qtest_get_mem_as(chr, attrs, &as)) {
+ return;
+ }
if (len) {
data = g_malloc(len);
memset(data, pattern, len);
- address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
- data, len);
+ address_space_write(as, addr, attrs, data, len);
g_free(data);
}
qtest_send(chr, "OK\n");
- } else if (strcmp(words[0], "b64write") == 0) {
+ } else if (strcmp(words[0], "b64write") == 0) {
uint64_t addr, len;
uint8_t *data;
size_t data_len;
diff --git a/tests/qtest/libqtest-single.h b/tests/qtest/libqtest-single.h
index 851724cbcb..38d05ee61e 100644
--- a/tests/qtest/libqtest-single.h
+++ b/tests/qtest/libqtest-single.h
@@ -291,6 +291,67 @@ static inline void memwrite(uint64_t addr, const void *data, size_t size)
qtest_memwrite(global_qtest, addr, data, size);
}
+/*
+ * Memory commands with optional attrs argument.
+ */
+static inline void writeb_attrs(uint64_t addr, uint8_t value, const char *attrs)
+{
+ qtest_writeb_attrs(global_qtest, addr, value, attrs);
+}
+
+static inline void writew_attrs(uint64_t addr, uint16_t value, const char *attrs)
+{
+ qtest_writew_attrs(global_qtest, addr, value, attrs);
+}
+
+static inline void writel_attrs(uint64_t addr, uint32_t value, const char *attrs)
+{
+ qtest_writel_attrs(global_qtest, addr, value, attrs);
+}
+
+static inline void writeq_attrs(uint64_t addr, uint64_t value, const char *attrs)
+{
+ qtest_writeq_attrs(global_qtest, addr, value, attrs);
+}
+
+static inline uint8_t readb_attrs(uint64_t addr, const char *attrs)
+{
+ return qtest_readb_attrs(global_qtest, addr, attrs);
+}
+
+static inline uint16_t readw_attrs(uint64_t addr, const char *attrs)
+{
+ return qtest_readw_attrs(global_qtest, addr, attrs);
+}
+
+static inline uint32_t readl_attrs(uint64_t addr, const char *attrs)
+{
+ return qtest_readl_attrs(global_qtest, addr, attrs);
+}
+
+static inline uint64_t readq_attrs(uint64_t addr, const char *attrs)
+{
+ return qtest_readq_attrs(global_qtest, addr, attrs);
+}
+
+static inline void memread_attrs(uint64_t addr, void *data, size_t size,
+ const char *attrs)
+{
+ qtest_memread_attrs(global_qtest, addr, data, size, attrs);
+}
+
+static inline void memwrite_attrs(uint64_t addr, const void *data, size_t size,
+ const char *attrs)
+{
+ qtest_memwrite_attrs(global_qtest, addr, data, size, attrs);
+}
+
+static inline void memset_attrs(uint64_t addr, uint8_t pattern, size_t size,
+ const char *attrs)
+{
+ qtest_memset_attrs(global_qtest, addr, pattern, size, attrs);
+}
+
/**
* clock_step_next:
*
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 051faf31e1..a548331f7c 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1447,6 +1447,154 @@ void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
qtest_rsp(s);
}
+static bool qtest_has_attrs(const char *attrs)
+{
+ return attrs && attrs[0];
+}
+
+static void qtest_write_attrs(QTestState *s, const char *cmd,
+ uint64_t addr, uint64_t value,
+ const char *attrs)
+{
+ if (qtest_has_attrs(attrs)) {
+ qtest_sendf(s, "%s 0x%" PRIx64 " 0x%" PRIx64 " %s\n",
+ cmd, addr, value, attrs);
+ } else {
+ qtest_sendf(s, "%s 0x%" PRIx64 " 0x%" PRIx64 "\n", cmd, addr, value);
+ }
+ qtest_rsp(s);
+}
+
+static uint64_t qtest_read_attrs(QTestState *s, const char *cmd,
+ uint64_t addr, const char *attrs)
+{
+ gchar **args;
+ int ret;
+ uint64_t value;
+
+ if (qtest_has_attrs(attrs)) {
+ qtest_sendf(s, "%s 0x%" PRIx64 " %s\n", cmd, addr, attrs);
+ } else {
+ qtest_sendf(s, "%s 0x%" PRIx64 "\n", cmd, addr);
+ }
+ args = qtest_rsp_args(s, 2);
+ ret = qemu_strtou64(args[1], NULL, 0, &value);
+ g_assert(!ret);
+ g_strfreev(args);
+
+ return value;
+}
+
+void qtest_writeb_attrs(QTestState *s, uint64_t addr, uint8_t value,
+ const char *attrs)
+{
+ qtest_write_attrs(s, "writeb", addr, value, attrs);
+}
+
+void qtest_writew_attrs(QTestState *s, uint64_t addr, uint16_t value,
+ const char *attrs)
+{
+ qtest_write_attrs(s, "writew", addr, value, attrs);
+}
+
+void qtest_writel_attrs(QTestState *s, uint64_t addr, uint32_t value,
+ const char *attrs)
+{
+ qtest_write_attrs(s, "writel", addr, value, attrs);
+}
+
+void qtest_writeq_attrs(QTestState *s, uint64_t addr, uint64_t value,
+ const char *attrs)
+{
+ qtest_write_attrs(s, "writeq", addr, value, attrs);
+}
+
+uint8_t qtest_readb_attrs(QTestState *s, uint64_t addr, const char *attrs)
+{
+ return qtest_read_attrs(s, "readb", addr, attrs);
+}
+
+uint16_t qtest_readw_attrs(QTestState *s, uint64_t addr, const char *attrs)
+{
+ return qtest_read_attrs(s, "readw", addr, attrs);
+}
+
+uint32_t qtest_readl_attrs(QTestState *s, uint64_t addr, const char *attrs)
+{
+ return qtest_read_attrs(s, "readl", addr, attrs);
+}
+
+uint64_t qtest_readq_attrs(QTestState *s, uint64_t addr, const char *attrs)
+{
+ return qtest_read_attrs(s, "readq", addr, attrs);
+}
+
+void qtest_memread_attrs(QTestState *s, uint64_t addr, void *data,
+ size_t size, const char *attrs)
+{
+ uint8_t *ptr = data;
+ gchar **args;
+ size_t i;
+
+ if (!size) {
+ return;
+ }
+
+ if (qtest_has_attrs(attrs)) {
+ qtest_sendf(s, "read 0x%" PRIx64 " 0x%zx %s\n", addr, size, attrs);
+ } else {
+ qtest_sendf(s, "read 0x%" PRIx64 " 0x%zx\n", addr, size);
+ }
+ args = qtest_rsp_args(s, 2);
+
+ for (i = 0; i < size; i++) {
+ ptr[i] = hex2nib(args[1][2 + (i * 2)]) << 4;
+ ptr[i] |= hex2nib(args[1][2 + (i * 2) + 1]);
+ }
+
+ g_strfreev(args);
+}
+
+void qtest_memwrite_attrs(QTestState *s, uint64_t addr, const void *data,
+ size_t size, const char *attrs)
+{
+ const uint8_t *ptr = data;
+ size_t i;
+ char *enc;
+
+ if (!size) {
+ return;
+ }
+
+ enc = g_malloc(2 * size + 1);
+
+ for (i = 0; i < size; i++) {
+ sprintf(&enc[i * 2], "%02x", ptr[i]);
+ }
+
+ if (qtest_has_attrs(attrs)) {
+ qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x%s %s\n",
+ addr, size, enc, attrs);
+ } else {
+ qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x%s\n", addr, size, enc);
+ }
+ qtest_rsp(s);
+ g_free(enc);
+}
+
+void qtest_memset_attrs(QTestState *s, uint64_t addr, uint8_t pattern,
+ size_t size, const char *attrs)
+{
+ if (qtest_has_attrs(attrs)) {
+ qtest_sendf(s, "memset 0x%" PRIx64 " 0x%zx 0x%02x %s\n",
+ addr, size, pattern, attrs);
+ } else {
+ qtest_sendf(s, "memset 0x%" PRIx64 " 0x%zx 0x%02x\n",
+ addr, size, pattern);
+ }
+ qtest_rsp(s);
+}
+
QDict *qtest_vqmp_assert_failure_ref(QTestState *qts,
const char *fmt, va_list args)
{
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 9c118c89ca..d00a83478a 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -705,6 +705,31 @@ void qtest_bufwrite(QTestState *s, uint64_t addr,
*/
void qtest_memset(QTestState *s, uint64_t addr, uint8_t patt, size_t size);
+/*
+ * Memory commands with optional attrs argument.
+ */
+
+void qtest_writeb_attrs(QTestState *s, uint64_t addr, uint8_t value,
+ const char *attrs);
+void qtest_writew_attrs(QTestState *s, uint64_t addr, uint16_t value,
+ const char *attrs);
+void qtest_writel_attrs(QTestState *s, uint64_t addr, uint32_t value,
+ const char *attrs);
+void qtest_writeq_attrs(QTestState *s, uint64_t addr, uint64_t value,
+ const char *attrs);
+
+uint8_t qtest_readb_attrs(QTestState *s, uint64_t addr, const char *attrs);
+uint16_t qtest_readw_attrs(QTestState *s, uint64_t addr, const char *attrs);
+uint32_t qtest_readl_attrs(QTestState *s, uint64_t addr, const char *attrs);
+uint64_t qtest_readq_attrs(QTestState *s, uint64_t addr, const char *attrs);
+
+void qtest_memread_attrs(QTestState *s, uint64_t addr, void *data, size_t size,
+ const char *attrs);
+void qtest_memwrite_attrs(QTestState *s, uint64_t addr, const void *data,
+ size_t size, const char *attrs);
+void qtest_memset_attrs(QTestState *s, uint64_t addr, uint8_t patt, size_t size,
+ const char *attrs);
+
/**
* qtest_clock_step_next:
* @s: #QTestState instance to operate on.
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC v2 2/2] tests/qtest: Add qtest-attrs-test for memory access with attrs
2026-03-11 15:49 [RFC v2 0/2] tests/qtest: Add memory-access attributes (secure/space) Tao Tang
2026-03-11 15:49 ` [RFC v2 1/2] tests/qtest: Add attrs argument support to memory access commands Tao Tang
@ 2026-03-11 15:49 ` Tao Tang
2026-03-12 19:23 ` Peter Maydell
1 sibling, 1 reply; 7+ messages in thread
From: Tao Tang @ 2026-03-11 15:49 UTC (permalink / raw)
To: Fabiano Rosas, Laurent Vivier, Paolo Bonzini
Cc: qemu-devel, qemu-arm, Peter Maydell, Chen Baozi, Pierrick Bouvier,
Chao Liu, Tao Tang
Add qtest-attrs-test to exercise qtest memory commands with optional attrs
on aarch64 (virt, secure=on) and x86 (pc, tcg).
The test covers:
- ARM (virt machine): exercises all supported ARM security spaces
(non-secure, secure, root, realm).
- x86 (pc machine): exercises secure attrs accesses
(SMM address-space path).
The test also covers libqtest-single *_attrs shortcut wrappers and
verifies that space=non-secure behaves like omitting attrs.
Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
---
tests/qtest/meson.build | 4 +-
tests/qtest/qtest-attrs-test.c | 234 +++++++++++++++++++++++++++++++++
2 files changed, 237 insertions(+), 1 deletion(-)
create mode 100644 tests/qtest/qtest-attrs-test.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index be4fa627b5..c11759e23f 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -115,6 +115,7 @@ qtests_i386 = \
'drive_del-test',
'cpu-plug-test',
'migration-test',
+ 'qtest-attrs-test',
]
if dbus_display and config_all_devices.has_key('CONFIG_VGA')
@@ -270,7 +271,8 @@ qtests_aarch64 = \
['arm-cpu-features',
'numa-test',
'boot-serial-test',
- 'migration-test']
+ 'migration-test',
+ 'qtest-attrs-test']
qtests_s390x = \
qtests_filter + \
diff --git a/tests/qtest/qtest-attrs-test.c b/tests/qtest/qtest-attrs-test.c
new file mode 100644
index 0000000000..ce204c2c95
--- /dev/null
+++ b/tests/qtest/qtest-attrs-test.c
@@ -0,0 +1,234 @@
+/*
+ * QTest for memory access with transaction attributes
+ *
+ * Verify optional attrs argument support for qtest memory commands.
+ *
+ * Copyright (c) 2026 Phytium Technology
+ *
+ * Author:
+ * Tao Tang <tangtao1634@phytium.com.cn>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "libqtest-single.h"
+
+/*
+ * Default RAM size is 128 MiB on both machines used below.
+ * Keep test addresses in low RAM and away from device MMIO regions.
+ */
+#define TEST_ADDR_OFFSET 0x1000ULL
+#define TEST_ARM_BASE 0x40000000ULL
+#define TEST_X86_BASE 0x0ULL
+
+#define TEST_ADDR_ARM (TEST_ARM_BASE + TEST_ADDR_OFFSET)
+#define TEST_ADDR_X86 (TEST_X86_BASE + TEST_ADDR_OFFSET)
+
+#define ARM_MACHINE_ARGS "-machine virt,secure=on -cpu cortex-a57"
+#define X86_MACHINE_ARGS "-machine pc -accel tcg"
+
+static void test_arm_scalar_attrs(void)
+{
+ QTestState *qts;
+ uint8_t val;
+
+ if (!qtest_has_machine("virt")) {
+ g_test_skip("virt machine not available");
+ return;
+ }
+
+ qts = qtest_init(ARM_MACHINE_ARGS);
+
+ qtest_writeb_attrs(qts, TEST_ADDR_ARM, 0x11, NULL);
+ val = qtest_readb_attrs(qts, TEST_ADDR_ARM, NULL);
+ g_assert_cmpuint(val, ==, 0x11);
+
+ qtest_writeb_attrs(qts, TEST_ADDR_ARM + 0x1, 0x22, "secure");
+ val = qtest_readb_attrs(qts, TEST_ADDR_ARM + 0x1, "secure");
+ g_assert_cmpuint(val, ==, 0x22);
+
+ qtest_writeb_attrs(qts, TEST_ADDR_ARM + 0x2, 0x33, "space=realm");
+ val = qtest_readb_attrs(qts, TEST_ADDR_ARM + 0x2, "space=realm");
+ g_assert_cmpuint(val, ==, 0x33);
+
+ qtest_writeb_attrs(qts, TEST_ADDR_ARM + 0x3, 0x44, "space=root");
+ val = qtest_readb_attrs(qts, TEST_ADDR_ARM + 0x3, "space=root");
+ g_assert_cmpuint(val, ==, 0x44);
+
+ qtest_writeb_attrs(qts, TEST_ADDR_ARM + 0x4, 0x55, "space=secure");
+ val = qtest_readb_attrs(qts, TEST_ADDR_ARM + 0x4, "space=secure");
+ g_assert_cmpuint(val, ==, 0x55);
+
+ /* space=non-secure is equivalent to no attrs argument */
+ qtest_writeb_attrs(qts, TEST_ADDR_ARM + 0x5, 0x66, "space=non-secure");
+ val = qtest_readb_attrs(qts, TEST_ADDR_ARM + 0x5, NULL);
+ g_assert_cmpuint(val, ==, 0x66);
+
+ qtest_writeb_attrs(qts, TEST_ADDR_ARM + 0x6, 0x77, NULL);
+ val = qtest_readb_attrs(qts, TEST_ADDR_ARM + 0x6, "space=non-secure");
+ g_assert_cmpuint(val, ==, 0x77);
+
+ qtest_quit(qts);
+}
+
+static void test_arm_bulk_attrs(void)
+{
+ QTestState *qts;
+ uint8_t wbuf[16] = {
+ 0x00, 0x11, 0x22, 0x33,
+ 0x44, 0x55, 0x66, 0x77,
+ 0x88, 0x99, 0xaa, 0xbb,
+ 0xcc, 0xdd, 0xee, 0xff,
+ };
+ uint8_t rbuf[16];
+ size_t i;
+
+ if (!qtest_has_machine("virt")) {
+ g_test_skip("virt machine not available");
+ return;
+ }
+
+ qts = qtest_init(ARM_MACHINE_ARGS);
+
+ qtest_memwrite_attrs(qts, TEST_ADDR_ARM + 0x100,
+ wbuf, sizeof(wbuf), NULL);
+ qtest_memread_attrs(qts, TEST_ADDR_ARM + 0x100,
+ rbuf, sizeof(rbuf), NULL);
+ g_assert(memcmp(wbuf, rbuf, sizeof(wbuf)) == 0);
+
+ qtest_memwrite_attrs(qts, TEST_ADDR_ARM + 0x200,
+ wbuf, sizeof(wbuf), "secure");
+ qtest_memread_attrs(qts, TEST_ADDR_ARM + 0x200,
+ rbuf, sizeof(rbuf), "secure");
+ g_assert(memcmp(wbuf, rbuf, sizeof(wbuf)) == 0);
+
+ qtest_memwrite_attrs(qts, TEST_ADDR_ARM + 0x300,
+ wbuf, sizeof(wbuf), "space=realm");
+ qtest_memread_attrs(qts, TEST_ADDR_ARM + 0x300,
+ rbuf, sizeof(rbuf), "space=realm");
+ g_assert(memcmp(wbuf, rbuf, sizeof(wbuf)) == 0);
+
+ qtest_memset_attrs(qts, TEST_ADDR_ARM + 0x400,
+ 0xa5, sizeof(rbuf), "space=root");
+ qtest_memread_attrs(qts, TEST_ADDR_ARM + 0x400,
+ rbuf, sizeof(rbuf), "space=root");
+ for (i = 0; i < sizeof(rbuf); i++) {
+ g_assert_cmpuint(rbuf[i], ==, 0xa5);
+ }
+
+ qtest_memset_attrs(qts, TEST_ADDR_ARM + 0x500,
+ 0x5a, sizeof(rbuf), "space=non-secure");
+ qtest_memread_attrs(qts, TEST_ADDR_ARM + 0x500,
+ rbuf, sizeof(rbuf), NULL);
+ for (i = 0; i < sizeof(rbuf); i++) {
+ g_assert_cmpuint(rbuf[i], ==, 0x5a);
+ }
+
+ qtest_quit(qts);
+}
+
+static void test_arm_single_shortcuts_attrs(void)
+{
+ uint8_t val;
+ uint8_t wbuf[4] = { 0x10, 0x20, 0x30, 0x40 };
+ uint8_t rbuf[4];
+
+ if (!qtest_has_machine("virt")) {
+ g_test_skip("virt machine not available");
+ return;
+ }
+
+ qtest_start(ARM_MACHINE_ARGS);
+
+ writeb_attrs(TEST_ADDR_ARM + 0x600, 0x5a, "secure");
+ val = readb_attrs(TEST_ADDR_ARM + 0x600, "secure");
+ g_assert_cmpuint(val, ==, 0x5a);
+
+ writel_attrs(TEST_ADDR_ARM + 0x604,
+ 0xa5a5a5a5, "space=realm");
+ g_assert_cmphex(readl_attrs(TEST_ADDR_ARM + 0x604, "space=realm"), ==,
+ 0xa5a5a5a5U);
+
+ memwrite_attrs(TEST_ADDR_ARM + 0x608,
+ wbuf, sizeof(wbuf), "space=non-secure");
+ memread_attrs(TEST_ADDR_ARM + 0x608,
+ rbuf, sizeof(rbuf), NULL);
+ g_assert(memcmp(wbuf, rbuf, sizeof(wbuf)) == 0);
+
+ qtest_end();
+}
+
+static void test_x86_scalar_attrs(void)
+{
+ QTestState *qts;
+ uint8_t val;
+
+ if (!qtest_has_machine("pc")) {
+ g_test_skip("pc machine not available");
+ return;
+ }
+
+ qts = qtest_init(X86_MACHINE_ARGS);
+
+ qtest_writeb_attrs(qts, TEST_ADDR_X86, 0x11, NULL);
+ val = qtest_readb_attrs(qts, TEST_ADDR_X86, NULL);
+ g_assert_cmpuint(val, ==, 0x11);
+
+ qtest_writeb_attrs(qts, TEST_ADDR_X86 + 0x1, 0xaa, "secure");
+ val = qtest_readb_attrs(qts, TEST_ADDR_X86 + 0x1, "secure");
+ g_assert_cmpuint(val, ==, 0xaa);
+
+ qtest_quit(qts);
+}
+
+static void test_x86_bulk_attrs(void)
+{
+ QTestState *qts;
+ uint8_t wbuf[8] = { 1, 2, 3, 4, 5, 6, 7, 8 };
+ uint8_t rbuf[8];
+ size_t i;
+
+ if (!qtest_has_machine("pc")) {
+ g_test_skip("pc machine not available");
+ return;
+ }
+
+ qts = qtest_init(X86_MACHINE_ARGS);
+
+ qtest_memwrite_attrs(qts, TEST_ADDR_X86 + 0x100, wbuf, sizeof(wbuf), NULL);
+ qtest_memread_attrs(qts, TEST_ADDR_X86 + 0x100, rbuf, sizeof(rbuf), NULL);
+ g_assert(memcmp(wbuf, rbuf, sizeof(wbuf)) == 0);
+
+ qtest_memwrite_attrs(qts, TEST_ADDR_X86 + 0x180,
+ wbuf, sizeof(wbuf), "secure");
+ qtest_memread_attrs(qts, TEST_ADDR_X86 + 0x180,
+ rbuf, sizeof(rbuf), "secure");
+ g_assert(memcmp(wbuf, rbuf, sizeof(wbuf)) == 0);
+
+ qtest_memset_attrs(qts, TEST_ADDR_X86 + 0x200,
+ 0x3c, sizeof(rbuf), "secure");
+ qtest_memread_attrs(qts, TEST_ADDR_X86 + 0x200,
+ rbuf, sizeof(rbuf), "secure");
+ for (i = 0; i < sizeof(rbuf); i++) {
+ g_assert_cmpuint(rbuf[i], ==, 0x3c);
+ }
+
+ qtest_quit(qts);
+}
+
+int main(int argc, char **argv)
+{
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_add_func("/qtest/arm/attrs/scalar", test_arm_scalar_attrs);
+ qtest_add_func("/qtest/arm/attrs/bulk", test_arm_bulk_attrs);
+ qtest_add_func("/qtest/arm/attrs/single_shortcuts",
+ test_arm_single_shortcuts_attrs);
+
+ qtest_add_func("/qtest/x86/attrs/scalar", test_x86_scalar_attrs);
+ qtest_add_func("/qtest/x86/attrs/bulk", test_x86_bulk_attrs);
+
+ return g_test_run();
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC v2 1/2] tests/qtest: Add attrs argument support to memory access commands
2026-03-11 15:49 ` [RFC v2 1/2] tests/qtest: Add attrs argument support to memory access commands Tao Tang
@ 2026-03-12 19:15 ` Peter Maydell
2026-03-14 7:34 ` Tao Tang
0 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2026-03-12 19:15 UTC (permalink / raw)
To: Tao Tang
Cc: Fabiano Rosas, Laurent Vivier, Paolo Bonzini, qemu-devel,
qemu-arm, Chen Baozi, Pierrick Bouvier, Chao Liu
On Wed, 11 Mar 2026 at 15:50, Tao Tang <tangtao1634@phytium.com.cn> wrote:
>
> Extend qtest memory access commands to accept an optional attrs argument.
>
> Supported attrs:
> - secure (ARM/x86-only, lowercase)
> - space=non-secure|secure|root|realm (ARM-only, lowercase)
>
> For memory commands, parse attrs and select AddressSpace via
> cpu_asidx_from_attrs(), then issue accesses with the corresponding
> MemTxAttrs.
>
> Expose matching libqtest APIs:
> - qtest_{read,write}{b,w,l,q}_attrs()
> - qtest_mem{read,write,set}_attrs()
>
> Also add libqtest-single wrappers for the *_attrs helpers.
>
> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
> ---
> system/qtest.c | 266 ++++++++++++++++++++++++++++------
> tests/qtest/libqtest-single.h | 61 ++++++++
> tests/qtest/libqtest.c | 148 +++++++++++++++++++
> tests/qtest/libqtest.h | 25 ++++
> 4 files changed, 453 insertions(+), 47 deletions(-)
This is a pretty large patch. Please could you split it in two?
(1) add support for the new commands to system/qtest.c
(2) add support to libqtest for sending the new commands
> diff --git a/system/qtest.c b/system/qtest.c
> index cf90cd53ad..dbf3a63c57 100644
> --- a/system/qtest.c
> +++ b/system/qtest.c
> @@ -22,6 +22,7 @@
> #include "hw/core/qdev.h"
> #include "hw/core/irq.h"
> #include "hw/core/cpu.h"
> +#include "hw/arm/arm-security.h"
> #include "qemu/accel.h"
> #include "system/cpu-timers.h"
> #include "qemu/config-file.h"
> @@ -218,6 +219,39 @@ static void *qtest_server_send_opaque;
> * B64_DATA is an arbitrarily long base64 encoded string.
> * If the sizes do not match, the data will be truncated.
> *
> + * Memory access with MemTxAttrs:
> + * """"""""""""""""""""""""""""""
> + *
> + * The following commands allow specifying memory transaction attributes,
> + * which is useful for testing devices that behave differently based on
> + * security state (e.g., ARM TrustZone/CCA or System Management Mode in x86).
A small nit, but "Arm" is not all-caps these days.
> + *
> + * The memory access commands above support one optional ATTRS argument:
> + *
> + * .. code-block:: none
> + *
> + * > writeb ADDR VALUE
> + * < OK
> + * > writeb ADDR VALUE secure
> + * < OK
> + * > writeb ADDR VALUE space=realm
> + * < OK
> + * > readb ADDR secure
> + * < OK VALUE
> + * > readb ADDR space=root
> + * < OK VALUE
> + * > read ADDR SIZE space=secure
> + * < OK DATA
> + * > write ADDR SIZE DATA secure
> + * < OK
> + * > memset ADDR SIZE VALUE space=non-secure
> + * < OK
> + *
> + * ``secure`` sets MemTxAttrs.secure=1 (x86/ARM).
> + * ``space=...`` is ARM-specific and accepts:
> + * non-secure, secure, root, realm.
> + * ``space=non-secure`` is equivalent to omitting ATTRS.
If the test specifically asks for the non-secure space, this is
not the same as "doesn't specify attributes".
> + *
> * IRQ management:
> * """""""""""""""
> *
> @@ -353,6 +387,135 @@ static void qtest_install_gpio_out_intercept(DeviceState *dev, const char *name,
> *disconnected = qdev_intercept_gpio_out(dev, icpt, name, n);
> }
>
> +static bool qtest_parse_mem_attrs(CharFrontend *chr, const char *arg,
> + MemTxAttrs *attrs)
> +{
> + if (!arg) {
> + *attrs = MEMTXATTRS_UNSPECIFIED;
> + return true;
> + }
> +
> + if (strcmp(arg, "secure") == 0) {
> + *attrs = (MemTxAttrs){ .secure = 1 };
> + return true;
> + }
> +
> + if (strncmp(arg, "space=", 6) == 0) {
> + const char *space = arg + 6;
> + ARMSecuritySpace sec_space;
> +
> + if (!target_arm() && !target_aarch64()) {
> + qtest_send(chr, "ERR space=<...> is ARM-specific\n");
> + return false;
> + }
> +
> + if (strcmp(space, "non-secure") == 0) {
> + *attrs = MEMTXATTRS_UNSPECIFIED;
> + return true;
Here we should set sec_space = ARMSS_NonSecure and determine
the attributes the same way as for the other spaces.
> + } else if (strcmp(space, "secure") == 0) {
> + sec_space = ARMSS_Secure;
> + } else if (strcmp(space, "root") == 0) {
> + sec_space = ARMSS_Root;
> + } else if (strcmp(space, "realm") == 0) {
> + sec_space = ARMSS_Realm;
> + } else {
> + qtest_send(chr, "ERR invalid space value. Valid space: "
> + "secure/non-secure/root/realm\n");
> + return false;
> + }
> +
> + *attrs = (MemTxAttrs){
> + .space = sec_space,
> + .secure = arm_space_is_secure(sec_space),
> + };
> + return true;
> + }
> +
> + qtest_send(chr, "ERR invalid attrs argument\n");
> + return false;
> +}
Other than that the system/qtest.c changes look good to me.
> diff --git a/tests/qtest/libqtest-single.h b/tests/qtest/libqtest-single.h
> index 851724cbcb..38d05ee61e 100644
> --- a/tests/qtest/libqtest-single.h
> +++ b/tests/qtest/libqtest-single.h
> @@ -291,6 +291,67 @@ static inline void memwrite(uint64_t addr, const void *data, size_t size)
> qtest_memwrite(global_qtest, addr, data, size);
> }
>
> +/*
> + * Memory commands with optional attrs argument.
> + */
> +static inline void writeb_attrs(uint64_t addr, uint8_t value, const char *attrs)
> +{
> + qtest_writeb_attrs(global_qtest, addr, value, attrs);
> +}
> +
> +static inline void writew_attrs(uint64_t addr, uint16_t value, const char *attrs)
> +{
> + qtest_writew_attrs(global_qtest, addr, value, attrs);
> +}
All the other functions in this file have their own individual
documentation comments describing them. Your new ones should too.
I wonder if we should have these functions take the attrs
as a MemTxAttrs rather than a string? Not sure.
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 051faf31e1..a548331f7c 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1447,6 +1447,154 @@ void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
> qtest_rsp(s);
> }
>
> +static bool qtest_has_attrs(const char *attrs)
> +{
> + return attrs && attrs[0];
> +}
> +
> +static void qtest_write_attrs(QTestState *s, const char *cmd,
> + uint64_t addr, uint64_t value,
> + const char *attrs)
> +{
> + if (qtest_has_attrs(attrs)) {
> + qtest_sendf(s, "%s 0x%" PRIx64 " 0x%" PRIx64 " %s\n",
> + cmd, addr, value, attrs);
> + } else {
> + qtest_sendf(s, "%s 0x%" PRIx64 " 0x%" PRIx64 "\n", cmd, addr, value);
> + }
> + qtest_rsp(s);
> +}
> +
> +static uint64_t qtest_read_attrs(QTestState *s, const char *cmd,
> + uint64_t addr, const char *attrs)
> +{
> + gchar **args;
> + int ret;
> + uint64_t value;
> +
> + if (qtest_has_attrs(attrs)) {
> + qtest_sendf(s, "%s 0x%" PRIx64 " %s\n", cmd, addr, attrs);
> + } else {
> + qtest_sendf(s, "%s 0x%" PRIx64 "\n", cmd, addr);
> + }
> + args = qtest_rsp_args(s, 2);
> + ret = qemu_strtou64(args[1], NULL, 0, &value);
> + g_assert(!ret);
> + g_strfreev(args);
> +
> + return value;
> +}
> +
> +void qtest_writeb_attrs(QTestState *s, uint64_t addr, uint8_t value,
> + const char *attrs)
> +{
> + qtest_write_attrs(s, "writeb", addr, value, attrs);
> +}
You should also make the existing qtest_write(), qtest_writeb(), etc
go through the new code path, by:
qtest_writeb() calls qtest_writeb_attrs(s, addr, value, NULL)
similarly qtest_writew etc etc
qtest_write() can be deleted as it will have no callers
> +void qtest_memread_attrs(QTestState *s, uint64_t addr, void *data,
> + size_t size, const char *attrs)
> +{
> + uint8_t *ptr = data;
> + gchar **args;
> + size_t i;
> +
> + if (!size) {
> + return;
> + }
> +
> + if (qtest_has_attrs(attrs)) {
> + qtest_sendf(s, "read 0x%" PRIx64 " 0x%zx %s\n", addr, size, attrs);
> + } else {
> + qtest_sendf(s, "read 0x%" PRIx64 " 0x%zx\n", addr, size);
> + }
> + args = qtest_rsp_args(s, 2);
> +
> + for (i = 0; i < size; i++) {
> + ptr[i] = hex2nib(args[1][2 + (i * 2)]) << 4;
> + ptr[i] |= hex2nib(args[1][2 + (i * 2) + 1]);
> + }
> +
> + g_strfreev(args);
> +}
Similarly, the qtest_memread(), qtest_memwrite(), qtest_memset()
functions should all become wrappers that pass NULL to the
_attrs version of the functions.
> +/*
> + * Memory commands with optional attrs argument.
> + */
> +
> +void qtest_writeb_attrs(QTestState *s, uint64_t addr, uint8_t value,
> + const char *attrs);
> +void qtest_writew_attrs(QTestState *s, uint64_t addr, uint16_t value,
> + const char *attrs);
> +void qtest_writel_attrs(QTestState *s, uint64_t addr, uint32_t value,
> + const char *attrs);
> +void qtest_writeq_attrs(QTestState *s, uint64_t addr, uint64_t value,
> + const char *attrs);
> +
> +uint8_t qtest_readb_attrs(QTestState *s, uint64_t addr, const char *attrs);
> +uint16_t qtest_readw_attrs(QTestState *s, uint64_t addr, const char *attrs);
> +uint32_t qtest_readl_attrs(QTestState *s, uint64_t addr, const char *attrs);
> +uint64_t qtest_readq_attrs(QTestState *s, uint64_t addr, const char *attrs);
> +
> +void qtest_memread_attrs(QTestState *s, uint64_t addr, void *data, size_t size,
> + const char *attrs);
> +void qtest_memwrite_attrs(QTestState *s, uint64_t addr, const void *data,
> + size_t size, const char *attrs);
> +void qtest_memset_attrs(QTestState *s, uint64_t addr, uint8_t patt, size_t size,
> + const char *attrs);
Here also you should give each function its own doc comment,
to match the way the existing functions in this header do it.
> +
> /**
> * qtest_clock_step_next:
> * @s: #QTestState instance to operate on.
> --
> 2.34.1
thanks
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v2 2/2] tests/qtest: Add qtest-attrs-test for memory access with attrs
2026-03-11 15:49 ` [RFC v2 2/2] tests/qtest: Add qtest-attrs-test for memory access with attrs Tao Tang
@ 2026-03-12 19:23 ` Peter Maydell
2026-03-14 13:43 ` Tao Tang
0 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2026-03-12 19:23 UTC (permalink / raw)
To: Tao Tang
Cc: Fabiano Rosas, Laurent Vivier, Paolo Bonzini, qemu-devel,
qemu-arm, Chen Baozi, Pierrick Bouvier, Chao Liu
On Wed, 11 Mar 2026 at 15:50, Tao Tang <tangtao1634@phytium.com.cn> wrote:
>
> Add qtest-attrs-test to exercise qtest memory commands with optional attrs
> on aarch64 (virt, secure=on) and x86 (pc, tcg).
>
> The test covers:
> - ARM (virt machine): exercises all supported ARM security spaces
> (non-secure, secure, root, realm).
> - x86 (pc machine): exercises secure attrs accesses
> (SMM address-space path).
>
> The test also covers libqtest-single *_attrs shortcut wrappers and
> verifies that space=non-secure behaves like omitting attrs.
>
> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
> ---
> tests/qtest/meson.build | 4 +-
> tests/qtest/qtest-attrs-test.c | 234 +++++++++++++++++++++++++++++++++
> 2 files changed, 237 insertions(+), 1 deletion(-)
> create mode 100644 tests/qtest/qtest-attrs-test.c
>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index be4fa627b5..c11759e23f 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -115,6 +115,7 @@ qtests_i386 = \
> 'drive_del-test',
> 'cpu-plug-test',
> 'migration-test',
> + 'qtest-attrs-test',
> ]
>
> if dbus_display and config_all_devices.has_key('CONFIG_VGA')
> @@ -270,7 +271,8 @@ qtests_aarch64 = \
> ['arm-cpu-features',
> 'numa-test',
> 'boot-serial-test',
> - 'migration-test']
> + 'migration-test',
> + 'qtest-attrs-test']
>
> qtests_s390x = \
> qtests_filter + \
> diff --git a/tests/qtest/qtest-attrs-test.c b/tests/qtest/qtest-attrs-test.c
> new file mode 100644
> index 0000000000..ce204c2c95
> --- /dev/null
> +++ b/tests/qtest/qtest-attrs-test.c
> @@ -0,0 +1,234 @@
> +/*
> + * QTest for memory access with transaction attributes
> + *
> + * Verify optional attrs argument support for qtest memory commands.
> + *
> + * Copyright (c) 2026 Phytium Technology
> + *
> + * Author:
> + * Tao Tang <tangtao1634@phytium.com.cn>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +#include "libqtest-single.h"
> +
> +/*
> + * Default RAM size is 128 MiB on both machines used below.
> + * Keep test addresses in low RAM and away from device MMIO regions.
> + */
> +#define TEST_ADDR_OFFSET 0x1000ULL
> +#define TEST_ARM_BASE 0x40000000ULL
> +#define TEST_X86_BASE 0x0ULL
> +
> +#define TEST_ADDR_ARM (TEST_ARM_BASE + TEST_ADDR_OFFSET)
> +#define TEST_ADDR_X86 (TEST_X86_BASE + TEST_ADDR_OFFSET)
> +
> +#define ARM_MACHINE_ARGS "-machine virt,secure=on -cpu cortex-a57"
> +#define X86_MACHINE_ARGS "-machine pc -accel tcg"
You start a virt machine with secure=on, but you are only
testing the RAM, which is mapped regardless of the security
space, so this will not notice any bugs where we don't actually
access with the correct attributes.
It would be more interesting to look at the secure-only
RAM (which is at 0x0e00_0000), which should only be accessible
via the secure or root spaces. (I'm not sure what qtest will
do for attempted access via non-secure and realm, where the
address will have nothing mapped there. You'll have to check.)
I don't know if the x86 PC machine has a similar bit of RAM
or whatever that behaves differently for secure and non-secure
accesses.
thanks
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v2 1/2] tests/qtest: Add attrs argument support to memory access commands
2026-03-12 19:15 ` Peter Maydell
@ 2026-03-14 7:34 ` Tao Tang
0 siblings, 0 replies; 7+ messages in thread
From: Tao Tang @ 2026-03-14 7:34 UTC (permalink / raw)
To: Peter Maydell
Cc: Fabiano Rosas, Laurent Vivier, Paolo Bonzini, qemu-devel,
qemu-arm, Chen Baozi, Pierrick Bouvier, Chao Liu
Hi Peter,
On 2026/3/13 03:15, Peter Maydell wrote:
> On Wed, 11 Mar 2026 at 15:50, Tao Tang <tangtao1634@phytium.com.cn> wrote:
>> Extend qtest memory access commands to accept an optional attrs argument.
>>
>> Supported attrs:
>> - secure (ARM/x86-only, lowercase)
>> - space=non-secure|secure|root|realm (ARM-only, lowercase)
>>
>> For memory commands, parse attrs and select AddressSpace via
>> cpu_asidx_from_attrs(), then issue accesses with the corresponding
>> MemTxAttrs.
>>
>> Expose matching libqtest APIs:
>> - qtest_{read,write}{b,w,l,q}_attrs()
>> - qtest_mem{read,write,set}_attrs()
>>
>> Also add libqtest-single wrappers for the *_attrs helpers.
>>
>> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
>> ---
>> system/qtest.c | 266 ++++++++++++++++++++++++++++------
>> tests/qtest/libqtest-single.h | 61 ++++++++
>> tests/qtest/libqtest.c | 148 +++++++++++++++++++
>> tests/qtest/libqtest.h | 25 ++++
>> 4 files changed, 453 insertions(+), 47 deletions(-)
> This is a pretty large patch. Please could you split it in two?
> (1) add support for the new commands to system/qtest.c
> (2) add support to libqtest for sending the new commands
Thanks for your suggestion.
Sure. I'll split it in V3.
>
>> diff --git a/system/qtest.c b/system/qtest.c
>> index cf90cd53ad..dbf3a63c57 100644
>> --- a/system/qtest.c
>> +++ b/system/qtest.c
>> @@ -22,6 +22,7 @@
>> #include "hw/core/qdev.h"
>> #include "hw/core/irq.h"
>> #include "hw/core/cpu.h"
>> +#include "hw/arm/arm-security.h"
>> #include "qemu/accel.h"
>> #include "system/cpu-timers.h"
>> #include "qemu/config-file.h"
>> @@ -218,6 +219,39 @@ static void *qtest_server_send_opaque;
>> * B64_DATA is an arbitrarily long base64 encoded string.
>> * If the sizes do not match, the data will be truncated.
>> *
>> + * Memory access with MemTxAttrs:
>> + * """"""""""""""""""""""""""""""
>> + *
>> + * The following commands allow specifying memory transaction attributes,
>> + * which is useful for testing devices that behave differently based on
>> + * security state (e.g., ARM TrustZone/CCA or System Management Mode in x86).
> A small nit, but "Arm" is not all-caps these days.
Good catch. I hadn't paid enough attention to this before, including in
some of my earlier series. I'll use the preferred "Arm"
capitalization going forward.
>
>> + *
>> + * The memory access commands above support one optional ATTRS argument:
>> + *
>> + * .. code-block:: none
>> + *
>> + * > writeb ADDR VALUE
>> + * < OK
>> + * > writeb ADDR VALUE secure
>> + * < OK
>> + * > writeb ADDR VALUE space=realm
>> + * < OK
>> + * > readb ADDR secure
>> + * < OK VALUE
>> + * > readb ADDR space=root
>> + * < OK VALUE
>> + * > read ADDR SIZE space=secure
>> + * < OK DATA
>> + * > write ADDR SIZE DATA secure
>> + * < OK
>> + * > memset ADDR SIZE VALUE space=non-secure
>> + * < OK
>> + *
>> + * ``secure`` sets MemTxAttrs.secure=1 (x86/ARM).
>> + * ``space=...`` is ARM-specific and accepts:
>> + * non-secure, secure, root, realm.
>> + * ``space=non-secure`` is equivalent to omitting ATTRS.
> If the test specifically asks for the non-secure space, this is
> not the same as "doesn't specify attributes".
....
>
>> + *
>> * IRQ management:
>> * """""""""""""""
>> *
>> @@ -353,6 +387,135 @@ static void qtest_install_gpio_out_intercept(DeviceState *dev, const char *name,
>> *disconnected = qdev_intercept_gpio_out(dev, icpt, name, n);
>> }
>>
>> +static bool qtest_parse_mem_attrs(CharFrontend *chr, const char *arg,
>> + MemTxAttrs *attrs)
>> +{
>> + if (!arg) {
>> + *attrs = MEMTXATTRS_UNSPECIFIED;
>> + return true;
>> + }
>> +
>> + if (strcmp(arg, "secure") == 0) {
>> + *attrs = (MemTxAttrs){ .secure = 1 };
>> + return true;
>> + }
>> +
>> + if (strncmp(arg, "space=", 6) == 0) {
>> + const char *space = arg + 6;
>> + ARMSecuritySpace sec_space;
>> +
>> + if (!target_arm() && !target_aarch64()) {
>> + qtest_send(chr, "ERR space=<...> is ARM-specific\n");
>> + return false;
>> + }
>> +
>> + if (strcmp(space, "non-secure") == 0) {
>> + *attrs = MEMTXATTRS_UNSPECIFIED;
>> + return true;
> Here we should set sec_space = ARMSS_NonSecure and determine
> the attributes the same way as for the other spaces.
I'll fix it so it is handled as an explicit Arm non-secure space.
>
>> + } else if (strcmp(space, "secure") == 0) {
>> + sec_space = ARMSS_Secure;
>> + } else if (strcmp(space, "root") == 0) {
>> + sec_space = ARMSS_Root;
>> + } else if (strcmp(space, "realm") == 0) {
>> + sec_space = ARMSS_Realm;
>> + } else {
>> + qtest_send(chr, "ERR invalid space value. Valid space: "
>> + "secure/non-secure/root/realm\n");
>> + return false;
>> + }
>> +
>> + *attrs = (MemTxAttrs){
>> + .space = sec_space,
>> + .secure = arm_space_is_secure(sec_space),
>> + };
>> + return true;
>> + }
>> +
>> + qtest_send(chr, "ERR invalid attrs argument\n");
>> + return false;
>> +}
> Other than that the system/qtest.c changes look good to me.
>
>> diff --git a/tests/qtest/libqtest-single.h b/tests/qtest/libqtest-single.h
>> index 851724cbcb..38d05ee61e 100644
>> --- a/tests/qtest/libqtest-single.h
>> +++ b/tests/qtest/libqtest-single.h
>> @@ -291,6 +291,67 @@ static inline void memwrite(uint64_t addr, const void *data, size_t size)
>> qtest_memwrite(global_qtest, addr, data, size);
>> }
>>
>> +/*
>> + * Memory commands with optional attrs argument.
>> + */
>> +static inline void writeb_attrs(uint64_t addr, uint8_t value, const char *attrs)
>> +{
>> + qtest_writeb_attrs(global_qtest, addr, value, attrs);
>> +}
>> +
>> +static inline void writew_attrs(uint64_t addr, uint16_t value, const char *attrs)
>> +{
>> + qtest_writew_attrs(global_qtest, addr, value, attrs);
>> +}
> All the other functions in this file have their own individual
> documentation comments describing them. Your new ones should too.
I'll add docs in v5.
>
> I wonder if we should have these functions take the attrs
> as a MemTxAttrs rather than a string? Not sure.
Just to make sure I understood your suggestion correctly: do you mean
that the qtest wire protocol would remain text-based, but the libqtest C
APIs would take a MemTxAttrs rather than a string, with libqtest doing
the conversion internally?
If so, that makes sense to me as an API-shape question, even if the wire
protocol itself still uses the textual attrs form.
>
>> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
>> index 051faf31e1..a548331f7c 100644
>> --- a/tests/qtest/libqtest.c
>> +++ b/tests/qtest/libqtest.c
>> @@ -1447,6 +1447,154 @@ void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
>> qtest_rsp(s);
>> }
>>
>> +static bool qtest_has_attrs(const char *attrs)
>> +{
>> + return attrs && attrs[0];
>> +}
>> +
>> +static void qtest_write_attrs(QTestState *s, const char *cmd,
>> + uint64_t addr, uint64_t value,
>> + const char *attrs)
>> +{
>> + if (qtest_has_attrs(attrs)) {
>> + qtest_sendf(s, "%s 0x%" PRIx64 " 0x%" PRIx64 " %s\n",
>> + cmd, addr, value, attrs);
>> + } else {
>> + qtest_sendf(s, "%s 0x%" PRIx64 " 0x%" PRIx64 "\n", cmd, addr, value);
>> + }
>> + qtest_rsp(s);
>> +}
>> +
>> +static uint64_t qtest_read_attrs(QTestState *s, const char *cmd,
>> + uint64_t addr, const char *attrs)
>> +{
>> + gchar **args;
>> + int ret;
>> + uint64_t value;
>> +
>> + if (qtest_has_attrs(attrs)) {
>> + qtest_sendf(s, "%s 0x%" PRIx64 " %s\n", cmd, addr, attrs);
>> + } else {
>> + qtest_sendf(s, "%s 0x%" PRIx64 "\n", cmd, addr);
>> + }
>> + args = qtest_rsp_args(s, 2);
>> + ret = qemu_strtou64(args[1], NULL, 0, &value);
>> + g_assert(!ret);
>> + g_strfreev(args);
>> +
>> + return value;
>> +}
>> +
>> +void qtest_writeb_attrs(QTestState *s, uint64_t addr, uint8_t value,
>> + const char *attrs)
>> +{
>> + qtest_write_attrs(s, "writeb", addr, value, attrs);
>> +}
> You should also make the existing qtest_write(), qtest_writeb(), etc
> go through the new code path, by:
> qtest_writeb() calls qtest_writeb_attrs(s, addr, value, NULL)
> similarly qtest_writew etc etc
> qtest_write() can be deleted as it will have no callers
.....
>
>> +void qtest_memread_attrs(QTestState *s, uint64_t addr, void *data,
>> + size_t size, const char *attrs)
>> +{
>> + uint8_t *ptr = data;
>> + gchar **args;
>> + size_t i;
>> +
>> + if (!size) {
>> + return;
>> + }
>> +
>> + if (qtest_has_attrs(attrs)) {
>> + qtest_sendf(s, "read 0x%" PRIx64 " 0x%zx %s\n", addr, size, attrs);
>> + } else {
>> + qtest_sendf(s, "read 0x%" PRIx64 " 0x%zx\n", addr, size);
>> + }
>> + args = qtest_rsp_args(s, 2);
>> +
>> + for (i = 0; i < size; i++) {
>> + ptr[i] = hex2nib(args[1][2 + (i * 2)]) << 4;
>> + ptr[i] |= hex2nib(args[1][2 + (i * 2) + 1]);
>> + }
>> +
>> + g_strfreev(args);
>> +}
> Similarly, the qtest_memread(), qtest_memwrite(), qtest_memset()
> functions should all become wrappers that pass NULL to the
> _attrs version of the functions.
Makes sense. I'll route the existing
qtest_write*()/qtest_read*()/qtest_mem* helpers through the new
*_attrs(..., NULL) path.
>
>> +/*
>> + * Memory commands with optional attrs argument.
>> + */
>> +
>> +void qtest_writeb_attrs(QTestState *s, uint64_t addr, uint8_t value,
>> + const char *attrs);
>> +void qtest_writew_attrs(QTestState *s, uint64_t addr, uint16_t value,
>> + const char *attrs);
>> +void qtest_writel_attrs(QTestState *s, uint64_t addr, uint32_t value,
>> + const char *attrs);
>> +void qtest_writeq_attrs(QTestState *s, uint64_t addr, uint64_t value,
>> + const char *attrs);
>> +
>> +uint8_t qtest_readb_attrs(QTestState *s, uint64_t addr, const char *attrs);
>> +uint16_t qtest_readw_attrs(QTestState *s, uint64_t addr, const char *attrs);
>> +uint32_t qtest_readl_attrs(QTestState *s, uint64_t addr, const char *attrs);
>> +uint64_t qtest_readq_attrs(QTestState *s, uint64_t addr, const char *attrs);
>> +
>> +void qtest_memread_attrs(QTestState *s, uint64_t addr, void *data, size_t size,
>> + const char *attrs);
>> +void qtest_memwrite_attrs(QTestState *s, uint64_t addr, const void *data,
>> + size_t size, const char *attrs);
>> +void qtest_memset_attrs(QTestState *s, uint64_t addr, uint8_t patt, size_t size,
>> + const char *attrs);
> Here also you should give each function its own doc comment,
> to match the way the existing functions in this header do it.
OK.
>
>> +
>> /**
>> * qtest_clock_step_next:
>> * @s: #QTestState instance to operate on.
>> --
>> 2.34.1
> thanks
> -- PMM
Thanks for the review!
Tao
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v2 2/2] tests/qtest: Add qtest-attrs-test for memory access with attrs
2026-03-12 19:23 ` Peter Maydell
@ 2026-03-14 13:43 ` Tao Tang
0 siblings, 0 replies; 7+ messages in thread
From: Tao Tang @ 2026-03-14 13:43 UTC (permalink / raw)
To: Peter Maydell
Cc: Fabiano Rosas, Laurent Vivier, Paolo Bonzini, qemu-devel,
qemu-arm, Chen Baozi, Pierrick Bouvier, Chao Liu
Hi Peter,
On 3/13/2026 3:23 AM, Peter Maydell wrote:
> On Wed, 11 Mar 2026 at 15:50, Tao Tang <tangtao1634@phytium.com.cn> wrote:
>> Add qtest-attrs-test to exercise qtest memory commands with optional attrs
>> on aarch64 (virt, secure=on) and x86 (pc, tcg).
>>
>> The test covers:
>> - ARM (virt machine): exercises all supported ARM security spaces
>> (non-secure, secure, root, realm).
>> - x86 (pc machine): exercises secure attrs accesses
>> (SMM address-space path).
>>
>> The test also covers libqtest-single *_attrs shortcut wrappers and
>> verifies that space=non-secure behaves like omitting attrs.
>>
>> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
>> ---
>> tests/qtest/meson.build | 4 +-
>> tests/qtest/qtest-attrs-test.c | 234 +++++++++++++++++++++++++++++++++
>> 2 files changed, 237 insertions(+), 1 deletion(-)
>> create mode 100644 tests/qtest/qtest-attrs-test.c
>>
>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
>> index be4fa627b5..c11759e23f 100644
>> --- a/tests/qtest/meson.build
>> +++ b/tests/qtest/meson.build
>> @@ -115,6 +115,7 @@ qtests_i386 = \
>> 'drive_del-test',
>> 'cpu-plug-test',
>> 'migration-test',
>> + 'qtest-attrs-test',
>> ]
>>
>> if dbus_display and config_all_devices.has_key('CONFIG_VGA')
>> @@ -270,7 +271,8 @@ qtests_aarch64 = \
>> ['arm-cpu-features',
>> 'numa-test',
>> 'boot-serial-test',
>> - 'migration-test']
>> + 'migration-test',
>> + 'qtest-attrs-test']
>>
>> qtests_s390x = \
>> qtests_filter + \
>> diff --git a/tests/qtest/qtest-attrs-test.c b/tests/qtest/qtest-attrs-test.c
>> new file mode 100644
>> index 0000000000..ce204c2c95
>> --- /dev/null
>> +++ b/tests/qtest/qtest-attrs-test.c
>> @@ -0,0 +1,234 @@
>> +/*
>> + * QTest for memory access with transaction attributes
>> + *
>> + * Verify optional attrs argument support for qtest memory commands.
>> + *
>> + * Copyright (c) 2026 Phytium Technology
>> + *
>> + * Author:
>> + * Tao Tang <tangtao1634@phytium.com.cn>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "libqtest.h"
>> +#include "libqtest-single.h"
>> +
>> +/*
>> + * Default RAM size is 128 MiB on both machines used below.
>> + * Keep test addresses in low RAM and away from device MMIO regions.
>> + */
>> +#define TEST_ADDR_OFFSET 0x1000ULL
>> +#define TEST_ARM_BASE 0x40000000ULL
>> +#define TEST_X86_BASE 0x0ULL
>> +
>> +#define TEST_ADDR_ARM (TEST_ARM_BASE + TEST_ADDR_OFFSET)
>> +#define TEST_ADDR_X86 (TEST_X86_BASE + TEST_ADDR_OFFSET)
>> +
>> +#define ARM_MACHINE_ARGS "-machine virt,secure=on -cpu cortex-a57"
>> +#define X86_MACHINE_ARGS "-machine pc -accel tcg"
> You start a virt machine with secure=on, but you are only
> testing the RAM, which is mapped regardless of the security
> space, so this will not notice any bugs where we don't actually
> access with the correct attributes.
>
> It would be more interesting to look at the secure-only
> RAM (which is at 0x0e00_0000), which should only be accessible
> via the secure or root spaces. (I'm not sure what qtest will
> do for attempted access via non-secure and realm, where the
> address will have nothing mapped there. You'll have to check.)
I just tested the secure-only RAM path as you suggested. For accesses
from the non-secure side, I now see:
"Invalid read/write at addr 0xE000000, size 1, region '(null)',
reason: rejected"
which is consistent with the secure-only RAM not being mapped in the
non-secure address space. So I agree that this is a much better target
for validating whether the requested attrs are actually being used.
While testing this, I also found another bug in the qtest implementation:
qtest_read/write_sized() currently ignore the MemTxResult returned by
address_space_read/write, so access failures are effectively handled
silently. That is not correct. For failed reads/writes, qtest should
return a non-OK response so the failure can be asserted properly in
qtest_rsp_args() instead of being treated as success.
I'll fix this in v3 as well.
>
> I don't know if the x86 PC machine has a similar bit of RAM
> or whatever that behaves differently for secure and non-secure
> accesses.
Unlike the Arm virt case, where secure=on gives a very clear secure-only
RAM target, the x86 side seems less straightforward to me too.
From the current x86 code, it looks like the normal and SMM address
spaces are both created ( tcg_cpu_realizefn in
target/i386/tcg/system/tcg-cpu.c ), with the SMM view built by
overlaying /machine/smram on top of the normal memory
view(tcg_cpu_machine_done in target/i386/tcg/system/tcg-cpu.c).
I also looked into the x86 code to see whether there is an obvious
SMM-specific MemoryRegion that would make a good test target, and I
found some SMM-related logic in hw/pci-host/q35.c ( such as function
mch_* ), but that path is complex enough that I am not confident I fully
understand the intended testing model.
Would it make sense to ask x86 experts what the best way is to test this
on the PC machine?
>
> thanks
> -- PMM
Thanks for the review,
Tao
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-03-14 13:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-11 15:49 [RFC v2 0/2] tests/qtest: Add memory-access attributes (secure/space) Tao Tang
2026-03-11 15:49 ` [RFC v2 1/2] tests/qtest: Add attrs argument support to memory access commands Tao Tang
2026-03-12 19:15 ` Peter Maydell
2026-03-14 7:34 ` Tao Tang
2026-03-11 15:49 ` [RFC v2 2/2] tests/qtest: Add qtest-attrs-test for memory access with attrs Tao Tang
2026-03-12 19:23 ` Peter Maydell
2026-03-14 13:43 ` Tao Tang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox