Re: [PATCH] efi_loader: remove EFI_BOUNCE_BUFFER

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 28.03.25 17:04, Tom Rini wrote:
> On Fri, Mar 28, 2025 at 02:26:39PM +0200, Ilias Apalodimas wrote:
>> On Fri, 28 Mar 2025 at 13:34, Simon Glass <sjg@chromium.org> wrote:
>>>
>>> Hi Ilias,
>>>
>>> On Thu, 27 Mar 2025 at 15:19, Ilias Apalodimas
>>> <ilias.apalodimas@linaro.org> wrote:
>>>>
>>>> Hi Simon
>>>>
>>>> On Thu, 27 Mar 2025 at 15:33, Simon Glass <sjg@chromium.org> wrote:
>>>>>
>>>>> Hi Ilias,
>>>>>
>>>>> On Wed, 26 Mar 2025 at 02:37, Ilias Apalodimas
>>>>> <ilias.apalodimas@linaro.org> wrote:
>>>>>>
>>>>>> Hi Heinrich,
>>>>>>
>>>>>> On Mon, 24 Mar 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>>>>
>>>>>>> On 17.03.25 14:38, Ilias Apalodimas wrote:
>>>>>>>
>>>>>>> %s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/
>>>>>>>
>>>>>>>> The EFI subsystem defines its own bounce buffer for devices that
>>>>>>>> can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
>>>>>>>> which can be reused instead of defining another symbol.
>>>>>>>> The only limitation for EFI is that we don't know how big the file a user
>>>>>>>> chooses to transfer is and as a result we can't depend on allocating the
>>>>>>>> memory from the malloc area, which can prove too small.
>>>>>>>>
>>>>>>>> So allocate an EFI buffer of the correct size and pass it to the DM,
>>>>>>>> which already supports bounce buffering via bounce_buffer_start_extalign()
>>>>>>>
>>>>>>> Looking at
>>>>>>>
>>>>>>>       if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {
>>>>>>>
>>>>>>> in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
>>>>>>> enabled by the device driver. Only the scsi drivers sets bb = true.
>>>>>>>
>>>>>>> Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
>>>>>>> to blk_desc")
>>>>>>>
>>>>>>> Which device-drivers of the boards mentioned below do actually need
>>>>>>> bounce buffering?
>>>>>>
>>>>>> Unfortunately, I don't have any of the hardware to test and I havent
>>>>>> worked with that platform much.
>>>>>> That 'bb' variable and the fact that EFI needs bigger allocations is
>>>>>> why I ended up allocationg properly aligned memory from the EFI
>>>>>> subsystem. But as Mark pointed out, the cache flush is a no go for
>>>>>> now, so I'll drop this and see if I find time to rework the bounce
>>>>>> buffer logic overall
>>>>>
>>>>> There was quite a bit of discussion about all this in the context of
>>>>> my attempt to just add a message to warn the user[1]
>>>>>
>>>>> We might consider adding an event to reserve memory before relocation,
>>>>> along with a way to discover (in board_r) where the memory was
>>>>> allocated. That would make the solution more generic.
>>>>
>>>> I am not sure what you are trying to solve here. The EFI bounce buffer
>>>> after the LMB patches can't overwrite memory, nor can it be
>>>> overwritten.
>>>
>>> I am thinking of we can create a single implementation of the
>>> bouncebuf logic which also works for EFI.
>>>
>>> I think the two sane things to do are:
>>> - restrict U-Boot to using memory below 4GB for platforms which have
>>> the DMA limitation
>>
>> You don't need that. The bounce buf code has a callback you can use to
>> define the limitations
>>
>>> - create (in board_f) a special region below 4GB for use with the
>>> bouncebuf logic
>>
>> The only problem with EFI is that you don't know how much memory it
>> needs and we can't use the existing memalign calls. So if we replace
>> that memalign in the bounce buffer code, with an lmb reservation we
>> have everything we need.
>
> It's not even an EFI problem is it? You could hit the same problem
> reading a file from a filesystem outside of EFI too. These specific
> SoCs just aren't heavily exercised is one of the challenges I think and
> so it's possible that we have a few things to yet improve in the
> bounce buffer code (which was added for other SoCs and done as generic
> enough starting point for others).
>

EFI or LMB allocate memory top down. So EFI applications have a good
chance of loading files into high memory. Other file-system operations
typically use predefined addresses line $loadaddr. This is why the
problem became more visible in EFI.

It is evident that the bounce-buffer functionality is not EFI specific
per se.

Best regards

Heinrich