* [PATCH v2 0/2] KVM: arm64: Fix and test MMIO sign-extending loads
@ 2026-06-25 14:48 Fuad Tabba
2026-06-25 14:48 ` [PATCH v2 1/2] KVM: arm64: Fix sign-extension of MMIO loads Fuad Tabba
2026-06-25 14:48 ` [PATCH v2 2/2] KVM: arm64: selftests: Add MMIO sign-extending load test Fuad Tabba
0 siblings, 2 replies; 5+ messages in thread
From: Fuad Tabba @ 2026-06-25 14:48 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton
Cc: Joey Gouly, Suzuki K Poulose, Zenghui Yu, Steffen Eiden,
Catalin Marinas, Will Deacon, Shuah Khan, Christoffer Dall,
Victor Kamensky, linux-arm-kernel, kvmarm, linux-kernel
Hi folks,
Changes since v1 [1]:
- Patch 1: rewrote the commit msg in the Arm ARM's terms, with the Mem
accessor performing the access keyed on the access size and SignExtend
handling the register width. No code change. (Marc)
- Patch 2: added a big-endian pass to the test. The big-endian loads run
at EL0 with SCTLR_EL1.E0E set, so the access is big-endian while the
stage-1 walk stays little-endian. (Marc)
Oliver's Reviewed-by is on patch 1 only: the code there is unchanged, while
the test in patch 2 gained the big-endian coverage above.
A sign-extending load (LDRSB/LDRSH/LDRSW) from emulated MMIO returns a
zero-extended value rather than the sign-extended one the architecture
requires; vcpu_data_host_to_guest() strips the sign bits when it masks
the data to the access width.
If my git archeology is right, the masking dates to 2014 (b30070862edbd,
big-endian support) and has been wrong ever since, but sign-extending
loads from device memory are rare enough that nobody hit it. Patch 1
fixes it; patch 2 adds a selftest so it doesn't regress.
Based on Linux 7.1 (8cd9520d35a6c).
Cheers,
/fuad
[1] https://lore.kernel.org/all/20260622190701.2039766-1-fuad.tabba@linux.dev/
Fuad Tabba (2):
KVM: arm64: Fix sign-extension of MMIO loads
KVM: arm64: selftests: Add MMIO sign-extending load test
arch/arm64/kvm/mmio.c | 7 +-
tools/testing/selftests/kvm/Makefile.kvm | 1 +
.../selftests/kvm/arm64/mmio_sign_ext.c | 259 ++++++++++++++++++
3 files changed, 264 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/kvm/arm64/mmio_sign_ext.c
--
2.39.5
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] KVM: arm64: Fix sign-extension of MMIO loads
2026-06-25 14:48 [PATCH v2 0/2] KVM: arm64: Fix and test MMIO sign-extending loads Fuad Tabba
@ 2026-06-25 14:48 ` Fuad Tabba
2026-06-25 14:48 ` [PATCH v2 2/2] KVM: arm64: selftests: Add MMIO sign-extending load test Fuad Tabba
1 sibling, 0 replies; 5+ messages in thread
From: Fuad Tabba @ 2026-06-25 14:48 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton
Cc: Joey Gouly, Suzuki K Poulose, Zenghui Yu, Steffen Eiden,
Catalin Marinas, Will Deacon, Shuah Khan, Christoffer Dall,
Victor Kamensky, linux-arm-kernel, kvmarm, linux-kernel
A sign-extending load (LDRSB, LDRSH, LDRSW) from MMIO returns a
zero-extended value to the guest. The architecture performs such a load
as a memory read of the access size, then a sign-extension to the
register width. For LDRSH (DDI 0487 M.b C6.2.225, with the Mem accessor
at J1.2.3.111):
data = Mem{16}(address, accdesc);
X{regsize}(t) = SignExtend{regsize}(data);
The byte order is handled inside the Mem accessor, keyed on the access
size; the register width is separate, applied afterwards by SignExtend().
kvm_handle_mmio_return() runs these in the wrong order: it sign-extends
the access-width data, then calls vcpu_data_host_to_guest(), which masks
the value back to the access width (the size-keyed byte-order step). The
mask drops the sign bits that sign-extension produced.
Reorder so vcpu_data_host_to_guest() runs first, with the sign-extension
to register width after it. trace_kvm_mmio() moves with it and now logs
the access-width data before sign-extension.
Fixes: b30070862edbd ("ARM64: KVM: MMIO support BE host running LE code")
Reviewed-by: Oliver Upton <oupton@kernel.org>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
arch/arm64/kvm/mmio.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kvm/mmio.c b/arch/arm64/kvm/mmio.c
index e2285ed8c91de..d1c3a352d5a22 100644
--- a/arch/arm64/kvm/mmio.c
+++ b/arch/arm64/kvm/mmio.c
@@ -126,6 +126,10 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu)
len = kvm_vcpu_dabt_get_as(vcpu);
data = kvm_mmio_read_buf(run->mmio.data, len);
+ trace_kvm_mmio(KVM_TRACE_MMIO_READ, len, run->mmio.phys_addr,
+ &data);
+ data = vcpu_data_host_to_guest(vcpu, data, len);
+
if (kvm_vcpu_dabt_issext(vcpu) &&
len < sizeof(unsigned long)) {
mask = 1U << ((len * 8) - 1);
@@ -135,9 +139,6 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu)
if (!kvm_vcpu_dabt_issf(vcpu))
data = data & 0xffffffff;
- trace_kvm_mmio(KVM_TRACE_MMIO_READ, len, run->mmio.phys_addr,
- &data);
- data = vcpu_data_host_to_guest(vcpu, data, len);
vcpu_set_reg(vcpu, kvm_vcpu_dabt_get_rd(vcpu), data);
}
--
2.39.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] KVM: arm64: selftests: Add MMIO sign-extending load test
2026-06-25 14:48 [PATCH v2 0/2] KVM: arm64: Fix and test MMIO sign-extending loads Fuad Tabba
2026-06-25 14:48 ` [PATCH v2 1/2] KVM: arm64: Fix sign-extension of MMIO loads Fuad Tabba
@ 2026-06-25 14:48 ` Fuad Tabba
2026-06-25 14:57 ` sashiko-bot
1 sibling, 1 reply; 5+ messages in thread
From: Fuad Tabba @ 2026-06-25 14:48 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton
Cc: Joey Gouly, Suzuki K Poulose, Zenghui Yu, Steffen Eiden,
Catalin Marinas, Will Deacon, Shuah Khan, Christoffer Dall,
Victor Kamensky, linux-arm-kernel, kvmarm, linux-kernel
Add a test for sign-extending MMIO loads (LDRSB, LDRSH, LDRSW) into Xt
and Wt destinations, with and without the sign bit set. The host supplies
the MMIO data and checks the guest register holds the sign-extended value.
Repeat the loads big-endian on a mixed-endian implementation. Issue those
at EL0: SCTLR_EL1.EE would make an EL1 load big-endian but also walk the
little-endian page tables big-endian, whereas SCTLR_EL1.E0E selects only
EL0 data endianness and leaves the walk little-endian.
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
tools/testing/selftests/kvm/Makefile.kvm | 1 +
.../selftests/kvm/arm64/mmio_sign_ext.c | 259 ++++++++++++++++++
2 files changed, 260 insertions(+)
create mode 100644 tools/testing/selftests/kvm/arm64/mmio_sign_ext.c
diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 9118a5a51b89f..0f5803a1092e1 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -171,6 +171,7 @@ TEST_GEN_PROGS_arm64 += arm64/hello_el2
TEST_GEN_PROGS_arm64 += arm64/host_sve
TEST_GEN_PROGS_arm64 += arm64/hypercalls
TEST_GEN_PROGS_arm64 += arm64/external_aborts
+TEST_GEN_PROGS_arm64 += arm64/mmio_sign_ext
TEST_GEN_PROGS_arm64 += arm64/page_fault_test
TEST_GEN_PROGS_arm64 += arm64/psci_test
TEST_GEN_PROGS_arm64 += arm64/sea_to_user
diff --git a/tools/testing/selftests/kvm/arm64/mmio_sign_ext.c b/tools/testing/selftests/kvm/arm64/mmio_sign_ext.c
new file mode 100644
index 0000000000000..06708ab6db8c0
--- /dev/null
+++ b/tools/testing/selftests/kvm/arm64/mmio_sign_ext.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * mmio_sign_ext - Test sign-extending MMIO load emulation (LDRSB/LDRSH/LDRSW)
+ *
+ * Copyright (c) 2026 Google LLC
+ */
+
+#include <asm/ptrace.h>
+
+#include "processor.h"
+#include "test_util.h"
+
+#define MMIO_ADDR 0x8000000ULL
+
+/* AP[1]: allow unprivileged (EL0) access to a mapping. */
+#define PTE_USER BIT(6)
+
+/* SPSR for ERET to EL0t with DAIF masked. */
+#define SPSR_EL0 (PSR_MODE_EL0t | PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
+
+struct mmio_test {
+ const char *name;
+ uint64_t data; /* access-width value, host byte order */
+ uint8_t len;
+ uint64_t expected; /* sign-extended result; same for LE and BE */
+};
+
+/* Paired 1:1, in order, with the loads in guest_loads_le() and el0_be_loads. */
+static const struct mmio_test tests[] = {
+ /* LDRSB Xt: byte sign-extended to 64 bits */
+ { "LDRSB Xt 0xFF", 0xFF, 1, 0xFFFFFFFFFFFFFFFFULL },
+ { "LDRSB Xt 0x7F", 0x7F, 1, 0x7FULL },
+
+ /* LDRSB Wt: byte sign-extended to 32 bits, upper 32 bits zeroed */
+ { "LDRSB Wt 0xFF", 0xFF, 1, 0xFFFFFFFFULL },
+ { "LDRSB Wt 0x7F", 0x7F, 1, 0x7FULL },
+
+ /* LDRSH Xt: halfword sign-extended to 64 bits */
+ { "LDRSH Xt 0x8001", 0x8001, 2, 0xFFFFFFFFFFFF8001ULL },
+ { "LDRSH Xt 0x7FFF", 0x7FFF, 2, 0x7FFFULL },
+
+ /* LDRSH Wt: halfword sign-extended to 32 bits, upper 32 bits zeroed */
+ { "LDRSH Wt 0x8001", 0x8001, 2, 0xFFFF8001ULL },
+ { "LDRSH Wt 0x7FFF", 0x7FFF, 2, 0x7FFFULL },
+
+ /* LDRSW Xt: word sign-extended to 64 bits (no Wt form) */
+ { "LDRSW Xt 0x80000001", 0x80000001, 4, 0xFFFFFFFF80000001ULL },
+ { "LDRSW Xt 0x7FFFFFFF", 0x7FFFFFFF, 4, 0x7FFFFFFFULL },
+};
+
+/* Issue one sign-extending load from MMIO and report the result. */
+#define GUEST_LDRS(load) do { \
+ uint64_t val; \
+ \
+ asm volatile(load : "=r"(val) : "r"(MMIO_ADDR) : "memory"); \
+ GUEST_SYNC(val); \
+} while (0)
+
+/* Little-endian pass: loads issued at EL1. */
+static void guest_loads_le(void)
+{
+ GUEST_LDRS("ldrsb %0, [%1]");
+ GUEST_LDRS("ldrsb %0, [%1]");
+ GUEST_LDRS("ldrsb %w0, [%1]");
+ GUEST_LDRS("ldrsb %w0, [%1]");
+ GUEST_LDRS("ldrsh %0, [%1]");
+ GUEST_LDRS("ldrsh %0, [%1]");
+ GUEST_LDRS("ldrsh %w0, [%1]");
+ GUEST_LDRS("ldrsh %w0, [%1]");
+ GUEST_LDRS("ldrsw %0, [%1]");
+ GUEST_LDRS("ldrsw %0, [%1]");
+}
+
+/*
+ * Run the big-endian loads at EL0, where SCTLR_EL1.E0E flips only the data
+ * endianness; at EL1, SCTLR_EL1.EE would also flip the page-table walk and
+ * fault on the little-endian tables. x0 holds MMIO_ADDR; results return in
+ * x19..x28 (tests[] order) via a single SVC.
+ */
+extern char el0_be_loads[], el0_be_loads_end[];
+asm(
+" .pushsection .text, \"ax\"\n"
+" .global el0_be_loads\n"
+"el0_be_loads:\n"
+" ldrsb x19, [x0]\n"
+" ldrsb x20, [x0]\n"
+" ldrsb w21, [x0]\n"
+" ldrsb w22, [x0]\n"
+" ldrsh x23, [x0]\n"
+" ldrsh x24, [x0]\n"
+" ldrsh w25, [x0]\n"
+" ldrsh w26, [x0]\n"
+" ldrsw x27, [x0]\n"
+" ldrsw x28, [x0]\n"
+" svc #0\n"
+" .global el0_be_loads_end\n"
+"el0_be_loads_end:\n"
+" .popsection\n"
+);
+
+/* EL1 handler for the EL0 SVC: report the results, then finish. */
+static void el0_svc_handler(struct ex_regs *regs)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(tests); i++)
+ GUEST_SYNC(regs->regs[19 + i]);
+
+ GUEST_DONE();
+}
+
+static bool guest_mixed_endian_el0(void)
+{
+ uint64_t mmfr0 = read_sysreg(id_aa64mmfr0_el1);
+
+ return SYS_FIELD_GET(ID_AA64MMFR0_EL1, BIGEND, mmfr0) ||
+ SYS_FIELD_GET(ID_AA64MMFR0_EL1, BIGENDEL0, mmfr0);
+}
+
+static void guest_code(void)
+{
+ guest_loads_le();
+
+ if (guest_mixed_endian_el0()) {
+ write_sysreg(read_sysreg(sctlr_el1) | SCTLR_EL1_E0E, sctlr_el1);
+ isb();
+
+ asm volatile(
+ " msr elr_el1, %[pc]\n"
+ " msr spsr_el1, %[spsr]\n"
+ " mov x0, %[mmio]\n"
+ " isb\n"
+ " eret\n"
+ :
+ : [pc] "r"(el0_be_loads),
+ [spsr] "r"((uint64_t)SPSR_EL0),
+ [mmio] "r"(MMIO_ADDR)
+ : "x0", "memory");
+ __builtin_unreachable(); /* el0_svc_handler ends the test */
+ }
+
+ GUEST_DONE();
+}
+
+static void handle_mmio(struct kvm_run *run, const struct mmio_test *t, bool be)
+{
+ int i;
+
+ TEST_ASSERT_EQ(run->mmio.phys_addr, MMIO_ADDR);
+ TEST_ASSERT(!run->mmio.is_write, "Expected MMIO read for %s", t->name);
+ TEST_ASSERT_EQ(run->mmio.len, t->len);
+
+ memset(run->mmio.data, 0, sizeof(run->mmio.data));
+ if (be) {
+ /* The guest reads the device bytes most-significant first. */
+ for (i = 0; i < t->len; i++)
+ run->mmio.data[i] = t->data >> (8 * (t->len - 1 - i));
+ } else {
+ /* Works because arm64 KVM hosts are always little-endian. */
+ memcpy(run->mmio.data, &t->data, t->len);
+ }
+}
+
+static void expect_sync(struct kvm_vcpu *vcpu, struct ucall *uc,
+ const struct mmio_test *t)
+{
+ switch (get_ucall(vcpu, uc)) {
+ case UCALL_SYNC:
+ TEST_ASSERT(uc->args[1] == t->expected,
+ "%s: got %#lx, want %#lx", t->name,
+ (unsigned long)uc->args[1], (unsigned long)t->expected);
+ break;
+ case UCALL_ABORT:
+ REPORT_GUEST_ASSERT(*uc);
+ break;
+ default:
+ TEST_FAIL("Unexpected ucall for %s", t->name);
+ }
+}
+
+/* OR PTE_USER into the leaf descriptors covering [gva, gva + len). */
+static void make_el0_accessible(struct kvm_vm *vm, uint64_t gva, uint64_t len)
+{
+ uint64_t addr;
+
+ for (addr = gva & ~((uint64_t)vm->page_size - 1); addr < gva + len;
+ addr += vm->page_size)
+ *virt_get_pte_hva(vm, addr) |= PTE_USER;
+}
+
+static bool vcpu_mixed_endian_el0(struct kvm_vcpu *vcpu)
+{
+ uint64_t mmfr0 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR0_EL1));
+
+ return SYS_FIELD_GET(ID_AA64MMFR0_EL1, BIGEND, mmfr0) ||
+ SYS_FIELD_GET(ID_AA64MMFR0_EL1, BIGENDEL0, mmfr0);
+}
+
+int main(void)
+{
+ struct kvm_vcpu *vcpu;
+ struct kvm_vm *vm;
+ struct ucall uc;
+ unsigned int i;
+ bool be;
+
+ vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+ virt_map(vm, MMIO_ADDR, MMIO_ADDR, 1);
+
+ vm_init_descriptor_tables(vm);
+ vcpu_init_descriptor_tables(vcpu);
+ vm_install_sync_handler(vm, VECTOR_SYNC_LOWER_64, ESR_ELx_EC_SVC64,
+ el0_svc_handler);
+
+ be = vcpu_mixed_endian_el0(vcpu);
+ if (be) {
+ make_el0_accessible(vm, MMIO_ADDR, vm->page_size);
+ make_el0_accessible(vm, (uint64_t)el0_be_loads,
+ el0_be_loads_end - el0_be_loads);
+ }
+
+ ksft_print_header();
+ ksft_set_plan(ARRAY_SIZE(tests) * (be ? 2 : 1));
+
+ /* Little-endian pass: one load and one result per iteration. */
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
+ const struct mmio_test *t = &tests[i];
+
+ vcpu_run(vcpu);
+ TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_MMIO);
+ handle_mmio(vcpu->run, t, false);
+
+ vcpu_run(vcpu);
+ expect_sync(vcpu, &uc, t);
+
+ ksft_test_result_pass("%s\n", t->name);
+ }
+
+ if (be) {
+ /* The EL0 stub issues all the loads, then reports the results. */
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
+ vcpu_run(vcpu);
+ TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_MMIO);
+ handle_mmio(vcpu->run, &tests[i], true);
+ }
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
+ vcpu_run(vcpu);
+ expect_sync(vcpu, &uc, &tests[i]);
+ ksft_test_result_pass("BE %s\n", tests[i].name);
+ }
+ }
+
+ vcpu_run(vcpu);
+ TEST_ASSERT(get_ucall(vcpu, &uc) == UCALL_DONE, "Expected UCALL_DONE");
+
+ kvm_vm_free(vm);
+
+ ksft_finished();
+}
--
2.39.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: selftests: Add MMIO sign-extending load test
2026-06-25 14:48 ` [PATCH v2 2/2] KVM: arm64: selftests: Add MMIO sign-extending load test Fuad Tabba
@ 2026-06-25 14:57 ` sashiko-bot
2026-06-25 16:14 ` Fuad Tabba
0 siblings, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-06-25 14:57 UTC (permalink / raw)
To: Fuad Tabba; +Cc: kvmarm, Oliver Upton, Marc Zyngier
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Guest EL1 will crash with an Instruction Abort because the `el0_be_loads` assembly block shares a page with EL1 test code and is marked as EL0-accessible.
--
commit 60c24b3648e8f92661ec2d484d27ac4580cad60a
Author: Fuad Tabba <fuad.tabba@linux.dev>
KVM: arm64: selftests: Add MMIO sign-extending load test
Adds a test for sign-extending MMIO loads (LDRSB, LDRSH, LDRSW) into Xt
and Wt destinations, with and without the sign bit set. The host supplies
the MMIO data and checks the guest register holds the sign-extended value.
> diff --git a/tools/testing/selftests/kvm/arm64/mmio_sign_ext.c b/tools/testing/selftests/kvm/arm64/mmio_sign_ext.c
> new file mode 100644
> index 0000000000000..06708ab6db8c0
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/arm64/mmio_sign_ext.c
[ ... ]
> +extern char el0_be_loads[], el0_be_loads_end[];
> +asm(
> +" .pushsection .text, \"ax\"\n"
> +" .global el0_be_loads\n"
> +"el0_be_loads:\n"
[Severity: Medium]
Will this crash guest EL1 with an Instruction Abort?
The el0_be_loads assembly block lacks page alignment directives, placing it
adjacent to EL1 executable code in the same page of the .text section.
When make_el0_accessible() is called in main():
make_el0_accessible(vm, (uint64_t)el0_be_loads,
el0_be_loads_end - el0_be_loads);
It makes the shared page EL0-accessible by setting PTE_USER. This
inadvertently makes the EL1 guest_code on the same page EL0-accessible. In
the ARMv8 EL1&0 translation regime, an instruction fetch by EL1 from an
EL0-accessible memory page unconditionally generates a Permission Fault.
> +" ldrsb x19, [x0]\n"
> +" ldrsb x20, [x0]\n"
> +" ldrsb w21, [x0]\n"
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625144807.2603272-1-fuad.tabba@linux.dev?part=2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: selftests: Add MMIO sign-extending load test
2026-06-25 14:57 ` sashiko-bot
@ 2026-06-25 16:14 ` Fuad Tabba
0 siblings, 0 replies; 5+ messages in thread
From: Fuad Tabba @ 2026-06-25 16:14 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvmarm, Oliver Upton, Marc Zyngier
On Thu, 25 Jun 2026 at 15:57, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Guest EL1 will crash with an Instruction Abort because the `el0_be_loads` assembly block shares a page with EL1 test code and is marked as EL0-accessible.
> --
>
> commit 60c24b3648e8f92661ec2d484d27ac4580cad60a
> Author: Fuad Tabba <fuad.tabba@linux.dev>
>
> KVM: arm64: selftests: Add MMIO sign-extending load test
>
> Adds a test for sign-extending MMIO loads (LDRSB, LDRSH, LDRSW) into Xt
> and Wt destinations, with and without the sign bit set. The host supplies
> the MMIO data and checks the guest register holds the sign-extended value.
>
> > diff --git a/tools/testing/selftests/kvm/arm64/mmio_sign_ext.c b/tools/testing/selftests/kvm/arm64/mmio_sign_ext.c
> > new file mode 100644
> > index 0000000000000..06708ab6db8c0
> > --- /dev/null
> > +++ b/tools/testing/selftests/kvm/arm64/mmio_sign_ext.c
>
> [ ... ]
>
> > +extern char el0_be_loads[], el0_be_loads_end[];
> > +asm(
> > +" .pushsection .text, \"ax\"\n"
> > +" .global el0_be_loads\n"
> > +"el0_be_loads:\n"
>
> [Severity: Medium]
> Will this crash guest EL1 with an Instruction Abort?
>
> The el0_be_loads assembly block lacks page alignment directives, placing it
> adjacent to EL1 executable code in the same page of the .text section.
>
> When make_el0_accessible() is called in main():
>
> make_el0_accessible(vm, (uint64_t)el0_be_loads,
> el0_be_loads_end - el0_be_loads);
>
> It makes the shared page EL0-accessible by setting PTE_USER. This
> inadvertently makes the EL1 guest_code on the same page EL0-accessible. In
> the ARMv8 EL1&0 translation regime, an instruction fetch by EL1 from an
> EL0-accessible memory page unconditionally generates a Permission Fault.
Nice catch! You're right: AP[2:1]=01 grants UnprivWrite, and the
architecture removes PrivExecute whenever UnprivWrite is present,
regardless of PXN (ARM ARM D8.4.1.2). The call is also unnecessary:
EL0 instruction fetch is controlled by UXN (already 0), not AP bits.
The EL0 stub only needs data access to the MMIO page, not to its own
code.
The fix is to drop the second make_el0_accessible call and the
now-unused el0_be_loads_end label. Re-tested with and without the
previous patch (patch 1/2), all tests pass with the fix and the first
sign-bit case fails without it.
Marc, can you fold this in or would you like me to respin?
--- a/tools/testing/selftests/kvm/arm64/mmio_sign_ext.c
+++ b/tools/testing/selftests/kvm/arm64/mmio_sign_ext.c
@@ -77,7 +77,7 @@ static void guest_loads_le(void)
* fault on the little-endian tables. x0 holds MMIO_ADDR; results return in
* x19..x28 (tests[] order) via a single SVC.
*/
-extern char el0_be_loads[], el0_be_loads_end[];
+extern char el0_be_loads[];
asm(
" .pushsection .text, \"ax\"\n"
" .global el0_be_loads\n"
@@ -93,8 +93,6 @@ asm(
" ldrsw x27, [x0]\n"
" ldrsw x28, [x0]\n"
" svc #0\n"
-" .global el0_be_loads_end\n"
-"el0_be_loads_end:\n"
" .popsection\n"
);
@@ -213,11 +211,8 @@ int main(void)
el0_svc_handler);
be = vcpu_mixed_endian_el0(vcpu);
- if (be) {
+ if (be)
make_el0_accessible(vm, MMIO_ADDR, vm->page_size);
- make_el0_accessible(vm, (uint64_t)el0_be_loads,
- el0_be_loads_end - el0_be_loads);
- }
ksft_print_header();
ksft_set_plan(ARRAY_SIZE(tests) * (be ? 2 : 1));
Cheers,
/fuad
>
> > +" ldrsb x19, [x0]\n"
> > +" ldrsb x20, [x0]\n"
> > +" ldrsb w21, [x0]\n"
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260625144807.2603272-1-fuad.tabba@linux.dev?part=2
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-06-25 16:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-25 14:48 [PATCH v2 0/2] KVM: arm64: Fix and test MMIO sign-extending loads Fuad Tabba
2026-06-25 14:48 ` [PATCH v2 1/2] KVM: arm64: Fix sign-extension of MMIO loads Fuad Tabba
2026-06-25 14:48 ` [PATCH v2 2/2] KVM: arm64: selftests: Add MMIO sign-extending load test Fuad Tabba
2026-06-25 14:57 ` sashiko-bot
2026-06-25 16:14 ` Fuad Tabba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.