On 12/02/2019 18:21, Philippe Mathieu-Daudé wrote: > On 2/12/19 6:50 PM, Mark Cave-Ayland wrote: >> 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 <[email protected]> >> > > Thanks! > > Phil.
No worries. David, are you able to update the commit message in your ppc-for-4.0 branch accordingly? ATB, Mark.
