From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:37392) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gtcgc-0006vh-Ps for qemu-devel@nongnu.org; Tue, 12 Feb 2019 13:21:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gtcgb-0007fi-9Y for qemu-devel@nongnu.org; Tue, 12 Feb 2019 13:21:38 -0500 Received: from mail-wr1-f66.google.com ([209.85.221.66]:32902) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gtcga-0007Wv-Tu for qemu-devel@nongnu.org; Tue, 12 Feb 2019 13:21:37 -0500 Received: by mail-wr1-f66.google.com with SMTP id i12so3862836wrw.0 for ; Tue, 12 Feb 2019 10:21:26 -0800 (PST) References: <20190210174421.22062-1-mark.cave-ayland@ilande.co.uk> <442139a2-5ab2-9259-03a0-11d2f3c86c59@redhat.com> <28a8cd58-51a9-6301-1d25-02289af8c81d@ilande.co.uk> <971ba38d-d883-d51c-55b2-531639336ed4@ilande.co.uk> From: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= Message-ID: Date: Tue, 12 Feb 2019 19:21:24 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark Cave-Ayland , BALATON Zoltan Cc: david@gibson.dropbear.id.au, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, hsp.cat7@gmail.com 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 > Thanks! Phil.