Re: {PATCH] accel/tcg: Fix CPU specific unaligned behaviour

[Richard Henderson <richard.henderson@linaro.org>]

On 10/2/24 08:35, Alex Bennée wrote:
> Helge Deller <deller@kernel.org> 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@gmx.de>
>> 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.

Correct.

> I guess the real question is where could we put this flag? My gut says
> we should expand the MO_ALIGN bits in MemOp to express the precedence or
> not of the alignment check in relation to permissions.

I suppose that could work.

I was hoping for a reorg of the target hooks that could allow the target to see 
misalignment and permission check simultaneously, then the target chooses the order in 
which the two faults are presented.  Given how complicated tlb_fill is though, I don't see 
that being an easy job.

I'll play around with something and report back.


r~