Return the type of UV hubless system for UV specific code that depends
on that. Use a define to indicate the change in arg type for this
function in uv.h. Add a function to convert UV system type to bit
pattern needed for is_uv_hubless().
Signed-off-by: Mike Travis <[email protected]>
Reviewed-by: Steve Wahl <[email protected]>
Reviewed-by: Dimitri Sivanich <[email protected]>
---
arch/x86/include/asm/uv/uv.h | 13 +++++++++++--
arch/x86/kernel/apic/x2apic_uv_x.c | 29 +++++++++++++++++++----------
2 files changed, 30 insertions(+), 12 deletions(-)
--- linux.orig/arch/x86/include/asm/uv/uv.h
+++ linux/arch/x86/include/asm/uv/uv.h
@@ -12,13 +12,21 @@ struct mm_struct;
#ifdef CONFIG_X86_UV
#include <linux/efi.h>
+static inline int uv(int uvtype)
+{
+ /* uv(0) is "any" */
+ if (uvtype >= 0 && uvtype <= 30)
+ return 1 << uvtype;
+ return 1;
+}
extern enum uv_system_type get_uv_system_type(void);
static inline bool is_early_uv_system(void)
{
return !((efi.uv_systab == EFI_INVALID_TABLE_ADDR) || !efi.uv_systab);
}
extern int is_uv_system(void);
-extern int is_uv_hubless(void);
+extern int _is_uv_hubless(int uvtype);
+#define is_uv_hubless _is_uv_hubless
extern void uv_cpu_init(void);
extern void uv_nmi_init(void);
extern void uv_system_init(void);
@@ -30,7 +38,8 @@ extern const struct cpumask *uv_flush_tl
static inline enum uv_system_type get_uv_system_type(void) { return UV_NONE; }
static inline bool is_early_uv_system(void) { return 0; }
static inline int is_uv_system(void) { return 0; }
-static inline int is_uv_hubless(void) { return 0; }
+static inline int _is_uv_hubless(int uv) { return 0; }
+#define is_uv_hubless _is_uv_hubless
static inline void uv_cpu_init(void) { }
static inline void uv_system_init(void) { }
static inline const struct cpumask *
--- linux.orig/arch/x86/kernel/apic/x2apic_uv_x.c
+++ linux/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -46,7 +46,7 @@
DEFINE_PER_CPU(int, x2apic_extra_bits);
static enum uv_system_type uv_system_type;
-static bool uv_hubless_system;
+static int uv_hubless_system;
static u64 gru_start_paddr, gru_end_paddr;
static u64 gru_dist_base, gru_first_node_paddr = -1LL, gru_last_node_paddr;
static u64 gru_dist_lmask, gru_dist_umask;
@@ -288,11 +288,20 @@ static int __init uv_acpi_madt_oem_check
uv_stringify(sizeof(oem_table_id), oem_table_id, _oem_table_id);
if (strncmp(oem_id, "SGI", 3) != 0) {
- if (strncmp(oem_id, "NSGI", 4) == 0) {
- uv_hubless_system = true;
- pr_info("UV: OEM IDs %s/%s, HUBLESS\n",
- oem_id, oem_table_id);
- }
+ if (strncmp(oem_id, "NSGI", 4) != 0)
+ return 0;
+
+ /* UV4 Hubless, CH, (0x11:UV4+Any) */
+ if (strncmp(oem_id, "NSGI4", 5) == 0)
+ uv_hubless_system = 0x11;
+
+ /* UV3 Hubless, UV300/MC990X w/o hub (0x9:UV3+Any) */
+ else
+ uv_hubless_system = 0x9;
+
+ pr_info("UV: OEM IDs %s/%s, HUBLESS(0x%x)\n",
+ oem_id, oem_table_id, uv_hubless_system);
+
return 0;
}
@@ -370,11 +379,11 @@ int is_uv_system(void)
}
EXPORT_SYMBOL_GPL(is_uv_system);
-int is_uv_hubless(void)
+int _is_uv_hubless(int uvtype)
{
- return uv_hubless_system;
+ return (uv_hubless_system & uvtype);
}
-EXPORT_SYMBOL_GPL(is_uv_hubless);
+EXPORT_SYMBOL_GPL(_is_uv_hubless);
void **__uv_hub_info_list;
EXPORT_SYMBOL_GPL(__uv_hub_info_list);
@@ -1612,7 +1621,7 @@ static void __init uv_system_init_hub(vo
*/
void __init uv_system_init(void)
{
- if (likely(!is_uv_system() && !is_uv_hubless()))
+ if (likely(!is_uv_system() && !is_uv_hubless(1)))
return;
if (is_uv_system())
--
> static inline bool is_early_uv_system(void)
> {
> return !((efi.uv_systab == EFI_INVALID_TABLE_ADDR) || !efi.uv_systab);
No need for the inner braces here.
But woudn't this be nicer as:
return efi.uv_systab != EFI_INVALID_TABLE_ADDR && efi.uv_systab;
anyway?
> +#define is_uv_hubless _is_uv_hubless
Why the weird macro indirection?
> -static inline int is_uv_hubless(void) { return 0; }
> +static inline int _is_uv_hubless(int uv) { return 0; }
> +#define is_uv_hubless _is_uv_hubless
And here again.
On 9/2/2019 11:49 PM, Christoph Hellwig wrote:
>> static inline bool is_early_uv_system(void)
>> {
>> return !((efi.uv_systab == EFI_INVALID_TABLE_ADDR) || !efi.uv_systab);
>
> No need for the inner braces here.
>
> But woudn't this be nicer as:
>
> return efi.uv_systab != EFI_INVALID_TABLE_ADDR && efi.uv_systab;
>
> anyway?
Yes, good catch. It somehow evolved to this but your suggestion is
much more clear.
>
>> +#define is_uv_hubless _is_uv_hubless
>
> Why the weird macro indirection?
>
>> -static inline int is_uv_hubless(void) { return 0; }
>> +static inline int _is_uv_hubless(int uv) { return 0; }
>> +#define is_uv_hubless _is_uv_hubless
>
> And here again.
>
Sorry, I should have explained this better. The problem arises because
we have a number of UV specific kernel modules that support multiple
distributions. And with back porting to earlier distros we cannot
rely on the KERNEL_VERSION macro to define whether the source is being
built for an earlier kernel. So this allows an ifdef on the function
name to discover if the kernel is before or after these changes.
The primary motivation is to avoid referencing the hub structures
when there aren't any, thus avoiding any NULL dereferences. (Similar
to patch 8/8.)
On Tue, Sep 03, 2019 at 07:12:28AM -0700, Mike Travis wrote:
> > > +#define is_uv_hubless _is_uv_hubless
> >
> > Why the weird macro indirection?
> >
> > > -static inline int is_uv_hubless(void) { return 0; }
> > > +static inline int _is_uv_hubless(int uv) { return 0; }
> > > +#define is_uv_hubless _is_uv_hubless
> >
> > And here again.
> >
>
> Sorry, I should have explained this better. The problem arises because
> we have a number of UV specific kernel modules that support multiple
> distributions. And with back porting to earlier distros we cannot
> rely on the KERNEL_VERSION macro to define whether the source is being
> built for an earlier kernel. So this allows an ifdef on the function
> name to discover if the kernel is before or after these changes.
And none of these matter for upstream. We'd rather not make the code
more convouluted than required. If you actually really cared about these
modules you would simply submit them upstream.
On 9/3/2019 8:41 AM, Christoph Hellwig wrote:
> On Tue, Sep 03, 2019 at 07:12:28AM -0700, Mike Travis wrote:
>>>> +#define is_uv_hubless _is_uv_hubless
>>>
>>> Why the weird macro indirection?
>>>
>>>> -static inline int is_uv_hubless(void) { return 0; }
>>>> +static inline int _is_uv_hubless(int uv) { return 0; }
>>>> +#define is_uv_hubless _is_uv_hubless
>>>
>>> And here again.
>>>
>>
>> Sorry, I should have explained this better. The problem arises because
>> we have a number of UV specific kernel modules that support multiple
>> distributions. And with back porting to earlier distros we cannot
>> rely on the KERNEL_VERSION macro to define whether the source is being
>> built for an earlier kernel. So this allows an ifdef on the function
>> name to discover if the kernel is before or after these changes.
>
> And none of these matter for upstream. We'd rather not make the code
> more convouluted than required. If you actually really cared about these
> modules you would simply submit them upstream.
>
That is always being considered for everything we include into the
community kernel source. The problem is a couple of the kernel modules
(hwperf being the prime example) is much more tied to hardware and
BIOS/FW updates so has to be updated much more often than the current
submittal/acceptance process allows. We do opensource these modules but
they are built from single source directories and have to be released as
a module into a package that can be installed on different distros.
There is not a source version for each kernel version.
I have seen this method (declare the function with a leading underscore
and a #define for the function reference) which is why I'm assuming it's
a standard kernel practice? (I'll find some examples if necessary?)
On Tue, Sep 03, 2019 at 11:49:53AM -0700, Mike Travis wrote:
>
> That is always being considered for everything we include into the community
> kernel source. The problem is a couple of the kernel modules (hwperf being
> the prime example) is much more tied to hardware and BIOS/FW updates so has
> to be updated much more often than the current submittal/acceptance process
> allows. We do opensource these modules but they are built from single
> source directories and have to be released as a module into a package that
> can be installed on different distros. There is not a source version for
> each kernel version.
Well, tought luck then. We do not support interface for out of tree
modules only. I actually found a few in uv and will send patches to
drop that dead weight.
> I have seen this method (declare the function with a leading underscore and
> a #define for the function reference) which is why I'm assuming it's a
> standard kernel practice? (I'll find some examples if necessary?)
No, it isn't.