From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-x22c.google.com (mail-lf0-x22c.google.com [IPv6:2a00:1450:4010:c07::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id BE26E1A1E0A for ; Mon, 12 Sep 2016 06:06:36 -0700 (PDT) Received: by mail-lf0-x22c.google.com with SMTP id u14so86521623lfd.1 for ; Mon, 12 Sep 2016 06:06:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Du59GnBR5Q62IfyguPaciR82Zvv0PR/Hi7y+I3M3i50=; b=G3k8ZQEG2SGS9kcCZZtsehmpQ9+ViCQekPxH9qRASmkgyincsfNfPmK8bI4fgXK8d1 nFKkNg54CcUssCF68ny8sVlOKL0V59n+pGHgK16Qyj/GlXMQZXUYuXMzSvGnBZXwaHQs 61aqMf8zrQ+oiX+e3CRIBjdJm5MvPP9lJ9BpA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Du59GnBR5Q62IfyguPaciR82Zvv0PR/Hi7y+I3M3i50=; b=WJpQ2GBigmjz+0q+PpSU88l1odRtjBFxeINMHfN2KkcfL9v8QQb9vcTfGCPimU/eKx 9yYG2Tgfy5qJYswG2NMm+1SX/GwhCX1qqvHmq6HJ+cLvFOE5Pf3AfNTsIovZR00sCUXe 5J0iF+xoKnKqsSKpTjry26ZjXK0+DcjGSTVMyrXXf7QWMZjSg7MHc7hfhDGw/mXCJBdk AuXPl1JoGlnr9aSRBEiym+HuhcjyGm4ESlv/Hk7ChcRpqFol7YHwqRFyUQxmbrElpYML 7gIeR9NxAyDudujDxwa3IO1UzAC18OERHEOrkITScu2a/pGxDrjEdEOQg1fix6jGL/Nd Vyxw== X-Gm-Message-State: AE9vXwMbIDPS0uYUoqm6Mgq1EcRJKnuR7aw1Dx8TAG4Hiz3gYAuRxaJG1CbhwUtTSrREIvxiSIl9Ld2+5TYhhO3v X-Received: by 10.25.147.17 with SMTP id v17mr5413409lfd.9.1473685594795; Mon, 12 Sep 2016 06:06:34 -0700 (PDT) MIME-Version: 1.0 Received: by 10.25.157.204 with HTTP; Mon, 12 Sep 2016 06:06:34 -0700 (PDT) In-Reply-To: References: <1473674479-20207-1-git-send-email-ard.biesheuvel@linaro.org> <1473674479-20207-2-git-send-email-ard.biesheuvel@linaro.org> <20160912102356.GS16080@bivouac.eciton.net> From: Ard Biesheuvel Date: Mon, 12 Sep 2016 14:06:34 +0100 Message-ID: To: Laszlo Ersek Cc: Leif Lindholm , "edk2-devel@lists.01.org" Subject: Re: [PATCH 1/3] ArmPkg: add driver to force 64-bit MMIO BARs to be allocated above 4 GB X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Sep 2016 13:06:37 -0000 Content-Type: text/plain; charset=UTF-8 On 12 September 2016 at 13:29, Laszlo Ersek wrote: > On 09/12/16 12:23, Leif Lindholm wrote: >> On Mon, Sep 12, 2016 at 11:01:17AM +0100, Ard Biesheuvel wrote: >>> By default, the generic PciBusDxe driver uses the presence of a ROM BAR >>> as a hint that all MMIO BARs should be allocated in the 32-bit region, >>> even the ones that could be allocated above 4 GB. The reason is that >>> such a ROM BAR may contain code that runs in the context of a legacy >>> BIOS (i.e., a CSM), and allocating the 64-bit BARs above 4 GB may put >>> them out of reach. >>> >>> Of course, none of this matters on ARM, and so we can unconditionally >>> override this decision. So take the OVMF implementation of the >>> IncompatiblePciDeviceSupportDxe driver, rip out the bits that care >>> about the presence of a legacy BIOS, and wire it up to the ArmPkg PCI >>> PCDs. >> >> While I agree this solves a real problem, we're now at a point where >> we're adding multiple new implementations of a library > > s/library/protocol/ > >> to give >> standards-compliant platforms the ability to circumvent legacy >> workarounds in core code. >> >> Should this not be the other way around? > > Yes, it should be. > > ... IMO, the specific degradation logic in the PCI Bus driver is > simplistic; for example, if it finds a PCI oprom for the device, it > could parse it to see if there's a legacy BIOS image (too) in that > oprom, before summarily degrading the resource. > > But, implementing that in PciBusDxe now, or even eliminating the quirk > completely, would require all downstreams to make up for it in their > IncompatiblePciDeviceSupportDxe drivers. Getting shortcuts into core > code is easy (initally), getting them out is not so much. > True, but I have sent out a patch nonetheless. Let's at least have the discussion. > So, I completely agree with you, especially that we're abusing this > protocol as a resource allocation hint -- as the patches themselves say, > there's nothing "incompatible" about the devices we handle. But, > changing the status quo at this stage looks really impractical. > >> And if not, is that not a policy decision that suggests the workaround >> belongs as a core library anyway? > > Based on the PI spec (1.4a, vol 5, "11.5 Incompatible PCI Device Support > Protocol"), the protocol should be provided by a platform driver. > > Now, PciHostBridgeDriverDxe is also a platform driver. I think it's > different from this case because PciHostBridgeDriverDxe has so much > common / shared logic that it could be meaningfully split into a > PciHostBridgeLib class, and a common driver base (linking against the > lib class). For IncompatiblePciDeviceSupportDxe however, at least on the > level that we work with it, there's zero common logic to factor out. > (Okay, yes, you could centralize one or two PcdGet() calls, and perhaps > one of the PCDs could be moved to a central .DEC file.) > > I don't like this situation either, but it's how ABI specs designed for > the proprietary world work in practice, I guess. >