From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web10.10352.1600857386553226080 for ; Wed, 23 Sep 2020 03:36:26 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=CtRHttFW; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1600857385; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=4ybCF4CCFMlO1XaAF6SB3CC6dmb2GPdEPOlsOdbYjTY=; b=CtRHttFWjjqUneM4MyL4Qd/921+aYnpJya4DUbD0ZpoF3YjMB1ehPkGtMVZTbtHZCj9xL8 /kxxN96faFb0OU9qvFbY6ENraZquCX94gPFHBIsk/8Z9ESr5TRFX3N8Gg30NEmykYgpWZW 6r85uGRQgnybUC3CKdny+LoGQyGmDLU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-324-L4pRtjMWPAKVBCnr-AlEkA-1; Wed, 23 Sep 2020 06:36:21 -0400 X-MC-Unique: L4pRtjMWPAKVBCnr-AlEkA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 213531021202; Wed, 23 Sep 2020 10:36:20 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-233.ams2.redhat.com [10.36.112.233]) by smtp.corp.redhat.com (Postfix) with ESMTP id BDBA59CBA; Wed, 23 Sep 2020 10:36:17 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/Mem: Memory paging related attributes must be separated from others To: devel@edk2.groups.io, jacek.kukiello@intel.com References: <20200923080829.6884-1-jacek.kukiello@intel.com> <20200923080829.6884-2-jacek.kukiello@intel.com> Cc: Michael D Kinney , Liming Gao , Zhiguang Liu , Oleksiy Yakovlev , "Ard Biesheuvel (ARM address)" , Jian J Wang , Hao A Wu , Dandan Bi From: "Laszlo Ersek" Message-ID: <448ebada-9cb5-3ece-a6c5-3004ee06e1c7@redhat.com> Date: Wed, 23 Sep 2020 12:36:16 +0200 MIME-Version: 1.0 In-Reply-To: <20200923080829.6884-2-jacek.kukiello@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US On 09/23/20 10:08, Malgorzata Kukiello wrote: > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2982 > > During a recent change (bda18f5ed04f5317ab2bdc9da4530c078b33cc63) that changed > the way memory attributes are handled it affected a function in Page.c that > would clear all memory attributes even security related ones > > Signed-off-by: Malgorzata Kukiello > --- > MdePkg/Include/Uefi/UefiSpec.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/MdePkg/Include/Uefi/UefiSpec.h b/MdePkg/Include/Uefi/UefiSpec.h > index 05b82e0be1..2b1b72d862 100644 > --- a/MdePkg/Include/Uefi/UefiSpec.h > +++ b/MdePkg/Include/Uefi/UefiSpec.h > @@ -113,7 +113,8 @@ typedef enum { > // Attributes bitmasks, grouped by type > // > #define EFI_CACHE_ATTRIBUTE_MASK (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP) > -#define EFI_MEMORY_ATTRIBUTE_MASK (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO | EFI_MEMORY_SP | EFI_MEMORY_CPU_CRYPTO) > +#define EFI_MEMORY_ACCESS_MASK (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO) > +#define EFI_MEMORY_ATTRIBUTE_MASK (EFI_MEMORY_ACCESS_MASK | EFI_MEMORY_SP | EFI_MEMORY_CPU_CRYPTO) > > /// > /// Memory descriptor version number. > (1) The subject line of this patch is wrong. The patch modifies MdePkg, not MdeModulePkg. I suggest: MdePkg/UefiSpec: separate page access bitmask from SP and CRYPTO caps (2) My points (1) through (4) that I made under the other patch in this series apply here as well, wrt. CC'ing maintainers / reviewers, commit hash reference, structuring this posting etc. Note that this patch will have a different set of required CC's. (3) The commit message is lacking, in my opinion. The point is that our workaround, in the UEFI memmap construction, must not clear the SP and CRYPTO bits, because OSes do correctly interpret SP and CRYPTO as capabilities. For this reason, the SP and CRYPTO bits should be separated from the bitmask that we use for hiding the page-access attributes. Thanks Laszlo