Skip to content

Commit 7af0145

Browse files
committed
x86/mm/cpa: Prevent large page split when ftrace flips RW on kernel text
ftrace does not use text_poke() for enabling trace functionality. It uses its own mechanism and flips the whole kernel text to RW and back to RO. The CPA rework removed a loop based check of 4k pages which tried to preserve a large page by checking each 4k page whether the change would actually cover all pages in the large page. This resulted in endless loops for nothing as in testing it turned out that it actually never preserved anything. Of course testing missed to include ftrace, which is the one and only case which benefitted from the 4k loop. As a consequence enabling function tracing or ftrace based kprobes results in a full 4k split of the kernel text, which affects iTLB performance. The kernel RO protection is the only valid case where this can actually preserve large pages. All other static protections (RO data, data NX, PCI, BIOS) are truly static. So a conflict with those protections which results in a split should only ever happen when a change of memory next to a protected region is attempted. But these conflicts are rightfully splitting the large page to preserve the protected regions. In fact a change to the protected regions itself is a bug and is warned about. Add an exception for the static protection check for kernel text RO when the to be changed region spawns a full large page which allows to preserve the large mappings. This also prevents the syslog to be spammed about CPA violations when ftrace is used. The exception needs to be removed once ftrace switched over to text_poke() which avoids the whole issue. Fixes: 585948f ("x86/mm/cpa: Avoid the 4k pages check completely") Reported-by: Song Liu <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Tested-by: Song Liu <[email protected]> Reviewed-by: Song Liu <[email protected]> Acked-by: Peter Zijlstra (Intel) <[email protected]> Cc: [email protected] Link: https://lkml.kernel.org/r/[email protected]
1 parent 42e0e95 commit 7af0145

File tree

1 file changed

+18
-8
lines changed

1 file changed

+18
-8
lines changed

arch/x86/mm/pageattr.c

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ static inline void check_conflict(int warnlvl, pgprot_t prot, pgprotval_t val,
516516
*/
517517
static inline pgprot_t static_protections(pgprot_t prot, unsigned long start,
518518
unsigned long pfn, unsigned long npg,
519-
int warnlvl)
519+
unsigned long lpsize, int warnlvl)
520520
{
521521
pgprotval_t forbidden, res;
522522
unsigned long end;
@@ -535,9 +535,17 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long start,
535535
check_conflict(warnlvl, prot, res, start, end, pfn, "Text NX");
536536
forbidden = res;
537537

538-
res = protect_kernel_text_ro(start, end);
539-
check_conflict(warnlvl, prot, res, start, end, pfn, "Text RO");
540-
forbidden |= res;
538+
/*
539+
* Special case to preserve a large page. If the change spawns the
540+
* full large page mapping then there is no point to split it
541+
* up. Happens with ftrace and is going to be removed once ftrace
542+
* switched to text_poke().
543+
*/
544+
if (lpsize != (npg * PAGE_SIZE) || (start & (lpsize - 1))) {
545+
res = protect_kernel_text_ro(start, end);
546+
check_conflict(warnlvl, prot, res, start, end, pfn, "Text RO");
547+
forbidden |= res;
548+
}
541549

542550
/* Check the PFN directly */
543551
res = protect_pci_bios(pfn, pfn + npg - 1);
@@ -819,7 +827,7 @@ static int __should_split_large_page(pte_t *kpte, unsigned long address,
819827
* extra conditional required here.
820828
*/
821829
chk_prot = static_protections(old_prot, lpaddr, old_pfn, numpages,
822-
CPA_CONFLICT);
830+
psize, CPA_CONFLICT);
823831

824832
if (WARN_ON_ONCE(pgprot_val(chk_prot) != pgprot_val(old_prot))) {
825833
/*
@@ -855,7 +863,7 @@ static int __should_split_large_page(pte_t *kpte, unsigned long address,
855863
* protection requirement in the large page.
856864
*/
857865
new_prot = static_protections(req_prot, lpaddr, old_pfn, numpages,
858-
CPA_DETECT);
866+
psize, CPA_DETECT);
859867

860868
/*
861869
* If there is a conflict, split the large page.
@@ -906,7 +914,8 @@ static void split_set_pte(struct cpa_data *cpa, pte_t *pte, unsigned long pfn,
906914
if (!cpa->force_static_prot)
907915
goto set;
908916

909-
prot = static_protections(ref_prot, address, pfn, npg, CPA_PROTECT);
917+
/* Hand in lpsize = 0 to enforce the protection mechanism */
918+
prot = static_protections(ref_prot, address, pfn, npg, 0, CPA_PROTECT);
910919

911920
if (pgprot_val(prot) == pgprot_val(ref_prot))
912921
goto set;
@@ -1503,7 +1512,8 @@ static int __change_page_attr(struct cpa_data *cpa, int primary)
15031512
pgprot_val(new_prot) |= pgprot_val(cpa->mask_set);
15041513

15051514
cpa_inc_4k_install();
1506-
new_prot = static_protections(new_prot, address, pfn, 1,
1515+
/* Hand in lpsize = 0 to enforce the protection mechanism */
1516+
new_prot = static_protections(new_prot, address, pfn, 1, 0,
15071517
CPA_PROTECT);
15081518

15091519
new_prot = pgprot_clear_protnone_bits(new_prot);

0 commit comments

Comments
 (0)