Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt

[Mark Cave-Ayland]

On 12/02/2019 17:21, Philippe Mathieu-Daudé wrote:

>>> If this delay is to prevent a bug which only happens in MacOS then that's 
>>> the hack
>>> not the normal code path to run without the delay that you've just removed. 
>>> So maybe
>>> this should be kept if possible to avoid unecessary delays for other guests.
>>> (Although if this only affects mac99,via=cuda but not mac99,via=pmu then I 
>>> don't care
>>> much as long as pmu works.)
>>
>> Well the reality is that the detection above doesn't actually seem to work 
>> anyway -
>> at least a quick boot test with Linux, MacOS X and MacOS 9 with a printf() 
>> added into
>> the if() shows nothing firing once the kernel takes over. So the slow path 
>> with the
>> delay included was always being taken within the OS anyway.
>>
>> And indeed, the code doesn't affect pmu so you won't see any difference 
>> there.
>>
>>>> As a plus it also prevents a guest OS from accidentally triggering the 
>>>> hack whilst
>>>> programming the VIA port.
>>>
>>> That may be a problem though. What's the issue exactly? Why is the delay 
>>> needed in
>>> the first place?
>>
>> It's some kind of racy polling with OS 9 (I wasn't involved in the technical 
>> details,
>> sorry) which causes OS 9 to hang on boot if the delay isn't present. And 
>> even better
>> the slow path that was previously always being taken has now been reduced 
>> from 300us
>> to 30us so whichever way you look at it, having this patch applied is a win.
> 
> Can you write a paragraph about this, that David can amend to your
> patch? That would stop worrying me about looking at this patch in
> various months...

Hmmmm well the existing description already describes the interrupt race in OS 
9 so I
guess the only part missing is the bit about the fast path. How about the 
revised
text below for the patch description?


    cuda: decrease time delay before raising VIA SR interrupt and remove fast 
path

    In order to handle a race condition in the MacOS 9 CUDA driver, a delay was
    introduced when raising the VIA SR interrupt inspired by similar code in
    MacOnLinux.

    During original testing of the MacOS 9 patches it was found that the 30us
    delay used in MacOnLinux did not work reliably within QEMU, and a value of
    300us was required to function correctly.

    Recent experiments have shown two things: firstly when booting Linux, MacOS
    9 and MacOS X the fast path which bypasses the delay is never triggered 
once the
    OS kernel is loaded making it effectively useless. Rather than leave this 
code
    in place where a guest could potentially enable it by accident and break 
itself,
    we might as well just remove it.

    Secondly the previous reliability issues are no longer present, and this 
value
    can be reduced down to 20us with no apparent ill effects. This has the 
benefit of
    considerably improving the responsiveness of the ADB keyboard and mouse 
within
    the guest.

    Signed-off-by: Mark Cave-Ayland <address@hidden>



ATB,

Mark.