[edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables

Jeremy Linton posted 9 patches 2 years, 3 months ago
Only 8 patches received!
There is a newer version of this series
Platform/RaspberryPi/AcpiTables/AcpiTables.inf     |   1 +
Platform/RaspberryPi/AcpiTables/Dsdt.asl           |   7 -
Platform/RaspberryPi/AcpiTables/GpuDevs.asl        | 125 ----
Platform/RaspberryPi/AcpiTables/SsdtGpio.asl       | 157 +++++
Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |  47 ++
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf    |   2 +
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni |  10 +
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr |  36 +-
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c       |  10 +-
Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf     |   1 +
.../Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c        |  62 +-
.../Drivers/VarBlockServiceDxe/FvbInfo.c           |   8 +-
.../Drivers/VarBlockServiceDxe/VarBlockService.c   | 654 ++++++++++++++++++++-
.../Drivers/VarBlockServiceDxe/VarBlockService.h   |  10 +
.../VarBlockServiceDxe/VarBlockServiceDxe.c        |  38 +-
.../VarBlockServiceDxe/VarBlockServiceDxe.inf      |   6 +
Platform/RaspberryPi/Include/ConfigVars.h          |   4 +
.../RaspberryPi/Include/IndustryStandard/RpiMbox.h |   1 +
.../RaspberryPi/Include/Protocol/RpiFirmware.h     |   7 +
Platform/RaspberryPi/RPi3/RPi3.dsc                 |  12 +
Platform/RaspberryPi/RPi4/RPi4.dsc                 |  14 +
Platform/RaspberryPi/RaspberryPi.dec               |   2 +
.../Bcm283x/Include/IndustryStandard/Bcm2836.h     |  34 ++
Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h |   6 +
Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c |  16 +-
25 files changed, 1107 insertions(+), 163 deletions(-)
create mode 100644 Platform/RaspberryPi/AcpiTables/SsdtGpio.asl
[edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables
Posted by Jeremy Linton 2 years, 3 months ago
The RPi4 has a SPI flash with unused capacity. This set detects if
that capacity is sufficient for a UEFI variable store and utilizes
it as such. This fixes a long list of problems, and along the way likely
also fixes a random boot failure caused by the FaultTolerantWriteDxe
garbage collecting, and erasing the flash volume header which is being
used to return information about the underlying variable storage capacity.

This set was dependent on an earlier, mostly ignored set of changes to
move the GPIO/etc devices into their own SSDT and disable them. Because
of that, the two sets have been merged.

Why is that? Because the SPI flash is mux'ed with the PWM used to play
audio out the 3.5mm audio jack on this device. This causes a long list
of problems we must try and avoid, starting with the fact that the pins
need to be controlled by the uefi runtime service. The other problem is
obviously that any time a variable is updated, if the user is utilizing
the 3.5mm audio they will hear clicks and pops. Turns out that behavior
isn't unique to this patch set because the low level boot/etc exhibits this
when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
fixes the majority of this, and the remaining runtime problems caused
by this patch actually are very slight and generally not noticeable unless
one goes looking for them. OTOH, we revert to the earlier non persisted
variable store if the firmware is running in a DT only mode, or the
user enables the ACPI GPIO block.


Jeremy Linton (9):
  Platform/RaspberryPi: Cleanup menu visibility
  Platform/RaspberryPi: Give the user control over the XHCI mailbox
  Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
  Platform/RaspberryPi: Add menu item to enable/disable GPIO
  Platform/RaspberryPi: Add constants for controlling SPI
  Platform/RaspberryPi: Add mailbox cmd to control audio amp
  Platform/RaspberryPi: Add SPI/GPIO to memory map
  Platform/RaspberryPi: Allow pin function selection at runtime
  Platform/RaspberryPi: Add SPI flash variable store.

 Platform/RaspberryPi/AcpiTables/AcpiTables.inf     |   1 +
 Platform/RaspberryPi/AcpiTables/Dsdt.asl           |   7 -
 Platform/RaspberryPi/AcpiTables/GpuDevs.asl        | 125 ----
 Platform/RaspberryPi/AcpiTables/SsdtGpio.asl       | 157 +++++
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |  47 ++
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf    |   2 +
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni |  10 +
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr |  36 +-
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c       |  10 +-
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf     |   1 +
 .../Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c        |  62 +-
 .../Drivers/VarBlockServiceDxe/FvbInfo.c           |   8 +-
 .../Drivers/VarBlockServiceDxe/VarBlockService.c   | 654 ++++++++++++++++++++-
 .../Drivers/VarBlockServiceDxe/VarBlockService.h   |  10 +
 .../VarBlockServiceDxe/VarBlockServiceDxe.c        |  38 +-
 .../VarBlockServiceDxe/VarBlockServiceDxe.inf      |   6 +
 Platform/RaspberryPi/Include/ConfigVars.h          |   4 +
 .../RaspberryPi/Include/IndustryStandard/RpiMbox.h |   1 +
 .../RaspberryPi/Include/Protocol/RpiFirmware.h     |   7 +
 Platform/RaspberryPi/RPi3/RPi3.dsc                 |  12 +
 Platform/RaspberryPi/RPi4/RPi4.dsc                 |  14 +
 Platform/RaspberryPi/RaspberryPi.dec               |   2 +
 .../Bcm283x/Include/IndustryStandard/Bcm2836.h     |  34 ++
 Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h |   6 +
 Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c |  16 +-
 25 files changed, 1107 insertions(+), 163 deletions(-)
 create mode 100644 Platform/RaspberryPi/AcpiTables/SsdtGpio.asl

-- 
2.13.7



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84253): https://edk2.groups.io/g/devel/message/84253
Mute This Topic: https://groups.io/mt/87456856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables
Posted by Ard Biesheuvel 2 years, 3 months ago
On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> The RPi4 has a SPI flash with unused capacity. This set detects if
> that capacity is sufficient for a UEFI variable store and utilizes
> it as such. This fixes a long list of problems, and along the way likely
> also fixes a random boot failure caused by the FaultTolerantWriteDxe
> garbage collecting, and erasing the flash volume header which is being
> used to return information about the underlying variable storage capacity.
>
> This set was dependent on an earlier, mostly ignored set of changes to
> move the GPIO/etc devices into their own SSDT and disable them. Because
> of that, the two sets have been merged.
>
> Why is that? Because the SPI flash is mux'ed with the PWM used to play
> audio out the 3.5mm audio jack on this device. This causes a long list
> of problems we must try and avoid, starting with the fact that the pins
> need to be controlled by the uefi runtime service. The other problem is
> obviously that any time a variable is updated, if the user is utilizing
> the 3.5mm audio they will hear clicks and pops. Turns out that behavior
> isn't unique to this patch set because the low level boot/etc exhibits this
> when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
> fixes the majority of this, and the remaining runtime problems caused
> by this patch actually are very slight and generally not noticeable unless
> one goes looking for them. OTOH, we revert to the earlier non persisted
> variable store if the firmware is running in a DT only mode, or the
> user enables the ACPI GPIO block.
>
>
> Jeremy Linton (9):
>   Platform/RaspberryPi: Cleanup menu visibility
>   Platform/RaspberryPi: Give the user control over the XHCI mailbox
>   Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
>   Platform/RaspberryPi: Add menu item to enable/disable GPIO
>   Platform/RaspberryPi: Add constants for controlling SPI
>   Platform/RaspberryPi: Add mailbox cmd to control audio amp
>   Platform/RaspberryPi: Add SPI/GPIO to memory map
>   Platform/RaspberryPi: Allow pin function selection at runtime
>   Platform/RaspberryPi: Add SPI flash variable store.
>

Very nice!

I am having trouble applying these patches, though. Could you please
resend without the random whitespace changes?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84263): https://edk2.groups.io/g/devel/message/84263
Mute This Topic: https://groups.io/mt/87456856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables
Posted by Ard Biesheuvel 2 years, 3 months ago
On Thu, 2 Dec 2021 at 18:03, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
> >
> > The RPi4 has a SPI flash with unused capacity. This set detects if
> > that capacity is sufficient for a UEFI variable store and utilizes
> > it as such. This fixes a long list of problems, and along the way likely
> > also fixes a random boot failure caused by the FaultTolerantWriteDxe
> > garbage collecting, and erasing the flash volume header which is being
> > used to return information about the underlying variable storage capacity.
> >
> > This set was dependent on an earlier, mostly ignored set of changes to
> > move the GPIO/etc devices into their own SSDT and disable them. Because
> > of that, the two sets have been merged.
> >
> > Why is that? Because the SPI flash is mux'ed with the PWM used to play
> > audio out the 3.5mm audio jack on this device. This causes a long list
> > of problems we must try and avoid, starting with the fact that the pins
> > need to be controlled by the uefi runtime service. The other problem is
> > obviously that any time a variable is updated, if the user is utilizing
> > the 3.5mm audio they will hear clicks and pops. Turns out that behavior
> > isn't unique to this patch set because the low level boot/etc exhibits this
> > when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
> > fixes the majority of this, and the remaining runtime problems caused
> > by this patch actually are very slight and generally not noticeable unless
> > one goes looking for them. OTOH, we revert to the earlier non persisted
> > variable store if the firmware is running in a DT only mode, or the
> > user enables the ACPI GPIO block.
> >
> >
> > Jeremy Linton (9):
> >   Platform/RaspberryPi: Cleanup menu visibility
> >   Platform/RaspberryPi: Give the user control over the XHCI mailbox
> >   Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
> >   Platform/RaspberryPi: Add menu item to enable/disable GPIO
> >   Platform/RaspberryPi: Add constants for controlling SPI
> >   Platform/RaspberryPi: Add mailbox cmd to control audio amp
> >   Platform/RaspberryPi: Add SPI/GPIO to memory map
> >   Platform/RaspberryPi: Allow pin function selection at runtime
> >   Platform/RaspberryPi: Add SPI flash variable store.
> >
>
> Very nice!
>
> I am having trouble applying these patches, though. Could you please
> resend without the random whitespace changes?

