* Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Thu, 6 Mar 2025 at 00:13, tip-bot2 for Andy Shevchenko
> <tip-bot2@xxxxxxxxxxxxx> wrote:
> >
> > x86/mm: Check if PTRS_PER_PMD is defined before use
>
> I'm not at all happy with this one.
>
> > -#if PTRS_PER_PMD > 1
> > +#if defined(PTRS_PER_PMD) && (PTRS_PER_PMD > 1)
>
> Honestly, I feel that if PTRS_PER_PMD isn't defined, we've missed some
> include, and now the code is making random decisions based on lack of
> information.
Yeah, so <asm/pgtable-2level_types.h> hasn't defined it historically,
because 2-level paging only has PGDs and PTE tables - and it relies on
<asm-generic/pgtable-nopmd.h> doing it:
#define PTRS_PER_PMD 1
<asm/pgtable_types.h> includes <asm-generic/pgtable-nopmd.h>, and with
it most of the MM headers.
But:
> It should be defined either by the architecture pgtable_types.h
> header, or if the PMD is folded away, the architecture should have
> included <asm-generic/pgtable-nopmd.h>.
>
> So I'm *really* thinking this patch is completely bogus and is hiding
> a serious problem, and making PAGE_TABLE_SIZE() have random values.
Yeah, so the MM headers cover the C case - but the bugreport was about
the assembly side (head_32.S):
In file included from arch/x86/kernel/head_32.S:29:
arch/x86/include/asm/pgtable_32.h:59:5: error: "PTRS_PER_PMD" is not defined, evaluates to 0 [-Werror=undef]
59 | #if PTRS_PER_PMD > 1
and AFAICS the assembly version of these headers doesn't define
PTRS_PER_PMD.
Separating out the assembler-compatible defines from the types headers
appears to be a bigger patch, since it's all mixed in with C syntax:
<=-----------------------------------===============================
typedef struct { pud_t pud; } pmd_t;
#define PMD_SHIFT PUD_SHIFT
#define PTRS_PER_PMD 1
#define PMD_SIZE (1UL << PMD_SHIFT)
#define PMD_MASK (~(PMD_SIZE-1))
/*
* The "pud_xxx()" functions here are trivial for a folded two-level
* setup: the pmd is never bad, and a pmd always exists (as it's folded
* into the pud entry)
*/
static inline int pud_none(pud_t pud) { return 0; }
static inline int pud_bad(pud_t pud) { return 0; }
static inline int pud_present(pud_t pud) { return 1; }
================================================================>
In any case I've removed the commit for the time being until this all
is cleared up.
Thanks,
Ingo
|