From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: "Yao, Jiewen" <jiewen.yao@intel.com>
Cc: "Tian, Feng" <feng.tian@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
Leif Lindholm <leif.lindholm@linaro.org>,
"Kinney, Michael D" <michael.d.kinney@intel.com>,
"Fan, Jeff" <jeff.fan@intel.com>,
"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH V3 0/4] DXE Memory Protection
Date: Thu, 9 Feb 2017 13:51:36 +0000 [thread overview]
Message-ID: <CAKv+Gu_djRnM6XMMufgHSgk885MbP7KHWjiyqtr08qkbQURFjQ@mail.gmail.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503A8EBEC3@shsmsx102.ccr.corp.intel.com>
On 9 February 2017 at 13:19, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Thank you Ard. I check the ARM code again.
>
>
>
> It seems the ARM CPU driver is similar as X86 CPU driver – it installs
> CpuArch protocol, then SyncCacheConfig.
>
> ========================
>
> Status = gBS->InstallMultipleProtocolInterfaces (
>
> &mCpuHandle,
>
> &gEfiCpuArchProtocolGuid, &mCpu,
>
> &gVirtualUncachedPagesProtocolGuid, &gVirtualUncachedPages,
>
> NULL
>
> );
>
>
>
> //
>
> // Make sure GCD and MMU settings match. This API calls
> gDS->SetMemorySpaceAttributes ()
>
> // and that calls EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes, so this code
> needs to go
>
> // after the protocol is installed
>
> //
>
> SyncCacheConfig (&mCpu);
>
> ========================
>
>
Thanks a lot for taking the time to look at this.
>
> In my update, the DxeCore registers a CPU ARCH callback function to setup PE
> code/data section attribute there.
>
> At that time, the GCD attribute is not ready. As such, the cache attribute
> is all 0.
>
>
>
> After SyncCacheConfig(), the rest attribute setting will carry expected GCD
> update.
>
So how does x86 deal with this situation if it follows the same basic approach?
>
>
> Here is my thought:
>
> 1) How about I set default PcdImageProtectionPolicy to be 0 for ARM in
> DEC file? As such the ARM platform is not impacted.
>
> 2) At same time, someone from ARM can help to enhance the CPU driver
> to make it work.
>
> In my opinion, ASSERT() here is not the best idea. According to PI spec V2,
> 12.3, EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes(), the EFI_UNSUPPORTED is
> expected.
>
>
>
>
>
>
>
> Just for your reference, below is the OVMF log for X86 CPU:
>
> It sets page attribute only before GCD sync, then set both page attribute
> and cache attributes after GCD sync.
>
> ============================
>
> Loading driver at 0x00007605000 EntryPoint=0x000076066E9 CpuDxe.efi
>
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9410
>
> ProtectUefiImageCommon - 0x70D91A8
>
> - 0x0000000007605000 - 0x000000000000E000
>
> CurrentPagingContext:
>
> MachineType - 0x14C
>
> PageTableBase - 0x76A9000
>
> Attributes - 0xC0000002
>
> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 7611128
>
> MemoryProtectionCpuArchProtocolNotify: ImageRecordCount - 0xA
>
> SetUefiImageMemoryAttributes - 0x00000000076CE000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x76CE023->0x80000000076CE023
>
> SetUefiImageMemoryAttributes - 0x00000000076CF000 - 0x0000000000016000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x76CF023->0x76CF021
>
> ConvertPageEntryAttribute 0x76D0023->0x76D0021
>
> ConvertPageEntryAttribute 0x76D1023->0x76D1021
>
> ConvertPageEntryAttribute 0x76D2023->0x76D2021
>
> ConvertPageEntryAttribute 0x76D3023->0x76D3021
>
> ConvertPageEntryAttribute 0x76D4023->0x76D4021
>
> ConvertPageEntryAttribute 0x76D5023->0x76D5021
>
> ConvertPageEntryAttribute 0x76D6023->0x76D6021
>
> ConvertPageEntryAttribute 0x76D7023->0x76D7021
>
> ConvertPageEntryAttribute 0x76D8023->0x76D8021
>
> ConvertPageEntryAttribute 0x76D9023->0x76D9021
>
> ConvertPageEntryAttribute 0x76DA023->0x76DA021
>
> ConvertPageEntryAttribute 0x76DB023->0x76DB021
>
> ConvertPageEntryAttribute 0x76DC023->0x76DC021
>
> ConvertPageEntryAttribute 0x76DD023->0x76DD021
>
> ConvertPageEntryAttribute 0x76DE023->0x76DE021
>
> ConvertPageEntryAttribute 0x76DF023->0x76DF021
>
> ConvertPageEntryAttribute 0x76E0023->0x76E0021
>
> ConvertPageEntryAttribute 0x76E1023->0x76E1021
>
> ConvertPageEntryAttribute 0x76E2003->0x76E2001
>
> ConvertPageEntryAttribute 0x76E3023->0x76E3021
>
> ConvertPageEntryAttribute 0x76E4023->0x76E4021
>
> SetUefiImageMemoryAttributes - 0x00000000076E5000 - 0x000000000000B000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x76E5023->0x80000000076E5023
>
> ConvertPageEntryAttribute 0x76E6023->0x80000000076E6023
>
> ConvertPageEntryAttribute 0x76E7023->0x80000000076E7023
>
> ConvertPageEntryAttribute 0x76E8023->0x80000000076E8023
>
> ConvertPageEntryAttribute 0x76E9023->0x80000000076E9023
>
> ConvertPageEntryAttribute 0x76EA023->0x80000000076EA023
>
> ConvertPageEntryAttribute 0x76EB063->0x80000000076EB063
>
> ConvertPageEntryAttribute 0x76EC063->0x80000000076EC063
>
> ConvertPageEntryAttribute 0x76ED063->0x80000000076ED063
>
> ConvertPageEntryAttribute 0x76EE003->0x80000000076EE003
>
> ConvertPageEntryAttribute 0x76EF003->0x80000000076EF003
>
> SetUefiImageMemoryAttributes - 0x0000000007637000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7637063->0x8000000007637063
>
> SetUefiImageMemoryAttributes - 0x0000000007638000 - 0x0000000000006000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7638063->0x7638061
>
> ConvertPageEntryAttribute 0x7639063->0x7639061
>
> ConvertPageEntryAttribute 0x763A063->0x763A061
>
> ConvertPageEntryAttribute 0x763B063->0x763B061
>
> ConvertPageEntryAttribute 0x763C063->0x763C061
>
> ConvertPageEntryAttribute 0x763D063->0x763D061
>
> SetUefiImageMemoryAttributes - 0x000000000763E000 - 0x0000000000005000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x763E063->0x800000000763E063
>
> ConvertPageEntryAttribute 0x763F063->0x800000000763F063
>
> ConvertPageEntryAttribute 0x7640063->0x8000000007640063
>
> ConvertPageEntryAttribute 0x7641063->0x8000000007641063
>
> ConvertPageEntryAttribute 0x7642063->0x8000000007642063
>
> SetUefiImageMemoryAttributes - 0x000000000762E000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x762E063->0x800000000762E063
>
> SetUefiImageMemoryAttributes - 0x000000000762F000 - 0x0000000000004000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x762F063->0x762F061
>
> ConvertPageEntryAttribute 0x7630063->0x7630061
>
> ConvertPageEntryAttribute 0x7631063->0x7631061
>
> ConvertPageEntryAttribute 0x7632063->0x7632061
>
> SetUefiImageMemoryAttributes - 0x0000000007633000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7633063->0x8000000007633063
>
> ConvertPageEntryAttribute 0x7634063->0x8000000007634063
>
> ConvertPageEntryAttribute 0x7635063->0x8000000007635063
>
> ConvertPageEntryAttribute 0x7636063->0x8000000007636063
>
> SetUefiImageMemoryAttributes - 0x000000000766C000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x766C063->0x800000000766C063
>
> SetUefiImageMemoryAttributes - 0x000000000766D000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x766D063->0x766D061
>
> ConvertPageEntryAttribute 0x766E063->0x766E061
>
> SetUefiImageMemoryAttributes - 0x000000000766F000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x766F063->0x800000000766F063
>
> ConvertPageEntryAttribute 0x7670063->0x8000000007670063
>
> ConvertPageEntryAttribute 0x7671063->0x8000000007671063
>
> ConvertPageEntryAttribute 0x7672063->0x8000000007672063
>
> SetUefiImageMemoryAttributes - 0x0000000007666000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7666063->0x8000000007666063
>
> SetUefiImageMemoryAttributes - 0x0000000007667000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7667063->0x7667061
>
> ConvertPageEntryAttribute 0x7668063->0x7668061
>
> SetUefiImageMemoryAttributes - 0x0000000007669000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7669063->0x8000000007669063
>
> ConvertPageEntryAttribute 0x766A063->0x800000000766A063
>
> ConvertPageEntryAttribute 0x766B063->0x800000000766B063
>
> SetUefiImageMemoryAttributes - 0x0000000007627000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7627063->0x8000000007627063
>
> SetUefiImageMemoryAttributes - 0x0000000007628000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7628063->0x7628061
>
> ConvertPageEntryAttribute 0x7629063->0x7629061
>
> SetUefiImageMemoryAttributes - 0x000000000762A000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x762A063->0x800000000762A063
>
> ConvertPageEntryAttribute 0x762B063->0x800000000762B063
>
> ConvertPageEntryAttribute 0x762C063->0x800000000762C063
>
> ConvertPageEntryAttribute 0x762D063->0x800000000762D063
>
> SetUefiImageMemoryAttributes - 0x000000000761F000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x761F063->0x800000000761F063
>
> SetUefiImageMemoryAttributes - 0x0000000007620000 - 0x0000000000004000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7620063->0x7620061
>
> ConvertPageEntryAttribute 0x7621063->0x7621061
>
> ConvertPageEntryAttribute 0x7622063->0x7622061
>
> ConvertPageEntryAttribute 0x7623063->0x7623061
>
> SetUefiImageMemoryAttributes - 0x0000000007624000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7624063->0x8000000007624063
>
> ConvertPageEntryAttribute 0x7625063->0x8000000007625063
>
> ConvertPageEntryAttribute 0x7626063->0x8000000007626063
>
> SetUefiImageMemoryAttributes - 0x0000000007619000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7619063->0x8000000007619063
>
> SetUefiImageMemoryAttributes - 0x000000000761A000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x761A063->0x761A061
>
> ConvertPageEntryAttribute 0x761B063->0x761B061
>
> SetUefiImageMemoryAttributes - 0x000000000761C000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x761C063->0x800000000761C063
>
> ConvertPageEntryAttribute 0x761D063->0x800000000761D063
>
> ConvertPageEntryAttribute 0x761E063->0x800000000761E063
>
> SetUefiImageMemoryAttributes - 0x0000000007613000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7613063->0x8000000007613063
>
> SetUefiImageMemoryAttributes - 0x0000000007614000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7614063->0x7614061
>
> ConvertPageEntryAttribute 0x7615063->0x7615061
>
> SetUefiImageMemoryAttributes - 0x0000000007616000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7616063->0x8000000007616063
>
> ConvertPageEntryAttribute 0x7617063->0x8000000007617063
>
> ConvertPageEntryAttribute 0x7618063->0x8000000007618063
>
> SetUefiImageMemoryAttributes - 0x0000000007605000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7605063->0x8000000007605063
>
> SetUefiImageMemoryAttributes - 0x0000000007606000 - 0x0000000000008000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7606063->0x7606061
>
> ConvertPageEntryAttribute 0x7607063->0x7607061
>
> ConvertPageEntryAttribute 0x7608063->0x7608061
>
> ConvertPageEntryAttribute 0x7609063->0x7609061
>
> ConvertPageEntryAttribute 0x760A063->0x760A061
>
> ConvertPageEntryAttribute 0x760B063->0x760B061
>
> ConvertPageEntryAttribute 0x760C063->0x760C061
>
> ConvertPageEntryAttribute 0x760D063->0x760D061
>
> SetUefiImageMemoryAttributes - 0x000000000760E000 - 0x0000000000005000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x760E063->0x800000000760E063
>
> ConvertPageEntryAttribute 0x760F063->0x800000000760F063
>
> ConvertPageEntryAttribute 0x7610063->0x8000000007610063
>
> ConvertPageEntryAttribute 0x7611063->0x8000000007611063
>
> ConvertPageEntryAttribute 0x7612063->0x8000000007612063
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> AP Loop Mode is 1
>
> Detect CPU count: 4
>
> InstallProtocolInterface: 3FDDA605-A76E-4F46-AD29-12F4531B3D08 76111D4
>
> Loading driver F6697AC4-A776-4EE1-B643-1FEFF2B615BB
>
> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 70D8CA8
>
> Loading driver at 0x000075FF000 EntryPoint=0x000076000B5
> IncompatiblePciDeviceSupportDxe.efi
>
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9010
>
> ProtectUefiImageCommon - 0x70D8CA8
>
> - 0x00000000075FF000 - 0x0000000000006000
>
> SetUefiImageMemoryAttributes - 0x00000000075FF000 - 0x0000000000001000
> (0x0000000000004008)
>
> Split - 0x6CA7000
>
> ConvertPageEntryAttribute 0x75FF063->0x80000000075FF063
>
> SetUefiImageMemoryAttributes - 0x0000000007600000 - 0x0000000000002000
> (0x0000000000020008)
>
> ConvertPageEntryAttribute 0x7600063->0x7600061
>
> ConvertPageEntryAttribute 0x7601063->0x7601061
>
> SetUefiImageMemoryAttributes - 0x0000000007602000 - 0x0000000000003000
> (0x0000000000004008)
>
> ConvertPageEntryAttribute 0x7602063->0x8000000007602063
>
> ConvertPageEntryAttribute 0x7603063->0x8000000007603063
>
> ConvertPageEntryAttribute 0x7604063->0x8000000007604063
>
> ============================
>
>
>
>
>
>
>
>
>
>
>
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Thursday, February 9, 2017 1:22 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm
> <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>
>
>
> On 9 February 2017 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
>
>> On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> wrote:
>>> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>>>> Hi Lindholm/Ard
>>>> This version 3 contains both of your feedback before.
>>>>
>>>> If you can do me a favor to evaluated the impact to ARM, that will be
>>>> great.
>>>>
>>>
>>> I will take a look right away.
>>>
>>
>> I am hitting the following assert as soon as ArmCpuDxe is loaded:
>>
>> Loading driver at 0x0005BE15000 EntryPoint=0x0005BE16048 ArmCpuDxe.efi
>> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 5B142398
>> ProtectUefiImageCommon - 0x5B142140
>> - 0x000000005BE15000 - 0x0000000000010000
>> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 5BE23008
>> InstallProtocolInterface: AD651C7D-3C22-4DBF-92E8-38A7CDAE87B2 5BE230B0
>> MemoryProtectionCpuArchProtocolNotify:
>> ProtectUefiImageCommon - 0x5EF02400
>> - 0x000000005EEC4000 - 0x0000000000042000
>> SetUefiImageMemoryAttributes - 0x000000005EEC4000 - 0x0000000000001000
>> (0x0000000000004000)
>> EfiAttributeToArmAttribute: 0x4000 attributes is not supported.
>> ASSERT [ArmCpuDxe]
>> /home/ard/build/uefi-next/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c(220): 0
>>
>> The reason appears to be that the GCD memory descriptor retrieved in
>> SetUefiImageMemoryAttributes() for this region has Attributes == 0
>>
>> I am digging further ...
>
> Attached is the output of DEBUG_GCD
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
next prev parent reply other threads:[~2017-02-09 13:51 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-09 7:20 [PATCH V3 0/4] DXE Memory Protection Jiewen Yao
2017-02-09 7:20 ` [PATCH V3 1/4] UefiCpuPkg/CpuDxe: Add memory attribute setting Jiewen Yao
2017-02-09 7:20 ` [PATCH V3 2/4] ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage Jiewen Yao
2017-02-09 7:20 ` [PATCH V3 3/4] MdeModulePkg/dec: add PcdImageProtectionPolicy Jiewen Yao
2017-02-09 7:20 ` [PATCH V3 4/4] MdeModulePkg/DxeCore: Add UEFI image protection Jiewen Yao
2017-02-09 7:43 ` [PATCH V3 0/4] DXE Memory Protection Yao, Jiewen
2017-02-09 8:49 ` Ard Biesheuvel
2017-02-09 9:09 ` Ard Biesheuvel
2017-02-09 9:22 ` Ard Biesheuvel
2017-02-09 13:19 ` Yao, Jiewen
2017-02-09 13:51 ` Ard Biesheuvel [this message]
2017-02-09 14:08 ` Yao, Jiewen
2017-02-09 14:55 ` Ard Biesheuvel
2017-02-09 15:27 ` Yao, Jiewen
2017-02-09 15:28 ` Ard Biesheuvel
2017-02-09 16:21 ` Ard Biesheuvel
2017-02-09 16:29 ` Yao, Jiewen
2017-02-09 16:30 ` Ard Biesheuvel
2017-02-09 16:48 ` Ard Biesheuvel
2017-02-10 2:26 ` Yao, Jiewen
2017-02-10 6:34 ` Ard Biesheuvel
2017-02-10 6:41 ` Ard Biesheuvel
2017-02-10 11:32 ` Yao, Jiewen
2017-02-10 11:42 ` Ard Biesheuvel
2017-02-10 12:59 ` Yao, Jiewen
2017-02-10 14:16 ` Ard Biesheuvel
2017-02-09 14:23 ` Yao, Jiewen
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-list 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=CAKv+Gu_djRnM6XMMufgHSgk885MbP7KHWjiyqtr08qkbQURFjQ@mail.gmail.com \
--to=devel@edk2.groups.io \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox