From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x231.google.com (mail-it0-x231.google.com [IPv6:2607:f8b0:4001:c0b::231]) (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 B9DBB21BC6A24 for ; Fri, 31 Mar 2017 03:48:49 -0700 (PDT) Received: by mail-it0-x231.google.com with SMTP id y18so9647768itc.1 for ; Fri, 31 Mar 2017 03:48:49 -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=BCq8WFwK4GRxgAd883Zz9ctN4i+tL/T5NGf19+3dKnk=; b=TRvWVegTAHPeI2aizS0ufZtzKKrW2IlhFknadSZNQf8VtBopWvjx3Qqf7LfOh+/rhE XaJbU4d6EuXvgQ/UxFVcb6G2ZvtrsjJZCnOquqo8CbYM5OK2XGSaRZYetFVUb6TGZ5b9 u3SPRB0VqtIgDjvPr8q/Bu8WOtyS8wVet9Xpo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=BCq8WFwK4GRxgAd883Zz9ctN4i+tL/T5NGf19+3dKnk=; b=BPgrmvdxkGBOmnm0jDORwSGkNsuy75FkJLqS9AO8ZsfW9dipTSO4SY88x60Pm26Ee6 zSM1ka5TB57kP8GMtZOSa1MxVK68p1/z79k3Zh4T69dzdLTGo8Bb/c3X8cTBh1PdhFnt mKe4Bqg07se99WJuEKJ3WeKb6oWyUGGmqGFwNWzTYWcLoZHMWCZjjLsrD2gVlkORgeBr thqye2y0fM1eYjyBX37BKZCUTuBtpYs4jQjlQWMaRv+q5Wsi74qRmpmGkIBq+qRdn6aI zgD1rK77J2rN/gjO5cTxMVa0bokpG4qx+ZXq9oriZ8xfd3RBvmV4rAR6syltRotYl9UD Xl5g== X-Gm-Message-State: AFeK/H0IXdJUAGoBsWUditsfFI5l2O55iNNoIUURg42FBr/t+f7dMpdIzGQb5/BJg+7JF5od8IrQg//ZBjLHJmI0 X-Received: by 10.36.46.69 with SMTP id i66mr2323959ita.59.1490957329057; Fri, 31 Mar 2017 03:48:49 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.10.27 with HTTP; Fri, 31 Mar 2017 03:48:48 -0700 (PDT) In-Reply-To: References: <20170329175039.29635-1-ard.biesheuvel@linaro.org> <18095962-76eb-7337-969d-4f6080dff4d7@redhat.com> <41a87740-7634-bfde-d2fb-3767a5c33140@redhat.com> From: Ard Biesheuvel Date: Fri, 31 Mar 2017 11:48:48 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Marc Zyngier , Mark Rutland , Drew Jones , Jon Masters 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:48:50 -0000 Content-Type: text/plain; charset=UTF-8 On 30 March 2017 at 17:16, Laszlo Ersek wrote: > 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. > Yeah good point. Keystrokes are cheap ...