Re: [edk2] [PATCH] ArmVirtPkg: ArmVirtQemu: disable PropertiesTable memory protection feature

2016-02-18 Thread Ard Biesheuvel 
On 18 February 2016 at 13:17, Laszlo Ersek  wrote:
> On 02/18/16 12:56, Ard Biesheuvel wrote:
>> The PropertiesTable feature is poorly named, since the feature this PCD
>> controls is only a single bit in its MemoryProtectionAttribute member,
>> called EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA.
>>
>> This feature causes breakage on legacy OSes that assume that each memory
>> region in the UEFI memory map is completely independent, which is no longer
>> the case with this feature enabled. For this reason, the UEFI spec now
>> recommends not to use this feature, and use the MemoryAttributes table
>> instead. For this reason, support for the feature will not be implemented
>> in arm64 or ARM Linux, and enabling it here is pointless.
>>
>> So set PcdPropertiesTableEnable to FALSE.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmVirtPkg/ArmVirtQemu.dsc | 12 
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
>> index e2641fd2c289..7ff1bd4074a8 100644
>> --- a/ArmVirtPkg/ArmVirtQemu.dsc
>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
>> @@ -162,6 +162,18 @@ [PcdsFixedAtBuild.common]
>>#
>>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>>
>> +  # The PropertiesTable feature is poorly named, since the feature this PCD
>> +  # controls is only a single bit in its MemoryProtectionAttribute member,
>> +  # called EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA.
>> +  #
>> +  # This feature causes breakage on legacy OSes that assume that each memory
>> +  # region in the UEFI memory map is completely independent, which is no 
>> longer
>> +  # the case with this feature enabled. For this reason, the UEFI spec now
>> +  # recommends not to use this feature, and use the MemoryAttributes table
>> +  # instead. For this reason, support for the feature will not be 
>> implemented in
>> +  # arm64 or ARM Linux, and enabling it here is pointless.
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
>> +
>>  [PcdsFixedAtBuild.AARCH64]
>>gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>>
>>
>
> You reviewed Jiewen's patch:
>
>   MdePkg: Change PcdPropertiesTableEnable default value to FALSE
>   http://thread.gmane.org/gmane.comp.bios.edk2.devel/7681
>
> So why is this necessary? Isn't it enough to inherit the FALSE default
> from "MdeModulePkg/MdeModulePkg.dec"?
>

Yes, it is. That completely slipped my mind, and I still noticed the
PROP= entry in the kernel boot log next to the MEMATTR= entry that I
am adding atm.

So indeed, please disregard.

-- 
Ard.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmVirtPkg: ArmVirtQemu: disable PropertiesTable memory protection feature

2016-02-18 Thread Laszlo Ersek 
On 02/18/16 12:56, Ard Biesheuvel wrote:
> The PropertiesTable feature is poorly named, since the feature this PCD
> controls is only a single bit in its MemoryProtectionAttribute member,
> called EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA.
> 
> This feature causes breakage on legacy OSes that assume that each memory
> region in the UEFI memory map is completely independent, which is no longer
> the case with this feature enabled. For this reason, the UEFI spec now
> recommends not to use this feature, and use the MemoryAttributes table
> instead. For this reason, support for the feature will not be implemented
> in arm64 or ARM Linux, and enabling it here is pointless.
> 
> So set PcdPropertiesTableEnable to FALSE.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirtQemu.dsc | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index e2641fd2c289..7ff1bd4074a8 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -162,6 +162,18 @@ [PcdsFixedAtBuild.common]
>#
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>  
> +  # The PropertiesTable feature is poorly named, since the feature this PCD
> +  # controls is only a single bit in its MemoryProtectionAttribute member,
> +  # called EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA.
> +  #
> +  # This feature causes breakage on legacy OSes that assume that each memory
> +  # region in the UEFI memory map is completely independent, which is no 
> longer
> +  # the case with this feature enabled. For this reason, the UEFI spec now
> +  # recommends not to use this feature, and use the MemoryAttributes table
> +  # instead. For this reason, support for the feature will not be 
> implemented in
> +  # arm64 or ARM Linux, and enabling it here is pointless.
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
> +
>  [PcdsFixedAtBuild.AARCH64]
>gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>  
> 

You reviewed Jiewen's patch:

  MdePkg: Change PcdPropertiesTableEnable default value to FALSE
  http://thread.gmane.org/gmane.comp.bios.edk2.devel/7681

So why is this necessary? Isn't it enough to inherit the FALSE default
from "MdeModulePkg/MdeModulePkg.dec"?

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ArmVirtPkg: ArmVirtQemu: disable PropertiesTable memory protection feature

2016-02-18 Thread Ard Biesheuvel 
The PropertiesTable feature is poorly named, since the feature this PCD
controls is only a single bit in its MemoryProtectionAttribute member,
called EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA.

This feature causes breakage on legacy OSes that assume that each memory
region in the UEFI memory map is completely independent, which is no longer
the case with this feature enabled. For this reason, the UEFI spec now
recommends not to use this feature, and use the MemoryAttributes table
instead. For this reason, support for the feature will not be implemented
in arm64 or ARM Linux, and enabling it here is pointless.

So set PcdPropertiesTableEnable to FALSE.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtQemu.dsc | 12 
 1 file changed, 12 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index e2641fd2c289..7ff1bd4074a8 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -162,6 +162,18 @@ [PcdsFixedAtBuild.common]
   #
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
 
+  # The PropertiesTable feature is poorly named, since the feature this PCD
+  # controls is only a single bit in its MemoryProtectionAttribute member,
+  # called EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA.
+  #
+  # This feature causes breakage on legacy OSes that assume that each memory
+  # region in the UEFI memory map is completely independent, which is no longer
+  # the case with this feature enabled. For this reason, the UEFI spec now
+  # recommends not to use this feature, and use the MemoryAttributes table
+  # instead. For this reason, support for the feature will not be implemented 
in
+  # arm64 or ARM Linux, and enabling it here is pointless.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
+
 [PcdsFixedAtBuild.AARCH64]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
 
-- 
2.5.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel