From c6445544d4cea2628fbad3bad09f3d3a03c749d3 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 25 Jul 2023 10:56:51 +0100 Subject: hw/arm/smmu: Handle big-endian hosts correctly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The implementation of the SMMUv3 has multiple places where it reads a data structure from the guest and directly operates on it without doing a guest-to-host endianness conversion. Since all SMMU data structures are little-endian, this means that the SMMU doesn't work on a big-endian host. In particular, this causes the Avocado test machine_aarch64_virt.py:Aarch64VirtMachine.test_alpine_virt_tcg_gic_max to fail on an s390x host. Add appropriate byte-swapping on reads and writes of guest in-memory data structures so that the device works correctly on big-endian hosts. As part of this we constrain queue_read() to operate only on Cmd structs and queue_write() on Evt structs, because in practice these are the only data structures the two functions are used with, and we need to know what the data structure is to be able to byte-swap its parts correctly. Signed-off-by: Peter Maydell Tested-by: Thomas Huth Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Eric Auger Message-id: 20230717132641.764660-1-peter.maydell@linaro.org Cc: qemu-stable@nongnu.org --- hw/arm/smmu-common.c | 3 +-- hw/arm/smmuv3.c | 39 +++++++++++++++++++++++++++++++-------- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index 5ab9d45..f35ae9a 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -216,8 +216,7 @@ static int get_pte(dma_addr_t baseaddr, uint32_t index, uint64_t *pte, dma_addr_t addr = baseaddr + index * sizeof(*pte); /* TODO: guarantee 64-bit single-copy atomicity */ - ret = dma_memory_read(&address_space_memory, addr, pte, sizeof(*pte), - MEMTXATTRS_UNSPECIFIED); + ret = ldq_le_dma(&address_space_memory, addr, pte, MEMTXATTRS_UNSPECIFIED); if (ret != MEMTX_OK) { info->type = SMMU_PTW_ERR_WALK_EABT; diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index 932f009..1e9be8e 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -102,20 +102,34 @@ static void smmuv3_write_gerrorn(SMMUv3State *s, uint32_t new_gerrorn) trace_smmuv3_write_gerrorn(toggled & pending, s->gerrorn); } -static inline MemTxResult queue_read(SMMUQueue *q, void *data) +static inline MemTxResult queue_read(SMMUQueue *q, Cmd *cmd) { dma_addr_t addr = Q_CONS_ENTRY(q); + MemTxResult ret; + int i; - return dma_memory_read(&address_space_memory, addr, data, q->entry_size, - MEMTXATTRS_UNSPECIFIED); + ret = dma_memory_read(&address_space_memory, addr, cmd, sizeof(Cmd), + MEMTXATTRS_UNSPECIFIED); + if (ret != MEMTX_OK) { + return ret; + } + for (i = 0; i < ARRAY_SIZE(cmd->word); i++) { + le32_to_cpus(&cmd->word[i]); + } + return ret; } -static MemTxResult queue_write(SMMUQueue *q, void *data) +static MemTxResult queue_write(SMMUQueue *q, Evt *evt_in) { dma_addr_t addr = Q_PROD_ENTRY(q); MemTxResult ret; + Evt evt = *evt_in; + int i; - ret = dma_memory_write(&address_space_memory, addr, data, q->entry_size, + for (i = 0; i < ARRAY_SIZE(evt.word); i++) { + cpu_to_le32s(&evt.word[i]); + } + ret = dma_memory_write(&address_space_memory, addr, &evt, sizeof(Evt), MEMTXATTRS_UNSPECIFIED); if (ret != MEMTX_OK) { return ret; @@ -298,7 +312,7 @@ static void smmuv3_init_regs(SMMUv3State *s) static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf, SMMUEventInfo *event) { - int ret; + int ret, i; trace_smmuv3_get_ste(addr); /* TODO: guarantee 64-bit single-copy atomicity */ @@ -311,6 +325,9 @@ static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf, event->u.f_ste_fetch.addr = addr; return -EINVAL; } + for (i = 0; i < ARRAY_SIZE(buf->word); i++) { + le32_to_cpus(&buf->word[i]); + } return 0; } @@ -320,7 +337,7 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid, CD *buf, SMMUEventInfo *event) { dma_addr_t addr = STE_CTXPTR(ste); - int ret; + int ret, i; trace_smmuv3_get_cd(addr); /* TODO: guarantee 64-bit single-copy atomicity */ @@ -333,6 +350,9 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid, event->u.f_ste_fetch.addr = addr; return -EINVAL; } + for (i = 0; i < ARRAY_SIZE(buf->word); i++) { + le32_to_cpus(&buf->word[i]); + } return 0; } @@ -569,7 +589,7 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, return -EINVAL; } if (s->features & SMMU_FEATURE_2LVL_STE) { - int l1_ste_offset, l2_ste_offset, max_l2_ste, span; + int l1_ste_offset, l2_ste_offset, max_l2_ste, span, i; dma_addr_t l1ptr, l2ptr; STEDesc l1std; @@ -593,6 +613,9 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, event->u.f_ste_fetch.addr = l1ptr; return -EINVAL; } + for (i = 0; i < ARRAY_SIZE(l1std.word); i++) { + le32_to_cpus(&l1std.word[i]); + } span = L1STD_SPAN(&l1std); -- cgit v1.1 From f9540bb1b2d35d2b0f7e831208c5f44854896cd0 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 25 Jul 2023 10:56:51 +0100 Subject: scripts/git-submodule.sh: Don't rely on non-POSIX 'read' behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The POSIX definition of the 'read' utility requires that you specify the variable name to set; omitting the name and having it default to 'REPLY' is a bashism. If your system sh is dash, then it will print an error message during build: qemu/pc-bios/s390-ccw/../../scripts/git-submodule.sh: 106: read: arg count Specify the variable name explicitly. Fixes: fdb8fd8cb915647b ("git-submodule: allow partial update of .git-submodule-status") Signed-off-by: Peter Maydell Reviewed-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Message-id: 20230720153038.1587196-1-peter.maydell@linaro.org --- scripts/git-submodule.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh index 335f7f5..bb1222c 100755 --- a/scripts/git-submodule.sh +++ b/scripts/git-submodule.sh @@ -103,7 +103,7 @@ update) check_updated $module || echo Updated "$module" done - (while read -r; do + (while read -r REPLY; do for module in $modules; do case $REPLY in *" $module "*) continue 2 ;; -- cgit v1.1 From 5d78893f39caf94c8587141e2219b57a7d63dd5c Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 25 Jul 2023 10:56:51 +0100 Subject: target/arm: Special case M-profile in debug_helper.c code A lot of the code called from helper_exception_bkpt_insn() is written assuming A-profile, but we will also call this helper on M-profile CPUs when they execute a BKPT insn. This used to work by accident, but recent changes mean that we will hit an assert when some of this code calls down into lower level functions that end up calling arm_security_space_below_el3(), arm_el_is_aa64(), and other functions that now explicitly assert that the guest CPU is not M-profile. Handle M-profile directly to avoid the assertions: * in arm_debug_target_el(), M-profile debug exceptions always go to EL1 * in arm_debug_exception_fsr(), M-profile always uses the short format FSR (compare commit d7fe699be54b2, though in this case the code in arm_v7m_cpu_do_interrupt() does not need to look at the FSR value at all) Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1775 Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20230721143239.1753066-1-peter.maydell@linaro.org --- target/arm/debug_helper.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index 8362462..abe72e3 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -21,6 +21,10 @@ static int arm_debug_target_el(CPUARMState *env) bool secure = arm_is_secure(env); bool route_to_el2 = false; + if (arm_feature(env, ARM_FEATURE_M)) { + return 1; + } + if (arm_is_el2_enabled(env)) { route_to_el2 = env->cp15.hcr_el2 & HCR_TGE || env->cp15.mdcr_el2 & MDCR_TDE; @@ -434,18 +438,20 @@ static uint32_t arm_debug_exception_fsr(CPUARMState *env) { ARMMMUFaultInfo fi = { .type = ARMFault_Debug }; int target_el = arm_debug_target_el(env); - bool using_lpae = false; + bool using_lpae; - if (target_el == 2 || arm_el_is_aa64(env, target_el)) { + if (arm_feature(env, ARM_FEATURE_M)) { + using_lpae = false; + } else if (target_el == 2 || arm_el_is_aa64(env, target_el)) { using_lpae = true; } else if (arm_feature(env, ARM_FEATURE_PMSA) && arm_feature(env, ARM_FEATURE_V8)) { using_lpae = true; + } else if (arm_feature(env, ARM_FEATURE_LPAE) && + (env->cp15.tcr_el[target_el] & TTBCR_EAE)) { + using_lpae = true; } else { - if (arm_feature(env, ARM_FEATURE_LPAE) && - (env->cp15.tcr_el[target_el] & TTBCR_EAE)) { - using_lpae = true; - } + using_lpae = false; } if (using_lpae) { -- cgit v1.1 From 9b579543d7b01666218d8ae01561a96757678eb1 Mon Sep 17 00:00:00 2001 From: Sean Estabrooks Date: Tue, 25 Jul 2023 10:56:51 +0100 Subject: For curses display, recognize a few more control keys The curses display handles most control-X keys, and translates them into their corresponding keycode. Here we recognize a few that are missing, Ctrl-@ (null), Ctrl-\ (backslash), Ctrl-] (right bracket), Ctrl-^ (caret), Ctrl-_ (underscore). Signed-off-by: Sean Estabrooks Message-id: CAHyVn3Bh9CRgDuOmf7G7Ngwamu8d4cVozAcB2i4ymnnggBXNmg@mail.gmail.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- ui/curses_keys.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ui/curses_keys.h b/ui/curses_keys.h index 71e04ac..88a2208 100644 --- a/ui/curses_keys.h +++ b/ui/curses_keys.h @@ -210,6 +210,12 @@ static const int _curses2keycode[CURSES_CHARS] = { ['N' - '@'] = 49 | CNTRL, /* Control + n */ /* Control + m collides with the keycode for Enter */ + ['@' - '@'] = 3 | CNTRL, /* Control + @ */ + /* Control + [ collides with the keycode for Escape */ + ['\\' - '@'] = 43 | CNTRL, /* Control + Backslash */ + [']' - '@'] = 27 | CNTRL, /* Control + ] */ + ['^' - '@'] = 7 | CNTRL, /* Control + ^ */ + ['_' - '@'] = 12 | CNTRL, /* Control + Underscore */ }; static const int _curseskey2keycode[CURSES_KEYS] = { -- cgit v1.1 From 78cc90346ec680a7f1bb9f138bf7c9654cf526d5 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 25 Jul 2023 10:56:52 +0100 Subject: tests/decode: Suppress "error: " string for expected-failure tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "expected failure" tests for decodetree result in the error messages from decodetree ending up in logs and in V=1 output: >>> MALLOC_PERTURB_=226 /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/pyvenv/bin/python3 /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/scripts/decodetree.py --output-null --test-for-error /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/../../tests/decode/err_argset1.decode ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀ ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86/../../tests/decode/err_argset1.decode:5: error: duplicate argument "a" ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― 1/44 qemu:decodetree / err_argset1 OK 0.05s This then produces false positives when scanning the logfiles for strings like "error: ". For the expected-failure tests, make decodetree print "detected:" instead of "error:". Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-id: 20230720131521.1325905-1-peter.maydell@linaro.org --- scripts/decodetree.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/decodetree.py b/scripts/decodetree.py index a8a6cb6..e8b72da 100644 --- a/scripts/decodetree.py +++ b/scripts/decodetree.py @@ -134,6 +134,10 @@ def error_with_file(file, lineno, *args): global output_file global output_fd + # For the test suite expected-errors case, don't print the + # string "error: ", so they don't turn up as false positives + # if you grep the meson logs for strings like that. + end = 'error: ' if not testforerror else 'detected: ' prefix = '' if file: prefix += f'{file}:' @@ -141,7 +145,7 @@ def error_with_file(file, lineno, *args): prefix += f'{lineno}:' if prefix: prefix += ' ' - print(prefix, end='error: ', file=sys.stderr) + print(prefix, end=end, file=sys.stderr) print(*args, file=sys.stderr) if output_file and output_fd: -- cgit v1.1