It appears only 2/9 is affected, the remaining ones apply cleanly,
with the exception of 9/9, which seems to be missing entirely. Could
you please resend that one as well?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84264): https://edk2.groups.io/g/devel/message/84264
Mute This Topic: https://groups.io/mt/87456856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables
Posted by Jeremy Linton 2 years, 3 months ago
On 12/2/21 11:09, Ard Biesheuvel wrote:
> On Thu, 2 Dec 2021 at 18:03, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>
>>> The RPi4 has a SPI flash with unused capacity. This set detects if
>>> that capacity is sufficient for a UEFI variable store and utilizes
>>> it as such. This fixes a long list of problems, and along the way likely
>>> also fixes a random boot failure caused by the FaultTolerantWriteDxe
>>> garbage collecting, and erasing the flash volume header which is being
>>> used to return information about the underlying variable storage capacity.
>>>
>>> This set was dependent on an earlier, mostly ignored set of changes to
>>> move the GPIO/etc devices into their own SSDT and disable them. Because
>>> of that, the two sets have been merged.
>>>
>>> Why is that? Because the SPI flash is mux'ed with the PWM used to play
>>> audio out the 3.5mm audio jack on this device. This causes a long list
>>> of problems we must try and avoid, starting with the fact that the pins
>>> need to be controlled by the uefi runtime service. The other problem is
>>> obviously that any time a variable is updated, if the user is utilizing
>>> the 3.5mm audio they will hear clicks and pops. Turns out that behavior
>>> isn't unique to this patch set because the low level boot/etc exhibits this
>>> when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
>>> fixes the majority of this, and the remaining runtime problems caused
>>> by this patch actually are very slight and generally not noticeable unless
>>> one goes looking for them. OTOH, we revert to the earlier non persisted
>>> variable store if the firmware is running in a DT only mode, or the
>>> user enables the ACPI GPIO block.
>>>
>>>
>>> Jeremy Linton (9):
>>>    Platform/RaspberryPi: Cleanup menu visibility
>>>    Platform/RaspberryPi: Give the user control over the XHCI mailbox
>>>    Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
>>>    Platform/RaspberryPi: Add menu item to enable/disable GPIO
>>>    Platform/RaspberryPi: Add constants for controlling SPI
>>>    Platform/RaspberryPi: Add mailbox cmd to control audio amp
>>>    Platform/RaspberryPi: Add SPI/GPIO to memory map
>>>    Platform/RaspberryPi: Allow pin function selection at runtime
>>>    Platform/RaspberryPi: Add SPI flash variable store.
>>>
>>
>> Very nice!
>>
>> I am having trouble applying these patches, though. Could you please
>> resend without the random whitespace changes?
> 
> It appears only 2/9 is affected, the remaining ones apply cleanly,
> with the exception of 9/9, which seems to be missing entirely. Could
> you please resend that one as well?
> 

