* [PATCH] target/riscv: pmp: Fixup TLB size calculation
@ 2022-10-12 1:14 Alistair Francis
2022-10-12 2:50 ` LIU Zhiwei
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Alistair Francis @ 2022-10-12 1:14 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: bmeng.cn, palmer, alistair.francis, alistair23, richard.henderson
From: Alistair Francis <alistair.francis@wdc.com>
Since commit 4047368938f6 "accel/tcg: Introduce tlb_set_page_full" we
have been seeing this assert
../accel/tcg/cputlb.c:1294: tlb_set_page_with_attrs: Assertion `is_power_of_2(size)' failed.
When running Tock on the OpenTitan machine.
The issue is that pmp_get_tlb_size() would return a TLB size that wasn't
a power of 2. The size was also smaller then TARGET_PAGE_SIZE.
This patch ensures that any TLB size less then TARGET_PAGE_SIZE is
rounded down to 1 to ensure it's a valid size.
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
This is based on advice from Richard:
https://patchwork.kernel.org/project/qemu-devel/patch/20221004141051.110653-9-richard.henderson@linaro.org/#25043166
target/riscv/pmp.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index ea2b67d947..2b43e399b8 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -628,6 +628,18 @@ bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa,
}
if (*tlb_size != 0) {
+ /*
+ * At this point we have a tlb_size that is the smallest possible size
+ * That fits within a TARGET_PAGE_SIZE and the PMP region.
+ *
+ * If the size is less then TARGET_PAGE_SIZE we drop the size to 1.
+ * This means the result isn't cached in the TLB and is only used for
+ * a single translation.
+ */
+ if (*tlb_size < TARGET_PAGE_SIZE) {
+ *tlb_size = 1;
+ }
+
return true;
}
--
2.37.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] target/riscv: pmp: Fixup TLB size calculation
2022-10-12 1:14 [PATCH] target/riscv: pmp: Fixup TLB size calculation Alistair Francis
@ 2022-10-12 2:50 ` LIU Zhiwei
2022-10-12 3:09 ` Alistair Francis
2022-10-13 18:16 ` Richard Henderson
2022-10-14 4:36 ` Alistair Francis
2 siblings, 1 reply; 6+ messages in thread
From: LIU Zhiwei @ 2022-10-12 2:50 UTC (permalink / raw)
To: Alistair Francis, qemu-devel, qemu-riscv
Cc: bmeng.cn, palmer, alistair.francis, alistair23, richard.henderson
Reviewed-by: LIU Zhiwei<zhiwei_liu@linux.alibaba.com>
By the way, we missed one related patch that once had been picked to riscv-next patch.
The patch v3:
https://lore.kernel.org/all/ceeb4037-6d17-0a09-f35a-eaf3280339bb@c-sky.com/T/#m183e4430bda408bc3a2b2751aa94eff7fc02e23c
The patch v4:
https://lists.gnu.org/archive/html/qemu-devel/2021-12/msg02854.html
I think the patch v4 should be taken at the same time with this patch.
Thanks,
Zhiwei
On 2022/10/12 9:14, Alistair Francis wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Since commit 4047368938f6 "accel/tcg: Introduce tlb_set_page_full" we
> have been seeing this assert
>
> ../accel/tcg/cputlb.c:1294: tlb_set_page_with_attrs: Assertion `is_power_of_2(size)' failed.
>
> When running Tock on the OpenTitan machine.
>
> The issue is that pmp_get_tlb_size() would return a TLB size that wasn't
> a power of 2. The size was also smaller then TARGET_PAGE_SIZE.
>
> This patch ensures that any TLB size less then TARGET_PAGE_SIZE is
> rounded down to 1 to ensure it's a valid size.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> This is based on advice from Richard:
> https://patchwork.kernel.org/project/qemu-devel/patch/20221004141051.110653-9-richard.henderson@linaro.org/#25043166
>
> target/riscv/pmp.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index ea2b67d947..2b43e399b8 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -628,6 +628,18 @@ bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa,
> }
>
> if (*tlb_size != 0) {
> + /*
> + * At this point we have a tlb_size that is the smallest possible size
> + * That fits within a TARGET_PAGE_SIZE and the PMP region.
> + *
> + * If the size is less then TARGET_PAGE_SIZE we drop the size to 1.
> + * This means the result isn't cached in the TLB and is only used for
> + * a single translation.
> + */
> + if (*tlb_size < TARGET_PAGE_SIZE) {
> + *tlb_size = 1;
> + }
> +
> return true;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] target/riscv: pmp: Fixup TLB size calculation
2022-10-12 2:50 ` LIU Zhiwei
@ 2022-10-12 3:09 ` Alistair Francis
2022-10-12 6:04 ` LIU Zhiwei
0 siblings, 1 reply; 6+ messages in thread
From: Alistair Francis @ 2022-10-12 3:09 UTC (permalink / raw)
To: LIU Zhiwei
Cc: Alistair Francis, qemu-devel, qemu-riscv, bmeng.cn, palmer,
alistair.francis, richard.henderson
On Wed, Oct 12, 2022 at 12:50 PM LIU Zhiwei
<zhiwei_liu@linux.alibaba.com> wrote:
>
> Reviewed-by: LIU Zhiwei<zhiwei_liu@linux.alibaba.com>
Thanks!
>
> By the way, we missed one related patch that once had been picked to riscv-next patch.
>
> The patch v3:
> https://lore.kernel.org/all/ceeb4037-6d17-0a09-f35a-eaf3280339bb@c-sky.com/T/#m183e4430bda408bc3a2b2751aa94eff7fc02e23c
So this was applied but caused boot failures so it was dropped from my
RISC-V tree
>
> The patch v4:
> https://lists.gnu.org/archive/html/qemu-devel/2021-12/msg02854.html
I think I misunderstood this comment [1] as applying to v4 and it
never got applied.
Do you mind resending the patch?
1: https://lore.kernel.org/all/ceeb4037-6d17-0a09-f35a-eaf3280339bb@c-sky.com/T/#m5e958d702d9905169a941f2ae59fdf7ac4a02383
Alistair
>
> I think the patch v4 should be taken at the same time with this patch.
>
> Thanks,
> Zhiwei
>
> On 2022/10/12 9:14, Alistair Francis wrote:
> > From: Alistair Francis <alistair.francis@wdc.com>
> >
> > Since commit 4047368938f6 "accel/tcg: Introduce tlb_set_page_full" we
> > have been seeing this assert
> >
> > ../accel/tcg/cputlb.c:1294: tlb_set_page_with_attrs: Assertion `is_power_of_2(size)' failed.
> >
> > When running Tock on the OpenTitan machine.
> >
> > The issue is that pmp_get_tlb_size() would return a TLB size that wasn't
> > a power of 2. The size was also smaller then TARGET_PAGE_SIZE.
> >
> > This patch ensures that any TLB size less then TARGET_PAGE_SIZE is
> > rounded down to 1 to ensure it's a valid size.
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> > This is based on advice from Richard:
> > https://patchwork.kernel.org/project/qemu-devel/patch/20221004141051.110653-9-richard.henderson@linaro.org/#25043166
> >
> > target/riscv/pmp.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> > index ea2b67d947..2b43e399b8 100644
> > --- a/target/riscv/pmp.c
> > +++ b/target/riscv/pmp.c
> > @@ -628,6 +628,18 @@ bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa,
> > }
> >
> > if (*tlb_size != 0) {
> > + /*
> > + * At this point we have a tlb_size that is the smallest possible size
> > + * That fits within a TARGET_PAGE_SIZE and the PMP region.
> > + *
> > + * If the size is less then TARGET_PAGE_SIZE we drop the size to 1.
> > + * This means the result isn't cached in the TLB and is only used for
> > + * a single translation.
> > + */
> > + if (*tlb_size < TARGET_PAGE_SIZE) {
> > + *tlb_size = 1;
> > + }
> > +
> > return true;
> > }
> >
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] target/riscv: pmp: Fixup TLB size calculation
2022-10-12 3:09 ` Alistair Francis
@ 2022-10-12 6:04 ` LIU Zhiwei
0 siblings, 0 replies; 6+ messages in thread
From: LIU Zhiwei @ 2022-10-12 6:04 UTC (permalink / raw)
To: Alistair Francis
Cc: Alistair Francis, qemu-devel, qemu-riscv, bmeng.cn, palmer,
alistair.francis, richard.henderson
On 2022/10/12 11:09, Alistair Francis wrote:
> On Wed, Oct 12, 2022 at 12:50 PM LIU Zhiwei
> <zhiwei_liu@linux.alibaba.com> wrote:
>> Reviewed-by: LIU Zhiwei<zhiwei_liu@linux.alibaba.com>
> Thanks!
>
>> By the way, we missed one related patch that once had been picked to riscv-next patch.
>>
>> The patch v3:
>> https://lore.kernel.org/all/ceeb4037-6d17-0a09-f35a-eaf3280339bb@c-sky.com/T/#m183e4430bda408bc3a2b2751aa94eff7fc02e23c
> So this was applied but caused boot failures so it was dropped from my
> RISC-V tree
>
>> The patch v4:
>> https://lists.gnu.org/archive/html/qemu-devel/2021-12/msg02854.html
> I think I misunderstood this comment [1] as applying to v4 and it
> never got applied.
>
> Do you mind resending the patch?
Sure. I have rebased it to your patch and sent it to the mail list.
Thanks,
Zhiwei
>
> 1: https://lore.kernel.org/all/ceeb4037-6d17-0a09-f35a-eaf3280339bb@c-sky.com/T/#m5e958d702d9905169a941f2ae59fdf7ac4a02383
>
> Alistair
>
>> I think the patch v4 should be taken at the same time with this patch.
>>
>> Thanks,
>> Zhiwei
>>
>> On 2022/10/12 9:14, Alistair Francis wrote:
>>> From: Alistair Francis <alistair.francis@wdc.com>
>>>
>>> Since commit 4047368938f6 "accel/tcg: Introduce tlb_set_page_full" we
>>> have been seeing this assert
>>>
>>> ../accel/tcg/cputlb.c:1294: tlb_set_page_with_attrs: Assertion `is_power_of_2(size)' failed.
>>>
>>> When running Tock on the OpenTitan machine.
>>>
>>> The issue is that pmp_get_tlb_size() would return a TLB size that wasn't
>>> a power of 2. The size was also smaller then TARGET_PAGE_SIZE.
>>>
>>> This patch ensures that any TLB size less then TARGET_PAGE_SIZE is
>>> rounded down to 1 to ensure it's a valid size.
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>>> ---
>>> This is based on advice from Richard:
>>> https://patchwork.kernel.org/project/qemu-devel/patch/20221004141051.110653-9-richard.henderson@linaro.org/#25043166
>>>
>>> target/riscv/pmp.c | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
>>> index ea2b67d947..2b43e399b8 100644
>>> --- a/target/riscv/pmp.c
>>> +++ b/target/riscv/pmp.c
>>> @@ -628,6 +628,18 @@ bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa,
>>> }
>>>
>>> if (*tlb_size != 0) {
>>> + /*
>>> + * At this point we have a tlb_size that is the smallest possible size
>>> + * That fits within a TARGET_PAGE_SIZE and the PMP region.
>>> + *
>>> + * If the size is less then TARGET_PAGE_SIZE we drop the size to 1.
>>> + * This means the result isn't cached in the TLB and is only used for
>>> + * a single translation.
>>> + */
>>> + if (*tlb_size < TARGET_PAGE_SIZE) {
>>> + *tlb_size = 1;
>>> + }
>>> +
>>> return true;
>>> }
>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: pmp: Fixup TLB size calculation
2022-10-12 1:14 [PATCH] target/riscv: pmp: Fixup TLB size calculation Alistair Francis
2022-10-12 2:50 ` LIU Zhiwei
@ 2022-10-13 18:16 ` Richard Henderson
2022-10-14 4:36 ` Alistair Francis
2 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2022-10-13 18:16 UTC (permalink / raw)
To: Alistair Francis, qemu-devel, qemu-riscv
Cc: bmeng.cn, palmer, alistair.francis, alistair23
On 10/12/22 18:14, Alistair Francis wrote:
> From: Alistair Francis<alistair.francis@wdc.com>
>
> Since commit 4047368938f6 "accel/tcg: Introduce tlb_set_page_full" we
> have been seeing this assert
>
> ../accel/tcg/cputlb.c:1294: tlb_set_page_with_attrs: Assertion `is_power_of_2(size)' failed.
>
> When running Tock on the OpenTitan machine.
>
> The issue is that pmp_get_tlb_size() would return a TLB size that wasn't
> a power of 2. The size was also smaller then TARGET_PAGE_SIZE.
>
> This patch ensures that any TLB size less then TARGET_PAGE_SIZE is
> rounded down to 1 to ensure it's a valid size.
>
> Signed-off-by: Alistair Francis<alistair.francis@wdc.com>
> ---
> This is based on advice from Richard:
> https://patchwork.kernel.org/project/qemu-devel/patch/20221004141051.110653-9-richard.henderson@linaro.org/#25043166
>
> target/riscv/pmp.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: pmp: Fixup TLB size calculation
2022-10-12 1:14 [PATCH] target/riscv: pmp: Fixup TLB size calculation Alistair Francis
2022-10-12 2:50 ` LIU Zhiwei
2022-10-13 18:16 ` Richard Henderson
@ 2022-10-14 4:36 ` Alistair Francis
2 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2022-10-14 4:36 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-devel, qemu-riscv, bmeng.cn, palmer, alistair.francis,
richard.henderson
On Wed, Oct 12, 2022 at 11:15 AM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Since commit 4047368938f6 "accel/tcg: Introduce tlb_set_page_full" we
> have been seeing this assert
>
> ../accel/tcg/cputlb.c:1294: tlb_set_page_with_attrs: Assertion `is_power_of_2(size)' failed.
>
> When running Tock on the OpenTitan machine.
>
> The issue is that pmp_get_tlb_size() would return a TLB size that wasn't
> a power of 2. The size was also smaller then TARGET_PAGE_SIZE.
>
> This patch ensures that any TLB size less then TARGET_PAGE_SIZE is
> rounded down to 1 to ensure it's a valid size.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> This is based on advice from Richard:
> https://patchwork.kernel.org/project/qemu-devel/patch/20221004141051.110653-9-richard.henderson@linaro.org/#25043166
>
> target/riscv/pmp.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index ea2b67d947..2b43e399b8 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -628,6 +628,18 @@ bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa,
> }
>
> if (*tlb_size != 0) {
> + /*
> + * At this point we have a tlb_size that is the smallest possible size
> + * That fits within a TARGET_PAGE_SIZE and the PMP region.
> + *
> + * If the size is less then TARGET_PAGE_SIZE we drop the size to 1.
> + * This means the result isn't cached in the TLB and is only used for
> + * a single translation.
> + */
> + if (*tlb_size < TARGET_PAGE_SIZE) {
> + *tlb_size = 1;
> + }
> +
> return true;
> }
>
> --
> 2.37.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-14 4:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-12 1:14 [PATCH] target/riscv: pmp: Fixup TLB size calculation Alistair Francis
2022-10-12 2:50 ` LIU Zhiwei
2022-10-12 3:09 ` Alistair Francis
2022-10-12 6:04 ` LIU Zhiwei
2022-10-13 18:16 ` Richard Henderson
2022-10-14 4:36 ` Alistair Francis
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).