Re: [repost PATCH v2] x86/mm/ident_map: Use gbpages only where full GB page should be mapped.

[Dave Hansen]

On 1/24/24 09:36, Steve Wahl wrote:
> Instead of using gbpages for all memory regions, which can include
> vast areas outside what's actually been requested, use them only when
> map creation requests include the full GB page of space; descend to
> using smaller 2M pages when only portions of a GB page are included in
> the request.

This kinda jumps immediately to the solution without stating the problem.

The problem is something like this:

	Right now, ident_pud_init() will use 1GB pages to map an area as
	long as 1G pages are supported.  It does not consider the size
	of the area being mapped.  Mapping 1G?  Use a 1GB mapping.
	Mapping 4k?  Also use a 1GB mapping.  On UV systems, this ends
	up mapping BIOS-reserved regions that will cause hardware to
	halt the system if accessed, even speculatively.

Right?

> +		/*
> +		 * To be eligible to use a gbpage:
> +		 *   - gbpages must be enabled
> +		 *   - addr must be gb aligned (start of region)
> +		 *   - next must be gb aligned (end of region)
> +		 *   - PUD must be empty (nothing already mapped in this region)
> +		 */

.. this also needs to say _why_.  As it stands, it kinda just rewrites
the code in English which isn't super helpful.  It's also awfully
awkward to write a multi-line comment above a multi-line if().

Why not refactor it to do something like:
	
		bool can_use_gbpages = info->direct_gbpages;

		/* Avoid using a gbpage when it would be too large: */
		can_use_gbpages &= (addr & ~PUD_MASK) ||
				   (next & ~PUD_MASK);

		/* Never overwrite existing mappings: */
		can_use_gbpages &= !pud_present(*pud);

		if (can_use_gbpages) {
			...