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

[BALATON Zoltan]

On Tue, 12 Feb 2019, Mark Cave-Ayland wrote:
> On 11/02/2019 23:35, Philippe Mathieu-Daudé wrote:
>
> > Hi Mark,
> >
> > On 2/10/19 6:44 PM, Mark Cave-Ayland wrote:
> > > In order to handle a race condition in MacOS 9, 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 that the previous reliability issues are no
> > > longer present, and this value can be reduced down to 20us with no apparent
> > > ill effects in my local tests. This has the benefit of considerably improving
> > > the responsiveness of the ADB keyboard and mouse with the guest.
> > >
> > > Signed-off-by: Mark Cave-Ayland <address@hidden>
> > > ---
> > >  hw/misc/macio/cuda.c | 11 +----------
> > >  1 file changed, 1 insertion(+), 10 deletions(-)
> > >
> > > diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> > > index c4f7a2f39b..3febacdd1e 100644
> > > --- a/hw/misc/macio/cuda.c
> > > +++ b/hw/misc/macio/cuda.c
> > > @@ -97,17 +97,8 @@ static void cuda_set_sr_int(void *opaque)
> > >
> > >  static void cuda_delay_set_sr_int(CUDAState *s)
> > >  {
> > > -    MOS6522CUDAState *mcs = &s->mos6522_cuda;
> > > -    MOS6522State *ms = MOS6522(mcs);
> > > -    MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(ms);
> > >      int64_t expire;
> > >
> > > -    if (ms->dirb == 0xff || s->sr_delay_ns == 0) {
> > > -        /* Disabled or not in Mac OS, fire the IRQ directly */
> > > -        mdc->set_sr_int(ms);
> > > -        return;
> > > -    }
> >
> > The change of sr_delay_ns below is well explained, but I don't
> > understand why you remove the previous if().
>
> IIRC it was a hack by Alex to try and restrict the delay on the interrupt just 
> to
> MacOS instead of Linux, but with the reduced value it doesn't really matter any 
> more.

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.)

> 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?

Regards,
BALATON Zoltan