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 014AD21DFA901 for ; Thu, 30 Mar 2017 09:17:02 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 69903C05B1D3; Thu, 30 Mar 2017 16:17:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 69903C05B1D3 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 69903C05B1D3 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-143.phx2.redhat.com [10.3.116.143]) by smtp.corp.redhat.com (Postfix) with ESMTP id DC602800C2; Thu, 30 Mar 2017 16:16:59 +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> Cc: "edk2-devel@lists.01.org" , Marc Zyngier , Mark Rutland , Drew Jones , Jon Masters From: Laszlo Ersek Message-ID: Date: Thu, 30 Mar 2017 18:16:58 +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.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Thu, 30 Mar 2017 16:17:01 +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: Thu, 30 Mar 2017 16:17:02 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 03/30/17 10: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. Good idea. In order to save some adrenaline down the line for both of us, I have some suggestions: - Please try clarify with the reporter of the regression what he or she prefers as a solution, before giving me a heart attack :) Regarding the "NACK" in all caps -- I wasn't yelling, that's just a way of formatting we use downstream (we mostly use ACK and NACK), which regrettably leaked into my upstream correspondence. Sorry about the confusion. (NB, I'm not apologizing for nacking v1 per se. There's a world of difference between exposing the exlusivity with some additional switch and getting cold feet on the exclusivity en bloc. In my opinion.) - Please always include an incremental v2, v3, ... notes / info section in the patch (or blurb, if there is one), so I can more easily find out about the inter-version changes near the end of a 14 hour work day. (When I finally went to bed my uptime was past 18 hours.) In this instance, there was no v2 info section, and I thought you only addressed the superficial technical suggestions that I made under v1. *Importantly*, this is not to say that I did not do a shit job at reviewing v2. I absolutely did. Lack of a v2 info section in the patch / blurb is no excuse for missing the -- happy! -- elefant in the room. It's quite embarrassing; I'm sorry about that. I'll strive to do the formal v1<->v2 diffing in the future unconditionally. Thanks, Laszlo