| Message ID | 20171114102205.30649-1-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 44d71c217ccbd87e8c42768b4e84b1c899d779e2 |
| Headers | show |
| Series | [edk2] ArmVirtPkg/ArmVirtQemu: use non-accelerated CopyMem for VariableRuntimeDxe | expand |
On 11/14/17 11:22, Ard Biesheuvel wrote: > The VariableRuntimeDxe driver may use CopyMem () on NOR flash regions, > assuming such regions always have full memory semantics. Given that > those regions cannot be mapped as ordinary memory on ARM (due to the > fact that the NOR flash requires device semantics while in write mode) > this prevents us from using BaseMemoryLibOptDxe in VariableRuntimeDxe, > since it may use unaligned accesses and/or DC ZVA instructions, both > of which are incompatible with mappings using device semantics. > > Note that there is no way we can work around this by changing the > mapping type between 'memory' and 'device' when switching from read to > write mode and back, because the runtime mapping is created by the OS, > and cannot be changed at will. > > So let's just switch to the unaccelerated version of BaseMemoryLib which > does not have the same problem. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmVirtPkg/ArmVirtQemu.dsc | 2 ++ > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc > index 8a60b61f2aa6..7b220d6e3c31 100644 > --- a/ArmVirtPkg/ArmVirtQemu.dsc > +++ b/ArmVirtPkg/ArmVirtQemu.dsc > @@ -261,6 +261,8 @@ [Components.common] > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { > <LibraryClasses> > NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf > + # don't use unaligned CopyMem () on the UEFI varstore NOR flash region > + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf > } > !if $(SECURE_BOOT_ENABLE) == TRUE > MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > index 9a31ec93ca06..7c032e1b07e0 100644 > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > @@ -252,6 +252,8 @@ [Components.common] > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { > <LibraryClasses> > NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf > + # don't use unaligned CopyMem () on the UEFI varstore NOR flash region > + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf > } > !if $(SECURE_BOOT_ENABLE) == TRUE > MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { > Reviewed-by: Laszlo Ersek <lersek@redhat.com> Given that we've never seen the symptom reported by Shannon, I must think Shannon is using some kind of new hardware. May I ask what hardware that is? Thanks, Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 15 November 2017 at 13:51, Laszlo Ersek <lersek@redhat.com> wrote: > On 11/14/17 11:22, Ard Biesheuvel wrote: >> The VariableRuntimeDxe driver may use CopyMem () on NOR flash regions, >> assuming such regions always have full memory semantics. Given that >> those regions cannot be mapped as ordinary memory on ARM (due to the >> fact that the NOR flash requires device semantics while in write mode) >> this prevents us from using BaseMemoryLibOptDxe in VariableRuntimeDxe, >> since it may use unaligned accesses and/or DC ZVA instructions, both >> of which are incompatible with mappings using device semantics. >> >> Note that there is no way we can work around this by changing the >> mapping type between 'memory' and 'device' when switching from read to >> write mode and back, because the runtime mapping is created by the OS, >> and cannot be changed at will. >> >> So let's just switch to the unaccelerated version of BaseMemoryLib which >> does not have the same problem. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> ArmVirtPkg/ArmVirtQemu.dsc | 2 ++ >> ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++ >> 2 files changed, 4 insertions(+) >> >> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc >> index 8a60b61f2aa6..7b220d6e3c31 100644 >> --- a/ArmVirtPkg/ArmVirtQemu.dsc >> +++ b/ArmVirtPkg/ArmVirtQemu.dsc >> @@ -261,6 +261,8 @@ [Components.common] >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { >> <LibraryClasses> >> NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf >> + # don't use unaligned CopyMem () on the UEFI varstore NOR flash region >> + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf >> } >> !if $(SECURE_BOOT_ENABLE) == TRUE >> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { >> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc >> index 9a31ec93ca06..7c032e1b07e0 100644 >> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc >> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc >> @@ -252,6 +252,8 @@ [Components.common] >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { >> <LibraryClasses> >> NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf >> + # don't use unaligned CopyMem () on the UEFI varstore NOR flash region >> + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf >> } >> !if $(SECURE_BOOT_ENABLE) == TRUE >> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { >> > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > Given that we've never seen the symptom reported by Shannon, I must > think Shannon is using some kind of new hardware. May I ask what > hardware that is? > I think this is equally likely to occur in any KVM hardware virtualization context. It is simply a regression after moving to the OptDxe BaseMemoryLib flavor. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 2017/11/15 22:03, Ard Biesheuvel wrote: > On 15 November 2017 at 13:51, Laszlo Ersek <lersek@redhat.com> wrote: >> > On 11/14/17 11:22, Ard Biesheuvel wrote: >>> >> The VariableRuntimeDxe driver may use CopyMem () on NOR flash regions, >>> >> assuming such regions always have full memory semantics. Given that >>> >> those regions cannot be mapped as ordinary memory on ARM (due to the >>> >> fact that the NOR flash requires device semantics while in write mode) >>> >> this prevents us from using BaseMemoryLibOptDxe in VariableRuntimeDxe, >>> >> since it may use unaligned accesses and/or DC ZVA instructions, both >>> >> of which are incompatible with mappings using device semantics. >>> >> >>> >> Note that there is no way we can work around this by changing the >>> >> mapping type between 'memory' and 'device' when switching from read to >>> >> write mode and back, because the runtime mapping is created by the OS, >>> >> and cannot be changed at will. >>> >> >>> >> So let's just switch to the unaccelerated version of BaseMemoryLib which >>> >> does not have the same problem. >>> >> >>> >> Contributed-under: TianoCore Contribution Agreement 1.1 >>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> >> --- >>> >> ArmVirtPkg/ArmVirtQemu.dsc | 2 ++ >>> >> ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++ >>> >> 2 files changed, 4 insertions(+) >>> >> >>> >> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc >>> >> index 8a60b61f2aa6..7b220d6e3c31 100644 >>> >> --- a/ArmVirtPkg/ArmVirtQemu.dsc >>> >> +++ b/ArmVirtPkg/ArmVirtQemu.dsc >>> >> @@ -261,6 +261,8 @@ [Components.common] >>> >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { >>> >> <LibraryClasses> >>> >> NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf >>> >> + # don't use unaligned CopyMem () on the UEFI varstore NOR flash region >>> >> + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf >>> >> } >>> >> !if $(SECURE_BOOT_ENABLE) == TRUE >>> >> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { >>> >> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc >>> >> index 9a31ec93ca06..7c032e1b07e0 100644 >>> >> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc >>> >> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc >>> >> @@ -252,6 +252,8 @@ [Components.common] >>> >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { >>> >> <LibraryClasses> >>> >> NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf >>> >> + # don't use unaligned CopyMem () on the UEFI varstore NOR flash region >>> >> + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf >>> >> } >>> >> !if $(SECURE_BOOT_ENABLE) == TRUE >>> >> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { >>> >> >> > >> > Reviewed-by: Laszlo Ersek <lersek@redhat.com> >> > >> > Given that we've never seen the symptom reported by Shannon, I must >> > think Shannon is using some kind of new hardware. May I ask what >> > hardware that is? >> > > I think this is equally likely to occur in any KVM hardware > virtualization context. It is simply a regression after moving to the > OptDxe BaseMemoryLib flavor. Exactly. I'm using Huawei D05. It works well when I use a older edk2 while fails when updating to UDK2017 branch. Tested-by: Shannon Zhao <zhaoshenglong@huawei.com> Thanks, -- Shannon _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 16 November 2017 at 01:01, Shannon Zhao <zhaoshenglong@huawei.com> wrote: > > > On 2017/11/15 22:03, Ard Biesheuvel wrote: >> On 15 November 2017 at 13:51, Laszlo Ersek <lersek@redhat.com> wrote: >>> > On 11/14/17 11:22, Ard Biesheuvel wrote: >>>> >> The VariableRuntimeDxe driver may use CopyMem () on NOR flash regions, >>>> >> assuming such regions always have full memory semantics. Given that >>>> >> those regions cannot be mapped as ordinary memory on ARM (due to the >>>> >> fact that the NOR flash requires device semantics while in write mode) >>>> >> this prevents us from using BaseMemoryLibOptDxe in VariableRuntimeDxe, >>>> >> since it may use unaligned accesses and/or DC ZVA instructions, both >>>> >> of which are incompatible with mappings using device semantics. >>>> >> >>>> >> Note that there is no way we can work around this by changing the >>>> >> mapping type between 'memory' and 'device' when switching from read to >>>> >> write mode and back, because the runtime mapping is created by the OS, >>>> >> and cannot be changed at will. >>>> >> >>>> >> So let's just switch to the unaccelerated version of BaseMemoryLib which >>>> >> does not have the same problem. >>>> >> >>>> >> Contributed-under: TianoCore Contribution Agreement 1.1 >>>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>>> >> --- >>>> >> ArmVirtPkg/ArmVirtQemu.dsc | 2 ++ >>>> >> ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++ >>>> >> 2 files changed, 4 insertions(+) >>>> >> >>>> >> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc >>>> >> index 8a60b61f2aa6..7b220d6e3c31 100644 >>>> >> --- a/ArmVirtPkg/ArmVirtQemu.dsc >>>> >> +++ b/ArmVirtPkg/ArmVirtQemu.dsc >>>> >> @@ -261,6 +261,8 @@ [Components.common] >>>> >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { >>>> >> <LibraryClasses> >>>> >> NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf >>>> >> + # don't use unaligned CopyMem () on the UEFI varstore NOR flash region >>>> >> + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf >>>> >> } >>>> >> !if $(SECURE_BOOT_ENABLE) == TRUE >>>> >> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { >>>> >> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc >>>> >> index 9a31ec93ca06..7c032e1b07e0 100644 >>>> >> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc >>>> >> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc >>>> >> @@ -252,6 +252,8 @@ [Components.common] >>>> >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { >>>> >> <LibraryClasses> >>>> >> NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf >>>> >> + # don't use unaligned CopyMem () on the UEFI varstore NOR flash region >>>> >> + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf >>>> >> } >>>> >> !if $(SECURE_BOOT_ENABLE) == TRUE >>>> >> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { >>>> >> >>> > >>> > Reviewed-by: Laszlo Ersek <lersek@redhat.com> >>> > >>> > Given that we've never seen the symptom reported by Shannon, I must >>> > think Shannon is using some kind of new hardware. May I ask what >>> > hardware that is? >>> > >> I think this is equally likely to occur in any KVM hardware >> virtualization context. It is simply a regression after moving to the >> OptDxe BaseMemoryLib flavor. > > Exactly. I'm using Huawei D05. It works well when I use a older edk2 > while fails when updating to UDK2017 branch. > > Tested-by: Shannon Zhao <zhaoshenglong@huawei.com> > Pushed as 44d71c217ccb Thanks. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc index 8a60b61f2aa6..7b220d6e3c31 100644 --- a/ArmVirtPkg/ArmVirtQemu.dsc +++ b/ArmVirtPkg/ArmVirtQemu.dsc @@ -261,6 +261,8 @@ [Components.common] MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { <LibraryClasses> NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf + # don't use unaligned CopyMem () on the UEFI varstore NOR flash region + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf } !if $(SECURE_BOOT_ENABLE) == TRUE MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc index 9a31ec93ca06..7c032e1b07e0 100644 --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc @@ -252,6 +252,8 @@ [Components.common] MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf { <LibraryClasses> NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf + # don't use unaligned CopyMem () on the UEFI varstore NOR flash region + BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf } !if $(SECURE_BOOT_ENABLE) == TRUE MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
The VariableRuntimeDxe driver may use CopyMem () on NOR flash regions, assuming such regions always have full memory semantics. Given that those regions cannot be mapped as ordinary memory on ARM (due to the fact that the NOR flash requires device semantics while in write mode) this prevents us from using BaseMemoryLibOptDxe in VariableRuntimeDxe, since it may use unaligned accesses and/or DC ZVA instructions, both of which are incompatible with mappings using device semantics. Note that there is no way we can work around this by changing the mapping type between 'memory' and 'device' when switching from read to write mode and back, because the runtime mapping is created by the OS, and cannot be changed at will. So let's just switch to the unaccelerated version of BaseMemoryLib which does not have the same problem. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmVirtPkg/ArmVirtQemu.dsc | 2 ++ ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++ 2 files changed, 4 insertions(+) -- 2.11.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel