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

[Steve Wahl]

Gentle ping.

Thanks,

--> Steve Wahl

On Fri, Nov 03, 2023 at 02:01:08PM -0500, 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.
> 
> No attempt is made to coalesce mapping requests. If a request requires
> a map entry at the 2M (pmd) level, subsequent mapping requests within
> the same 1G region will also be at the pmd level, even if adjacent or
> overlapping such requests could theoretically have been combined to
> map a full gbpage.  Existing usage starts with larger regions and then
> adds smaller regions, so this should not have any great consequence.
> 
> When gbpages are used exclusively to create identity maps, large
> ranges of addresses not actually requested can be included in the
> resulting table.  On UV systems, this ends up including regions that
> will cause hardware to halt the system if accessed (these are marked
> "reserved" by BIOS).  Even though code does not actually make
> references to these addresses, including them in an active map allows
> processor speculation into this region, which is enough to trigger the
> system halt.
> 
> The kernel option "nogbpages" will disallow use of gbpages entirely
> and avoid this problem, but uses a lot of extra memory for page tables
> that are not really needed.
> 
> Signed-off-by: Steve Wahl <steve.wahl@xxxxxxx>
> ---
> v2: per Dave Hanson review: Additional changelog info,
>     moved pud_large() check earlier in the code, and
>     improved the comment describing the conditions
>     that restrict gbpage usage.
>    
>  arch/x86/mm/ident_map.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/mm/ident_map.c b/arch/x86/mm/ident_map.c
> index 968d7005f4a7..5c88c3a7d12a 100644
> --- a/arch/x86/mm/ident_map.c
> +++ b/arch/x86/mm/ident_map.c
> @@ -31,13 +31,23 @@ static int ident_pud_init(struct x86_mapping_info *info, pud_t *pud_page,
>  		if (next > end)
>  			next = end;
>  
> -		if (info->direct_gbpages) {
> -			pud_t pudval;
> +		/* if this is already a gbpage, this portion is already mapped */
> +		if (pud_large(*pud))
> +			continue;
>  
> -			if (pud_present(*pud))
> -				continue;
> +		/*
> +		 * 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)
> +		 */
> +		if (info->direct_gbpages
> +		    && !(addr & ~PUD_MASK)
> +		    && !(next & ~PUD_MASK)
> +		    && !pud_present(*pud)) {
> +			pud_t pudval;
>  
> -			addr &= PUD_MASK;
>  			pudval = __pud((addr - info->offset) | info->page_flag);
>  			set_pud(pud, pudval);
>  			continue;
> -- 
> 2.26.2
>