Re: [PATCHv5 08/12] x86/mm: Provide helpers for unaccepted memory

Borislav Petkov
2022-05-04T11:12:11Z

On Mon, Apr 25, 2022 at 06:39:30AM +0300, Kirill A. Shutemov wrote:
> +/* Protects unaccepted memory bitmap */
> +static DEFINE_SPINLOCK(unaccepted_memory_lock);
> +
> +void accept_memory(phys_addr_t start, phys_addr_t end)
> +{
> + unsigned long *unaccepted_memory;

shorten that name.

> + unsigned long flags;
> + unsigned long range_start, range_end;

The tip-tree preferred ordering of variable declarations at the
beginning of a function is reverse fir tree order::

struct long_struct_name *descriptive_name;
unsigned long foo, bar;
unsigned int tmp;
int ret;

The above is faster to parse than the reverse ordering::

int ret;
unsigned int tmp;
unsigned long foo, bar;
struct long_struct_name *descriptive_name;

And even more so than random ordering::

unsigned long foo, bar;
int ret;
struct long_struct_name *descriptive_name;
unsigned int tmp;

> +
> + if (!boot_params.unaccepted_memory)
> + return;
> +
> + unaccepted_memory = __va(boot_params.unaccepted_memory);
> + range_start = start / PMD_SIZE;
> +
> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
> + for_each_set_bitrange_from(range_start, range_end, unaccepted_memory,
> + DIV_ROUND_UP(end, PMD_SIZE)) {
> + unsigned long len = range_end - range_start;
> +
> + /* Platform-specific memory-acceptance call goes here */
> + panic("Cannot accept memory");

Yeah, no, WARN_ON_ONCE() pls.

> + bitmap_clear(unaccepted_memory, range_start, len);
> + }
> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
> +}
> +
> +bool memory_is_unaccepted(phys_addr_t start, phys_addr_t end)
> +{
> + unsigned long *unaccepted_memory = __va(boot_params.unaccepted_memory);

As above, shorten that one.

> + unsigned long flags;
> + bool ret = false;
> +
> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
> + while (start < end) {
> + if (test_bit(start / PMD_SIZE, unaccepted_memory)) {
> + ret = true;

Wait, what?

That thing is lying: it'll return true for *some* PMD which is accepted
but not the whole range of [start, end].

--
Regards/Gruss,
Boris.