All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
To: Sai Praneeth Prakhya
	<sai.praneeth.prakhya-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jlee-IBi9RG/b67k@public.gmane.org,
	bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org,
	ricardo.neri-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	ravi.v.shankar-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	fenghua.yu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Subject: Re: [PATCH 4/4] efi: Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE is detected
Date: Wed, 7 Dec 2016 13:36:45 +0000	[thread overview]
Message-ID: <20161207133645.GB17720@codeblueprint.co.uk> (raw)
In-Reply-To: <1481051763-8705-5-git-send-email-sai.praneeth.prakhya-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

On Tue, 06 Dec, at 11:16:03AM, Sai Praneeth Prakhya wrote:
> From: Sai Praneeth <sai.praneeth.prakhya-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> UEFI specification v2.6 recommends not to use
> "EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA"
> attribute of EFI_PROPERTIES_TABLE. Presently, this is the *only* bit
> defined in EFI_PROPERTIES_TABLE. This bit implies that EFI Runtime code
> and data regions of an executable image are separate and are aligned as
> specified in spec. Please refer to "EFI_PROPERTIES_TABLE" in section 4.6
> of UEFI specification v2.6 for more information on this table.
> 
> UEFI v2.6 introduces EFI_MEMORY_ATTRIBUTES_TABLE and is intended to
> replace EFI_PROPERTIES_TABLE. If EFI_MEMORY_ATTRIBUTES_TABLE is found we
> skip updating of efi runtime region mappings based on
> EFI_PROPERTIES_TABLE, so let's also skip parsing of EFI_PROPERTIES_TABLE
> if we find EFI_MEMORY_ATTRIBUTES_TABLE because we are not using this
> table anyways. The only caveat here is, if further versions of UEFI spec
> adds some more bits (hence some more attributes) to EFI_PROPERTIES_TABLE
> then we might need to parse it again, otherwise there is no good in
> doing that. We can also expect that the same attributes might be reflected in
> EFI_MEMORY_ATTRIBUTES_TABLE and hence saving us from parsing
> EFI_PROPERTIES_TABLE again.
> 
> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Lee, Chun-Yi <jlee-IBi9RG/b67k@public.gmane.org>
> Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
> Cc: Ricardo Neri <ricardo.neri-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Ravi Shankar <ravi.v.shankar-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Fenghua Yu <fenghua.yu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/firmware/efi/efi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

I see where you're coming from with this patch, but I think it's
unnecessary. Turning on/off parsing of Table A based on existence of
Table B just seems like extra work.

WARNING: multiple messages have this Message-ID (diff)
From: Matt Fleming <matt@codeblueprint.co.uk>
To: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org,
	jlee@suse.com, bp@alien8.de, ricardo.neri@intel.com,
	ard.biesheuvel@linaro.org, ravi.v.shankar@intel.com,
	fenghua.yu@intel.com
Subject: Re: [PATCH 4/4] efi: Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE is detected
Date: Wed, 7 Dec 2016 13:36:45 +0000	[thread overview]
Message-ID: <20161207133645.GB17720@codeblueprint.co.uk> (raw)
In-Reply-To: <1481051763-8705-5-git-send-email-sai.praneeth.prakhya@intel.com>

On Tue, 06 Dec, at 11:16:03AM, Sai Praneeth Prakhya wrote:
> From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> 
> UEFI specification v2.6 recommends not to use
> "EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA"
> attribute of EFI_PROPERTIES_TABLE. Presently, this is the *only* bit
> defined in EFI_PROPERTIES_TABLE. This bit implies that EFI Runtime code
> and data regions of an executable image are separate and are aligned as
> specified in spec. Please refer to "EFI_PROPERTIES_TABLE" in section 4.6
> of UEFI specification v2.6 for more information on this table.
> 
> UEFI v2.6 introduces EFI_MEMORY_ATTRIBUTES_TABLE and is intended to
> replace EFI_PROPERTIES_TABLE. If EFI_MEMORY_ATTRIBUTES_TABLE is found we
> skip updating of efi runtime region mappings based on
> EFI_PROPERTIES_TABLE, so let's also skip parsing of EFI_PROPERTIES_TABLE
> if we find EFI_MEMORY_ATTRIBUTES_TABLE because we are not using this
> table anyways. The only caveat here is, if further versions of UEFI spec
> adds some more bits (hence some more attributes) to EFI_PROPERTIES_TABLE
> then we might need to parse it again, otherwise there is no good in
> doing that. We can also expect that the same attributes might be reflected in
> EFI_MEMORY_ATTRIBUTES_TABLE and hence saving us from parsing
> EFI_PROPERTIES_TABLE again.
> 
> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> Cc: Lee, Chun-Yi <jlee@suse.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Ricardo Neri <ricardo.neri@intel.com>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ravi Shankar <ravi.v.shankar@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  drivers/firmware/efi/efi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

I see where you're coming from with this patch, but I think it's
unnecessary. Turning on/off parsing of Table A based on existence of
Table B just seems like extra work.

  parent reply	other threads:[~2016-12-07 13:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-06 19:15 [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Sai Praneeth Prakhya
2016-12-06 19:15 ` Sai Praneeth Prakhya
2016-12-06 19:16 ` [PATCH 1/4] efi: Make EFI_MEMORY_ATTRIBUTES_TABLE initialization common across all architectures Sai Praneeth Prakhya
2016-12-07 13:28   ` Matt Fleming
2016-12-06 19:16 ` [PATCH 2/4] efi: Introduce EFI_MEM_ATTR bit and set it from memory attributes table Sai Praneeth Prakhya
2016-12-06 19:16 ` [PATCH 3/4] x86/efi: Add support for EFI_MEMORY_ATTRIBUTES_TABLE Sai Praneeth Prakhya
2016-12-06 19:16 ` [PATCH 4/4] efi: Skip parsing of EFI_PROPERTIES_TABLE if EFI_MEMORY_ATTRIBUTES_TABLE is detected Sai Praneeth Prakhya
     [not found]   ` <1481051763-8705-5-git-send-email-sai.praneeth.prakhya-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-12-07 13:36     ` Matt Fleming [this message]
2016-12-07 13:36       ` Matt Fleming
     [not found] ` <1481051763-8705-1-git-send-email-sai.praneeth.prakhya-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-12-07 13:26   ` [PATCH 0/4] UEFI: EFI_MEMORY_ATTRIBUTES_TABLE support for x86 Matt Fleming
2016-12-07 13:26     ` Matt Fleming
2016-12-07 13:56   ` Matt Fleming
2016-12-07 13:56     ` Matt Fleming
     [not found]     ` <20161207135646.GC17720-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-12-07 19:01       ` Sai Praneeth Prakhya
2016-12-07 19:01         ` Sai Praneeth Prakhya
2016-12-07 20:04         ` Matt Fleming
2016-12-07 20:13           ` Sai Praneeth Prakhya
     [not found]             ` <1481141608.15606.133.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-12-13  6:47               ` joeyli
2016-12-13  6:47                 ` joeyli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161207133645.GB17720@codeblueprint.co.uk \
    --to=matt-mf/unelci9gs6ibeejttw/xrex20p6io@public.gmane.org \
    --cc=ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org \
    --cc=fenghua.yu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=jlee-IBi9RG/b67k@public.gmane.org \
    --cc=linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=ravi.v.shankar-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=ricardo.neri-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=sai.praneeth.prakhya-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.