From: Alistair Francis <alistair23@gmail.com> To: guoren@kernel.org Cc: Alistair Francis <alistair.francis@wdc.com>, Palmer Dabbelt <palmer@sifive.com>, "open list:RISC-V" <qemu-riscv@nongnu.org>, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>, Guo Ren <ren_guo@c-sky.com> Subject: Re: [PATCH V2] target/riscv: Bugfix reserved bits in PTE for RV64 Date: Tue, 24 Sep 2019 16:33:51 -0700 [thread overview] Message-ID: <CAKmqyKMzpTKBT+urX_7qFASqcAd4kkfJmf6LUk-0V=0LOuHLxw@mail.gmail.com> (raw) In-Reply-To: <1569311902-12173-1-git-send-email-guoren@kernel.org> On Tue, Sep 24, 2019 at 12:58 AM <guoren@kernel.org> wrote: > > From: Guo Ren <ren_guo@c-sky.com> > > Highest 10 bits of PTE are reserved in riscv-privileged, ref: [1], so we > need to ignore them. They can not be a part of ppn. > > 1: The RISC-V Instruction Set Manual, Volume II: Privileged Architecture > 4.4 Sv39: Page-Based 39-bit Virtual-Memory System > 4.5 Sv48: Page-Based 48-bit Virtual-Memory System Thanks for the patch! The spec says "must be zeroed by software for forward compatibility" so I don't think it's correct for QEMU to zero out the bits. Does this fix a bug you are seeing? > > Changelog V2: > - Bugfix pte destroyed cause boot fail > - Change to AND with a mask instead of shifting both directions The changelog shouldn't be in the commit, it should be kept under the line line below. > > Signed-off-by: Guo Ren <ren_guo@c-sky.com> > Reviewed-by: Liu Zhiwei <zhiwei_liu@c-sky.com> > --- The change log should go here. > target/riscv/cpu_bits.h | 3 +++ > target/riscv/cpu_helper.c | 3 ++- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h > index e998348..ae8aa0f 100644 > --- a/target/riscv/cpu_bits.h > +++ b/target/riscv/cpu_bits.h > @@ -470,6 +470,9 @@ > #define PTE_D 0x080 /* Dirty */ > #define PTE_SOFT 0x300 /* Reserved for Software */ > > +/* Reserved highest 10 bits in PTE */ > +#define PTE_RESERVED ((target_ulong)0x3ff << 54) I think it's just easier to define this as 0xFFC0000000000000ULL and remove the cast. > + > /* Page table PPN shift amount */ > #define PTE_PPN_SHIFT 10 > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 87dd6a6..7a540cc 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -258,10 +258,11 @@ restart: > } > #if defined(TARGET_RISCV32) > target_ulong pte = ldl_phys(cs->as, pte_addr); > + hwaddr ppn = pte >> PTE_PPN_SHIFT; > #elif defined(TARGET_RISCV64) > target_ulong pte = ldq_phys(cs->as, pte_addr); > + hwaddr ppn = (pte & ~PTE_RESERVED) >> PTE_PPN_SHIFT; > #endif > - hwaddr ppn = pte >> PTE_PPN_SHIFT; You don't have to move this shift > > if (!(pte & PTE_V)) { > /* Invalid PTE */ > -- > 2.7.4 >
WARNING: multiple messages have this Message-ID (diff)
From: Alistair Francis <alistair23@gmail.com> To: guoren@kernel.org Cc: "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>, "open list:RISC-V" <qemu-riscv@nongnu.org>, Alistair Francis <alistair.francis@wdc.com>, Palmer Dabbelt <palmer@sifive.com>, Guo Ren <ren_guo@c-sky.com> Subject: Re: [PATCH V2] target/riscv: Bugfix reserved bits in PTE for RV64 Date: Tue, 24 Sep 2019 16:33:51 -0700 [thread overview] Message-ID: <CAKmqyKMzpTKBT+urX_7qFASqcAd4kkfJmf6LUk-0V=0LOuHLxw@mail.gmail.com> (raw) In-Reply-To: <1569311902-12173-1-git-send-email-guoren@kernel.org> On Tue, Sep 24, 2019 at 12:58 AM <guoren@kernel.org> wrote: > > From: Guo Ren <ren_guo@c-sky.com> > > Highest 10 bits of PTE are reserved in riscv-privileged, ref: [1], so we > need to ignore them. They can not be a part of ppn. > > 1: The RISC-V Instruction Set Manual, Volume II: Privileged Architecture > 4.4 Sv39: Page-Based 39-bit Virtual-Memory System > 4.5 Sv48: Page-Based 48-bit Virtual-Memory System Thanks for the patch! The spec says "must be zeroed by software for forward compatibility" so I don't think it's correct for QEMU to zero out the bits. Does this fix a bug you are seeing? > > Changelog V2: > - Bugfix pte destroyed cause boot fail > - Change to AND with a mask instead of shifting both directions The changelog shouldn't be in the commit, it should be kept under the line line below. > > Signed-off-by: Guo Ren <ren_guo@c-sky.com> > Reviewed-by: Liu Zhiwei <zhiwei_liu@c-sky.com> > --- The change log should go here. > target/riscv/cpu_bits.h | 3 +++ > target/riscv/cpu_helper.c | 3 ++- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h > index e998348..ae8aa0f 100644 > --- a/target/riscv/cpu_bits.h > +++ b/target/riscv/cpu_bits.h > @@ -470,6 +470,9 @@ > #define PTE_D 0x080 /* Dirty */ > #define PTE_SOFT 0x300 /* Reserved for Software */ > > +/* Reserved highest 10 bits in PTE */ > +#define PTE_RESERVED ((target_ulong)0x3ff << 54) I think it's just easier to define this as 0xFFC0000000000000ULL and remove the cast. > + > /* Page table PPN shift amount */ > #define PTE_PPN_SHIFT 10 > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 87dd6a6..7a540cc 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -258,10 +258,11 @@ restart: > } > #if defined(TARGET_RISCV32) > target_ulong pte = ldl_phys(cs->as, pte_addr); > + hwaddr ppn = pte >> PTE_PPN_SHIFT; > #elif defined(TARGET_RISCV64) > target_ulong pte = ldq_phys(cs->as, pte_addr); > + hwaddr ppn = (pte & ~PTE_RESERVED) >> PTE_PPN_SHIFT; > #endif > - hwaddr ppn = pte >> PTE_PPN_SHIFT; You don't have to move this shift > > if (!(pte & PTE_V)) { > /* Invalid PTE */ > -- > 2.7.4 >
next prev parent reply other threads:[~2019-09-24 23:40 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-09-24 7:58 [PATCH V2] target/riscv: Bugfix reserved bits in PTE for RV64 guoren 2019-09-24 7:58 ` guoren 2019-09-24 8:02 ` Guo Ren 2019-09-24 23:33 ` Alistair Francis [this message] 2019-09-24 23:33 ` Alistair Francis 2019-09-25 0:13 ` Guo Ren 2019-09-25 0:13 ` Guo Ren 2019-09-25 0:21 ` Alistair Francis 2019-09-25 0:21 ` Alistair Francis 2019-09-25 1:02 ` Guo Ren 2019-09-25 1:02 ` Guo Ren
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='CAKmqyKMzpTKBT+urX_7qFASqcAd4kkfJmf6LUk-0V=0LOuHLxw@mail.gmail.com' \ --to=alistair23@gmail.com \ --cc=alistair.francis@wdc.com \ --cc=guoren@kernel.org \ --cc=palmer@sifive.com \ --cc=qemu-devel@nongnu.org \ --cc=qemu-riscv@nongnu.org \ --cc=ren_guo@c-sky.com \ /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.