Re: [PATCH 4/4] x86/platform/uv: Add gap hole end size

[Steve Wahl <steve.wahl@hpe.com>]

Mike,

I know you're trying to get this out and don't really need another
delta, and I'd be holding it back if I didn't think it might make
things smoother upstream.

But what I'd consider for this one is:  Add the word log to the
subject line, perhaps "Add gap hole end size to log", or just "Log gap
hole end size".  Without it, the reviewer has to ask "add to *where*?"

And I believe the second sentence of the description, "The structure
stores PA bits 56:26, for > 64MB granularity, up to 64PB max size," is
perhaps not necessary, and I think it may slow down somebody trying to
read the patch quickly.  So I'd consider deleting it.

With those two changes the description still matches the code, and
seems simpler and easier to accept.

Your call on either / both, of course.

--> Steve

On Mon, Mar 07, 2022 at 07:05:37PM -0600, Mike Travis wrote:
> Show value of gap end in kernel log which equates to number of physical
> address bits used by system.  The structure stores PA bits 56:26, for
> 64MB granularity, up to 64PB max size.
> 
> Signed-off-by: Mike Travis <mike.travis@hpe.com>
> Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
> ---
>  arch/x86/kernel/apic/x2apic_uv_x.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
> index 387d6533549a..146f0f63a43b 100644
> --- a/arch/x86/kernel/apic/x2apic_uv_x.c
> +++ b/arch/x86/kernel/apic/x2apic_uv_x.c
> @@ -1346,7 +1346,7 @@ static void __init decode_gam_params(unsigned long ptr)
>  static void __init decode_gam_rng_tbl(unsigned long ptr)
>  {
>  	struct uv_gam_range_entry *gre = (struct uv_gam_range_entry *)ptr;
> -	unsigned long lgre = 0;
> +	unsigned long lgre = 0, gend = 0;
>  	int index = 0;
>  	int sock_min = 999999, pnode_min = 99999;
>  	int sock_max = -1, pnode_max = -1;
> @@ -1380,6 +1380,9 @@ static void __init decode_gam_rng_tbl(unsigned long ptr)
>  			flag, size, suffix[order],
>  			gre->type, gre->nasid, gre->sockid, gre->pnode);
>  
> +		if (gre->type == UV_GAM_RANGE_TYPE_HOLE)
> +			gend = (unsigned long)gre->limit << UV_GAM_RANGE_SHFT;
> +
>  		/* update to next range start */
>  		lgre = gre->limit;
>  		if (sock_min > gre->sockid)
> @@ -1397,7 +1400,8 @@ static void __init decode_gam_rng_tbl(unsigned long ptr)
>  	_max_pnode	= pnode_max;
>  	_gr_table_len	= index;
>  
> -	pr_info("UV: GRT: %d entries, sockets(min:%x,max:%x) pnodes(min:%x,max:%x)\n", index, _min_socket, _max_socket, _min_pnode, _max_pnode);
> +	pr_info("UV: GRT: %d entries, sockets(min:%x,max:%x), pnodes(min:%x,max:%x), gap_end(%d)\n",
> +	  index, _min_socket, _max_socket, _min_pnode, _max_pnode, fls64(gend));
>  }
>  
>  /* Walk through UVsystab decoding the fields */
> -- 
> 2.26.2
> 

-- 
Steve Wahl, Hewlett Packard Enterprise