* [Qemu-devel] [PATCH] riscv: rv32: Root page table address can be larger than 32-bit @ 2019-07-31 12:45 Bin Meng 2019-07-31 17:35 ` Richard Henderson 2019-08-08 2:49 ` [Qemu-devel] [PATCH v2] " Bin Meng 0 siblings, 2 replies; 12+ messages in thread From: Bin Meng @ 2019-07-31 12:45 UTC (permalink / raw) To: Alistair Francis, Palmer Dabbelt, Bastian Koppelmann, Sagar Karandikar, QEMU devel, QEMU riscv For RV32, the root page table's PPN has 22 bits hence its address bits could be larger than the maximum bits that target_ulong is able to represent. Use hwaddr instead. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- target/riscv/cpu_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index e32b612..3150a6a 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -176,7 +176,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical, *prot = 0; - target_ulong base; + hwaddr base; int levels, ptidxbits, ptesize, vm, sum; int mxr = get_field(env->mstatus, MSTATUS_MXR); @@ -239,7 +239,7 @@ restart: ((1 << ptidxbits) - 1); /* check that physical address of PTE is legal */ - target_ulong pte_addr = base + idx * ptesize; + hwaddr pte_addr = base + idx * ptesize; if (riscv_feature(env, RISCV_FEATURE_PMP) && !pmp_hart_has_privs(env, pte_addr, sizeof(target_ulong), -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] riscv: rv32: Root page table address can be larger than 32-bit 2019-07-31 12:45 [Qemu-devel] [PATCH] riscv: rv32: Root page table address can be larger than 32-bit Bin Meng @ 2019-07-31 17:35 ` Richard Henderson 2019-08-01 1:53 ` Bin Meng 2019-08-08 2:49 ` [Qemu-devel] [PATCH v2] " Bin Meng 1 sibling, 1 reply; 12+ messages in thread From: Richard Henderson @ 2019-07-31 17:35 UTC (permalink / raw) To: Bin Meng, Alistair Francis, Palmer Dabbelt, Bastian Koppelmann, Sagar Karandikar, QEMU devel, QEMU riscv On 7/31/19 5:45 AM, Bin Meng wrote: > - target_ulong base; > + hwaddr base; ... > - target_ulong pte_addr = base + idx * ptesize; > + hwaddr pte_addr = base + idx * ptesize; I believe that you either need base + (hwaddr)idx * ptesize or change the type of idx to hwaddr above. Otherwise the multiply overflows before it gets promoted with the add. r~ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] riscv: rv32: Root page table address can be larger than 32-bit 2019-07-31 17:35 ` Richard Henderson @ 2019-08-01 1:53 ` Bin Meng 2019-08-01 14:16 ` Richard Henderson 0 siblings, 1 reply; 12+ messages in thread From: Bin Meng @ 2019-08-01 1:53 UTC (permalink / raw) To: Richard Henderson Cc: QEMU riscv, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt, QEMU devel, Alistair Francis Hi Richard, On Thu, Aug 1, 2019 at 1:35 AM Richard Henderson <richard.henderson@linaro.org> wrote: > > On 7/31/19 5:45 AM, Bin Meng wrote: > > - target_ulong base; > > + hwaddr base; > ... > > - target_ulong pte_addr = base + idx * ptesize; > > + hwaddr pte_addr = base + idx * ptesize; > > I believe that you either need > > base + (hwaddr)idx * ptesize > > or change the type of idx to hwaddr above. > > Otherwise the multiply overflows before it gets promoted with the add. > I am not sure how (idx * ptesize) could overflow. It represents the offset by a page table which is [0, 4096). Regards, Bin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] riscv: rv32: Root page table address can be larger than 32-bit 2019-08-01 1:53 ` Bin Meng @ 2019-08-01 14:16 ` Richard Henderson 2019-08-01 14:57 ` Bin Meng 0 siblings, 1 reply; 12+ messages in thread From: Richard Henderson @ 2019-08-01 14:16 UTC (permalink / raw) To: Bin Meng Cc: QEMU riscv, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt, QEMU devel, Alistair Francis On 7/31/19 6:53 PM, Bin Meng wrote: > I am not sure how (idx * ptesize) could overflow. It represents the > offset by a page table which is [0, 4096). You're right, I mis-read what was going on there. However, lower down, "target_ulong ppn" needs to be promoted to hwaddr, so that ppn = pte >> PTE_PPN_SHIFT; ... base = ppn << PGSHIFT; does not overflow. (Which is the part of the page table walk that I thought I had gleaned from the patch without actually reading the entire function.) r~ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] riscv: rv32: Root page table address can be larger than 32-bit 2019-08-01 14:16 ` Richard Henderson @ 2019-08-01 14:57 ` Bin Meng 2019-08-07 20:55 ` Palmer Dabbelt 0 siblings, 1 reply; 12+ messages in thread From: Bin Meng @ 2019-08-01 14:57 UTC (permalink / raw) To: Richard Henderson Cc: QEMU riscv, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt, QEMU devel, Alistair Francis On Thu, Aug 1, 2019 at 10:16 PM Richard Henderson <richard.henderson@linaro.org> wrote: > > On 7/31/19 6:53 PM, Bin Meng wrote: > > I am not sure how (idx * ptesize) could overflow. It represents the > > offset by a page table which is [0, 4096). > > You're right, I mis-read what was going on there. > > However, lower down, "target_ulong ppn" needs to be promoted to hwaddr, so that > > ppn = pte >> PTE_PPN_SHIFT; > ... > base = ppn << PGSHIFT; > > does not overflow. (Which is the part of the page table walk that I thought I > had gleaned from the patch without actually reading the entire function.) Ah, yes. ppn should be promoted. Thanks for the review! Regards, Bin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] riscv: rv32: Root page table address can be larger than 32-bit 2019-08-01 14:57 ` Bin Meng @ 2019-08-07 20:55 ` Palmer Dabbelt 2019-08-08 1:49 ` Bin Meng 0 siblings, 1 reply; 12+ messages in thread From: Palmer Dabbelt @ 2019-08-07 20:55 UTC (permalink / raw) To: Bin Meng Cc: QEMU riscv, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt, Richard Henderson, QEMU devel, Alistair Francis On Thu, Aug 1, 2019 at 7:58 AM Bin Meng <bmeng.cn@gmail.com> wrote: > On Thu, Aug 1, 2019 at 10:16 PM Richard Henderson > <richard.henderson@linaro.org> wrote: > > > > On 7/31/19 6:53 PM, Bin Meng wrote: > > > I am not sure how (idx * ptesize) could overflow. It represents the > > > offset by a page table which is [0, 4096). > > > > You're right, I mis-read what was going on there. > > > > However, lower down, "target_ulong ppn" needs to be promoted to hwaddr, > so that > > > > ppn = pte >> PTE_PPN_SHIFT; > > ... > > base = ppn << PGSHIFT; > > > > does not overflow. (Which is the part of the page table walk that I > thought I > > had gleaned from the patch without actually reading the entire function.) > > Ah, yes. ppn should be promoted. Thanks for the review! > Did I miss a v2? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] riscv: rv32: Root page table address can be larger than 32-bit 2019-08-07 20:55 ` Palmer Dabbelt @ 2019-08-08 1:49 ` Bin Meng 0 siblings, 0 replies; 12+ messages in thread From: Bin Meng @ 2019-08-08 1:49 UTC (permalink / raw) To: Palmer Dabbelt Cc: QEMU riscv, Sagar Karandikar, Bastian Koppelmann, Richard Henderson, QEMU devel, Alistair Francis Hi Palmer, On Thu, Aug 8, 2019 at 4:55 AM Palmer Dabbelt <palmer@sifive.com> wrote: > > On Thu, Aug 1, 2019 at 7:58 AM Bin Meng <bmeng.cn@gmail.com> wrote: >> >> On Thu, Aug 1, 2019 at 10:16 PM Richard Henderson >> <richard.henderson@linaro.org> wrote: >> > >> > On 7/31/19 6:53 PM, Bin Meng wrote: >> > > I am not sure how (idx * ptesize) could overflow. It represents the >> > > offset by a page table which is [0, 4096). >> > >> > You're right, I mis-read what was going on there. >> > >> > However, lower down, "target_ulong ppn" needs to be promoted to hwaddr, so that >> > >> > ppn = pte >> PTE_PPN_SHIFT; >> > ... >> > base = ppn << PGSHIFT; >> > >> > does not overflow. (Which is the part of the page table walk that I thought I >> > had gleaned from the patch without actually reading the entire function.) >> >> Ah, yes. ppn should be promoted. Thanks for the review! > > > Did I miss a v2? No, I will send a v2 soon. Regards, Bin ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2] riscv: rv32: Root page table address can be larger than 32-bit 2019-07-31 12:45 [Qemu-devel] [PATCH] riscv: rv32: Root page table address can be larger than 32-bit Bin Meng 2019-07-31 17:35 ` Richard Henderson @ 2019-08-08 2:49 ` Bin Meng 2019-08-10 1:49 ` Alistair Francis 1 sibling, 1 reply; 12+ messages in thread From: Bin Meng @ 2019-08-08 2:49 UTC (permalink / raw) To: Alistair Francis, Bastian Koppelmann, Palmer Dabbelt, Sagar Karandikar, Richard Henderson, qemu-devel, qemu-riscv For RV32, the root page table's PPN has 22 bits hence its address bits could be larger than the maximum bits that target_ulong is able to represent. Use hwaddr instead. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- Changes in v2: - promote ppn, env->satp/env->sptbl to hwaddr otherwise the page table base will not be correctly calculated target/riscv/cpu_helper.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index e32b612..b2b4f3a 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -176,12 +176,12 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical, *prot = 0; - target_ulong base; + hwaddr base; int levels, ptidxbits, ptesize, vm, sum; int mxr = get_field(env->mstatus, MSTATUS_MXR); if (env->priv_ver >= PRIV_VERSION_1_10_0) { - base = get_field(env->satp, SATP_PPN) << PGSHIFT; + base = (hwaddr)get_field(env->satp, SATP_PPN) << PGSHIFT; sum = get_field(env->mstatus, MSTATUS_SUM); vm = get_field(env->satp, SATP_MODE); switch (vm) { @@ -201,7 +201,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical, g_assert_not_reached(); } } else { - base = env->sptbr << PGSHIFT; + base = (hwaddr)(env->sptbr) << PGSHIFT; sum = !get_field(env->mstatus, MSTATUS_PUM); vm = get_field(env->mstatus, MSTATUS_VM); switch (vm) { @@ -239,7 +239,7 @@ restart: ((1 << ptidxbits) - 1); /* check that physical address of PTE is legal */ - target_ulong pte_addr = base + idx * ptesize; + hwaddr pte_addr = base + idx * ptesize; if (riscv_feature(env, RISCV_FEATURE_PMP) && !pmp_hart_has_privs(env, pte_addr, sizeof(target_ulong), @@ -251,7 +251,7 @@ restart: #elif defined(TARGET_RISCV64) target_ulong pte = ldq_phys(cs->as, pte_addr); #endif - target_ulong ppn = pte >> PTE_PPN_SHIFT; + hwaddr ppn = pte >> PTE_PPN_SHIFT; if (!(pte & PTE_V)) { /* Invalid PTE */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2] riscv: rv32: Root page table address can be larger than 32-bit 2019-08-08 2:49 ` [Qemu-devel] [PATCH v2] " Bin Meng @ 2019-08-10 1:49 ` Alistair Francis 2019-08-14 9:46 ` Bin Meng 0 siblings, 1 reply; 12+ messages in thread From: Alistair Francis @ 2019-08-10 1:49 UTC (permalink / raw) To: Bin Meng Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt, Richard Henderson, qemu-devel@nongnu.org Developers, Alistair Francis On Wed, Aug 7, 2019 at 7:50 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > For RV32, the root page table's PPN has 22 bits hence its address > bits could be larger than the maximum bits that target_ulong is > able to represent. Use hwaddr instead. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > > --- > > Changes in v2: > - promote ppn, env->satp/env->sptbl to hwaddr otherwise the page > table base will not be correctly calculated > > target/riscv/cpu_helper.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index e32b612..b2b4f3a 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -176,12 +176,12 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical, > > *prot = 0; > > - target_ulong base; > + hwaddr base; > int levels, ptidxbits, ptesize, vm, sum; > int mxr = get_field(env->mstatus, MSTATUS_MXR); > > if (env->priv_ver >= PRIV_VERSION_1_10_0) { > - base = get_field(env->satp, SATP_PPN) << PGSHIFT; > + base = (hwaddr)get_field(env->satp, SATP_PPN) << PGSHIFT; > sum = get_field(env->mstatus, MSTATUS_SUM); > vm = get_field(env->satp, SATP_MODE); > switch (vm) { > @@ -201,7 +201,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical, > g_assert_not_reached(); > } > } else { > - base = env->sptbr << PGSHIFT; > + base = (hwaddr)(env->sptbr) << PGSHIFT; > sum = !get_field(env->mstatus, MSTATUS_PUM); > vm = get_field(env->mstatus, MSTATUS_VM); > switch (vm) { > @@ -239,7 +239,7 @@ restart: > ((1 << ptidxbits) - 1); > > /* check that physical address of PTE is legal */ > - target_ulong pte_addr = base + idx * ptesize; > + hwaddr pte_addr = base + idx * ptesize; > > if (riscv_feature(env, RISCV_FEATURE_PMP) && > !pmp_hart_has_privs(env, pte_addr, sizeof(target_ulong), > @@ -251,7 +251,7 @@ restart: > #elif defined(TARGET_RISCV64) > target_ulong pte = ldq_phys(cs->as, pte_addr); > #endif > - target_ulong ppn = pte >> PTE_PPN_SHIFT; > + hwaddr ppn = pte >> PTE_PPN_SHIFT; > > if (!(pte & PTE_V)) { > /* Invalid PTE */ > -- > 2.7.4 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2] riscv: rv32: Root page table address can be larger than 32-bit 2019-08-10 1:49 ` Alistair Francis @ 2019-08-14 9:46 ` Bin Meng 2019-08-19 6:00 ` Bin Meng 0 siblings, 1 reply; 12+ messages in thread From: Bin Meng @ 2019-08-14 9:46 UTC (permalink / raw) To: Alistair Francis Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt, Richard Henderson, qemu-devel@nongnu.org Developers, Alistair Francis Hi Palmer, On Sat, Aug 10, 2019 at 9:49 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Wed, Aug 7, 2019 at 7:50 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > For RV32, the root page table's PPN has 22 bits hence its address > > bits could be larger than the maximum bits that target_ulong is > > able to represent. Use hwaddr instead. > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > Would you take this one too? Regards, Bin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2] riscv: rv32: Root page table address can be larger than 32-bit 2019-08-14 9:46 ` Bin Meng @ 2019-08-19 6:00 ` Bin Meng 2019-08-27 23:37 ` Palmer Dabbelt 0 siblings, 1 reply; 12+ messages in thread From: Bin Meng @ 2019-08-19 6:00 UTC (permalink / raw) To: Alistair Francis Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann, Palmer Dabbelt, Richard Henderson, qemu-devel@nongnu.org Developers, Alistair Francis On Wed, Aug 14, 2019 at 5:46 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Palmer, > > On Sat, Aug 10, 2019 at 9:49 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Wed, Aug 7, 2019 at 7:50 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > For RV32, the root page table's PPN has 22 bits hence its address > > > bits could be larger than the maximum bits that target_ulong is > > > able to represent. Use hwaddr instead. > > > > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > > > Would you take this one too? > Ping? What's the status of this patch? Regards, Bin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2] riscv: rv32: Root page table address can be larger than 32-bit 2019-08-19 6:00 ` Bin Meng @ 2019-08-27 23:37 ` Palmer Dabbelt 0 siblings, 0 replies; 12+ messages in thread From: Palmer Dabbelt @ 2019-08-27 23:37 UTC (permalink / raw) To: bmeng.cn Cc: qemu-riscv, sagark, Bastian Koppelmann, richard.henderson, qemu-devel, Alistair Francis, alistair23 On Sun, 18 Aug 2019 23:00:40 PDT (-0700), bmeng.cn@gmail.com wrote: > On Wed, Aug 14, 2019 at 5:46 PM Bin Meng <bmeng.cn@gmail.com> wrote: >> >> Hi Palmer, >> >> On Sat, Aug 10, 2019 at 9:49 AM Alistair Francis <alistair23@gmail.com> wrote: >> > >> > On Wed, Aug 7, 2019 at 7:50 PM Bin Meng <bmeng.cn@gmail.com> wrote: >> > > >> > > For RV32, the root page table's PPN has 22 bits hence its address >> > > bits could be larger than the maximum bits that target_ulong is >> > > able to represent. Use hwaddr instead. >> > > >> > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >> > >> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> >> > >> >> Would you take this one too? >> > > Ping? > > What's the status of this patch? Also in the patch queue. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-08-27 23:37 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-31 12:45 [Qemu-devel] [PATCH] riscv: rv32: Root page table address can be larger than 32-bit Bin Meng 2019-07-31 17:35 ` Richard Henderson 2019-08-01 1:53 ` Bin Meng 2019-08-01 14:16 ` Richard Henderson 2019-08-01 14:57 ` Bin Meng 2019-08-07 20:55 ` Palmer Dabbelt 2019-08-08 1:49 ` Bin Meng 2019-08-08 2:49 ` [Qemu-devel] [PATCH v2] " Bin Meng 2019-08-10 1:49 ` Alistair Francis 2019-08-14 9:46 ` Bin Meng 2019-08-19 6:00 ` Bin Meng 2019-08-27 23:37 ` Palmer Dabbelt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).