Peter Maydell <peter.maydell@xxxxxxxxxx> writes: > 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. > > There's also something curious going on here -- this patch > says "we check alignment before permissions, and that's wrong > on PARISC". 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. > > So do we check alignment first, or permissions first, or does > the order vary depending on what we're doing? If it varies by architecture and operation that is even more reason to encode the wanted behaviour in the MemOp. -- Alex Bennée Virtualisation Tech Lead @ Linaro