Yah, so it looks like 2/9 picked up a couple tab/trailing whitespace 
cleanups when I ran it through the edk2 checkpatch right before sending 
it. I can kill those, although, that doesn't really explain why it 
doesn't apply. Both the copies I got look ok, although the groups.io 
version changed the content-type from text/plain/7bit to 
text/plain/quoted-printable, which is disturbing if it decided to quote 
parts of the patch.

9/9 though seems to be my side, because it got rejected due to UTF-8 
whitespace on the last line of the patch not being 7-bit.

My mail process on this ML seems to continue to be a pain, and i'm not 
entirely sure why its special (I post patches elseswhere with a lot less 
trouble).



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84266): https://edk2.groups.io/g/devel/message/84266
Mute This Topic: https://groups.io/mt/87456856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables
Posted by Jeremy Linton 2 years, 3 months ago
Hi,


On 12/2/21 11:09, Ard Biesheuvel wrote:
> On Thu, 2 Dec 2021 at 18:03, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>
>>> The RPi4 has a SPI flash with unused capacity. This set detects if
>>> that capacity is sufficient for a UEFI variable store and utilizes
>>> it as such. This fixes a long list of problems, and along the way likely
>>> also fixes a random boot failure caused by the FaultTolerantWriteDxe
>>> garbage collecting, and erasing the flash volume header which is being
>>> used to return information about the underlying variable storage capacity.
>>>
>>> This set was dependent on an earlier, mostly ignored set of changes to
>>> move the GPIO/etc devices into their own SSDT and disable them. Because
>>> of that, the two sets have been merged.
>>>
>>> Why is that? Because the SPI flash is mux'ed with the PWM used to play
>>> audio out the 3.5mm audio jack on this device. This causes a long list
>>> of problems we must try and avoid, starting with the fact that the pins
>>> need to be controlled by the uefi runtime service. The other problem is
>>> obviously that any time a variable is updated, if the user is utilizing
>>> the 3.5mm audio they will hear clicks and pops. Turns out that behavior
>>> isn't unique to this patch set because the low level boot/etc exhibits this
>>> when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
>>> fixes the majority of this, and the remaining runtime problems caused
>>> by this patch actually are very slight and generally not noticeable unless
>>> one goes looking for them. OTOH, we revert to the earlier non persisted
>>> variable store if the firmware is running in a DT only mode, or the
>>> user enables the ACPI GPIO block.
>>>
>>>
>>> Jeremy Linton (9):
>>>    Platform/RaspberryPi: Cleanup menu visibility
>>>    Platform/RaspberryPi: Give the user control over the XHCI mailbox
>>>    Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
>>>    Platform/RaspberryPi: Add menu item to enable/disable GPIO
>>>    Platform/RaspberryPi: Add constants for controlling SPI
>>>    Platform/RaspberryPi: Add mailbox cmd to control audio amp
>>>    Platform/RaspberryPi: Add SPI/GPIO to memory map
>>>    Platform/RaspberryPi: Allow pin function selection at runtime
>>>    Platform/RaspberryPi: Add SPI flash variable store.
>>>
>>
>> Very nice!
>>
>> I am having trouble applying these patches, though. Could you please
>> resend without the random whitespace changes?
> 
> It appears only 2/9 is affected, the remaining ones apply cleanly,
> with the exception of 9/9, which seems to be missing entirely. Could
> you please resend that one as well?
> 

