[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] "tcg: Clean up direct block chaining safety checks" bre
From: |
Sergey Fedorov |
Subject: |
Re: [Qemu-devel] "tcg: Clean up direct block chaining safety checks" breaks target-xtensa mmu test |
Date: |
Mon, 16 May 2016 17:36:56 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 |
On 15/05/16 23:54, Sergey Fedorov wrote:
> On 15/05/16 22:56, Sergey Fedorov wrote:
>> On 15/05/16 22:53, Max Filippov wrote:
>>> On Sun, May 15, 2016 at 10:38:46PM +0300, Sergey Fedorov wrote:
>>>> On 15/05/16 21:58, Max Filippov wrote:
>>>>> I've noticed that the commit 5b053a4a28278 (tcg: Clean up direct block
>>>>> chaining safety checks) has broken tearget-xtensa test cross_page_tb
>>>>> from the tests/tcg/xtensa/test_mmu.S. The test runs a TB that spans two
>>>>> adjacent pages, then unmaps the second page and runs it again. It
>>>>> expects an instruction fetch exception on the second run, but with the
>>>>> said commit doesn't get it. Reverting that commit fixes the test.
>>>>> Any suggestions?
>>>> That's too strange. How do I run the test?
>>> I've minimized the test case, the source and the binary are available
>>> here:
>>> http://jcmvbkbc.spb.ru/~jcmvbkbc/tmp/201605152245/
>>>
>>> You can run it as
>>> qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting -kernel
>>> ./test_mmu.tst
>>>
>> Thank you for this. I'll try it tomorrow and figure out what's going wrong.
>
> I couldn't sleep without first trying the test :) Now I really
> understand why things went wrong. I mixed up 'next_tb' and 'tb' in
> this piece of code:
>
> /* see if we can patch the calling TB. When the TB
> spans two pages, we cannot safely do a direct
> jump. */
> if (next_tb != 0 && tb->page_addr[1] == -1
> && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
> tb_add_jump((TranslationBlock *)(next_tb & ~TB_EXIT_MASK),
> next_tb & TB_EXIT_MASK, tb);
> }
>
> So I removed 'tb->page_addr[1] == -1' check thinking it's for the last
> executed TB. But actually, it checks the next TB despite there's also
> the variable called 'next_tb'. Indeed, we cannot safely direct jump
> *to* the TB spanning pages in system emulation because we don't take
> care of direct jumps when address mapping changes. However we can do
> this in user emulation because there's only static address translation
> and TBs get always invalidated properly.
>
> I'll prepare a patch and fix this tomorrow then.
http://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg02506.html
Thanks,
Sergey