On Wed, Nov 22, 2017 at 7:33 AM, Borislav Petkov <b...@suse.de> wrote:
> On Tue, Nov 21, 2017 at 08:44:00PM -0800, Andy Lutomirski wrote:
>> Currently, the GDT is an ad-hoc array of pages, one per CPU, in the
>> fixmap. Generalize it to be an array of a new struct cpu_entry_area
>> so that we can cleanly add new things to it.
>>
>> Signed-off-by: Andy Lutomirski <l...@kernel.org>
>> ---
>> arch/x86/include/asm/desc.h | 9 +--------
>> arch/x86/include/asm/fixmap.h | 36 ++++++++++++++++++++++++++++++++++--
>> arch/x86/kernel/cpu/common.c | 14 +++++++-------
>> arch/x86/xen/mmu_pv.c | 2 +-
>> 4 files changed, 43 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
>> index 95cd95eb7285..194ffab00ebe 100644
>> --- a/arch/x86/include/asm/desc.h
>> +++ b/arch/x86/include/asm/desc.h
>> @@ -60,17 +60,10 @@ static inline struct desc_struct
>> *get_current_gdt_rw(void)
>> return this_cpu_ptr(&gdt_page)->gdt;
>> }
>>
>> -/* Get the fixmap index for a specific processor */
>> -static inline unsigned int get_cpu_gdt_ro_index(int cpu)
>> -{
>> - return FIX_GDT_REMAP_END - cpu;
>> -}
>> -
>> /* Provide the fixmap address of the remapped GDT */
>> static inline struct desc_struct *get_cpu_gdt_ro(int cpu)
>> {
>> - unsigned int idx = get_cpu_gdt_ro_index(cpu);
>> - return (struct desc_struct *)__fix_to_virt(idx);
>> + return (struct desc_struct *)&get_cpu_entry_area(cpu)->gdt;
>> }
>>
>> /* Provide the current read-only GDT */
>> diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
>> index b0c505fe9a95..038b8474c7f8 100644
>> --- a/arch/x86/include/asm/fixmap.h
>> +++ b/arch/x86/include/asm/fixmap.h
>
> Btw, the whole cpu_entry_area machinery looks like it should belong more
> in asm/processor.h and not in fixmap.h
Agreed, except that the fixmap enum needs to know
sizeof(cpu_entry_area), and I'm really hesitant to add yet another
header dependency.
>
>> @@ -44,6 +44,17 @@ extern unsigned long __FIXADDR_TOP;
>> PAGE_SIZE)
>> #endif
>>
>> +/*
>> + * cpu_entry_area is a percpu region in the fixmap that contains things
>> + * needed by the CPU and early entry/exit code. Real types aren't used
>> + * for all fields here to about circular header dependencies.
>
> s/about/avoid/
>
>> + */
>> +struct cpu_entry_area
>> +{
>
> ERROR: open brace '{' following struct go on the same line
> #67: FILE: arch/x86/include/asm/fixmap.h:53:
> +struct cpu_entry_area
> +{
>
>> + char gdt[PAGE_SIZE];
>> +};
>> +
>> +#define CPU_ENTRY_AREA_PAGES (sizeof(struct cpu_entry_area) / PAGE_SIZE)
>>
>> /*
>> * Here we define all the compile-time 'special' virtual
>> @@ -101,8 +112,8 @@ enum fixed_addresses {
>> FIX_LNW_VRTC,
>> #endif
>> /* Fixmap entries to remap the GDTs, one per processor. */
>> - FIX_GDT_REMAP_BEGIN,
>> - FIX_GDT_REMAP_END = FIX_GDT_REMAP_BEGIN + NR_CPUS - 1,
>> + FIX_CPU_ENTRY_AREA_TOP,
>> + FIX_CPU_ENTRY_AREA_BOTTOM = FIX_CPU_ENTRY_AREA_TOP +
>> (CPU_ENTRY_AREA_PAGES * NR_CPUS) - 1,
>>
>> #ifdef CONFIG_ACPI_APEI_GHES
>> /* Used for GHES mapping from assorted contexts */
>> @@ -191,5 +202,26 @@ void __init
>> *early_memremap_decrypted_wp(resource_size_t phys_addr,
>> void __early_set_fixmap(enum fixed_addresses idx,
>> phys_addr_t phys, pgprot_t flags);
>>
>> +static inline unsigned int __get_cpu_entry_area_page_index(int cpu, int
>> page)
>> +{
>> + BUILD_BUG_ON(sizeof(struct cpu_entry_area) % PAGE_SIZE != 0);
>
> BUILD_BUG_ON(sizeof(struct cpu_entry_area) % PAGE_SIZE);
>
My general habit is that I like the != 0 here because I'm doing
arithmetic rather than thinking of % as some kind of logical operator.
I.e. I find it easier to understand the way I wrote it.
>> +
>> + return FIX_CPU_ENTRY_AREA_BOTTOM - cpu*CPU_ENTRY_AREA_PAGES - page;
>> +}
>> +
>> +#define __get_cpu_entry_area_offset_index(cpu, offset) ({ \
>> + BUILD_BUG_ON(offset % PAGE_SIZE != 0); \
>
> BUILD_BUG_ON(offset % PAGE_SIZE);
Ditto.
>
>> + __get_cpu_entry_area_page_index(cpu, offset / PAGE_SIZE); \
>> + })
>> +
>> +#define get_cpu_entry_area_index(cpu, field) \
>> + __get_cpu_entry_area_offset_index((cpu), offsetof(struct
>> cpu_entry_area, field))
>> +
>> +static inline struct cpu_entry_area *get_cpu_entry_area(int cpu)
>> +{
>> + return (struct cpu_entry_area *)
>> + __fix_to_virt(__get_cpu_entry_area_page_index(cpu, 0));
>
> Just let it stick out.
Sure.
>
> --
> Regards/Gruss,
> Boris.
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB
> 21284 (AG Nürnberg)
> --