Hi,

So, 2/2 was probably me too, I resent it as well with the same subject 
but of course the email thread id isn't right.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84270): https://edk2.groups.io/g/devel/message/84270
Mute This Topic: https://groups.io/mt/87456856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables
Posted by Ard Biesheuvel 2 years, 3 months ago
On Thu, 2 Dec 2021 at 18:55, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Hi,
>
>
> On 12/2/21 11:09, Ard Biesheuvel wrote:
> > On Thu, 2 Dec 2021 at 18:03, Ard Biesheuvel <ardb@kernel.org> wrote:
> >>
> >> On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
> >>>
> >>> The RPi4 has a SPI flash with unused capacity. This set detects if
> >>> that capacity is sufficient for a UEFI variable store and utilizes
> >>> it as such. This fixes a long list of problems, and along the way likely
> >>> also fixes a random boot failure caused by the FaultTolerantWriteDxe
> >>> garbage collecting, and erasing the flash volume header which is being
> >>> used to return information about the underlying variable storage capacity.
> >>>
> >>> This set was dependent on an earlier, mostly ignored set of changes to
> >>> move the GPIO/etc devices into their own SSDT and disable them. Because
> >>> of that, the two sets have been merged.
> >>>
> >>> Why is that? Because the SPI flash is mux'ed with the PWM used to play
> >>> audio out the 3.5mm audio jack on this device. This causes a long list
> >>> of problems we must try and avoid, starting with the fact that the pins
> >>> need to be controlled by the uefi runtime service. The other problem is
> >>> obviously that any time a variable is updated, if the user is utilizing
> >>> the 3.5mm audio they will hear clicks and pops. Turns out that behavior
> >>> isn't unique to this patch set because the low level boot/etc exhibits this
> >>> when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
> >>> fixes the majority of this, and the remaining runtime problems caused
> >>> by this patch actually are very slight and generally not noticeable unless
> >>> one goes looking for them. OTOH, we revert to the earlier non persisted
> >>> variable store if the firmware is running in a DT only mode, or the
> >>> user enables the ACPI GPIO block.
> >>>
> >>>
> >>> Jeremy Linton (9):
> >>>    Platform/RaspberryPi: Cleanup menu visibility
> >>>    Platform/RaspberryPi: Give the user control over the XHCI mailbox
> >>>    Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
> >>>    Platform/RaspberryPi: Add menu item to enable/disable GPIO
> >>>    Platform/RaspberryPi: Add constants for controlling SPI
> >>>    Platform/RaspberryPi: Add mailbox cmd to control audio amp
> >>>    Platform/RaspberryPi: Add SPI/GPIO to memory map
> >>>    Platform/RaspberryPi: Allow pin function selection at runtime
> >>>    Platform/RaspberryPi: Add SPI flash variable store.
> >>>
> >>
> >> Very nice!
> >>
> >> I am having trouble applying these patches, though. Could you please
> >> resend without the random whitespace changes?
> >
> > It appears only 2/9 is affected, the remaining ones apply cleanly,
> > with the exception of 9/9, which seems to be missing entirely. Could
> > you please resend that one as well?
> >
>
> Hi,
>
> So, 2/2 was probably me too, I resent it as well with the same subject
> but of course the email thread id isn't right.

