[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before rai
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt |
Date: |
Tue, 12 Feb 2019 18:21:07 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
On 2/12/19 5:51 PM, Mark Cave-Ayland wrote:
> On 12/02/2019 11:03, BALATON Zoltan wrote:
>
>> 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.)
>
> 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...
Thanks!
Phil.
- [Qemu-devel] [PATCH] cuda: decrease time delay before raising VIA SR interrupt, Mark Cave-Ayland, 2019/02/10
- Re: [Qemu-devel] [PATCH] cuda: decrease time delay before raising VIA SR interrupt, David Gibson, 2019/02/10
- Re: [Qemu-devel] [PATCH] cuda: decrease time delay before raising VIA SR interrupt, Philippe Mathieu-Daudé, 2019/02/11
- Re: [Qemu-devel] [PATCH] cuda: decrease time delay before raising VIA SR interrupt, Mark Cave-Ayland, 2019/02/12
- Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt, BALATON Zoltan, 2019/02/12
- Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt, Mark Cave-Ayland, 2019/02/12
- Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt,
Philippe Mathieu-Daudé <=
- Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt, Mark Cave-Ayland, 2019/02/12
- Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt, Philippe Mathieu-Daudé, 2019/02/12
- Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt, Mark Cave-Ayland, 2019/02/12
- Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt, David Gibson, 2019/02/12
- Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt, Mark Cave-Ayland, 2019/02/13