Re: [Qemu-devel] [PATCH v3 0/3] softmmu demacro

[Mark Cave-Ayland]

On 26/02/2019 09:24, Alex Bennée wrote:

>> Presumably the issue here is somehow related to the compiler incorrectly
>> extending/reducing the shift when the larger type is involved? Also during 
>> my tests
>> the visual corruption was only present for 32-bit accesses, but presumably 
>> all the
>> access sizes will need a similar fix.
> 
> So I did fix this in the third patch when I out of lined the unaligned
> helpers so:
> 
>      const tcg_target_ulong size_mask = MAKE_64BIT_MASK(0, size * 8);
> 
> and
> 
>      /* Big-endian combine.  */
>      r2 &= size_mask;
> 
> or
> 
>      /* Little-endian combine.  */
>      r1 &= size_mask;
> 
> I guess I should also apply that to patch 1 for bisectability.

I've done that locally, and while things have improved with progress bars I'm 
still
seeing some strange blank fills appearing on the display in MacOS. I'll have 
another
dig further and see what's going on...

> Thanks for digging into the failures. It does make me think that we
> really could do with some system mode tests to properly exercise all
> softmmu code paths:
> 
>   * each size access, load and store
>   * unaligned accesses, load and store (+ potential arch specific handling)
>   * page striding accesses faulting and non-faulting
> 
> I'm not sure how much work it would be to get a minimal bare metal
> kernel that would be able to do these and report success/fail. When I
> did the MTTCG work I used kvm-unit-tests as a fairly complete mini-os
> but maybe that's overkill for what we need here. After all we already
> have migration kernels that just do a simple tick-tock for testing
> migration. It would be nice to have the tests in C with maybe a minimal
> per-arch assembly so we can share the tests across various platforms.

Right. Having unrolled the macros myself whilst softmmu_template.h appears on 
the
surface to be deceptively simple, it hides quite a few subtle implementation 
details
which is why I think your work is so valuable here.


ATB,

Mark.