From: Matt Evans <matt@ozlabs.org> To: Alexander Graf <graf@amazon.com> Cc: kvm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, Christophe Leroy <christophe.leroy@csgroup.eu>, Paul Mackerras <paulus@samba.org>, Ben Herrenschmitt <benh@kernel.crashing.org>, Michael Ellerman <mpe@ellerman.id.au>, stable@vger.kernel.org Subject: Re: [PATCH] KVM: PPC: Book3S PR: Enable MSR_DR for switch_mmu_context() Date: Tue, 10 May 2022 11:33:45 +0100 [thread overview] Message-ID: <5CBF19EB-F2DA-4DBA-9BD0-D38E3A3F959A@ozlabs.org> (raw) In-Reply-To: <20220509202355.13985-1-graf@amazon.com> Hi Alex, > On 9 May 2022, at 21:23, Alexander Graf <graf@amazon.com> wrote: > > Commit 863771a28e27 ("powerpc/32s: Convert switch_mmu_context() to C") > moved the switch_mmu_context() to C. While in principle a good idea, it > meant that the function now uses the stack. The stack is not accessible > from real mode though. > > So to keep calling the function, let's turn on MSR_DR while we call it. > That way, all pointer references to the stack are handled virtually. > > Reported-by: Matt Evans <matt@ozlabs.org> > Fixes: 863771a28e27 ("powerpc/32s: Convert switch_mmu_context() to C") > Signed-off-by: Alexander Graf <graf@amazon.com> > Cc: stable@vger.kernel.org Many thanks - this addresses the issue I saw, and has been... Tested-by: Matt Evans <matt@ozlabs.org> ...on a G4 host. One comment though: > — > arch/powerpc/kvm/book3s_32_sr.S | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_32_sr.S b/arch/powerpc/kvm/book3s_32_sr.S > index e3ab9df6cf19..bd4f798f7a46 100644 > --- a/arch/powerpc/kvm/book3s_32_sr.S > +++ b/arch/powerpc/kvm/book3s_32_sr.S > @@ -122,11 +122,21 @@ > > /* 0x0 - 0xb */ > > - /* 'current->mm' needs to be in r4 */ > - tophys(r4, r2) > - lwz r4, MM(r4) > - tophys(r4, r4) > - /* This only clobbers r0, r3, r4 and r5 */ > + /* switch_mmu_context() needs paging, let's enable it */ > + mfmsr r9 > + ori r11, r9, MSR_DR > + mtmsr r11 > + sync > + > + /* Calling switch_mmu_context(<inv>, current->mm, <inv>); */ > + lwz r4, MM(r2) > bl switch_mmu_context Of the volatile registers, I believe r12 is still valuable here and would need to be preserved. (I can’t spot any others but would defer to your judgement here.) For example: diff --git a/arch/powerpc/kvm/book3s_32_sr.S b/arch/powerpc/kvm/book3s_32_sr.S index e3ab9df6cf19..41fc9ca12d38 100644 --- a/arch/powerpc/kvm/book3s_32_sr.S +++ b/arch/powerpc/kvm/book3s_32_sr.S @@ -122,11 +122,23 @@ /* 0x0 - 0xb */ - /* 'current->mm' needs to be in r4 */ - tophys(r4, r2) - lwz r4, MM(r4) - tophys(r4, r4) - /* This only clobbers r0, r3, r4 and r5 */ + /* switch_mmu_context() needs paging, let's enable it */ + mfmsr r9 + ori r11, r9, MSR_DR + mtmsr r11 + sync + + SAVE_GPR(12, r1) + /* Calling switch_mmu_context(<inv>, current->mm, <inv>); */ + lwz r4, MM(r2) bl switch_mmu_context + REST_GPR(12, r1) + + /* Disable paging again */ + mfmsr r9 + li r6, MSR_DR + andc r9, r9, r6 + mtmsr r9 + sync .endm Matt > > + /* Disable paging again */ > + mfmsr r9 > + li r6, MSR_DR > + andc r9, r9, r6 > + mtmsr r9 > + sync > + > .endm > -- > 2.28.0.394.ge197136389 > > > > > Amazon Development Center Germany GmbH > Krausenstr. 38 > 10117 Berlin > Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss > Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B > Sitz: Berlin > Ust-ID: DE 289 237 879 > > >
WARNING: multiple messages have this Message-ID (diff)
From: Matt Evans <matt@ozlabs.org> To: Alexander Graf <graf@amazon.com> Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>, stable@vger.kernel.org, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH] KVM: PPC: Book3S PR: Enable MSR_DR for switch_mmu_context() Date: Tue, 10 May 2022 11:33:45 +0100 [thread overview] Message-ID: <5CBF19EB-F2DA-4DBA-9BD0-D38E3A3F959A@ozlabs.org> (raw) In-Reply-To: <20220509202355.13985-1-graf@amazon.com> Hi Alex, > On 9 May 2022, at 21:23, Alexander Graf <graf@amazon.com> wrote: > > Commit 863771a28e27 ("powerpc/32s: Convert switch_mmu_context() to C") > moved the switch_mmu_context() to C. While in principle a good idea, it > meant that the function now uses the stack. The stack is not accessible > from real mode though. > > So to keep calling the function, let's turn on MSR_DR while we call it. > That way, all pointer references to the stack are handled virtually. > > Reported-by: Matt Evans <matt@ozlabs.org> > Fixes: 863771a28e27 ("powerpc/32s: Convert switch_mmu_context() to C") > Signed-off-by: Alexander Graf <graf@amazon.com> > Cc: stable@vger.kernel.org Many thanks - this addresses the issue I saw, and has been... Tested-by: Matt Evans <matt@ozlabs.org> ...on a G4 host. One comment though: > — > arch/powerpc/kvm/book3s_32_sr.S | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_32_sr.S b/arch/powerpc/kvm/book3s_32_sr.S > index e3ab9df6cf19..bd4f798f7a46 100644 > --- a/arch/powerpc/kvm/book3s_32_sr.S > +++ b/arch/powerpc/kvm/book3s_32_sr.S > @@ -122,11 +122,21 @@ > > /* 0x0 - 0xb */ > > - /* 'current->mm' needs to be in r4 */ > - tophys(r4, r2) > - lwz r4, MM(r4) > - tophys(r4, r4) > - /* This only clobbers r0, r3, r4 and r5 */ > + /* switch_mmu_context() needs paging, let's enable it */ > + mfmsr r9 > + ori r11, r9, MSR_DR > + mtmsr r11 > + sync > + > + /* Calling switch_mmu_context(<inv>, current->mm, <inv>); */ > + lwz r4, MM(r2) > bl switch_mmu_context Of the volatile registers, I believe r12 is still valuable here and would need to be preserved. (I can’t spot any others but would defer to your judgement here.) For example: diff --git a/arch/powerpc/kvm/book3s_32_sr.S b/arch/powerpc/kvm/book3s_32_sr.S index e3ab9df6cf19..41fc9ca12d38 100644 --- a/arch/powerpc/kvm/book3s_32_sr.S +++ b/arch/powerpc/kvm/book3s_32_sr.S @@ -122,11 +122,23 @@ /* 0x0 - 0xb */ - /* 'current->mm' needs to be in r4 */ - tophys(r4, r2) - lwz r4, MM(r4) - tophys(r4, r4) - /* This only clobbers r0, r3, r4 and r5 */ + /* switch_mmu_context() needs paging, let's enable it */ + mfmsr r9 + ori r11, r9, MSR_DR + mtmsr r11 + sync + + SAVE_GPR(12, r1) + /* Calling switch_mmu_context(<inv>, current->mm, <inv>); */ + lwz r4, MM(r2) bl switch_mmu_context + REST_GPR(12, r1) + + /* Disable paging again */ + mfmsr r9 + li r6, MSR_DR + andc r9, r9, r6 + mtmsr r9 + sync .endm Matt > > + /* Disable paging again */ > + mfmsr r9 > + li r6, MSR_DR > + andc r9, r9, r6 > + mtmsr r9 > + sync > + > .endm > -- > 2.28.0.394.ge197136389 > > > > > Amazon Development Center Germany GmbH > Krausenstr. 38 > 10117 Berlin > Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss > Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B > Sitz: Berlin > Ust-ID: DE 289 237 879 > > >
next prev parent reply other threads:[~2022-05-10 10:34 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-09 20:23 [PATCH] KVM: PPC: Book3S PR: Enable MSR_DR for switch_mmu_context() Alexander Graf 2022-05-09 20:23 ` Alexander Graf 2022-05-10 10:33 ` Matt Evans [this message] 2022-05-10 10:33 ` Matt Evans
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=5CBF19EB-F2DA-4DBA-9BD0-D38E3A3F959A@ozlabs.org \ --to=matt@ozlabs.org \ --cc=benh@kernel.crashing.org \ --cc=christophe.leroy@csgroup.eu \ --cc=graf@amazon.com \ --cc=kvm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mpe@ellerman.id.au \ --cc=paulus@samba.org \ --cc=stable@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.