On 13/02/2019 00:21, David Gibson wrote: > On Tue, Feb 12, 2019 at 08:01:22PM +0000, Mark Cave-Ayland wrote: >> 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 <mark.cave-ayl...@ilande.co.uk> >>>> >>> >>> Thanks! >>> >>> Phil. >> >> No worries. David, are you able to update the commit message in your >> ppc-for-4.0 >> branch accordingly? > > Done.
Great, thanks! ATB, Mark.
