[PATCH 0/4] x86/platform/uv: UV Kernel support for UV5

[PATCH 0/4] x86/platform/uv: UV Kernel support for UV5

[Mike Travis]

    Remove obsolete scratch5 NMI handler
	Removes obsolete scratch5 NMI handler only used in UV1 and early UV2
	systems.

    Update NMI setup for UV5
	Update NMI handler to interface with UV5 hardware. This involves
	changing the EVENT_OCCURRED MMR used by the hardware and removes
	the check for the newer NMI function supported by UV BIOS.

    Update TSC sync check for UV5
	Update TSC to not check TSC sync state for uv5+ as it is not
	available.  Therefore it is assumed that TSC will always be in
	sync for multiple chassis and will pass the tests for the kernel
	to accept it as the clocksource.  To disable this check use the
	kernel start options tsc=reliable clocksource=tsc.

    Add gap hole end size
	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.


Mike Travis (4):
  x86/platform/uv: Remove Obsolete Scratch5 NMI handler
  x86/platform/uv: Update NMI Handler for UV5
  x86/platform/uv: Update TSC sync state for UV5
  x86/platform/uv: Add gap hole end size

 arch/x86/include/asm/uv/uv_hub.h   |  6 ------
 arch/x86/kernel/apic/x2apic_uv_x.c | 20 +++++++++++++++-----
 arch/x86/platform/uv/uv_nmi.c      | 27 ++++++++++++---------------
 3 files changed, 27 insertions(+), 26 deletions(-)

-- 
2.26.2

[PATCH 1/4] x86/platform/uv: Remove Obsolete Scratch5 NMI handler

[Mike Travis]

Removes obsolete scratch5 NMI handler only used in UV1 and early UV2
systems.

Signed-off-by: Mike Travis <mike.travis@hpe.com>
Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
---
 arch/x86/include/asm/uv/uv_hub.h | 6 ------
 arch/x86/platform/uv/uv_nmi.c    | 5 +----
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/uv/uv_hub.h b/arch/x86/include/asm/uv/uv_hub.h
index d3e3197917be..958c18f94146 100644
--- a/arch/x86/include/asm/uv/uv_hub.h
+++ b/arch/x86/include/asm/uv/uv_hub.h
@@ -729,12 +729,6 @@ extern void uv_nmi_setup_hubless(void);
 #define UVH_TSC_SYNC_VALID	3	/* 0011 */
 #define UVH_TSC_SYNC_UNKNOWN	0	/* 0000 */
 
-/* BMC sets a bit this MMR non-zero before sending an NMI */
-#define UVH_NMI_MMR		UVH_BIOS_KERNEL_MMR
-#define UVH_NMI_MMR_CLEAR	UVH_BIOS_KERNEL_MMR_ALIAS
-#define UVH_NMI_MMR_SHIFT	63
-#define UVH_NMI_MMR_TYPE	"SCRATCH5"
-
 struct uv_hub_nmi_s {
 	raw_spinlock_t	nmi_lock;
 	atomic_t	in_nmi;		/* flag this node in UV NMI IRQ */
diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c
index 1e9ff28bc2e0..926a4e006e5a 100644
--- a/arch/x86/platform/uv/uv_nmi.c
+++ b/arch/x86/platform/uv/uv_nmi.c
@@ -280,10 +280,7 @@ static void uv_nmi_setup_mmrs(void)
 		nmi_mmr_pending = 1UL << uvh_nmi_mmrx_shift;
 		pr_info("UV: SMI NMI support: %s\n", uvh_nmi_mmrx_type);
 	} else {
-		nmi_mmr = UVH_NMI_MMR;
-		nmi_mmr_clear = UVH_NMI_MMR_CLEAR;
-		nmi_mmr_pending = 1UL << UVH_NMI_MMR_SHIFT;
-		pr_info("UV: SMI NMI support: %s\n", UVH_NMI_MMR_TYPE);
+		pr_warn("UV: NMI handler not present or not supported\n");
 	}
 }
 
-- 
2.26.2

Re: [PATCH 1/4] x86/platform/uv: Remove Obsolete Scratch5 NMI handler

[Hans de Goede]

Hi,

I just noticed this series only touches files under arch/x86,
so reviewing + merging this really is up to the x86 maintainers,
but I was already looking at it anyways so here is a review
from me too.