Thanks

I gave this a spin, and Boot#### variables created by the Debian
installer persisted as expected, so

Tested-by: Ard Biesheuvel <ardb@kernel.org>

Before I merge this, though, could you elaborate on how playing with
the SPI flash like this is not going to brick my RPi4? Also, others,
please chime in as well.

-- 
Ard.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84333): https://edk2.groups.io/g/devel/message/84333
Mute This Topic: https://groups.io/mt/87456856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables
Posted by Jeremy Linton 2 years, 3 months ago
Hi,

On 12/3/21 12:12, Ard Biesheuvel wrote:
> On Thu, 2 Dec 2021 at 18:55, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>
>> Hi,
>>
>>
>> On 12/2/21 11:09, Ard Biesheuvel wrote:
>>> On Thu, 2 Dec 2021 at 18:03, Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>
>>>> On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>>>
>>>>> The RPi4 has a SPI flash with unused capacity. This set detects if
>>>>> that capacity is sufficient for a UEFI variable store and utilizes
>>>>> it as such. This fixes a long list of problems, and along the way likely
>>>>> also fixes a random boot failure caused by the FaultTolerantWriteDxe
>>>>> garbage collecting, and erasing the flash volume header which is being
>>>>> used to return information about the underlying variable storage capacity.
>>>>>
>>>>> This set was dependent on an earlier, mostly ignored set of changes to
>>>>> move the GPIO/etc devices into their own SSDT and disable them. Because
>>>>> of that, the two sets have been merged.
>>>>>
>>>>> Why is that? Because the SPI flash is mux'ed with the PWM used to play
>>>>> audio out the 3.5mm audio jack on this device. This causes a long list
>>>>> of problems we must try and avoid, starting with the fact that the pins
>>>>> need to be controlled by the uefi runtime service. The other problem is
>>>>> obviously that any time a variable is updated, if the user is utilizing
>>>>> the 3.5mm audio they will hear clicks and pops. Turns out that behavior
>>>>> isn't unique to this patch set because the low level boot/etc exhibits this
>>>>> when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
>>>>> fixes the majority of this, and the remaining runtime problems caused
>>>>> by this patch actually are very slight and generally not noticeable unless
>>>>> one goes looking for them. OTOH, we revert to the earlier non persisted
>>>>> variable store if the firmware is running in a DT only mode, or the
>>>>> user enables the ACPI GPIO block.
>>>>>
>>>>>
>>>>> Jeremy Linton (9):
>>>>>     Platform/RaspberryPi: Cleanup menu visibility
>>>>>     Platform/RaspberryPi: Give the user control over the XHCI mailbox
>>>>>     Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
>>>>>     Platform/RaspberryPi: Add menu item to enable/disable GPIO
>>>>>     Platform/RaspberryPi: Add constants for controlling SPI
>>>>>     Platform/RaspberryPi: Add mailbox cmd to control audio amp
>>>>>     Platform/RaspberryPi: Add SPI/GPIO to memory map
>>>>>     Platform/RaspberryPi: Allow pin function selection at runtime
>>>>>     Platform/RaspberryPi: Add SPI flash variable store.
>>>>>
>>>>
>>>> Very nice!
>>>>
>>>> I am having trouble applying these patches, though. Could you please
>>>> resend without the random whitespace changes?
>>>
>>> It appears only 2/9 is affected, the remaining ones apply cleanly,
>>> with the exception of 9/9, which seems to be missing entirely. Could
>>> you please resend that one as well?
>>>
>>
>> Hi,
>>
>> So, 2/2 was probably me too, I resent it as well with the same subject
>> but of course the email thread id isn't right.
> 
> Thanks
> 
> I gave this a spin, and Boot#### variables created by the Debian
> installer persisted as expected, so
> 
> Tested-by: Ard Biesheuvel <ardb@kernel.org>
> 
> Before I merge this, though, could you elaborate on how playing with
> the SPI flash like this is not going to brick my RPi4? Also, others,
> please chime in as well.
> 


