Re: [PATCHv13 4/9] x86/boot/compressed: Handle unaccepted memory

From: Borislav Petkov
Date: Fri Jun 02 2023 - 12:09:08 EST


On Fri, Jun 02, 2023 at 06:36:44PM +0300, Kirill A. Shutemov wrote:
> I don't see why it is simpler. It looks unnecessary noisy to me.

Noisy?

I have no clue what you mean.

It is regular:

if (bla && flu)

vs

if (bla)
return flu();

It is about having regular patterns which can be recognized at a quick
glance by those who get to stare at that code constantly.

> Configuration table suppose to be present, even if unaccepted memory is
> not supported. Something is very wrong if it is missing.

I am not sure if it is the decompressor's job to do such validation
- I guess this is something the EFI code should do.

> I will downgrade it warn().

Yes, or simply return here without accepting memory - plain and simple.

> I wanted to keep unaccepted_table private to the libstub/unaccepted_memory.c.
> The setter provides a good spot for documentation to guide unaccepted
> memory enablers for other archs.
>
> Still want replace it with direct assignment?

No clue. Why would you want to keep a variable in the libstub private
which is not even in kernel proper, AFAICT?

> Okay, I will make init_unaccepted_memory() return true if unaccepted
> memory is present and hide defined it always-false for !UNACCEPTED_MEMORY.
> So this hunk will look this way:
>
> if (init_unaccepted_memory()) {
> debug_putstr("Accepting memory... ");
> accept_memory(__pa(output), __pa(output) + needed_size);
> }

Yap, thanks.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette