From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22b.google.com (mail-it0-x22b.google.com [IPv6:2607:f8b0:4001:c0b::22b]) (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 E37E01A1EFA for ; Thu, 15 Sep 2016 08:53:54 -0700 (PDT) Received: by mail-it0-x22b.google.com with SMTP id o3so102435119ita.1 for ; Thu, 15 Sep 2016 08:53:54 -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=BhqWIM1nI5OkqVYnXOJ+cq3/ZPxaCUm+iIq7P4EjIz4=; b=YH7i8m5z8QkZpEFV0bjksbt29b8ovBQnM1d/pspb3lybPgsjg4yYfDzRt382ILaRF+ V9V+PPcHFDqBQj/704RMkTXOngPyFdW8cKsh22uqg8LRFrgZqYydb/gCIWIXzfh+v2aB 9/fWTkZBL1Nybmazi8rouYbHPievpO6s2riak= 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=BhqWIM1nI5OkqVYnXOJ+cq3/ZPxaCUm+iIq7P4EjIz4=; b=WK7CAKv+Mdp7AnzU2Tbkcystnye+n7F34orrXuwcW960OZKnrkBatIvJJWnDwDCkSH yOqEJ8XT0hEESQHsuP+vLDhsbx3vrHGGB3fbtLGG0W1fkAp07REuAlfI/ach0qQuls5L 1RtnjNu10NYcegf+pCkCqlDL5kYAY3RXv+Z8BOYAR6UbpgFUKqjXqjaNRjTplDK6HFEL 8bPrCUuShfh3yElkrupdTn6vJgnRnhwv1ESeyr0ldpwV4nTG/wqY5/L6GXdppUgOonW4 +lP6Wa2uWcs8cXZSHHAJp7docIxEEErT18/ST5fsa/0gGIjwm61dc9QsVVRD13GoJNfz yLYQ== X-Gm-Message-State: AE9vXwONzVNUC8Jc2Hbz/rlJHJLx4R7ru5rozoX3FHwU2i48AmCNAdo5qiq+FIgQHuoFhq+rTSDQpy/33IJtg94A X-Received: by 10.107.133.17 with SMTP id h17mr17859236iod.148.1473954834018; Thu, 15 Sep 2016 08:53:54 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Thu, 15 Sep 2016 08:53:53 -0700 (PDT) In-Reply-To: <9892B892-284A-40F8-953C-C7830A0C7C55@apple.com> References: <1473951296-19120-1-git-send-email-ard.biesheuvel@linaro.org> <20160915152831.GH16080@bivouac.eciton.net> <9892B892-284A-40F8-953C-C7830A0C7C55@apple.com> From: Ard Biesheuvel Date: Thu, 15 Sep 2016 16:53:53 +0100 Message-ID: To: Andrew Fish Cc: Leif Lindholm , Ruiyu Ni , "Tian, Feng" , edk2-devel-01 , "Gao, Liming" , "Yao, Jiewen" , Mike Kinney , Laszlo Ersek , "Zeng, Star" Subject: Re: [PATCH] MdeModulePkg/PciBusDxe: make OPROM BAR degradation X64 specific 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: Thu, 15 Sep 2016 15:53:55 -0000 Content-Type: text/plain; charset=UTF-8 On 15 September 2016 at 16:48, Andrew Fish wrote: > >> On Sep 15, 2016, at 8:28 AM, Leif Lindholm wrote: >> >> On Thu, Sep 15, 2016 at 03:54:56PM +0100, Ard Biesheuvel wrote: >>> The 'universal' PCI bus driver in MdeModulePkg contains a quirk to >>> degrade 64-bit PCI MMIO BARs to 32-bit in the presence of an option >>> ROM on the same PCI controller. >>> >>> This quirk is highly specific to not just the X64 architecture in general, >>> but to the PC platform in particular, given that only X64 platforms that >>> require legacy PC BIOS compatibility require it. However, making the >>> quirk dependent on the presence of the legacy BIOS protocol met with >>> resistance, due to the fact that it introduces a dependency on the >>> IntelFrameworkModulePkg package. >>> >>> So instead, make the quirk dependent on whether we are compiling for the >>> X64 architecture, by putting the #ifdef MDE_CPU_X64/#endif preprocessor >>> directives around it. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Ard Biesheuvel >> >> I'm happy with the change, but it may be worth putting a comment in > > Do we have to use an #ifdef? Seems like it should be a PCD. Is this for legacy BIOS Option ROM support, or for existing EFI ROMs? > Strictly for being able to execute a legacy BIOS Option ROM in the context of a CSM. Hence my assertion that this is specific to the 64-bit variant of the PC platform. Getting new PCDs introduced is also met with fierce resistance, to the extent that (as this code testifies), we'd rather keep quirks hidden in the code like this, and enable them by default rather than acknowledging their existence by adding explicit controls to enable/disable them. Others have suggested a dynamic PCD, which can be flicked when the legacy BIOS protocol installed. But since X64 is not my primary interested, I will settle for a feature PCD, and #ifdef or anything else that results in this quirk to be disabled by default on 64-bit ARM. Thanks, Ard.