Re: [Qemu-devel] [PULL 23/32] tcg: Support MMU protection regions smaller than TARGET_PAGE_SIZE

[Laurent Vivier]

Le 28/06/2018 à 22:05, Peter Maydell a écrit :
> On 28 June 2018 at 20:23, Laurent Vivier <address@hidden> wrote:
>> Le 28/06/2018 à 15:23, Peter Maydell a écrit :
>>> On 28 June 2018 at 14:03, Laurent Vivier <address@hidden> wrote:
>>>> Le 26/06/2018 à 18:56, Peter Maydell a écrit :
>>>>> Add support for MMU protection regions that are smaller than
>>>>> TARGET_PAGE_SIZE. We do this by marking the TLB entry for those
>>>>> pages with a flag TLB_RECHECK. This flag causes us to always
>>>>> take the slow-path for accesses. In the slow path we can then
>>>>> special case them to always call tlb_fill() again, so we have
>>>>> the correct information for the exact address being accessed.
>>>>>
>>>>> This change allows us to handle reading and writing from small
>>>>> regions; we cannot deal with execution from the small region.
>>>>>
>>>>> Signed-off-by: Peter Maydell <address@hidden>
>>>>> Reviewed-by: Richard Henderson <address@hidden>
>>>>> Message-id: address@hidden
>>>>> ---
>>>>>  accel/tcg/softmmu_template.h |  24 ++++---
>>>>>  include/exec/cpu-all.h       |   5 +-
>>>>>  accel/tcg/cputlb.c           | 131 +++++++++++++++++++++++++++++------
>>>>>  3 files changed, 130 insertions(+), 30 deletions(-)
>>>>
>>>> This patch breaks Quadra 800 emulation, any idea why?
>>>>
>>>> ABCFGHIJK
>>>> qemu: fatal: Unable to handle guest executing from RAM within a small
>>>> MPU region at 0x0014cb5a
>>>
>>> Hmm, that shouldn't happen unless your target code was
>>> incorrectly returning a too-small page size. (I say
>>> "incorrectly" because before this patchseries that was
>>> unsupported and would have had weird effects depending on
>>> exactly what the order of guest accesses to the page was.)
>>>
>>> You could look at whether the m68k code is calling tlb_set_page()
>>> with a wrong page_size value and why that happens. You can
>>> get back the old behaviour by having your code do
>>>    if (page_size < TARGET_PAGE_SIZE) {
>>>        page_size = TARGET_PAGE_SIZE;
>>>    }
>>>
>>> but that is definitely a bit of a hack.
>>
>> Thank you to have had a look at this.
>>
>> I've added traces and tlb_set_page() is always called with page_size ==
>> TARGET_PAGE_SIZE.
>>
>> m68k linux kernel always use 4 kB page that is the value of
>> TARGET_PAGE_SIZE.
>> 68040 MMU can also use 8 kB page, but in our case it doesn't (and of
>> course 8 kB > TARGET_PAGE_SIZE).
> 
>> I'm not aware of subpage in m68k MMU. but we have TLB entries that are
>> separated between code and data: does it change something in your code?
>> Accessing an address as a data access and then as an instruction access
>> could appear like a TLB_RECHECK?
> 
> If you never pass a page_size < TARGET_PAGE_SIZE to
> tlb_set_page() then we should never mark anything as TLB_RECHECK:
> the theory was no behaviour change for the currently-being-used case.
> 
> Do you have a repro case (images, command line) that I can
> use to investigate ?
- checkout the branch q800-dev-part1 from
  git://github.com/vivier/qemu-m68k.git

- configure and build

 './configure' '--target-list=m68k-softmmu' '--enable-debug' \
               '--enable-debug-tcg' '--enable-debug-info'

  my gcc is from Fedora 27, version 7.3.1 20180303 (Red Hat 7.3.1-5)

- get the kernel from the debian installer:

wget
https://cdimage.debian.org/mirror/cdimage/ports/9.0/m68k/iso-cd/debian-9.0-m68k-NETINST-1.iso

guestfish --add debian-9.0-m68k-NETINST-1.iso --ro \
          --mount /dev/sda:/ <<_EOF_
copy-out /install/kernels/vmlinux-4.15.0-2-m68k .
_EOF_

- and run

./m68k-softmmu/qemu-system-m68k -M q800 \
    -serial none -serial mon:stdio \
    -kernel vmlinux-4.15.0-2-m68k \
    -nographic

ABCFGHIJK
qemu: fatal: Unable to handle guest executing from RAM within a small
MPU region at 0x0029bb2c
D0 = 003ca111   A0 = 003ca111   F0 = 7fff ffffffffffffffff  (         nan)
D1 = 00000000   A1 = 0000000a   F1 = 7fff ffffffffffffffff  (         nan)
D2 = 00000000   A2 = 00395314   F2 = 7fff ffffffffffffffff  (         nan)
D3 = 00000001   A3 = 003ca110   F3 = 7fff ffffffffffffffff  (         nan)
D4 = 000003e0   A4 = 003ca4e8   F4 = 7fff ffffffffffffffff  (         nan)
D5 = 00393fc8   A5 = 0033d77b   F5 = 7fff ffffffffffffffff  (         nan)
D6 = 003ca108   A6 = 00393fc4   F6 = 7fff ffffffffffffffff  (         nan)
D7 = 00000002   A7 = 00393ef8   F7 = 7fff ffffffffffffffff  (         nan)
PC = 0029bb2c   SR = 2700 T:0 I:7 SI -----
FPSR = 00000000 ---- -------- -----  FPCR = 0000 X RN --------
  A7(MSP) = 00000000   A7(USP) = 00000000 ->A7(ISP) = 00393f68
VBR = 0x003bfce8
SFC = 5 DFC 5
SSW 00000000 TCR 00008000 URP 00000000 SRP 00001000
DTTR0/1: 00000000/f807a040 ITTR0/1: 00000000/f807a040
MMUSR 00000000, fault at 00000000
Aborted (core dumped)

Thanks,
Laurent