From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 375B721DFA7B7 for ; Fri, 31 Mar 2017 03:46:39 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 69C8961D38; Fri, 31 Mar 2017 10:46:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 69C8961D38 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 69C8961D38 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-31.phx2.redhat.com [10.3.116.31]) by smtp.corp.redhat.com (Postfix) with ESMTP id A40501712C; Fri, 31 Mar 2017 10:46:36 +0000 (UTC) To: Ard Biesheuvel References: <20170329175039.29635-1-ard.biesheuvel@linaro.org> <18095962-76eb-7337-969d-4f6080dff4d7@redhat.com> <41a87740-7634-bfde-d2fb-3767a5c33140@redhat.com> <6d0f6a54-c893-00a9-2181-29a5d766f4a3@redhat.com> Cc: Marc Zyngier , "edk2-devel@lists.01.org" , Mark Rutland , Drew Jones , Jon Masters From: Laszlo Ersek Message-ID: <01cc3201-1dbd-3cd8-1b19-270117adbdc9@redhat.com> Date: Fri, 31 Mar 2017 12:46:35 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 31 Mar 2017 10:46:38 +0000 (UTC) Subject: Re: [PATCH v2] ArmVirtPkg/PlatformHasAcpiDtDxe: allow guest level ACPI disable override X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 Mar 2017 10:46:39 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 03/31/17 12:16, Ard Biesheuvel wrote: > On 31 March 2017 at 10:59, Laszlo Ersek wrote: >> On 03/31/17 11:20, Marc Zyngier wrote: >>> On 30/03/17 09:40, Ard Biesheuvel wrote: >>>> On 29 March 2017 at 20:35, Laszlo Ersek wrote: >>>>> On 03/29/17 21:10, Ard Biesheuvel wrote: >>>> [...] >>>>>> >>>>>> How on earth is having two ways to disable ACPI rather than one going >>>>>> to cause fragmentation? Unlike v1, this patch does not allow you to >>>>>> expose both DT and ACPI tables at the same time. >>>>> >>>>> Oopsie daisy. You actually updated the commit message too. (I have now >>>>> formally diffed v1 vs. v2, including commit msg -- I generally do that >>>>> when reviewing incremental versions of patches, but it has been a very >>>>> long day, and I failed to get my mind off the track set up by v1). I got >>>>> really no good explanation for missing the fundamental logic change >>>>> between v1 and v2. As you say, version 2 preserves the mutual exclusion >>>>> between DT and ACPI that I'm so annoyingly obsessed about. Thank you for >>>>> the update. >>>>> >>>>> Reviewed-by: Laszlo Ersek >>>>> >>>> >>>> Thanks Laszlo. I am glad we have a solution we can both live with. >>>> >>>> I will wait for Marc to confirm that this works as expected for him. >>> >>> [ This email won't make it on the edk2 list because it filters out >>> non-subscribers. Boo! ] >> >> I fully agree: boo. I've raised this several times in the past, because >> it is rude to people with occasional interest, and diverges sharply from >> the custom on most other open source development lists. But, I had no >> success in changing the policy. I really don't understand what other >> subscribers are worried about. Spam is generally not a problem. >> >>> This patch does a very good job at restoring a functionality I'd have >>> otherwise lost. So for the record: >>> >>> Tested-by: Marc Zyngier >>> Acked-by: Marc Zyngier >>> >>> My only comment is that it that the enabling method is slightly cryptic, >>> and suffers from a lack of documentation. For example, it doesn't seem >>> to appear in Build/ArmVirtQemu-AARCH64/RELEASE_GCC5/FV/Guid.xref, which >>> is where I'd have expected to find it. >> >> That happens because BaseTools approaches the Guid.xref file generation >> from the direction of modules -- it goes over all modules and checks >> what plain GUIDs, protocol GUIDs, and PPI GUIDs it uses. >> >> However, in this case, gArmVirtVariableGuid is never directly used in >> PlatformHasAcpiDtDxe -- the driver never directly reads the UEFI >> variable in question, so it doesn't need the GUID for naming the >> variable's namespace. Instead, the connection to UEFI variables is made >> in the "ArmVirtQemu.dsc" platform description file; that's where the >> GUID is also named. >> >> [PcdsDynamicHii] >> gArmVirtTokenSpaceGuid.PcdForceNoAcpi|L"ForceNoAcpi"|gArmVirtVariableGuid|0x0|FALSE|NV,BS >> >> I guess we could file a BaseTools enhancement request to print out the >> variable namespace GUIDs used with dynamic HII PCDs. Ard, what do you think? >> > > I guess this is an omission, but I have never used Guid.xref in my > life so I have no strong feelings as to how this should be fixed. > I filed ; we'll see how far it goes. Thanks Laszlo