qemu-devel
[Top][All Lists]
Advanced

[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: Sun, 15 May 2016 23:54:11 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2

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.

Nice test and nice catch!

Thanks,
Sergey


reply via email to

[Prev in Thread] Current Thread [Next in Thread]