lkml.org 
[lkml]   [2017]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 RESEND 1/2] x86/UV: Introduce a helper function to check UV system at earlier stage
On 09/14/17 at 03:49pm, Dave Young wrote:
> > > diff --git a/arch/x86/include/asm/uv/uv.h b/arch/x86/include/asm/uv/uv.h
> > > index b5a32231abd8..93d7ad8763ba 100644
> > > --- a/arch/x86/include/asm/uv/uv.h
> > > +++ b/arch/x86/include/asm/uv/uv.h
> > > @@ -18,6 +18,11 @@ extern void uv_nmi_init(void);
> > > extern void uv_system_init(void);
> > > extern const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
> > > const struct flush_tlb_info *info);
> > > +#include <linux/efi.h>
> > > +static inline int is_early_uv_system(void)
> > > +{
> > > + return !((efi.uv_systab == EFI_INVALID_TABLE_ADDR) || !efi.uv_systab);
> > > +}
>

Thanks for looking into this, Dave!

>
> Sorry for jumping in late, I have two questions about the patch:
>
> 1) For efi tables, the only invalid value is EFI_INVALID_TABLE_ADDR, and
> efi struct is initialized as EFI_INVALID_TABLE_ADDR by default so no
> need to check "|| !efi.uv_systab". Do we have any UV firmware specific
> assumption that "0" is also possible to be assigned?

Hmm, in uv_bios_init() it also checks the !efi.uv_systab case. And
EFI_INVALID_TABLE_ADDR checking is earlier, it won't affect the result
if it's EFI_INVALID_TABLE_ADDR. And !efi.uv_systab can make it safer
since it doesn't work either if efi.uv_systab is 0. Mainly it's not
harmful.

Mike, what's your thought? Should I only check the (efi.uv_systab ==
EFI_INVALID_TABLE_ADDR) case?

>
> 2) It seems adding this function in uv.h for separating this for uv
> system only purpose. But I feel it is better to put it in efi.h instead.

At the beginning I put it in efi.c, later Mike suggested putting it in
asm/uv/uv.h. You can also find the discussion in below link.
https://patchwork.kernel.org/patch/9732787/

Thanks
Baoquan

>
> uv_systab is already a member of struct efi, it is in efi.h so it is
> natural to check the table exist or not. Then just include efi.h in
> kaslr.c and use the function.
>
> something like drivers/firmware/efi/esrt.c: esrt_table_exists()
>
> Anyway I have no strong opinon, it looks more natural to me though.
>
> > >
> > > #else /* X86_UV */
> > >
> > > @@ -30,6 +35,7 @@ static inline const struct cpumask *
> > > uv_flush_tlb_others(const struct cpumask *cpumask,
> > > const struct flush_tlb_info *info)
> > > { return cpumask; }
> > > +static inline int is_early_uv_system(void) { return 0; }
> > >
> > > #endif /* X86_UV */
> > >
> > > --
> > > 2.5.5
> > >
>
> Thanks
> Dave

\
 
 \ /
  Last update: 2017-09-14 10:08    [W:0.088 / U:0.280 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site