On 3/8/22 02:05, Mike Travis wrote:
> Removes obsolete scratch5 NMI handler only used in UV1 and early UV2
> systems.
> 
> Signed-off-by: Mike Travis <mike.travis@hpe.com>
> Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
> ---
>  arch/x86/include/asm/uv/uv_hub.h | 6 ------
>  arch/x86/platform/uv/uv_nmi.c    | 5 +----
>  2 files changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/arch/x86/include/asm/uv/uv_hub.h b/arch/x86/include/asm/uv/uv_hub.h
> index d3e3197917be..958c18f94146 100644
> --- a/arch/x86/include/asm/uv/uv_hub.h
> +++ b/arch/x86/include/asm/uv/uv_hub.h
> @@ -729,12 +729,6 @@ extern void uv_nmi_setup_hubless(void);
>  #define UVH_TSC_SYNC_VALID	3	/* 0011 */
>  #define UVH_TSC_SYNC_UNKNOWN	0	/* 0000 */
>  
> -/* BMC sets a bit this MMR non-zero before sending an NMI */
> -#define UVH_NMI_MMR		UVH_BIOS_KERNEL_MMR
> -#define UVH_NMI_MMR_CLEAR	UVH_BIOS_KERNEL_MMR_ALIAS
> -#define UVH_NMI_MMR_SHIFT	63
> -#define UVH_NMI_MMR_TYPE	"SCRATCH5"
> -
>  struct uv_hub_nmi_s {
>  	raw_spinlock_t	nmi_lock;
>  	atomic_t	in_nmi;		/* flag this node in UV NMI IRQ */
> diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c
> index 1e9ff28bc2e0..926a4e006e5a 100644
> --- a/arch/x86/platform/uv/uv_nmi.c
> +++ b/arch/x86/platform/uv/uv_nmi.c
> @@ -280,10 +280,7 @@ static void uv_nmi_setup_mmrs(void)
>  		nmi_mmr_pending = 1UL << uvh_nmi_mmrx_shift;
>  		pr_info("UV: SMI NMI support: %s\n", uvh_nmi_mmrx_type);
>  	} else {
> -		nmi_mmr = UVH_NMI_MMR;
> -		nmi_mmr_clear = UVH_NMI_MMR_CLEAR;
> -		nmi_mmr_pending = 1UL << UVH_NMI_MMR_SHIFT;
> -		pr_info("UV: SMI NMI support: %s\n", UVH_NMI_MMR_TYPE);
> +		pr_warn("UV: NMI handler not present or not supported\n");
>  	}
>  }
>  

This seems like a bad idea, just because HPE is no longer actively maintaining
these older UV versions does not mean that they are automatically without
users and we usually don't go around and start dropping support for hw
which is still in active use.

So NACK from me on this one.

Regards,

Hans

[PATCH 2/4] x86/platform/uv: Update NMI Handler for UV5

[Mike Travis]

Update NMI handler to interface with UV5 hardware. This involves
changing the EVENT_OCCURRED MMR used by the hardware and removes
the check for the newer NMI function supported by UV BIOS.

Signed-off-by: Mike Travis <mike.travis@hpe.com>
Reviewed-by: Dimitri Sivanich <dimitri.sivanich@hpe.com>
Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
---
 arch/x86/platform/uv/uv_nmi.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c
