* [PATCH v2] ArmVirtPkg/NorFlashQemuLib: disable NOR flash DT nodes upon discovery
@ 2020-06-24 11:15 Ard Biesheuvel
2020-06-24 11:48 ` Laszlo Ersek
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2020-06-24 11:15 UTC (permalink / raw)
To: devel; +Cc: lersek, philmd, Ard Biesheuvel
Our UEFI guest firmware takes ownership of the emulated NOR flash in
order to support the variable runtime services, and it does not expect
the OS to interfere with the underlying storage directly. So disable
the NOR flash DT nodes as we discover them, in a way similar to how we
disable the PL031 RTC in the device tree when we attach our RTC runtime
driver to it.
Note that this also hides the NOR flash bank that carries the UEFI
executable code, but this is not intended to be updatable from inside
the guest anyway, and if it was, we should use capsule update to do so.
Also, the first -pflash argument that defines the backing for this flash
bank is often issued with the 'readonly' modifier, in order to prevent
any changes whatsoever to be made to the executable firmware image by
the guest.
This issue has become relevant due to the following Linux changes,
which enable the flash driver stack for default build configurations
targetting arm64 and 32-bit ARM.
ce693fc2a877
("arm64: defconfig: Enable flash device drivers for QorIQ boards", 2020-03-16).
5f068190cc10
("ARM: multi_v7_defconfig: Enable support for CFI NOR FLASH", 2019-04-03)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
index 9b1d1184bdd3..425e36f2d127 100644
--- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
+++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
@@ -86,6 +86,22 @@ NorFlashPlatformGetDevices (
mNorFlashDevices[Num].BlockSize = QEMU_NOR_BLOCK_SIZE;
Num++;
}
+
+ //
+ // UEFI takes ownership of the NOR flash, and exposes its functionality
+ // through the UEFI Runtime Services GetVariable, SetVariable, etc. This
+ // means we need to disable it in the device tree to prevent the OS from
+ // attaching its device driver as well.
+ // Note that this also hides other flash banks, but the only other flash
+ // bank we expect to encounter is the one that carries the UEFI executable
+ // code, which is not intended to be guest updatable, and is usually backed
+ // in a readonly manner by QEMU anyway.
+ //
+ Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
+ "disabled", sizeof ("disabled"));
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "Failed to set NOR flash status to 'disabled'\n"));
+ }
}
*NorFlashDescriptions = mNorFlashDevices;
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] ArmVirtPkg/NorFlashQemuLib: disable NOR flash DT nodes upon discovery
2020-06-24 11:15 [PATCH v2] ArmVirtPkg/NorFlashQemuLib: disable NOR flash DT nodes upon discovery Ard Biesheuvel
@ 2020-06-24 11:48 ` Laszlo Ersek
2020-06-24 12:19 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2020-06-24 11:48 UTC (permalink / raw)
To: Ard Biesheuvel, devel; +Cc: philmd
On 06/24/20 13:15, Ard Biesheuvel wrote:
> Our UEFI guest firmware takes ownership of the emulated NOR flash in
> order to support the variable runtime services, and it does not expect
> the OS to interfere with the underlying storage directly. So disable
> the NOR flash DT nodes as we discover them, in a way similar to how we
> disable the PL031 RTC in the device tree when we attach our RTC runtime
> driver to it.
>
> Note that this also hides the NOR flash bank that carries the UEFI
> executable code, but this is not intended to be updatable from inside
> the guest anyway, and if it was, we should use capsule update to do so.
> Also, the first -pflash argument that defines the backing for this flash
> bank is often issued with the 'readonly' modifier, in order to prevent
> any changes whatsoever to be made to the executable firmware image by
> the guest.
>
> This issue has become relevant due to the following Linux changes,
> which enable the flash driver stack for default build configurations
> targetting arm64 and 32-bit ARM.
>
> ce693fc2a877
> ("arm64: defconfig: Enable flash device drivers for QorIQ boards", 2020-03-16).
>
> 5f068190cc10
> ("ARM: multi_v7_defconfig: Enable support for CFI NOR FLASH", 2019-04-03)
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
> ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> index 9b1d1184bdd3..425e36f2d127 100644
> --- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> +++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> @@ -86,6 +86,22 @@ NorFlashPlatformGetDevices (
> mNorFlashDevices[Num].BlockSize = QEMU_NOR_BLOCK_SIZE;
> Num++;
> }
> +
> + //
> + // UEFI takes ownership of the NOR flash, and exposes its functionality
> + // through the UEFI Runtime Services GetVariable, SetVariable, etc. This
> + // means we need to disable it in the device tree to prevent the OS from
> + // attaching its device driver as well.
> + // Note that this also hides other flash banks, but the only other flash
> + // bank we expect to encounter is the one that carries the UEFI executable
> + // code, which is not intended to be guest updatable, and is usually backed
> + // in a readonly manner by QEMU anyway.
> + //
> + Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
> + "disabled", sizeof ("disabled"));
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_WARN, "Failed to set NOR flash status to 'disabled'\n"));
> + }
> }
>
> *NorFlashDescriptions = mNorFlashDevices;
>
Thank you!
Laszlo
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] ArmVirtPkg/NorFlashQemuLib: disable NOR flash DT nodes upon discovery
2020-06-24 11:48 ` Laszlo Ersek
@ 2020-06-24 12:19 ` Ard Biesheuvel
2020-06-24 13:15 ` Philippe Mathieu-Daudé
2020-06-24 18:47 ` Laszlo Ersek
0 siblings, 2 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-06-24 12:19 UTC (permalink / raw)
To: Laszlo Ersek, devel; +Cc: philmd
On 6/24/20 1:48 PM, Laszlo Ersek wrote:
> On 06/24/20 13:15, Ard Biesheuvel wrote:
>> Our UEFI guest firmware takes ownership of the emulated NOR flash in
>> order to support the variable runtime services, and it does not expect
>> the OS to interfere with the underlying storage directly. So disable
>> the NOR flash DT nodes as we discover them, in a way similar to how we
>> disable the PL031 RTC in the device tree when we attach our RTC runtime
>> driver to it.
>>
>> Note that this also hides the NOR flash bank that carries the UEFI
>> executable code, but this is not intended to be updatable from inside
>> the guest anyway, and if it was, we should use capsule update to do so.
>> Also, the first -pflash argument that defines the backing for this flash
>> bank is often issued with the 'readonly' modifier, in order to prevent
>> any changes whatsoever to be made to the executable firmware image by
>> the guest.
>>
>> This issue has become relevant due to the following Linux changes,
>> which enable the flash driver stack for default build configurations
>> targetting arm64 and 32-bit ARM.
>>
>> ce693fc2a877
>> ("arm64: defconfig: Enable flash device drivers for QorIQ boards", 2020-03-16).
>>
>> 5f068190cc10
>> ("ARM: multi_v7_defconfig: Enable support for CFI NOR FLASH", 2019-04-03)
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> ---
>> ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>> index 9b1d1184bdd3..425e36f2d127 100644
>> --- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>> +++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>> @@ -86,6 +86,22 @@ NorFlashPlatformGetDevices (
>> mNorFlashDevices[Num].BlockSize = QEMU_NOR_BLOCK_SIZE;
>> Num++;
>> }
>> +
>> + //
>> + // UEFI takes ownership of the NOR flash, and exposes its functionality
>> + // through the UEFI Runtime Services GetVariable, SetVariable, etc. This
>> + // means we need to disable it in the device tree to prevent the OS from
>> + // attaching its device driver as well.
>> + // Note that this also hides other flash banks, but the only other flash
>> + // bank we expect to encounter is the one that carries the UEFI executable
>> + // code, which is not intended to be guest updatable, and is usually backed
>> + // in a readonly manner by QEMU anyway.
>> + //
>> + Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
>> + "disabled", sizeof ("disabled"));
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_WARN, "Failed to set NOR flash status to 'disabled'\n"));
>> + }
>> }
>>
>> *NorFlashDescriptions = mNorFlashDevices;
>>
Just noticed that both flash banks are actually emitted as a single DT
node, i.e.
flash@0 {
compatible = "cfi-flash";
reg = < 0x00 0x00 0x00 0x4000000 0x00 0x4000000 0x00 0x4000000 >;│
bank-width = < 0x04 >;
}
so in our case, the only straight-forward way to disable one of them is
to disable all of them (unless we want to poke around in the 'reg'
property).
This doesn't really make a difference for the reasoning above, so I
propose to apply the patch as is.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] ArmVirtPkg/NorFlashQemuLib: disable NOR flash DT nodes upon discovery
2020-06-24 12:19 ` Ard Biesheuvel
@ 2020-06-24 13:15 ` Philippe Mathieu-Daudé
2020-06-24 15:14 ` Ard Biesheuvel
2020-06-24 18:47 ` Laszlo Ersek
1 sibling, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-24 13:15 UTC (permalink / raw)
To: Ard Biesheuvel, Laszlo Ersek, devel
On 6/24/20 2:19 PM, Ard Biesheuvel wrote:
> On 6/24/20 1:48 PM, Laszlo Ersek wrote:
>> On 06/24/20 13:15, Ard Biesheuvel wrote:
>>> Our UEFI guest firmware takes ownership of the emulated NOR flash in
>>> order to support the variable runtime services, and it does not expect
>>> the OS to interfere with the underlying storage directly. So disable
>>> the NOR flash DT nodes as we discover them, in a way similar to how we
>>> disable the PL031 RTC in the device tree when we attach our RTC runtime
>>> driver to it.
>>>
>>> Note that this also hides the NOR flash bank that carries the UEFI
>>> executable code, but this is not intended to be updatable from inside
>>> the guest anyway, and if it was, we should use capsule update to do so.
>>> Also, the first -pflash argument that defines the backing for this flash
>>> bank is often issued with the 'readonly' modifier, in order to prevent
>>> any changes whatsoever to be made to the executable firmware image by
>>> the guest.
>>>
>>> This issue has become relevant due to the following Linux changes,
>>> which enable the flash driver stack for default build configurations
>>> targetting arm64 and 32-bit ARM.
>>>
>>> ce693fc2a877
>>> ("arm64: defconfig: Enable flash device drivers for QorIQ boards",
>>> 2020-03-16).
>>>
>>> 5f068190cc10
>>> ("ARM: multi_v7_defconfig: Enable support for CFI NOR FLASH",
>>> 2019-04-03)
>>>
>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>> ---
>>> ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c | 16
>>> ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>> b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>> index 9b1d1184bdd3..425e36f2d127 100644
>>> --- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>> +++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>> @@ -86,6 +86,22 @@ NorFlashPlatformGetDevices (
>>> mNorFlashDevices[Num].BlockSize = QEMU_NOR_BLOCK_SIZE;
>>> Num++;
>>> }
>>> +
>>> + //
>>> + // UEFI takes ownership of the NOR flash, and exposes its
>>> functionality
>>> + // through the UEFI Runtime Services GetVariable, SetVariable,
>>> etc. This
>>> + // means we need to disable it in the device tree to prevent the
>>> OS from
>>> + // attaching its device driver as well.
>>> + // Note that this also hides other flash banks, but the only
>>> other flash
>>> + // bank we expect to encounter is the one that carries the UEFI
>>> executable
>>> + // code, which is not intended to be guest updatable, and is
>>> usually backed
>>> + // in a readonly manner by QEMU anyway.
>>> + //
>>> + Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
>>> + "disabled", sizeof ("disabled"));
>>> + if (EFI_ERROR (Status)) {
>>> + DEBUG ((DEBUG_WARN, "Failed to set NOR flash status to
>>> 'disabled'\n"));
>>> + }
>>> }
>>> *NorFlashDescriptions = mNorFlashDevices;
>>>
>
> Just noticed that both flash banks are actually emitted as a single DT
> node, i.e.
>
> flash@0 {
> compatible = "cfi-flash";
> reg = < 0x00 0x00 0x00 0x4000000 0x00 0x4000000 0x00 0x4000000 >;│
> bank-width = < 0x04 >;
> }
IIRC the idea was to use protected banks, but QEMU doesn't model them,
so it was easier to use 1 ROM and 1 FLASH, but we accidentally ended
using 2 FLASH (the CODE one being 'read-only').
In one of the first Tianocore call I participated, someone from Intel
said they like the idea of having it FLASH and not ROM so they could
test the Capsule Update feature when QEMU support multiple banks &
locking.
The QEMU community is reluctant to change the pflash device as "it
just works for our needs".
I wonder how this DT node is consumed on the kernel side.
Ah, I suppose it trust the firmware, and doesn't CFI-query the flash
to verify its size. This is certainly fragile...
>
> so in our case, the only straight-forward way to disable one of them is
> to disable all of them (unless we want to poke around in the 'reg'
> property).
>
> This doesn't really make a difference for the reasoning above, so I
> propose to apply the patch as is.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] ArmVirtPkg/NorFlashQemuLib: disable NOR flash DT nodes upon discovery
2020-06-24 13:15 ` Philippe Mathieu-Daudé
@ 2020-06-24 15:14 ` Ard Biesheuvel
0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-06-24 15:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Laszlo Ersek, devel
On 6/24/20 3:15 PM, Philippe Mathieu-Daudé wrote:
> On 6/24/20 2:19 PM, Ard Biesheuvel wrote:
>> On 6/24/20 1:48 PM, Laszlo Ersek wrote:
>>> On 06/24/20 13:15, Ard Biesheuvel wrote:
>>>> Our UEFI guest firmware takes ownership of the emulated NOR flash in
>>>> order to support the variable runtime services, and it does not expect
>>>> the OS to interfere with the underlying storage directly. So disable
>>>> the NOR flash DT nodes as we discover them, in a way similar to how we
>>>> disable the PL031 RTC in the device tree when we attach our RTC runtime
>>>> driver to it.
>>>>
>>>> Note that this also hides the NOR flash bank that carries the UEFI
>>>> executable code, but this is not intended to be updatable from inside
>>>> the guest anyway, and if it was, we should use capsule update to do so.
>>>> Also, the first -pflash argument that defines the backing for this flash
>>>> bank is often issued with the 'readonly' modifier, in order to prevent
>>>> any changes whatsoever to be made to the executable firmware image by
>>>> the guest.
>>>>
>>>> This issue has become relevant due to the following Linux changes,
>>>> which enable the flash driver stack for default build configurations
>>>> targetting arm64 and 32-bit ARM.
>>>>
>>>> ce693fc2a877
>>>> ("arm64: defconfig: Enable flash device drivers for QorIQ boards",
>>>> 2020-03-16).
>>>>
>>>> 5f068190cc10
>>>> ("ARM: multi_v7_defconfig: Enable support for CFI NOR FLASH",
>>>> 2019-04-03)
>>>>
>>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>>> ---
>>>> ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c | 16
>>>> ++++++++++++++++
>>>> 1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>>> b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>>> index 9b1d1184bdd3..425e36f2d127 100644
>>>> --- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>>> +++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>>> @@ -86,6 +86,22 @@ NorFlashPlatformGetDevices (
>>>> mNorFlashDevices[Num].BlockSize = QEMU_NOR_BLOCK_SIZE;
>>>> Num++;
>>>> }
>>>> +
>>>> + //
>>>> + // UEFI takes ownership of the NOR flash, and exposes its
>>>> functionality
>>>> + // through the UEFI Runtime Services GetVariable, SetVariable,
>>>> etc. This
>>>> + // means we need to disable it in the device tree to prevent the
>>>> OS from
>>>> + // attaching its device driver as well.
>>>> + // Note that this also hides other flash banks, but the only
>>>> other flash
>>>> + // bank we expect to encounter is the one that carries the UEFI
>>>> executable
>>>> + // code, which is not intended to be guest updatable, and is
>>>> usually backed
>>>> + // in a readonly manner by QEMU anyway.
>>>> + //
>>>> + Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
>>>> + "disabled", sizeof ("disabled"));
>>>> + if (EFI_ERROR (Status)) {
>>>> + DEBUG ((DEBUG_WARN, "Failed to set NOR flash status to
>>>> 'disabled'\n"));
>>>> + }
>>>> }
>>>> *NorFlashDescriptions = mNorFlashDevices;
>>>>
>>
>> Just noticed that both flash banks are actually emitted as a single DT
>> node, i.e.
>>
>> flash@0 {
>> compatible = "cfi-flash";
>> reg = < 0x00 0x00 0x00 0x4000000 0x00 0x4000000 0x00 0x4000000 >;│
>> bank-width = < 0x04 >;
>> }
>
> IIRC the idea was to use protected banks, but QEMU doesn't model them,
> so it was easier to use 1 ROM and 1 FLASH, but we accidentally ended
> using 2 FLASH (the CODE one being 'read-only').
>
> In one of the first Tianocore call I participated, someone from Intel
> said they like the idea of having it FLASH and not ROM so they could
> test the Capsule Update feature when QEMU support multiple banks &
> locking.
> The QEMU community is reluctant to change the pflash device as "it
> just works for our needs".
>
> I wonder how this DT node is consumed on the kernel side.
> Ah, I suppose it trust the firmware, and doesn't CFI-query the flash
> to verify its size. This is certainly fragile...
>
Is it? The reg property contains two (base, size) tuples, and the driver
seems to treat them as two individual flash chips.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ArmVirtPkg/NorFlashQemuLib: disable NOR flash DT nodes upon discovery
2020-06-24 12:19 ` Ard Biesheuvel
2020-06-24 13:15 ` Philippe Mathieu-Daudé
@ 2020-06-24 18:47 ` Laszlo Ersek
1 sibling, 0 replies; 6+ messages in thread
From: Laszlo Ersek @ 2020-06-24 18:47 UTC (permalink / raw)
To: Ard Biesheuvel, devel; +Cc: philmd
On 06/24/20 14:19, Ard Biesheuvel wrote:
> On 6/24/20 1:48 PM, Laszlo Ersek wrote:
>> On 06/24/20 13:15, Ard Biesheuvel wrote:
>>> Our UEFI guest firmware takes ownership of the emulated NOR flash in
>>> order to support the variable runtime services, and it does not expect
>>> the OS to interfere with the underlying storage directly. So disable
>>> the NOR flash DT nodes as we discover them, in a way similar to how we
>>> disable the PL031 RTC in the device tree when we attach our RTC runtime
>>> driver to it.
>>>
>>> Note that this also hides the NOR flash bank that carries the UEFI
>>> executable code, but this is not intended to be updatable from inside
>>> the guest anyway, and if it was, we should use capsule update to do so.
>>> Also, the first -pflash argument that defines the backing for this flash
>>> bank is often issued with the 'readonly' modifier, in order to prevent
>>> any changes whatsoever to be made to the executable firmware image by
>>> the guest.
>>>
>>> This issue has become relevant due to the following Linux changes,
>>> which enable the flash driver stack for default build configurations
>>> targetting arm64 and 32-bit ARM.
>>>
>>> ce693fc2a877
>>> ("arm64: defconfig: Enable flash device drivers for QorIQ boards",
>>> 2020-03-16).
>>>
>>> 5f068190cc10
>>> ("ARM: multi_v7_defconfig: Enable support for CFI NOR FLASH",
>>> 2019-04-03)
>>>
>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>> ---
>>> ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c | 16
>>> ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>> b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>> index 9b1d1184bdd3..425e36f2d127 100644
>>> --- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>> +++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
>>> @@ -86,6 +86,22 @@ NorFlashPlatformGetDevices (
>>> mNorFlashDevices[Num].BlockSize = QEMU_NOR_BLOCK_SIZE;
>>> Num++;
>>> }
>>> +
>>> + //
>>> + // UEFI takes ownership of the NOR flash, and exposes its
>>> functionality
>>> + // through the UEFI Runtime Services GetVariable, SetVariable,
>>> etc. This
>>> + // means we need to disable it in the device tree to prevent the
>>> OS from
>>> + // attaching its device driver as well.
>>> + // Note that this also hides other flash banks, but the only
>>> other flash
>>> + // bank we expect to encounter is the one that carries the UEFI
>>> executable
>>> + // code, which is not intended to be guest updatable, and is
>>> usually backed
>>> + // in a readonly manner by QEMU anyway.
>>> + //
>>> + Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
>>> + "disabled", sizeof ("disabled"));
>>> + if (EFI_ERROR (Status)) {
>>> + DEBUG ((DEBUG_WARN, "Failed to set NOR flash status to
>>> 'disabled'\n"));
>>> + }
>>> }
>>> *NorFlashDescriptions = mNorFlashDevices;
>>>
>
> Just noticed that both flash banks are actually emitted as a single DT
> node, i.e.
>
> flash@0 {
> compatible = "cfi-flash";
> reg = < 0x00 0x00 0x00 0x4000000 0x00 0x4000000 0x00 0x4000000 >;│
> bank-width = < 0x04 >;
> }
>
> so in our case, the only straight-forward way to disable one of them is
> to disable all of them (unless we want to poke around in the 'reg'
> property).
>
> This doesn't really make a difference for the reasoning above, so I
> propose to apply the patch as is.
>
>
I'm OK with that!
Thanks
Laszlo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-24 18:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-24 11:15 [PATCH v2] ArmVirtPkg/NorFlashQemuLib: disable NOR flash DT nodes upon discovery Ard Biesheuvel
2020-06-24 11:48 ` Laszlo Ersek
2020-06-24 12:19 ` Ard Biesheuvel
2020-06-24 13:15 ` Philippe Mathieu-Daudé
2020-06-24 15:14 ` Ard Biesheuvel
2020-06-24 18:47 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox