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

[Borislav Petkov <bp@alien8.de>]

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.

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