First though, in the constant tweaking of patches, I noticed that 6/9 
"Add mailbox command to control audio amp" should probably have the LDO 
state DEBUG_ERROR's removed/reduced (I just removed them). NBD either 
way I guess.

So, back to how you won't permanently brick your rpi. Bricking it seems 
a lot harder than random SPI corruption, which I managed to achieve a 
few times while developing this set. More than once I corrupted it 
sufficiently to keep the low level bootloader from running. In those 
cases the rpi foundation's 
https://www.raspberrypi.com/news/raspberry-pi-imager-imaging-utility/ 
imaging tool has an option "Bootloader EEPOM configuration", which 
creates an SD image that the SoC will prefer to boot from over the SPI 
flash. That utility erases the entire flash and writes the latest 
bootloader image to it. The whole process takes a few seconds if one 
keeps the recovery disk handy.

So, I think we are good if someone decides to run that utility to 
upgrade their "EEPROM", or we have bugs that corrupt it. My larger worry 
is that we create upgrade problems with the EFI firmware itself, but I 
don't see any evidence of that happening yet, we just need to be careful 
about how we initialize new variables to avoid a situation where the 
user has to use that utility to reset the EFI variable portion of the flash.

The other issue is that the rpi foundation hasn't made any guarantees 
that this space will remain available in the future, which this code 
should deal with as is, by reverting to the previous behavior. If/when 
they do that we can trim some of their fat, or ask them politely to 
create a reduced feature version for us (say by removing nvme boot/etc), 
or simply keep using the older versions until we find legitimate 
problems with them.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84335): https://edk2.groups.io/g/devel/message/84335
Mute This Topic: https://groups.io/mt/87456856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-