* SetMemorySpaceAttributes with EFI_MEMORY_XP @ 2017-03-20 10:32 Michael Zimmermann 2017-03-20 11:04 ` Ard Biesheuvel 2017-03-20 11:06 ` Laszlo Ersek 0 siblings, 2 replies; 11+ messages in thread From: Michael Zimmermann @ 2017-03-20 10:32 UTC (permalink / raw) To: edk2-devel-01, Laszlo Ersek, Ard Biesheuvel Hi, I didn't test ArmVirtQemuKernel but I'm trying to use some of the code for another platform. So does this call ever succeed with PcdDxeNxMemoryProtectionPolicy being enabled? https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L95 Status = gDS->SetMemorySpaceAttributes (CurBase, CurSize, Attributes); Neither the memory that was added by this Dxe nor the one added automatically by GCD has the EFI_MEMORY_XP capability which causes SetMemorySpaceAttributes to return EFI_UNSUPPORTED. Thanks Michael ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SetMemorySpaceAttributes with EFI_MEMORY_XP 2017-03-20 10:32 SetMemorySpaceAttributes with EFI_MEMORY_XP Michael Zimmermann @ 2017-03-20 11:04 ` Ard Biesheuvel 2017-03-20 11:16 ` Michael Zimmermann 2017-03-20 11:06 ` Laszlo Ersek 1 sibling, 1 reply; 11+ messages in thread From: Ard Biesheuvel @ 2017-03-20 11:04 UTC (permalink / raw) To: Michael Zimmermann; +Cc: edk2-devel-01, Laszlo Ersek On 20 March 2017 at 10:32, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: > Hi, > > I didn't test ArmVirtQemuKernel but I'm trying to use some of the code > for another platform. > So does this call ever succeed with PcdDxeNxMemoryProtectionPolicy > being enabled? > https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L95 > Status = gDS->SetMemorySpaceAttributes (CurBase, CurSize, Attributes); > > Neither the memory that was added by this Dxe nor the one added > automatically by GCD has the EFI_MEMORY_XP capability which causes > SetMemorySpaceAttributes to return EFI_UNSUPPORTED. > That is a very good point. I have been caught by this more than once already (and I did test this, but not as thoroughly as I should have, apparently) This is caused by the unfortunate situation in EDK2 that GCD permission attributes are ambiguous: it does not distinguish between 'the memory controller allows this range to be configured as non-executable' and 'the nature of the contents of this memory region allows it to be mapped without executable attributes', and therefore, RO/XP are never used in the GCD memory space map. The solution is to use the CPU_ARCH_PROTOCOL interface explicitly to set the XP attribute on the memory itself (but not on the descriptors in the GCD or UEFI memory maps). I will spin a patch to fix this. Thanks, Ard. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SetMemorySpaceAttributes with EFI_MEMORY_XP 2017-03-20 11:04 ` Ard Biesheuvel @ 2017-03-20 11:16 ` Michael Zimmermann 2017-03-20 11:20 ` Ard Biesheuvel 0 siblings, 1 reply; 11+ messages in thread From: Michael Zimmermann @ 2017-03-20 11:16 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: edk2-devel-01, Laszlo Ersek Ard, why is SetMSetMemorySpaceAttributes being called in first place? (ignoring the recent NX patch) Looking at the initial GCD, it looks like unused memory usually doesn't have any attributes set anyway. Thanks Michael On Mon, Mar 20, 2017 at 12:04 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 20 March 2017 at 10:32, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: >> Hi, >> >> I didn't test ArmVirtQemuKernel but I'm trying to use some of the code >> for another platform. >> So does this call ever succeed with PcdDxeNxMemoryProtectionPolicy >> being enabled? >> https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L95 >> Status = gDS->SetMemorySpaceAttributes (CurBase, CurSize, Attributes); >> >> Neither the memory that was added by this Dxe nor the one added >> automatically by GCD has the EFI_MEMORY_XP capability which causes >> SetMemorySpaceAttributes to return EFI_UNSUPPORTED. >> > > That is a very good point. I have been caught by this more than once > already (and I did test this, but not as thoroughly as I should have, > apparently) > > This is caused by the unfortunate situation in EDK2 that GCD > permission attributes are ambiguous: it does not distinguish between > 'the memory controller allows this range to be configured as > non-executable' and 'the nature of the contents of this memory region > allows it to be mapped without executable attributes', and therefore, > RO/XP are never used in the GCD memory space map. > > The solution is to use the CPU_ARCH_PROTOCOL interface explicitly to > set the XP attribute on the memory itself (but not on the descriptors > in the GCD or UEFI memory maps). I will spin a patch to fix this. > > Thanks, > Ard. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SetMemorySpaceAttributes with EFI_MEMORY_XP 2017-03-20 11:16 ` Michael Zimmermann @ 2017-03-20 11:20 ` Ard Biesheuvel 2017-03-20 11:38 ` Laszlo Ersek 0 siblings, 1 reply; 11+ messages in thread From: Ard Biesheuvel @ 2017-03-20 11:20 UTC (permalink / raw) To: Michael Zimmermann; +Cc: edk2-devel-01, Laszlo Ersek On 20 March 2017 at 11:16, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: > Ard, why is SetMSetMemorySpaceAttributes being called in first place? > (ignoring the recent NX patch) > Looking at the initial GCD, it looks like unused memory usually > doesn't have any attributes set anyway. > Originally, we added the new memory with EFI_MEMORY_WB|EFI_MEMORY_WT|EFI_MEMORY_WC|EFI_MEMORY_UC capabilities, and selected the EFI_MEMORY_WB attribute with the call to gDS->SetMemorySpaceAttributes. Later, we removed all capabilities expect EFI_MEMORY_WB, since the other ones cannot be supported under virtualization with KVM. Whether that makes the SetMemorySpaceAttributes redundant is not entirely clear to me, but it does appear the adding the memory does the right thing wrt non-exec permissions if the policy is enabled. So perhaps we can simply drop this call? > On Mon, Mar 20, 2017 at 12:04 PM, Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: >> On 20 March 2017 at 10:32, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: >>> Hi, >>> >>> I didn't test ArmVirtQemuKernel but I'm trying to use some of the code >>> for another platform. >>> So does this call ever succeed with PcdDxeNxMemoryProtectionPolicy >>> being enabled? >>> https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L95 >>> Status = gDS->SetMemorySpaceAttributes (CurBase, CurSize, Attributes); >>> >>> Neither the memory that was added by this Dxe nor the one added >>> automatically by GCD has the EFI_MEMORY_XP capability which causes >>> SetMemorySpaceAttributes to return EFI_UNSUPPORTED. >>> >> >> That is a very good point. I have been caught by this more than once >> already (and I did test this, but not as thoroughly as I should have, >> apparently) >> >> This is caused by the unfortunate situation in EDK2 that GCD >> permission attributes are ambiguous: it does not distinguish between >> 'the memory controller allows this range to be configured as >> non-executable' and 'the nature of the contents of this memory region >> allows it to be mapped without executable attributes', and therefore, >> RO/XP are never used in the GCD memory space map. >> >> The solution is to use the CPU_ARCH_PROTOCOL interface explicitly to >> set the XP attribute on the memory itself (but not on the descriptors >> in the GCD or UEFI memory maps). I will spin a patch to fix this. >> >> Thanks, >> Ard. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SetMemorySpaceAttributes with EFI_MEMORY_XP 2017-03-20 11:20 ` Ard Biesheuvel @ 2017-03-20 11:38 ` Laszlo Ersek 2017-03-20 14:08 ` Ard Biesheuvel 0 siblings, 1 reply; 11+ messages in thread From: Laszlo Ersek @ 2017-03-20 11:38 UTC (permalink / raw) To: Ard Biesheuvel, Michael Zimmermann; +Cc: edk2-devel-01 On 03/20/17 12:20, Ard Biesheuvel wrote: > On 20 March 2017 at 11:16, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: >> Ard, why is SetMSetMemorySpaceAttributes being called in first place? >> (ignoring the recent NX patch) >> Looking at the initial GCD, it looks like unused memory usually >> doesn't have any attributes set anyway. >> > > Originally, we added the new memory with > EFI_MEMORY_WB|EFI_MEMORY_WT|EFI_MEMORY_WC|EFI_MEMORY_UC capabilities, > and selected the EFI_MEMORY_WB attribute with the call to > gDS->SetMemorySpaceAttributes. Later, we removed all capabilities > expect EFI_MEMORY_WB, since the other ones cannot be supported under > virtualization with KVM. > > Whether that makes the SetMemorySpaceAttributes redundant is not > entirely clear to me, but it does appear the adding the memory does > the right thing wrt non-exec permissions if the policy is enabled. So > perhaps we can simply drop this call? Won't that turn off caching for the memory just added? Thanks Laszlo > > >> On Mon, Mar 20, 2017 at 12:04 PM, Ard Biesheuvel >> <ard.biesheuvel@linaro.org> wrote: >>> On 20 March 2017 at 10:32, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: >>>> Hi, >>>> >>>> I didn't test ArmVirtQemuKernel but I'm trying to use some of the code >>>> for another platform. >>>> So does this call ever succeed with PcdDxeNxMemoryProtectionPolicy >>>> being enabled? >>>> https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L95 >>>> Status = gDS->SetMemorySpaceAttributes (CurBase, CurSize, Attributes); >>>> >>>> Neither the memory that was added by this Dxe nor the one added >>>> automatically by GCD has the EFI_MEMORY_XP capability which causes >>>> SetMemorySpaceAttributes to return EFI_UNSUPPORTED. >>>> >>> >>> That is a very good point. I have been caught by this more than once >>> already (and I did test this, but not as thoroughly as I should have, >>> apparently) >>> >>> This is caused by the unfortunate situation in EDK2 that GCD >>> permission attributes are ambiguous: it does not distinguish between >>> 'the memory controller allows this range to be configured as >>> non-executable' and 'the nature of the contents of this memory region >>> allows it to be mapped without executable attributes', and therefore, >>> RO/XP are never used in the GCD memory space map. >>> >>> The solution is to use the CPU_ARCH_PROTOCOL interface explicitly to >>> set the XP attribute on the memory itself (but not on the descriptors >>> in the GCD or UEFI memory maps). I will spin a patch to fix this. >>> >>> Thanks, >>> Ard. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SetMemorySpaceAttributes with EFI_MEMORY_XP 2017-03-20 11:38 ` Laszlo Ersek @ 2017-03-20 14:08 ` Ard Biesheuvel 2017-03-20 15:22 ` Yao, Jiewen 2017-03-20 15:24 ` Laszlo Ersek 0 siblings, 2 replies; 11+ messages in thread From: Ard Biesheuvel @ 2017-03-20 14:08 UTC (permalink / raw) To: Laszlo Ersek; +Cc: Michael Zimmermann, edk2-devel-01 On 20 March 2017 at 11:38, Laszlo Ersek <lersek@redhat.com> wrote: > On 03/20/17 12:20, Ard Biesheuvel wrote: >> On 20 March 2017 at 11:16, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: >>> Ard, why is SetMSetMemorySpaceAttributes being called in first place? >>> (ignoring the recent NX patch) >>> Looking at the initial GCD, it looks like unused memory usually >>> doesn't have any attributes set anyway. >>> >> >> Originally, we added the new memory with >> EFI_MEMORY_WB|EFI_MEMORY_WT|EFI_MEMORY_WC|EFI_MEMORY_UC capabilities, >> and selected the EFI_MEMORY_WB attribute with the call to >> gDS->SetMemorySpaceAttributes. Later, we removed all capabilities >> expect EFI_MEMORY_WB, since the other ones cannot be supported under >> virtualization with KVM. >> >> Whether that makes the SetMemorySpaceAttributes redundant is not >> entirely clear to me, but it does appear the adding the memory does >> the right thing wrt non-exec permissions if the policy is enabled. So >> perhaps we can simply drop this call? > > Won't that turn off caching for the memory just added? > I think it may not map the memory at all in this case, so we need to do something. It looks like calling mCpu->SetMemoryAttributes() should be sufficient here, and so I wonder whether we violate anything by replacing gDS->SetMemorySpaceAttributes with mCpu->SetMemoryAttributes here. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SetMemorySpaceAttributes with EFI_MEMORY_XP 2017-03-20 14:08 ` Ard Biesheuvel @ 2017-03-20 15:22 ` Yao, Jiewen 2017-03-20 15:24 ` Laszlo Ersek 1 sibling, 0 replies; 11+ messages in thread From: Yao, Jiewen @ 2017-03-20 15:22 UTC (permalink / raw) To: Ard Biesheuvel, Laszlo Ersek; +Cc: edk2-devel-01, Michael Zimmermann We had some internal discussion on using gDS or using mCpu. We decided to choose mCpu purposely at that moment, because we wanted to keep UEFI memory map unchanged to avoid any potential compatibility issue. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel Sent: Monday, March 20, 2017 10:09 PM To: Laszlo Ersek <lersek@redhat.com> Cc: edk2-devel-01 <edk2-devel@lists.01.org>; Michael Zimmermann <sigmaepsilon92@gmail.com> Subject: Re: [edk2] SetMemorySpaceAttributes with EFI_MEMORY_XP On 20 March 2017 at 11:38, Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> wrote: > On 03/20/17 12:20, Ard Biesheuvel wrote: >> On 20 March 2017 at 11:16, Michael Zimmermann <sigmaepsilon92@gmail.com<mailto:sigmaepsilon92@gmail.com>> wrote: >>> Ard, why is SetMSetMemorySpaceAttributes being called in first place? >>> (ignoring the recent NX patch) >>> Looking at the initial GCD, it looks like unused memory usually >>> doesn't have any attributes set anyway. >>> >> >> Originally, we added the new memory with >> EFI_MEMORY_WB|EFI_MEMORY_WT|EFI_MEMORY_WC|EFI_MEMORY_UC capabilities, >> and selected the EFI_MEMORY_WB attribute with the call to >> gDS->SetMemorySpaceAttributes. Later, we removed all capabilities >> expect EFI_MEMORY_WB, since the other ones cannot be supported under >> virtualization with KVM. >> >> Whether that makes the SetMemorySpaceAttributes redundant is not >> entirely clear to me, but it does appear the adding the memory does >> the right thing wrt non-exec permissions if the policy is enabled. So >> perhaps we can simply drop this call? > > Won't that turn off caching for the memory just added? > I think it may not map the memory at all in this case, so we need to do something. It looks like calling mCpu->SetMemoryAttributes() should be sufficient here, and so I wonder whether we violate anything by replacing gDS->SetMemorySpaceAttributes with mCpu->SetMemoryAttributes here. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SetMemorySpaceAttributes with EFI_MEMORY_XP 2017-03-20 14:08 ` Ard Biesheuvel 2017-03-20 15:22 ` Yao, Jiewen @ 2017-03-20 15:24 ` Laszlo Ersek 2017-03-20 19:31 ` Ard Biesheuvel 1 sibling, 1 reply; 11+ messages in thread From: Laszlo Ersek @ 2017-03-20 15:24 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: Michael Zimmermann, edk2-devel-01 On 03/20/17 15:08, Ard Biesheuvel wrote: > On 20 March 2017 at 11:38, Laszlo Ersek <lersek@redhat.com> wrote: >> On 03/20/17 12:20, Ard Biesheuvel wrote: >>> On 20 March 2017 at 11:16, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: >>>> Ard, why is SetMSetMemorySpaceAttributes being called in first place? >>>> (ignoring the recent NX patch) >>>> Looking at the initial GCD, it looks like unused memory usually >>>> doesn't have any attributes set anyway. >>>> >>> >>> Originally, we added the new memory with >>> EFI_MEMORY_WB|EFI_MEMORY_WT|EFI_MEMORY_WC|EFI_MEMORY_UC capabilities, >>> and selected the EFI_MEMORY_WB attribute with the call to >>> gDS->SetMemorySpaceAttributes. Later, we removed all capabilities >>> expect EFI_MEMORY_WB, since the other ones cannot be supported under >>> virtualization with KVM. >>> >>> Whether that makes the SetMemorySpaceAttributes redundant is not >>> entirely clear to me, but it does appear the adding the memory does >>> the right thing wrt non-exec permissions if the policy is enabled. So >>> perhaps we can simply drop this call? >> >> Won't that turn off caching for the memory just added? >> > > I think it may not map the memory at all in this case, so we need to > do something. It looks like calling mCpu->SetMemoryAttributes() should > be sufficient here, and so I wonder whether we violate anything by > replacing gDS->SetMemorySpaceAttributes with mCpu->SetMemoryAttributes > here. Earlier you mentioned that we cannot get some piece of information from the GCD map, which limits what we can do here. Looking at GetMemorySpaceDescriptor() and GetMemorySpaceMap(), they should return both Capabilities and Attributes. Also, looking at vol2 in PI 1.5, I find: * GetMemorySpaceDescriptor() returns EFI_NOT_AVAILABLE_YET if "The attributes cannot be set because CPU architectural protocol is not available yet." * 9.7.3.2 SetMemorySpaceAttributes() -- When the DXE Foundation is notified that the EFI_CPU_ARCH_PROTOCOL has been installed, then the DXE Service SetMemorySpaceAttributes() can be made available. The DXE Foundation can then use the SetMemoryAttributes() service of the EFI_CPU_ARCH_PROTOCOL to implement the DXE Service SetMemorySpaceAttributes(). * 9.7.3.3 GetMemorySpaceMap() -- When the DXE Foundation is notified that the EFI_CPU_ARCH_PROTOCOL has been installed, then the DXE Service GetMemorySpaceMap() is fully functional. This function is made available when the memory-based services are initialized. However, the Attributes field of the array of EFI_GCD_MEMORY_SPACE_DESCRIPTORs is not valid until the EFI_CPU_ARCH_PROTOCOL is installed. So, assuming that you have tested GetMemorySpaceMap() earlier, and have found Attributes useless for the protection (or other) purposes, may that have happened because the CPU arch protocol was not available yet? ... I guess my email is a bit confusing. My points are, (a) we should likely not mess directly with the CPU arch protocol if we know (and we do know) that the GCD services use them internally, (b) are we absolutely sure that the GCD services can't help us here? If nothing else works, I agree we can mess with the CPU arch protocol directly. Thanks Laszlo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SetMemorySpaceAttributes with EFI_MEMORY_XP 2017-03-20 15:24 ` Laszlo Ersek @ 2017-03-20 19:31 ` Ard Biesheuvel 0 siblings, 0 replies; 11+ messages in thread From: Ard Biesheuvel @ 2017-03-20 19:31 UTC (permalink / raw) To: Laszlo Ersek; +Cc: Michael Zimmermann, edk2-devel-01 On 20 March 2017 at 15:24, Laszlo Ersek <lersek@redhat.com> wrote: > On 03/20/17 15:08, Ard Biesheuvel wrote: >> On 20 March 2017 at 11:38, Laszlo Ersek <lersek@redhat.com> wrote: >>> On 03/20/17 12:20, Ard Biesheuvel wrote: >>>> On 20 March 2017 at 11:16, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote: >>>>> Ard, why is SetMSetMemorySpaceAttributes being called in first place? >>>>> (ignoring the recent NX patch) >>>>> Looking at the initial GCD, it looks like unused memory usually >>>>> doesn't have any attributes set anyway. >>>>> >>>> >>>> Originally, we added the new memory with >>>> EFI_MEMORY_WB|EFI_MEMORY_WT|EFI_MEMORY_WC|EFI_MEMORY_UC capabilities, >>>> and selected the EFI_MEMORY_WB attribute with the call to >>>> gDS->SetMemorySpaceAttributes. Later, we removed all capabilities >>>> expect EFI_MEMORY_WB, since the other ones cannot be supported under >>>> virtualization with KVM. >>>> >>>> Whether that makes the SetMemorySpaceAttributes redundant is not >>>> entirely clear to me, but it does appear the adding the memory does >>>> the right thing wrt non-exec permissions if the policy is enabled. So >>>> perhaps we can simply drop this call? >>> >>> Won't that turn off caching for the memory just added? >>> >> >> I think it may not map the memory at all in this case, so we need to >> do something. It looks like calling mCpu->SetMemoryAttributes() should >> be sufficient here, and so I wonder whether we violate anything by >> replacing gDS->SetMemorySpaceAttributes with mCpu->SetMemoryAttributes >> here. > > Earlier you mentioned that we cannot get some piece of information from > the GCD map, which limits what we can do here. > > Looking at GetMemorySpaceDescriptor() and GetMemorySpaceMap(), they > should return both Capabilities and Attributes. > > Also, looking at vol2 in PI 1.5, I find: > > * GetMemorySpaceDescriptor() returns EFI_NOT_AVAILABLE_YET if "The > attributes cannot be set because CPU architectural protocol is not > available yet." > > * 9.7.3.2 SetMemorySpaceAttributes() -- When the DXE Foundation is > notified that the EFI_CPU_ARCH_PROTOCOL has been installed, then the DXE > Service SetMemorySpaceAttributes() can be made available. The DXE > Foundation can then use the SetMemoryAttributes() service of the > EFI_CPU_ARCH_PROTOCOL to implement the DXE Service > SetMemorySpaceAttributes(). > > * 9.7.3.3 GetMemorySpaceMap() -- When the DXE Foundation is notified > that the EFI_CPU_ARCH_PROTOCOL has been installed, then the DXE Service > GetMemorySpaceMap() is fully functional. This function is made available > when the memory-based services are initialized. However, the Attributes > field of the array of EFI_GCD_MEMORY_SPACE_DESCRIPTORs is not valid > until the EFI_CPU_ARCH_PROTOCOL is installed. > OK, that's the theory. The reality (as Jiewen confirms) is that the RO/XP capability and attribute bits are never used on the GCD side, because they may cause compatibility issues. The problem is that UEFI memory map entries inherit all attributes from the GCD memory space entries they are carved out of. This means that all UEFI memory map entries will have RO and XP set, which is clearly not what we want. I guess it would be feasible to filter out RO and XP when calling CoreAddRange(), but that does not happen currently. > So, assuming that you have tested GetMemorySpaceMap() earlier, and have > found Attributes useless for the protection (or other) purposes, may > that have happened because the CPU arch protocol was not available yet? > No > ... I guess my email is a bit confusing. My points are, (a) we should > likely not mess directly with the CPU arch protocol if we know (and we > do know) that the GCD services use them internally, (b) are we > absolutely sure that the GCD services can't help us here? > > If nothing else works, I agree we can mess with the CPU arch protocol > directly. > I'm afraid that is our only option. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SetMemorySpaceAttributes with EFI_MEMORY_XP 2017-03-20 10:32 SetMemorySpaceAttributes with EFI_MEMORY_XP Michael Zimmermann 2017-03-20 11:04 ` Ard Biesheuvel @ 2017-03-20 11:06 ` Laszlo Ersek 2017-03-20 11:10 ` Michael Zimmermann 1 sibling, 1 reply; 11+ messages in thread From: Laszlo Ersek @ 2017-03-20 11:06 UTC (permalink / raw) To: Michael Zimmermann, edk2-devel-01, Ard Biesheuvel On 03/20/17 11:32, Michael Zimmermann wrote: > Hi, > > I didn't test ArmVirtQemuKernel but I'm trying to use some of the code > for another platform. > So does this call ever succeed with PcdDxeNxMemoryProtectionPolicy > being enabled? > https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L95 > Status = gDS->SetMemorySpaceAttributes (CurBase, CurSize, Attributes); > > Neither the memory that was added by this Dxe nor the one added > automatically by GCD has the EFI_MEMORY_XP capability which causes > SetMemorySpaceAttributes to return EFI_UNSUPPORTED. See commit 413edd470932 ("ArmVirtPkg/HighMemDxe: preserve non-exec permissions on newly added regions", 2017-02-28). EFI_MEMORY_XP is only requested if EfiConventionalMemory's bit is set in PcdDxeNxMemoryProtectionPolicy, but then (I think) the DXE_CORE must have set the same bit in the "supported" bitmask too, when the memory was added. Thanks Laszlo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: SetMemorySpaceAttributes with EFI_MEMORY_XP 2017-03-20 11:06 ` Laszlo Ersek @ 2017-03-20 11:10 ` Michael Zimmermann 0 siblings, 0 replies; 11+ messages in thread From: Michael Zimmermann @ 2017-03-20 11:10 UTC (permalink / raw) To: Laszlo Ersek; +Cc: edk2-devel-01, Ard Biesheuvel > but then (I think) the DXE_CORE must have set the same bit in the "supported" bitmask too, when the memory was added. that's the point, if my port behaves the same as ArmVirt, it doesn't. for all memory, capabilities are 8000000000000008 and attributes are 0000000000000000 for unused memory and 0000000000000008 for used memory. This seems to confirm what Ard said about the GCD not being altered for NX permissions. Thanks Michael On Mon, Mar 20, 2017 at 12:06 PM, Laszlo Ersek <lersek@redhat.com> wrote: > On 03/20/17 11:32, Michael Zimmermann wrote: >> Hi, >> >> I didn't test ArmVirtQemuKernel but I'm trying to use some of the code >> for another platform. >> So does this call ever succeed with PcdDxeNxMemoryProtectionPolicy >> being enabled? >> https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L95 >> Status = gDS->SetMemorySpaceAttributes (CurBase, CurSize, Attributes); >> >> Neither the memory that was added by this Dxe nor the one added >> automatically by GCD has the EFI_MEMORY_XP capability which causes >> SetMemorySpaceAttributes to return EFI_UNSUPPORTED. > > See commit 413edd470932 ("ArmVirtPkg/HighMemDxe: preserve non-exec > permissions on newly added regions", 2017-02-28). EFI_MEMORY_XP is only > requested if EfiConventionalMemory's bit is set in > PcdDxeNxMemoryProtectionPolicy, but then (I think) the DXE_CORE must > have set the same bit in the "supported" bitmask too, when the memory > was added. > > Thanks > Laszlo > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-03-20 19:31 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-03-20 10:32 SetMemorySpaceAttributes with EFI_MEMORY_XP Michael Zimmermann 2017-03-20 11:04 ` Ard Biesheuvel 2017-03-20 11:16 ` Michael Zimmermann 2017-03-20 11:20 ` Ard Biesheuvel 2017-03-20 11:38 ` Laszlo Ersek 2017-03-20 14:08 ` Ard Biesheuvel 2017-03-20 15:22 ` Yao, Jiewen 2017-03-20 15:24 ` Laszlo Ersek 2017-03-20 19:31 ` Ard Biesheuvel 2017-03-20 11:06 ` Laszlo Ersek 2017-03-20 11:10 ` Michael Zimmermann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox