public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
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:19:51 +0000	[thread overview]
Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503A8EBEC3@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <CAKv+Gu8tiGxcXO_AZh98CFR0FeSz00HpkN6Ea-wszy9TFfRD-w@mail.gmail.com>

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);
========================

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.

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<mailto:ard.biesheuvel@linaro.org>> wrote:
> On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
>> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com<mailto: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<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel


  reply	other threads:[~2017-02-09 13:19 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 [this message]
2017-02-09 13:51           ` Ard Biesheuvel
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=74D8A39837DF1E4DA445A8C0B3885C503A8EBEC3@shsmsx102.ccr.corp.intel.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