From: Guo Ren <ren_guo@c-sky.com> To: Alistair Francis <alistair23@gmail.com> Cc: "open list:RISC-V" <qemu-riscv@nongnu.org>, Alistair Francis <alistair.francis@wdc.com>, Palmer Dabbelt <palmer@sifive.com>, guoren@kernel.org, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org> Subject: Re: [PATCH V2] target/riscv: Bugfix reserved bits in PTE for RV64 Date: Wed, 25 Sep 2019 08:13:17 +0800 [thread overview] Message-ID: <8E7A78A5-5E6F-49A2-89BC-85D2506229C6@c-sky.com> (raw) In-Reply-To: <CAKmqyKMzpTKBT+urX_7qFASqcAd4kkfJmf6LUk-0V=0LOuHLxw@mail.gmail.com> > 在 2019年9月25日,上午7:33,Alistair Francis <alistair23@gmail.com> 写道: > > 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. QEMU don’t zero out the bits, QEMU just ignore the bits for ppn. > > Does this fix a bug you are seeing? Yes, because we try to reuse these bits as attributes. > >> >> 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. I just prefer to save them in commit. > >> >> 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. OK, but git am we’ll lose them. > >> 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. OK follow your rule, but I still prefer prior. > >> + >> /* 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 En… Do you want this: ? #if defined(TARGET_RISCV32) target_ulong pte = ldl_phys(cs->as, pte_addr); + hwaddr ppn = pte; #elif defined(TARGET_RISCV64) target_ulong pte = ldq_phys(cs->as, pte_addr); + hwaddr ppn = (pte & ~PTE_RESERVED); #endif + ppn = ppn >> PTE_PPN_SHIFT; The pte couldn’t be destroyed, just ppn ignore the RESERVED bits.
WARNING: multiple messages have this Message-ID (diff)
From: Guo Ren <ren_guo@c-sky.com> To: Alistair Francis <alistair23@gmail.com> Cc: guoren@kernel.org, "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> Subject: Re: [PATCH V2] target/riscv: Bugfix reserved bits in PTE for RV64 Date: Wed, 25 Sep 2019 08:13:17 +0800 [thread overview] Message-ID: <8E7A78A5-5E6F-49A2-89BC-85D2506229C6@c-sky.com> (raw) In-Reply-To: <CAKmqyKMzpTKBT+urX_7qFASqcAd4kkfJmf6LUk-0V=0LOuHLxw@mail.gmail.com> > 在 2019年9月25日,上午7:33,Alistair Francis <alistair23@gmail.com> 写道: > > 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. QEMU don’t zero out the bits, QEMU just ignore the bits for ppn. > > Does this fix a bug you are seeing? Yes, because we try to reuse these bits as attributes. > >> >> 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. I just prefer to save them in commit. > >> >> 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. OK, but git am we’ll lose them. > >> 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. OK follow your rule, but I still prefer prior. > >> + >> /* 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 En… Do you want this: ? #if defined(TARGET_RISCV32) target_ulong pte = ldl_phys(cs->as, pte_addr); + hwaddr ppn = pte; #elif defined(TARGET_RISCV64) target_ulong pte = ldq_phys(cs->as, pte_addr); + hwaddr ppn = (pte & ~PTE_RESERVED); #endif + ppn = ppn >> PTE_PPN_SHIFT; The pte couldn’t be destroyed, just ppn ignore the RESERVED bits.
next prev parent reply other threads:[~2019-09-25 0:53 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 2019-09-24 23:33 ` Alistair Francis 2019-09-25 0:13 ` Guo Ren [this message] 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=8E7A78A5-5E6F-49A2-89BC-85D2506229C6@c-sky.com \ --to=ren_guo@c-sky.com \ --cc=alistair.francis@wdc.com \ --cc=alistair23@gmail.com \ --cc=guoren@kernel.org \ --cc=palmer@sifive.com \ --cc=qemu-devel@nongnu.org \ --cc=qemu-riscv@nongnu.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.