From patchwork Mon Jul 8 06:43:56 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rob Landley X-Patchwork-Id: 18280 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-gg0-f199.google.com (mail-gg0-f199.google.com [209.85.161.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 9A4A225DF6 for ; Mon, 8 Jul 2013 06:44:02 +0000 (UTC) Received: by mail-gg0-f199.google.com with SMTP id o1sf4752500ggn.6 for ; Sun, 07 Jul 2013 23:44:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-beenthere:x-forwarded-to:x-forwarded-for:delivered-to:date:from :subject:to:cc:in-reply-to:x-mailer:message-id:mime-version :x-gm-message-state:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :x-google-group-id:list-post:list-help:list-archive:list-unsubscribe :content-type:content-disposition:content-transfer-encoding; bh=hk7R8/6pJO50lQKYqg7j0agPjefuJdPc06sdfnwEqOo=; b=WItn2VsUHwLKJluM+1A66937rRdWnNePY5FwJ1K+ArbaJ0cCB3ETvMJtKu+F3RVVaf giJDKacoLfXEEP8tubaCdVsNgP3x/hzJlRMc7wAkRV/mpGjgmcioLpzzvHMPx6J8Dr8L FCbDqR67nTSPT2hmjCAl46na0vp4ebXK73puSczyPJbGgJblHkKOg0rGBE31xdnGI9qd RRkywaoYuQLU3LgAgeZJAJ2SQqJ7ZXP1dAdhzzLRHeUbOG+RkBr2Uq74/kzKDWZbtNdW kTMdSUVJhjlWzEVjmQx+0Ql9omyvPVhfAbKqzSu987y1Lz7xlo0vj4ldvZqaFM5mSeOC +t7w== X-Received: by 10.224.129.196 with SMTP id p4mr23630477qas.6.1373265841736; Sun, 07 Jul 2013 23:44:01 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.39.164 with SMTP id q4ls1733558qek.31.gmail; Sun, 07 Jul 2013 23:44:01 -0700 (PDT) X-Received: by 10.58.40.16 with SMTP id t16mr13174317vek.64.1373265841587; Sun, 07 Jul 2013 23:44:01 -0700 (PDT) Received: from mail-vc0-f178.google.com (mail-vc0-f178.google.com [209.85.220.178]) by mx.google.com with ESMTPS id aj7si5817407vec.21.2013.07.07.23.44.01 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sun, 07 Jul 2013 23:44:01 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.220.178 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.178; Received: by mail-vc0-f178.google.com with SMTP id m17so3019283vca.37 for ; Sun, 07 Jul 2013 23:44:01 -0700 (PDT) X-Received: by 10.52.158.164 with SMTP id wv4mr10540530vdb.106.1373265841444; Sun, 07 Jul 2013 23:44:01 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.149.77 with SMTP id s13csp157641vcv; Sun, 7 Jul 2013 23:44:00 -0700 (PDT) X-Received: by 10.236.111.162 with SMTP id w22mr11260786yhg.213.1373265840720; Sun, 07 Jul 2013 23:44:00 -0700 (PDT) Received: from mail-yh0-f52.google.com (mail-yh0-f52.google.com [209.85.213.52]) by mx.google.com with ESMTPS id z27si9144079yha.201.2013.07.07.23.44.00 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sun, 07 Jul 2013 23:44:00 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.213.52 is neither permitted nor denied by best guess record for domain of rob@landley.net) client-ip=209.85.213.52; Received: by mail-yh0-f52.google.com with SMTP id f10so1778767yha.39 for ; Sun, 07 Jul 2013 23:44:00 -0700 (PDT) X-Received: by 10.236.190.169 with SMTP id e29mr11310445yhn.17.1373265840303; Sun, 07 Jul 2013 23:44:00 -0700 (PDT) Received: from driftwood (c-50-148-218-118.hsd1.mn.comcast.net. [50.148.218.118]) by mx.google.com with ESMTPSA id i27sm35129620yhj.24.2013.07.07.23.43.57 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sun, 07 Jul 2013 23:43:58 -0700 (PDT) Date: Mon, 08 Jul 2013 01:43:56 -0500 From: Rob Landley Subject: Re: [Qemu-devel] [PATCH v4 00/11] Fix versatile_pci To: Peter Maydell Cc: qemu-devel@nongnu.org, Arnd Bergmann , "Michael S. Tsirkin" , patches@linaro.org, Will Deacon , Paul Brook , Andreas =?iso-8859-1?q?F=E4rber?= In-Reply-To: (from peter.maydell@linaro.org on Sat Jun 29 06:03:26 2013) X-Mailer: Balsa 2.4.11 Message-Id: <1373265836.27613.14@driftwood> MIME-Version: 1.0 X-Gm-Message-State: ALoCoQm7LxQnYlAClywQcceaJgkrlu1sCw1XH9L7QBhxdNZDLigg1zYTilEtSM/tpJs8UqZkcaRs X-Original-Sender: rob@landley.net X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.220.178 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Content-Disposition: inline On 06/29/2013 06:03:26 AM, Peter Maydell wrote: > On 28 June 2013 08:01, Rob Landley wrote: > > Now that the next kernel's about to come out, I'm trying to get my > arm > > versatile image to work under qemu 1.5.0. The old kernel doesn't > work, and > > the current vanilla kernel doesn't work. This change broke it. > > > > I'm testing current linux-git both with and without this patch: > > > > --- linux/arch/arm/mach-versatile/pci.c 2013-04-28 > 19:36:01.000000000 -0500 > > +++ linux.bak/arch/arm/mach-versatile/pci.c 2013-04-29 > > 19:09:44.857097553 -0500 > > @@ -333,7 +333,7 @@ > > * 26 1 IRQ_SIC_PCI2 > > * 27 1 IRQ_SIC_PCI3 > > */ > > - irq = IRQ_SIC_PCI0 + ((slot - 24 + pin - 1) & 3); > > + irq = 59; //IRQ_SIC_PCI0 + ((slot - 24 + pin - 1) & 3); > > > > return irq; > > } > > > > With the patch, qemu 1.2.0 works, but qemu 1.5 versatile does not > work with > > or without the patch. > > The "with the patch" case is uninteresting, because it's not > actually a fix for anything, it's just a change that happened > to work with old QEMU, and it's not a change that is in upstream > Linux. I want to create a kernel image that works with the existing QEMU image people are likely to have installed on their systems, _and_ with current versions of QEMU. Is that what you consider "uninteresting"? > > Here's a test image to demonstrate the problem: it works fine under > qemu > > 1.2.0, but doesn't under 1.5.0: > > > > http://landley.net/aboriginal/bin/system-image-armv5l.tar.bz2 > > > > Extract it and ./run-emulator.sh. You get a shell prompt from > 1.2.0, from > > 1.5.0 it never gets a scsi interrupt, and after a timeout goes into > an > > abort/reset loop. > > Is this an image with your patch or without it? Does it work > if you use the -global option to force legacy IRQ mapping? Under qemu 1.2? It goes: qemu-system-arm: Property '.broken-irq-mapping' not found See "backwards compatability", above. I am trying to produce "an image that runs under qemu". That level of genericity worked for several years, at least ever since 1.0. When it stopped, I complained. You seem to be expressing incredulity at the concept. > >> >> This version of the patchset avoids breaking legacy Linux guests > >> >> by automatically detecting those broken kernels and switching > back > >> >> to the old mapping. > > > > > > As testing with the above image confirms: it does not. > > I tested with several separate variants of the Linux kernel. I tried building a vanilla linux 3.10 kernel, and it doesn't work on qemu 1.5. I confirmed their current interrupt mapping does not match any known hardware or emulator, and that it changed more than once in incompatible ways as they did wild-ass-guess du jour about what it should be now. The fact it was obviously untested (or it wouldn't keep changing, since there presumably is just one right answer) says to me that this hardware is difficult ot come by. (The fact the IRQ spent over 5 years in the old state before anyone noticed would also imply a lack of test hardware.) However, it's useful to emulate because you can stick a PCI bus in it, meaning you can stick arbitrary devices (hard drive and network card) into said bus. So I did the obvious-to-me thing and patched it back to how it _was_, which is the only thing that works with previous versions of QEMU. You seem to find this approach shocking and unexpected. Do you have a better way to achieve compatability with older versions of qemu _and_ current versions? (I'm open to suggestions...) > >> >> This works by looking at the values the kernel > >> >> writes to the PCI_INTERRUPT_LINE register in the config space, > which > >> >> is effectively the interrupt number the kernel expects the > device > >> >> to be using. If this doesn't match reality we use the broken > mapping. > >> >> (Thanks to Michael S. Tsirkin for this suggestion.) > > > > Define "reality". > > "would work on real hardware". The stuff that's so rare the kernel guys couldn't find any to test on? Maybe I haven't been clear: I want something that works with current QEMU _and_ old QEMU. Your paragraph above implies that there's smoething _different_ I can write to PCI_INTERRUPT_LINE that will trigger the old behavior. So my patch to do the old IRQ (which worked fine with qemu 1.2) needs to be bigger to humor QEMU's heuristic. I'll try to figure out how to extend my patch to avoid triggering your new "does not work" mode. Let's see, stick a printf into qemu, and: =====Write 60=12,59===== So it's requesting IRQ 59, and QEMU's reponse is to _not_ give it IRQ 59 unless we trigger the "broken" flag. So... real hardware doesn't get the IRQ it requests? Using the reqested IRQ is "broken"? (Apparently your patch for a real hardware kernel doesn't request the IRQ it's going to use? I'm confused...) Right, easy enough to fix: + // for compatability with old versions. + irq = 59; //IRQ_SIC_PCI0 + ((slot - 24 + pin - 1) & 3); return irq; } That should work with new _and_ old QEMU. > > The kernel changed what irqs it was expecting _three_times_ last > year, and > > as far as I can tell none of them were what they were _trying_ to > do. > > > > Here's my blog entry and the source control comments where I > diagnosed this > > stuff to show that the kernel guys have no flipping CLUE how an arm > > versatile board actually works: > > I agree that the kernel has made a bit of a mess in this area, > so we don't need to have that argument again. Can we have the backwards compatability argument? > > The kernel code in this area is CRAP, has changed multiple times in > > semi-random ways, has obviously NEVER been tested on real hardware, > and if > > you've implemented what the actual versatile documentation says > it's clearly > > not what the kernel is doing. > > I tested these changes against a patchset which Arnd wrote which > I can guarantee was tested on real hardware because I did the > testing myself. Where is this patch? (Was it submitted upstream to linux-kernel?) > > Do you have a kernel that runs under current qemu arm versatile > emulation? I > > can poke around and figure out which irqs it expects _now_, but > it's not > > "right" and presumably you're just going to change it again when > you realize > > what qemu is doing and what the unpatched kernel is doing don't > match. > > The ones on http://people.debian.org/~aurel32/qemu/armel/ > should work. I'm not seeing a patch there. Rob --- a/arch/arm/mach-versatile/pci.c +++ b/arch/arm/mach-versatile/pci.c @@ -333,7 +333,13 @@ static int __init versatile_map_irq(const struct pci_dev *dev, u8 slot, u8 pin) * 26 1 IRQ_SIC_PCI2 * 27 1 IRQ_SIC_PCI3 */ - irq = IRQ_SIC_PCI0 + ((slot - 24 + pin - 1) & 3); + // Hit QEMU 1.5.0 and later with a brick so it uses the IRQ we say. + dev->bus->ops->write(dev->bus, dev->devfn, PCI_INTERRUPT_LINE, 1, 27); + + // The kernel has no clue where IRQs are, and its current assignments + // match neither the hardware nor historic QEMU. Use historic QEMU