From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web08.923.1615578744702495508 for ; Fri, 12 Mar 2021 11:52:24 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=FAfy3Et4; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id E270E64DC4 for ; Fri, 12 Mar 2021 19:52:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1615578744; bh=+Wsuuc+qT/z9jUXJpKKbUuvg+lcQLtuK2ckSu4hg47I=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=FAfy3Et4aiyDUsA2XQ6tq6MPdS0xy+wfY42awB2DmFoYkjCJx+/eb8A8aeldUa7Ob W6N6UxBXbGQFJLrYvMqVvnLWwtKpeMDr2EQFwFmz/TxWFwvr8i6or0uhQU7RaPdVdm q7qErSgcsgTa/wLHy/HP0IXbOO6J4jmtHuAasw8zwcz+nHq8JQAOID3vv/ZJ9xlHwI FB3WS5ipBE9A56l+ajxaOnK/4E6xZhjr6vFe0EiIRyST3+RFRtAALORXxPint6wnBc H6mv2ldMg9N84rOKEdP5wuOZnj3oxRj1fkNklKNsNrzZScw6Rh6TQinvH/sdprzLxb 4/oprSfgz4Ymg== Received: by mail-oi1-f174.google.com with SMTP id w65so28025388oie.7 for ; Fri, 12 Mar 2021 11:52:23 -0800 (PST) X-Gm-Message-State: AOAM532AL9pwlOLyTzXMre/6ZqYxhdVhk6o6Uk610ZSGrjy4zcqmiu0/ 00Dm0VBS9wFnzsBWpXV81nIyzN//xCZl/UKymOo= X-Google-Smtp-Source: ABdhPJxswfmNMr/Lr1EV7kScFdY/+sSC38FGehy5WvtUgwsvWc65w2Ma2tze7JUVUZ0/JTyqEjBYkq/OKb/RSng46m4= X-Received: by 2002:aca:538c:: with SMTP id h134mr10963080oib.174.1615578743073; Fri, 12 Mar 2021 11:52:23 -0800 (PST) MIME-Version: 1.0 References: <5363bdf0-afac-73bf-d001-77949916f511@redhat.com> <166B374585A9D8FC.18699@groups.io> <290a35ce-9116-af00-85f4-8df1c5228680@redhat.com> <4841241f-fc6d-6185-efe6-ed9a536534dd@redhat.com> <166B792D1514133B.31346@groups.io> In-Reply-To: From: "Ard Biesheuvel" Date: Fri, 12 Mar 2021 20:52:12 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] Conflicting virtual addresses causing Runtime Services issues To: Jon Nettleton Cc: devel@edk2.groups.io, Laszlo Ersek , Hao A Wu , Liming Gao , "Ard Biesheuvel (TianoCore)" , "Leif Lindholm (Nuvia address)" Content-Type: text/plain; charset="UTF-8" On Fri, 12 Mar 2021 at 07:00, Jon Nettleton wrote: > > On Fri, Mar 12, 2021 at 4:02 AM Jon Nettleton via groups.io > wrote: > > > > On Thu, Mar 11, 2021 at 11:39 PM Ard Biesheuvel wrote: > > > > > > On Thu, 11 Mar 2021 at 23:25, Laszlo Ersek wrote: > > > > > > > > Adding Ard and Leif, comments below: > > > > ... > > > Thanks for inviting me to this party! > > > > > > So the tl;dr here is that some points get converted twice, which > > > usually is not a problem because the virtual address resulting from > > > the conversion is rarely mistaken for a physical address living in a > > > EFI_MEMORY_RUNTIME region. > > > > > > So I agree with Laszlo's assertion that the consumer of a protocol has > > > no business updating its protocol pointers, so this should definitely > > > be fixed in the core VariableRuntime driver. However, given the > > > typical nature of the variable stack, i.e., a platform specfic NOR > > > flash driver combined with the generic FTW and variable drivers, doing > > > so would likely break many out of tree platforms where the NOR flash > > > driver does not bother to update its pointers at all. > > > > Not ideal, but we could add a flag to Runtime Services and let the platform > > specify if it is remapping the FVB. This would allow us to not break legacy > > drivers, but still easily let properly designed platforms bypass this > > behaviour. As time progressed this could be used to for a deprecation > > warning, until it became the default handling of FVB pointer conversion. > > > Something like the attached patch possibly? Frankly, I don't think adding a PCD that does one or the other is the right way to go here. Instead, what we might do is: - record the original values of the pointers at ExitBootServices() time - don't touch the original values but the copies during SetVirtualAddressMap() - in each exposed RuntimeService(), do a runtime check whether the addresses in the protocol struct are equal to the converted copies, and if not, update them. This functionality should be enabled by default, but can be opted out of by a platform if we dedicate a PCD to it that defaults to enabled. At some point, we could add some kind of deprecation warning if the PCD is enabled, and have a path forward to actually retiring it completely.