* [PATCH] ArmVirtPkg/ArmVirtQemu: use non-accelerated CopyMem for VariableRuntimeDxe
@ 2017-11-14 10:22 Ard Biesheuvel
2017-11-15 13:51 ` Laszlo Ersek
0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2017-11-14 10:22 UTC (permalink / raw)
To: edk2-devel, lersek; +Cc: zhaoshenglong, Ard Biesheuvel
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 {
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] ArmVirtPkg/ArmVirtQemu: use non-accelerated CopyMem for VariableRuntimeDxe
2017-11-14 10:22 [PATCH] ArmVirtPkg/ArmVirtQemu: use non-accelerated CopyMem for VariableRuntimeDxe Ard Biesheuvel
@ 2017-11-15 13:51 ` Laszlo Ersek
2017-11-15 14:03 ` Ard Biesheuvel
0 siblings, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2017-11-15 13:51 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ArmVirtPkg/ArmVirtQemu: use non-accelerated CopyMem for VariableRuntimeDxe
2017-11-15 13:51 ` Laszlo Ersek
@ 2017-11-15 14:03 ` Ard Biesheuvel
2017-11-16 1:01 ` Shannon Zhao
0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2017-11-15 14:03 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel@lists.01.org, Shannon Zhao
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ArmVirtPkg/ArmVirtQemu: use non-accelerated CopyMem for VariableRuntimeDxe
2017-11-15 14:03 ` Ard Biesheuvel
@ 2017-11-16 1:01 ` Shannon Zhao
2017-11-16 9:39 ` Ard Biesheuvel
0 siblings, 1 reply; 5+ messages in thread
From: Shannon Zhao @ 2017-11-16 1:01 UTC (permalink / raw)
To: Ard Biesheuvel, Laszlo Ersek; +Cc: edk2-devel@lists.01.org
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ArmVirtPkg/ArmVirtQemu: use non-accelerated CopyMem for VariableRuntimeDxe
2017-11-16 1:01 ` Shannon Zhao
@ 2017-11-16 9:39 ` Ard Biesheuvel
0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2017-11-16 9:39 UTC (permalink / raw)
To: Shannon Zhao; +Cc: Laszlo Ersek, edk2-devel@lists.01.org
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-16 9:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-14 10:22 [PATCH] ArmVirtPkg/ArmVirtQemu: use non-accelerated CopyMem for VariableRuntimeDxe Ard Biesheuvel
2017-11-15 13:51 ` Laszlo Ersek
2017-11-15 14:03 ` Ard Biesheuvel
2017-11-16 1:01 ` Shannon Zhao
2017-11-16 9:39 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox