> On Jun 11, 2021, at 7:24 AM, Alex Bennée <alex.ben...@linaro.org> wrote:
>
>
> Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> writes:
>
>> On 10/06/2021 14:14, Peter Maydell wrote:
>>
>>> On Thu, 10 Jun 2021 at 14:02, Programmingkid <programmingk...@gmail.com>
>>> wrote:
>>>>
>>>> Hi Richard,
>>>>
>>>> There is a function called breakpoint_invalidate() in cpu.c that
>>>> calls a function called tb_flush(). I have determined that this
>>>> call is being made over 200,000 times when Windows XP boots.
>>>> Disabling this function makes Windows XP boot way faster than
>>>> before. The time went down from around 3 minutes to 20 seconds when
>>>> I applied the patch below.
>>>>
>>>> After I applied the patch I ran several tests in my VM's to see if
>>>> anything broke. I could not find any problems. Here is the list my VM's I
>>>> tested:
>>>>
>>>> Mac OS 10.8 in qemu-system-x86_64
>>>> Windows 7 in qemu-system-x86_64
>>>> Windows XP in qemu-system-i386
>>>> Mac OS 10.4 in qemu-system-ppc
>>>>
>>>> I would be happy if the patch below was accepted but I would like to know
>>>> your thoughts.
>>>
>>>> cpu.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/cpu.c b/cpu.c
>>>> index bfbe5a66f9..297c2e4281 100644
>>>> --- a/cpu.c
>>>> +++ b/cpu.c
>>>> @@ -253,7 +253,7 @@ static void breakpoint_invalidate(CPUState *cpu,
>>>> target_ulong pc)
>>>> * Flush the whole TB cache to force re-translation of such TBs.
>>>> * This is heavyweight, but we're debugging anyway.
>>>> */
>>>> - tb_flush(cpu);
>>>> + /* tb_flush(cpu); */
>>>> }
>>>> #endif
>>> The patch is clearly wrong -- this function is called when a CPU
>>> breakpoint
>>> is added or removed, and we *must* drop generated code which either
>>> (a) includes code to take the breakpoint exception and now should not
>>> or (b) doesn't include code to take the breakpoint exception and now should.
>>> Otherwise we will incorrectly take/not take a breakpoint exception when
>>> that stale code is executed.
>>> As the comment notes, the assumption is that we won't be adding and
>>> removing breakpoints except when we're debugging and therefore
>>> performance is not critical. Windows XP is clearly doing something
>>> we weren't expecting, so we should ideally have a look at whether
>>> we can be a bit more efficient about not throwing the whole TB
>>> cache away.
>>
>> FWIW this was reported a while back on Launchpad as
>> https://bugs.launchpad.net/qemu/+bug/1883593 where the performance
>> regression was traced back to:
>>
>> commit b55f54bc965607c45b5010a107a792ba333ba654
>> Author: Max Filippov <jcmvb...@gmail.com>
>> Date: Wed Nov 27 14:06:01 2019 -0800
>>
>> exec: flush CPU TB cache in breakpoint_invalidate
>>
>> When a breakpoint is inserted at location for which there's currently no
>> virtual to physical translation no action is taken on CPU TB cache. If a
>> TB for that virtual address already exists but is not visible ATM the
>> breakpoint won't be hit next time an instruction at that address will be
>> executed.
>>
>> Flush entire CPU TB cache in breakpoint_invalidate to force
>> re-translation of all TBs for the breakpoint address.
>>
>> This change fixes the following scenario:
>> - linux user application is running
>> - a breakpoint is inserted from QEMU gdbstub for a user address that is
>> not currently present in the target CPU TLB
>> - an instruction at that address is executed, but the external debugger
>> doesn't get control.
>>
>> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
>> Signed-off-by: Max Filippov <jcmvb...@gmail.com>
>> Message-Id: <20191127220602.10827-2-jcmvb...@gmail.com>
>> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>
> This was a reversion of a reversion (see 406bc339b0 and a9353fe89). So
> we've bounced between solutions several times. Fundamentally if it gets
> tricky to ensure your translated state matches the actual machine state
> the easiest option is to throw everything away and start again.
>
>> Perhaps Windows XP is constantly executing some kind of breakpoint
>> instruction or updating some CPU debug registers during boot?
>
> It would be useful to identify what exactly is triggering the changes
> here. If it's old fashioned breakpoint instructions being inserted into
> memory we will need to ensure our invalidating of old translations is
> rock solid. If we are updating our internal breakpoint/watchpoint
> tracking as a result of the guest messing with debug registers maybe we
> can do something a bit more finessed?
>
>>
>>
>> ATB,
>>
>> Mark.
>
>
> --
> Alex Bennée
Hello Alex,
The good news is the source code to Windows XP is available online:
https://github.com/cryptoAlgorithm/nt5src
