lkml.org 
[lkml]   [2016]   [Aug]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [Resend][PATCH] x86/power/64: Always create temporary identity mapping correctly
    Date
    On Wednesday, August 10, 2016 09:50:15 AM Jiri Kosina wrote:
    > On Wed, 10 Aug 2016, Rafael J. Wysocki wrote:
    >
    > > For the lack of better ideas, below is a patch to try.
    > >
    > > It avoids the possible issue with the restore kernel's identity mapping overlap
    > > with restore_jump_address by creating special super-simple page tables just
    > > for the final jump to the image kernel.
    > >
    > > It is on top of the $subject patch. My test box still works with this applied,
    > > but then it worked without it as well.
    > >
    > > If it doesn't help, the identity mapping created by set_up_temporary_mappings()
    > > is still not adequate for some reason most likely and we'll need to find out
    > > why.
    >
    > Unfortunately, still with $subject patch + this one, triple fault and
    > reboot after reading the hibernation image.

    The last patch I sent had a problem, because if restore_jump_address really
    overlapped with the identity mapping of the restore kernel, it might share
    PGD or PUD entries with that mapping and that should have been taken into
    account.

    Here goes an update. Again, this works on my test machine, but then the
    previous version worked on it too ...

    ---
    arch/x86/power/hibernate_64.c | 53 ++++++++++++++++++++++++++++++--------
    arch/x86/power/hibernate_asm_64.S | 10 +++++++
    2 files changed, 53 insertions(+), 10 deletions(-)

    Index: linux-pm/arch/x86/power/hibernate_64.c
    ===================================================================
    --- linux-pm.orig/arch/x86/power/hibernate_64.c
    +++ linux-pm/arch/x86/power/hibernate_64.c
    @@ -38,14 +38,22 @@ unsigned long jump_address_phys;
    unsigned long restore_cr3 __visible;

    unsigned long temp_level4_pgt __visible;
    +unsigned long jump_level4_pgt __visible;

    unsigned long relocated_restore_code __visible;

    -static int set_up_temporary_text_mapping(pgd_t *pgd)
    +static int set_up_temporary_text_mapping(void)
    {
    + unsigned long pgd_idx = pgd_index(restore_jump_address);
    + unsigned long pud_idx = pud_index(restore_jump_address);
    + pgd_t *pgd;
    pmd_t *pmd;
    pud_t *pud;

    + pgd = (pgd_t *)get_safe_page(GFP_ATOMIC);
    + if (!pgd)
    + return -ENOMEM;
    +
    /*
    * The new mapping only has to cover the page containing the image
    * kernel's entry point (jump_address_phys), because the switch over to
    @@ -69,10 +77,32 @@ static int set_up_temporary_text_mapping

    set_pmd(pmd + pmd_index(restore_jump_address),
    __pmd((jump_address_phys & PMD_MASK) | __PAGE_KERNEL_LARGE_EXEC));
    - set_pud(pud + pud_index(restore_jump_address),
    + set_pud(pud + pud_idx, __pud(__pa(pmd) | _KERNPG_TABLE));
    + set_pgd(pgd + pgd_idx, __pgd(__pa(pud) | _KERNPG_TABLE));
    +
    + if (pgd_idx != pgd_index(relocated_restore_code)) {
    + pud = (pud_t *)get_safe_page(GFP_ATOMIC);
    + if (!pud)
    + return -ENOMEM;
    +
    + set_pgd(pgd + pgd_index(relocated_restore_code),
    + __pgd(__pa(pud) | _KERNPG_TABLE));
    + } else if (pud_idx == pud_index(relocated_restore_code)) {
    + goto set_pmd;
    + }
    +
    + pmd = (pmd_t *)get_safe_page(GFP_ATOMIC);
    + if (!pmd)
    + return -ENOMEM;
    +
    + set_pud(pud + pud_index(relocated_restore_code),
    __pud(__pa(pmd) | _KERNPG_TABLE));
    - set_pgd(pgd + pgd_index(restore_jump_address),
    - __pgd(__pa(pud) | _KERNPG_TABLE));
    +
    + set_pmd:
    + set_pmd(pmd + pmd_index(relocated_restore_code),
    + __pmd((__pa(relocated_restore_code) & PMD_MASK) | __PAGE_KERNEL_LARGE_EXEC));
    +
    + jump_level4_pgt = __pa(pgd);

    return 0;
    }
    @@ -98,11 +128,6 @@ static int set_up_temporary_mappings(voi
    if (!pgd)
    return -ENOMEM;

    - /* Prepare a temporary mapping for the kernel text */
    - result = set_up_temporary_text_mapping(pgd);
    - if (result)
    - return result;
    -
    /* Set up the direct mapping from scratch */
    for (i = 0; i < nr_pfn_mapped; i++) {
    mstart = pfn_mapped[i].start << PAGE_SHIFT;
    @@ -122,7 +147,10 @@ static int relocate_restore_code(void)
    pgd_t *pgd;
    pud_t *pud;

    - relocated_restore_code = get_safe_page(GFP_ATOMIC);
    + do
    + relocated_restore_code = get_safe_page(GFP_ATOMIC);
    + while ((relocated_restore_code & PMD_MASK) == (restore_jump_address & PMD_MASK));
    +
    if (!relocated_restore_code)
    return -ENOMEM;

    @@ -162,6 +190,11 @@ int swsusp_arch_resume(void)
    if (error)
    return error;

    + /* Prepare a temporary mapping for the jump to the image kernel */
    + error = set_up_temporary_text_mapping();
    + if (error)
    + return error;
    +
    restore_image();
    return 0;
    }
    Index: linux-pm/arch/x86/power/hibernate_asm_64.S
    ===================================================================
    --- linux-pm.orig/arch/x86/power/hibernate_asm_64.S
    +++ linux-pm/arch/x86/power/hibernate_asm_64.S
    @@ -57,6 +57,7 @@ ENTRY(restore_image)
    /* prepare to jump to the image kernel */
    movq restore_jump_address(%rip), %r8
    movq restore_cr3(%rip), %r9
    + movq jump_level4_pgt(%rip), %r10

    /* prepare to switch to temporary page tables */
    movq temp_level4_pgt(%rip), %rax
    @@ -96,6 +97,15 @@ ENTRY(core_restore_code)
    jmp .Lloop

    .Ldone:
    + /* switch to jump page tables */
    + movq %r10, %cr3
    + /* flush TLB */
    + movq %rbx, %rcx
    + andq $~(X86_CR4_PGE), %rcx
    + movq %rcx, %cr4; # turn off PGE
    + movq %cr3, %rcx; # flush TLB
    + movq %rcx, %cr3;
    + movq %rbx, %cr4; # turn PGE back on
    /* jump to the restore_registers address from the image header */
    jmpq *%r8

    \
     
     \ /
      Last update: 2016-08-10 22:41    [W:2.076 / U:0.288 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site