Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <b9fec908-5f57-4f77-72f1-7d5eb0898e8f@redhat.com>
Date:   Thu, 10 Mar 2022 17:23:26 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.0
Subject: Re: [PATCH 4/4] x86/platform/uv: Add gap hole end size
Content-Language: en-US
To:     Steve Wahl <steve.wahl@hpe.com>, Mike Travis <mike.travis@hpe.com>
Cc:     Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>, x86@kernel.org,
        Andy Shevchenko <andy@infradead.org>,
        Darren Hart <dvhart@infradead.org>,
        Dimitri Sivanich <dimitri.sivanich@hpe.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Russ Anderson <russ.anderson@hpe.com>,
        linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org
References: <20220308010537.70150-1-mike.travis@hpe.com>
 <20220308010537.70150-5-mike.travis@hpe.com>
 <Yid/x8NUqOP8fO+7@swahl-home.5wahls.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <Yid/x8NUqOP8fO+7@swahl-home.5wahls.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 3/8/22 17:09, Steve Wahl wrote:
> 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*?"

I agree that "Log gap hole end size" would be a better subject for this patch.

Regards,

Hans


> 
> 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
>>
>