[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 01/28] accel/tcg: Introduce translator_use_goto_tb
From: |
Wu, Fei |
Subject: |
Re: [PATCH v2 01/28] accel/tcg: Introduce translator_use_goto_tb |
Date: |
Thu, 16 Mar 2023 10:07:58 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 |
On 3/15/2023 2:15 AM, Richard Henderson wrote:
> On 3/14/23 06:47, Wu, Fei wrote:
>> On 3/13/2023 11:00 PM, Richard Henderson wrote:
>>> On 3/13/23 07:13, Wu, Fei2 wrote:
>>>> Hi Richard,
>>>>
>>>> Sorry for disturbing you. I'm doing some perf profiling on
>>>> qemu-riscv64,
>>>> I see 10%+ faster to build stress-ng without the following patch. I
>>>> know
>>>> it's incorrect to just skip this patch, I'm wondering if we can do
>>>> something on intercepting mmap/mprotect (very rare), e.g. even
>>>> invalidating all the TBs, but keep the cross-page block chaining.
>>>
>>> It also affects breakpoints.
>>>
>>> I have no good ideas for how to keep cross-page block chaining without
>>> breaking either of these use cases. If you come up with a good idea,
>>> please post on qemu-devel for discussion.
>>>
>> Thank you for reply. I am new to qemu/tcg, lots of details and
>> backgrounds need to catch up.
>>
>> If we only want to address user-mode qemu, and assume this cross-page
>> chain, first page -> second page:
>>
>> * breakpoints. If a new bp is added to second page, the chain is hard to
>> maintain, but it looks acceptable to flush all TBs and fall back to
>> current non-cross-page implementation during debugging? I think It's
>> different from the full system situation here:
>> https://gitlab.com/qemu-project/qemu/-/issues/404
>>
>> * mprotect. If the 2nd page remains 'X' permission after mprotect, the
>> chain is still valid, if it's changed to non-X, then the syscall
>> interceptor will change the permission of corresponding host page to
>> non-X, it will be segfault as expected?
>>
>> * mmap. I cannot figure out the situation. Is there any unit test for
>> this, or could you please shed some light?
> Also munmap, but handled via the same path through page_set_flags, see
>
> if (inval_tb) {
> tb_invalidate_phys_range(start, end);
> }
>
> There is no unit test for mmap over an existing code page.
> I believe we do have one for mprotect.
>
> You could plausibly add a global variable choosing between
> link-all-pages and link-one-page modes; it would be protected by
> mmap_lock. For link-all-pages mode, the above tb_invalidate_phys_range
> becomes tb_flush. We probably want to start in link-one-page mode if
> gdbstub is active, which is the only way to set breakpoints in user-only
> mode.
>
> I expect mprotect/mmap over existing executable pages to be extremely
> rare. I expect munmap of existing executable pages to be rare-ish, with
> dlclose() being the most common case. You might wish to change from
> link-all-pages mode to link-one-page mode after one or more instances.
>
> And as I said, this discussion should happen on qemu-devel.
>
My fault. I didn't notice the cc list, and initialized another thread:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg949625.html
Would you prefer commenting there, or I move the content here?
Thanks,
Fei.
>
> r~