On 10/2/24 17:47, Peter Maydell wrote:
On Wed, 2 Oct 2024 at 16:35, Alex Bennée <alex.bennee@xxxxxxxxxx> wrote:
Helge Deller <deller@xxxxxxxxxx> writes:
When the emulated CPU reads or writes to a memory location
a) for which no read/write permissions exists, *and*
b) the access happens unaligned (non-natural alignment),
then the CPU should either
- trigger a permission fault, or
- trigger an unalign access fault.
In the current code the alignment check happens before the memory
permission checks, so only unalignment faults will be triggered.
This behaviour breaks the emulation of the PARISC architecture, where the CPU
does a memory verification first. The behaviour can be tested with the testcase
from the bugzilla report.
Add the necessary code to allow PARISC and possibly other architectures to
trigger a memory fault instead.
Signed-off-by: Helge Deller <deller@xxxxxx>
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=219339
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 117b516739..dd1da358fb 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1684,6 +1684,26 @@ static void mmu_watch_or_dirty(CPUState *cpu, MMULookupPageData *data,
data->flags = flags;
}
+/* when accessing unreadable memory unaligned, will the CPU issue
+ * a alignment trap or a memory access trap ? */
+#ifdef TARGET_HPPA
+# define CPU_ALIGNMENT_CHECK_AFTER_MEMCHECK 1
+#else
+# define CPU_ALIGNMENT_CHECK_AFTER_MEMCHECK 0
+#endif
I'm pretty certain we don't want to be introducing per-guest hacks into
the core cputlb.c code when we are aiming to make it a compile once
object.
Ah, I didn't know.
I'm fine with either implementation people agree on and
which gets accepted in the end.
There's also something curious going on here -- this patch
says "we check alignment before permissions, and that's wrong
on PARISC".
Yes, that's correct.
The first hunk of code in mmu_lookup() which I remove is the first
alignment check.
Then the memory access permissions are checked.
But there's a comment in target/arm/ptw.c that
says "we check permissions before alignment, and that's
wrong on Arm":
* Enable alignment checks on Device memory.
*
* Per R_XCHFJ, this check is mis-ordered. The correct ordering
* for alignment, permission, and stage 2 faults should be:
* - Alignment fault caused by the memory type
* - Permission fault
* - A stage 2 fault on the memory access
* but due to the way the TCG softmmu TLB operates, we will have
* implicitly done the permission check and the stage2 lookup in
* finding the TLB entry, so the alignment check cannot be done sooner.
I'm no arm expert. Note there is a second ARM-related aligment check
in that function. See TLB_CHECK_ALIGNED.
So do we check alignment first, or permissions first, or does
the order vary depending on what we're doing?
currently alignment first.
Helge
