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.
>
>
> ATB,
>
> Mark.
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