index 926a4e006e5a..38f4beae9fab 100644
--- a/arch/x86/platform/uv/uv_nmi.c
+++ b/arch/x86/platform/uv/uv_nmi.c
@@ -245,7 +245,7 @@ static inline bool uv_nmi_action_is(const char *action)
 static void uv_nmi_setup_mmrs(void)
 {
 	/* First determine arch specific MMRs to handshake with BIOS */
-	if (UVH_EVENT_OCCURRED0_EXTIO_INT0_MASK) {
+	if (UVH_EVENT_OCCURRED0_EXTIO_INT0_MASK) {	/* UV2,3,4 setup */
 		uvh_nmi_mmrx = UVH_EVENT_OCCURRED0;
 		uvh_nmi_mmrx_clear = UVH_EVENT_OCCURRED0_ALIAS;
 		uvh_nmi_mmrx_shift = UVH_EVENT_OCCURRED0_EXTIO_INT0_SHFT;
@@ -255,26 +255,26 @@ static void uv_nmi_setup_mmrs(void)
 		uvh_nmi_mmrx_req = UVH_BIOS_KERNEL_MMR_ALIAS_2;
 		uvh_nmi_mmrx_req_shift = 62;
 
-	} else if (UVH_EVENT_OCCURRED1_EXTIO_INT0_MASK) {
+	} else if (UVH_EVENT_OCCURRED1_EXTIO_INT0_MASK) { /* UV5 setup */
 		uvh_nmi_mmrx = UVH_EVENT_OCCURRED1;
 		uvh_nmi_mmrx_clear = UVH_EVENT_OCCURRED1_ALIAS;
 		uvh_nmi_mmrx_shift = UVH_EVENT_OCCURRED1_EXTIO_INT0_SHFT;
 		uvh_nmi_mmrx_type = "OCRD1-EXTIO_INT0";
 
-		uvh_nmi_mmrx_supported = UVH_EXTIO_INT0_BROADCAST;
-		uvh_nmi_mmrx_req = UVH_BIOS_KERNEL_MMR_ALIAS_2;
-		uvh_nmi_mmrx_req_shift = 62;
+		uvh_nmi_mmrx_supported = 1;
+		uvh_nmi_mmrx_req = 0;
 
 	} else {
-		pr_err("UV:%s:cannot find EVENT_OCCURRED*_EXTIO_INT0\n",
-			__func__);
+		pr_err("UV:%s:NMI support not available on this system\n", __func__);
 		return;
 	}
 
-	/* Then find out if new NMI is supported */
-	if (likely(uv_read_local_mmr(uvh_nmi_mmrx_supported))) {
-		uv_write_local_mmr(uvh_nmi_mmrx_req,
-					1UL << uvh_nmi_mmrx_req_shift);
+	/* Then find out if new NMI is supported (assumed on UV5) */
+	if (likely(uvh_nmi_mmrx_supported == 1) ||
+	   (uv_read_local_mmr(uvh_nmi_mmrx_supported) & 1UL << uvh_nmi_mmrx_req_shift)) {
+		if (uvh_nmi_mmrx_req)
+			uv_write_local_mmr(uvh_nmi_mmrx_req,
+						1UL << uvh_nmi_mmrx_req_shift);
 		nmi_mmr = uvh_nmi_mmrx;
 		nmi_mmr_clear = uvh_nmi_mmrx_clear;
 		nmi_mmr_pending = 1UL << uvh_nmi_mmrx_shift;
-- 
2.26.2

Re: [PATCH 2/4] x86/platform/uv: Update NMI Handler for UV5

[Hans de Goede]

Hi,

On 3/8/22 02:05, Mike Travis wrote:
> Update NMI handler to interface with UV5 hardware. This involves
> changing the EVENT_OCCURRED MMR used by the hardware and removes
> the check for the newer NMI function supported by UV BIOS.
> 
> Signed-off-by: Mike Travis <mike.travis@hpe.com>
> Reviewed-by: Dimitri Sivanich <dimitri.sivanich@hpe.com>
> Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
> ---
>  arch/x86/platform/uv/uv_nmi.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c
> index 926a4e006e5a..38f4beae9fab 100644
> --- a/arch/x86/platform/uv/uv_nmi.c
> +++ b/arch/x86/platform/uv/uv_nmi.c
> @@ -245,7 +245,7 @@ static inline bool uv_nmi_action_is(const char *action)
>  static void uv_nmi_setup_mmrs(void)
>  {
>  	/* First determine arch specific MMRs to handshake with BIOS */
> -	if (UVH_EVENT_OCCURRED0_EXTIO_INT0_MASK) {
> +	if (UVH_EVENT_OCCURRED0_EXTIO_INT0_MASK) {	/* UV2,3,4 setup */
>  		uvh_nmi_mmrx = UVH_EVENT_OCCURRED0;
>  		uvh_nmi_mmrx_clear = UVH_EVENT_OCCURRED0_ALIAS;
>  		uvh_nmi_mmrx_shift = UVH_EVENT_OCCURRED0_EXTIO_INT0_SHFT;
> @@ -255,26 +255,26 @@ static void uv_nmi_setup_mmrs(void)
>  		uvh_nmi_mmrx_req = UVH_BIOS_KERNEL_MMR_ALIAS_2;
>  		uvh_nmi_mmrx_req_shift = 62;
>  
> -	} else if (UVH_EVENT_OCCURRED1_EXTIO_INT0_MASK) {
> +	} else if (UVH_EVENT_OCCURRED1_EXTIO_INT0_MASK) { /* UV5 setup */
>  		uvh_nmi_mmrx = UVH_EVENT_OCCURRED1;
>  		uvh_nmi_mmrx_clear = UVH_EVENT_OCCURRED1_ALIAS;
>  		uvh_nmi_mmrx_shift = UVH_EVENT_OCCURRED1_EXTIO_INT0_SHFT;
>  		uvh_nmi_mmrx_type = "OCRD1-EXTIO_INT0";
>  
> -		uvh_nmi_mmrx_supported = UVH_EXTIO_INT0_BROADCAST;
> -		uvh_nmi_mmrx_req = UVH_BIOS_KERNEL_MMR_ALIAS_2;
> -		uvh_nmi_mmrx_req_shift = 62;

The dropping of setting uvh_nmi_mmrx_req and uvh_nmi_mmrx_req_shift here
looks weird, this seems like it might break things.

A closer look shows that that is not the case because these 2 *global*
variables are only used inside this 1 function and the added if for
uvh_nmi_mmrx_req being set causes them to now no longer be used in
this code path.

And before this change these 2 global variables where always set to:
UVH_BIOS_KERNEL_MMR_ALIAS_2 resp 62. This is even mentioned in
a comment where they are declared.

IMHO this really should be replaced with a preparation patch
which just adds a #define for the 62 and uses UVH_BIOS_KERNEL_MMR_ALIAS_2
directly, which it seems they really should have been from the start...

> +		uvh_nmi_mmrx_supported = 1;

So now your abusing a value used to store a register address/offset
as bool as well, using a special value for it being a bool rather
then a register offset and just hoping that the offset never is 1.

This is not the way to write maintainable code if you need both
a variable to store a register address (*) and a bool, please
use 2 separate variables for this.

*) Note you don't seem to need a variable for the register
address, this seems to be another case where you should just be
better of directly using the UVH_EXTIO_INT0_BROADCAST #define,
you could even change things to a #define in the same preparation
patch.



> +		uvh_nmi_mmrx_req = 0;>  
>  	} else {
> -		pr_err("UV:%s:cannot find EVENT_OCCURRED*_EXTIO_INT0\n",
> -			__func__);
> +		pr_err("UV:%s:NMI support not available on this system\n", __func__);
>  		return;
>  	}
>  
> -	/* Then find out if new NMI is supported */
> -	if (likely(uv_read_local_mmr(uvh_nmi_mmrx_supported))) {
> -		uv_write_local_mmr(uvh_nmi_mmrx_req,
> -					1UL << uvh_nmi_mmrx_req_shift);
> +	/* Then find out if new NMI is supported (assumed on UV5) */
> +	if (likely(uvh_nmi_mmrx_supported == 1) ||
> +	   (uv_read_local_mmr(uvh_nmi_mmrx_supported) & 1UL << uvh_nmi_mmrx_req_shift)) {
> +		if (uvh_nmi_mmrx_req)
> +			uv_write_local_mmr(uvh_nmi_mmrx_req,
> +						1UL << uvh_nmi_mmrx_req_shift);

The whole switch of just assuming "new NMI" support in one case and
of no longer writing the mmr seems like something which should be
much more clearly described in the commit message.

>  		nmi_mmr = uvh_nmi_mmrx;
>  		nmi_mmr_clear = uvh_nmi_mmrx_clear;
>  		nmi_mmr_pending = 1UL << uvh_nmi_mmrx_shift;


Regards,

Hans

[PATCH 3/4] x86/platform/uv: Update TSC sync state for UV5

[Mike Travis]

Update TSC to not check TSC sync state for uv5+ as it is not
available.  Therefore it is assumed that TSC will always be in
sync for multiple chassis and will pass the tests for the kernel
to accept it as the clocksource.  To disable this check use the
kernel start options tsc=reliable clocksource=tsc.

Signed-off-by: Mike Travis <mike.travis@hpe.com>
Reviewed-by: Dimitri Sivanich <dimitri.sivanich@hpe.com>
Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
---
 arch/x86/kernel/apic/x2apic_uv_x.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index f5a48e66e4f5..387d6533549a 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -199,10 +199,16 @@ static void __init uv_tsc_check_sync(void)
 	int mmr_shift;
 	char *state;
 
-	/* Different returns from different UV BIOS versions */
+	/* UV5+, sync state from bios not available, assumed valid */
+	if (!is_uv(UV2|UV3|UV4)) {
+		pr_debug("UV: TSC sync state for UV5+ assumed valid\n");
+		mark_tsc_async_resets("UV5+");
+		return;
+	}
+
+	/* UV2,3,4, UV BIOS TSC sync state available */
 	mmr = uv_early_read_mmr(UVH_TSC_SYNC_MMR);
-	mmr_shift =
-		is_uv2_hub() ? UVH_TSC_SYNC_SHIFT_UV2K : UVH_TSC_SYNC_SHIFT;
+	mmr_shift = is_uv2_hub() ? UVH_TSC_SYNC_SHIFT_UV2K : UVH_TSC_SYNC_SHIFT;
 	sync_state = (mmr >> mmr_shift) & UVH_TSC_SYNC_MASK;
 
 	/* Check if TSC is valid for all sockets */
-- 
2.26.2

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

[Mike Travis]

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

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

[Steve Wahl]

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

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

[Hans de Goede]

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