* [PATCH] target/riscv: raise exception to HS-mode at get_physical_address
@ 2020-08-24 8:41 ` Yifei Jiang
0 siblings, 0 replies; 10+ messages in thread
From: Yifei Jiang @ 2020-08-24 8:41 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: zhang.zhanghailiang, sagark, kbastian, victor.zhangxiaofeng,
Yifei Jiang, Alistair.Francis, yinyipeng1, palmer, wu.wubin,
dengkai1
VS-stage translation at get_physical_address needs to translate pte
address by G-stage translation. But the G-stage translation error
can not be distinguished from VS-stage translation error in
riscv_cpu_tlb_fill. On migration, destination needs to rebuild pte,
and this G-stage translation error must be handled by HS-mode. So
introduce TRANSLATE_STAGE2_FAIL so that riscv_cpu_tlb_fill could
distinguish and raise it to HS-mode.
Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
---
target/riscv/cpu.h | 1 +
target/riscv/cpu_helper.c | 12 ++++++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a804a5d0ba..8b3b368d6a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -85,6 +85,7 @@ enum {
#define TRANSLATE_FAIL 1
#define TRANSLATE_SUCCESS 0
#define MMU_USER_IDX 3
+#define TRANSLATE_G_STAGE_FAIL 4
#define MAX_RISCV_PMPS (16)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index fd1d373b6f..1635b09c41 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -440,7 +440,10 @@ restart:
mmu_idx, false, true);
if (vbase_ret != TRANSLATE_SUCCESS) {
- return vbase_ret;
+ env->guest_phys_fault_addr = (base |
+ (addr &
+ (TARGET_PAGE_SIZE - 1))) >> 2;
+ return TRANSLATE_G_STAGE_FAIL;
}
pte_addr = vbase + idx * ptesize;
@@ -728,12 +731,17 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
ret = get_physical_address(env, &pa, &prot, address, access_type,
mmu_idx, true, true);
+ if (ret == TRANSLATE_G_STAGE_FAIL) {
+ first_stage_error = false;
+ access_type = MMU_DATA_LOAD;
+ }
+
qemu_log_mask(CPU_LOG_MMU,
"%s 1st-stage address=%" VADDR_PRIx " ret %d physical "
TARGET_FMT_plx " prot %d\n",
__func__, address, ret, pa, prot);
- if (ret != TRANSLATE_FAIL) {
+ if (ret != TRANSLATE_FAIL && ret != TRANSLATE_G_STAGE_FAIL) {
/* Second stage lookup */
im_address = pa;
--
2.19.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] target/riscv: raise exception to HS-mode at get_physical_address
@ 2020-08-24 8:41 ` Yifei Jiang
0 siblings, 0 replies; 10+ messages in thread
From: Yifei Jiang @ 2020-08-24 8:41 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: palmer, Alistair.Francis, sagark, kbastian, victor.zhangxiaofeng,
zhang.zhanghailiang, wu.wubin, dengkai1, yinyipeng1, Yifei Jiang
VS-stage translation at get_physical_address needs to translate pte
address by G-stage translation. But the G-stage translation error
can not be distinguished from VS-stage translation error in
riscv_cpu_tlb_fill. On migration, destination needs to rebuild pte,
and this G-stage translation error must be handled by HS-mode. So
introduce TRANSLATE_STAGE2_FAIL so that riscv_cpu_tlb_fill could
distinguish and raise it to HS-mode.
Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
---
target/riscv/cpu.h | 1 +
target/riscv/cpu_helper.c | 12 ++++++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a804a5d0ba..8b3b368d6a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -85,6 +85,7 @@ enum {
#define TRANSLATE_FAIL 1
#define TRANSLATE_SUCCESS 0
#define MMU_USER_IDX 3
+#define TRANSLATE_G_STAGE_FAIL 4
#define MAX_RISCV_PMPS (16)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index fd1d373b6f..1635b09c41 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -440,7 +440,10 @@ restart:
mmu_idx, false, true);
if (vbase_ret != TRANSLATE_SUCCESS) {
- return vbase_ret;
+ env->guest_phys_fault_addr = (base |
+ (addr &
+ (TARGET_PAGE_SIZE - 1))) >> 2;
+ return TRANSLATE_G_STAGE_FAIL;
}
pte_addr = vbase + idx * ptesize;
@@ -728,12 +731,17 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
ret = get_physical_address(env, &pa, &prot, address, access_type,
mmu_idx, true, true);
+ if (ret == TRANSLATE_G_STAGE_FAIL) {
+ first_stage_error = false;
+ access_type = MMU_DATA_LOAD;
+ }
+
qemu_log_mask(CPU_LOG_MMU,
"%s 1st-stage address=%" VADDR_PRIx " ret %d physical "
TARGET_FMT_plx " prot %d\n",
__func__, address, ret, pa, prot);
- if (ret != TRANSLATE_FAIL) {
+ if (ret != TRANSLATE_FAIL && ret != TRANSLATE_G_STAGE_FAIL) {
/* Second stage lookup */
im_address = pa;
--
2.19.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] target/riscv: raise exception to HS-mode at get_physical_address
2020-08-24 8:41 ` Yifei Jiang
@ 2020-09-25 22:24 ` Alistair Francis
-1 siblings, 0 replies; 10+ messages in thread
From: Alistair Francis @ 2020-09-25 22:24 UTC (permalink / raw)
To: Yifei Jiang
Cc: open list:RISC-V, zhang.zhanghailiang, Sagar Karandikar,
Bastian Koppelmann, victor.zhangxiaofeng,
qemu-devel@nongnu.org Developers, Alistair Francis, yinyipeng1,
Palmer Dabbelt, wu.wubin, dengkai (A)
On Mon, Aug 24, 2020 at 1:43 AM Yifei Jiang <jiangyifei@huawei.com> wrote:
>
> VS-stage translation at get_physical_address needs to translate pte
> address by G-stage translation. But the G-stage translation error
> can not be distinguished from VS-stage translation error in
> riscv_cpu_tlb_fill. On migration, destination needs to rebuild pte,
> and this G-stage translation error must be handled by HS-mode. So
> introduce TRANSLATE_STAGE2_FAIL so that riscv_cpu_tlb_fill could
> distinguish and raise it to HS-mode.
>
> Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
Thanks for the patch!
Sorry for the delay here.
> ---
> target/riscv/cpu.h | 1 +
> target/riscv/cpu_helper.c | 12 ++++++++++--
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index a804a5d0ba..8b3b368d6a 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -85,6 +85,7 @@ enum {
> #define TRANSLATE_FAIL 1
> #define TRANSLATE_SUCCESS 0
> #define MMU_USER_IDX 3
> +#define TRANSLATE_G_STAGE_FAIL 4
>
> #define MAX_RISCV_PMPS (16)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index fd1d373b6f..1635b09c41 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -440,7 +440,10 @@ restart:
> mmu_idx, false, true);
>
> if (vbase_ret != TRANSLATE_SUCCESS) {
> - return vbase_ret;
> + env->guest_phys_fault_addr = (base |
> + (addr &
> + (TARGET_PAGE_SIZE - 1))) >> 2;
Can we set guest_phys_fault_addr in riscv_cpu_tlb_fill() instead?
> + return TRANSLATE_G_STAGE_FAIL;
> }
>
> pte_addr = vbase + idx * ptesize;
> @@ -728,12 +731,17 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> ret = get_physical_address(env, &pa, &prot, address, access_type,
> mmu_idx, true, true);
>
> + if (ret == TRANSLATE_G_STAGE_FAIL) {
> + first_stage_error = false;
> + access_type = MMU_DATA_LOAD;
> + }
> +
> qemu_log_mask(CPU_LOG_MMU,
> "%s 1st-stage address=%" VADDR_PRIx " ret %d physical "
> TARGET_FMT_plx " prot %d\n",
> __func__, address, ret, pa, prot);
>
> - if (ret != TRANSLATE_FAIL) {
> + if (ret != TRANSLATE_FAIL && ret != TRANSLATE_G_STAGE_FAIL) {
Otherwise this patch looks correct.
Alistair
> /* Second stage lookup */
> im_address = pa;
>
> --
> 2.19.1
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] target/riscv: raise exception to HS-mode at get_physical_address
@ 2020-09-25 22:24 ` Alistair Francis
0 siblings, 0 replies; 10+ messages in thread
From: Alistair Francis @ 2020-09-25 22:24 UTC (permalink / raw)
To: Yifei Jiang
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V,
zhang.zhanghailiang, Sagar Karandikar, Bastian Koppelmann,
victor.zhangxiaofeng, Alistair Francis, yinyipeng1,
Palmer Dabbelt, wu.wubin, dengkai (A)
On Mon, Aug 24, 2020 at 1:43 AM Yifei Jiang <jiangyifei@huawei.com> wrote:
>
> VS-stage translation at get_physical_address needs to translate pte
> address by G-stage translation. But the G-stage translation error
> can not be distinguished from VS-stage translation error in
> riscv_cpu_tlb_fill. On migration, destination needs to rebuild pte,
> and this G-stage translation error must be handled by HS-mode. So
> introduce TRANSLATE_STAGE2_FAIL so that riscv_cpu_tlb_fill could
> distinguish and raise it to HS-mode.
>
> Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
Thanks for the patch!
Sorry for the delay here.
> ---
> target/riscv/cpu.h | 1 +
> target/riscv/cpu_helper.c | 12 ++++++++++--
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index a804a5d0ba..8b3b368d6a 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -85,6 +85,7 @@ enum {
> #define TRANSLATE_FAIL 1
> #define TRANSLATE_SUCCESS 0
> #define MMU_USER_IDX 3
> +#define TRANSLATE_G_STAGE_FAIL 4
>
> #define MAX_RISCV_PMPS (16)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index fd1d373b6f..1635b09c41 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -440,7 +440,10 @@ restart:
> mmu_idx, false, true);
>
> if (vbase_ret != TRANSLATE_SUCCESS) {
> - return vbase_ret;
> + env->guest_phys_fault_addr = (base |
> + (addr &
> + (TARGET_PAGE_SIZE - 1))) >> 2;
Can we set guest_phys_fault_addr in riscv_cpu_tlb_fill() instead?
> + return TRANSLATE_G_STAGE_FAIL;
> }
>
> pte_addr = vbase + idx * ptesize;
> @@ -728,12 +731,17 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> ret = get_physical_address(env, &pa, &prot, address, access_type,
> mmu_idx, true, true);
>
> + if (ret == TRANSLATE_G_STAGE_FAIL) {
> + first_stage_error = false;
> + access_type = MMU_DATA_LOAD;
> + }
> +
> qemu_log_mask(CPU_LOG_MMU,
> "%s 1st-stage address=%" VADDR_PRIx " ret %d physical "
> TARGET_FMT_plx " prot %d\n",
> __func__, address, ret, pa, prot);
>
> - if (ret != TRANSLATE_FAIL) {
> + if (ret != TRANSLATE_FAIL && ret != TRANSLATE_G_STAGE_FAIL) {
Otherwise this patch looks correct.
Alistair
> /* Second stage lookup */
> im_address = pa;
>
> --
> 2.19.1
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] target/riscv: raise exception to HS-mode at get_physical_address
2020-09-25 22:24 ` Alistair Francis
@ 2020-09-27 7:54 ` Jiangyifei
-1 siblings, 0 replies; 10+ messages in thread
From: Jiangyifei @ 2020-09-27 7:54 UTC (permalink / raw)
To: Alistair Francis
Cc: open list:RISC-V, Zhanghailiang, Sagar Karandikar,
Bastian Koppelmann, Zhangxiaofeng (F),
qemu-devel@nongnu.org Developers, Alistair Francis, yinyipeng,
Palmer Dabbelt, Wubin (H), dengkai (A)
> -----Original Message-----
> From: Alistair Francis [mailto:alistair23@gmail.com]
> Sent: Saturday, September 26, 2020 6:24 AM
> To: Jiangyifei <jiangyifei@huawei.com>
> Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> list:RISC-V <qemu-riscv@nongnu.org>; Zhanghailiang
> <zhang.zhanghailiang@huawei.com>; Sagar Karandikar
> <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> <kbastian@mail.uni-paderborn.de>; Zhangxiaofeng (F)
> <victor.zhangxiaofeng@huawei.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; yinyipeng <yinyipeng1@huawei.com>; Palmer
> Dabbelt <palmer@dabbelt.com>; Wubin (H) <wu.wubin@huawei.com>;
> dengkai (A) <dengkai1@huawei.com>
> Subject: Re: [PATCH] target/riscv: raise exception to HS-mode at
> get_physical_address
>
> On Mon, Aug 24, 2020 at 1:43 AM Yifei Jiang <jiangyifei@huawei.com> wrote:
> >
> > VS-stage translation at get_physical_address needs to translate pte
> > address by G-stage translation. But the G-stage translation error can
> > not be distinguished from VS-stage translation error in
> > riscv_cpu_tlb_fill. On migration, destination needs to rebuild pte,
> > and this G-stage translation error must be handled by HS-mode. So
> > introduce TRANSLATE_STAGE2_FAIL so that riscv_cpu_tlb_fill could
> > distinguish and raise it to HS-mode.
> >
> > Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> > Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
>
> Thanks for the patch!
>
> Sorry for the delay here.
>
> > ---
> > target/riscv/cpu.h | 1 +
> > target/riscv/cpu_helper.c | 12 ++++++++++--
> > 2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index
> > a804a5d0ba..8b3b368d6a 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -85,6 +85,7 @@ enum {
> > #define TRANSLATE_FAIL 1
> > #define TRANSLATE_SUCCESS 0
> > #define MMU_USER_IDX 3
> > +#define TRANSLATE_G_STAGE_FAIL 4
> >
> > #define MAX_RISCV_PMPS (16)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index fd1d373b6f..1635b09c41 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -440,7 +440,10 @@ restart:
> > mmu_idx,
> false,
> > true);
> >
> > if (vbase_ret != TRANSLATE_SUCCESS) {
> > - return vbase_ret;
> > + env->guest_phys_fault_addr = (base |
> > + (addr &
> > +
> (TARGET_PAGE_SIZE -
> > + 1))) >> 2;
>
> Can we set guest_phys_fault_addr in riscv_cpu_tlb_fill() instead?
Hi Alistair,
It's not easy to do that. The key is that the wrong address(the `base` variable) is not visible to riscv_cpu_tlb_fill().
Because the wrong address may be from any level of PTE which can't be easily obtained by riscv_cpu_tlb_fill().
The alternative is to add an out parameter in get_physical_address(). But it is not either elegant.
What is your advice?
Best Regards,
Yifei
>
> > + return TRANSLATE_G_STAGE_FAIL;
> > }
> >
> > pte_addr = vbase + idx * ptesize; @@ -728,12 +731,17 @@
> > bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > ret = get_physical_address(env, &pa, &prot, address,
> access_type,
> > mmu_idx, true, true);
> >
> > + if (ret == TRANSLATE_G_STAGE_FAIL) {
> > + first_stage_error = false;
> > + access_type = MMU_DATA_LOAD;
> > + }
> > +
> > qemu_log_mask(CPU_LOG_MMU,
> > "%s 1st-stage address=%" VADDR_PRIx "
> ret %d physical "
> > TARGET_FMT_plx " prot %d\n",
> > __func__, address, ret, pa, prot);
> >
> > - if (ret != TRANSLATE_FAIL) {
> > + if (ret != TRANSLATE_FAIL && ret != TRANSLATE_G_STAGE_FAIL) {
>
> Otherwise this patch looks correct.
>
> Alistair
>
> > /* Second stage lookup */
> > im_address = pa;
> >
> > --
> > 2.19.1
> >
> >
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] target/riscv: raise exception to HS-mode at get_physical_address
@ 2020-09-27 7:54 ` Jiangyifei
0 siblings, 0 replies; 10+ messages in thread
From: Jiangyifei @ 2020-09-27 7:54 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V,
Zhanghailiang, Sagar Karandikar, Bastian Koppelmann,
Zhangxiaofeng (F),
Alistair Francis, yinyipeng, Palmer Dabbelt, Wubin (H),
dengkai (A)
> -----Original Message-----
> From: Alistair Francis [mailto:alistair23@gmail.com]
> Sent: Saturday, September 26, 2020 6:24 AM
> To: Jiangyifei <jiangyifei@huawei.com>
> Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> list:RISC-V <qemu-riscv@nongnu.org>; Zhanghailiang
> <zhang.zhanghailiang@huawei.com>; Sagar Karandikar
> <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> <kbastian@mail.uni-paderborn.de>; Zhangxiaofeng (F)
> <victor.zhangxiaofeng@huawei.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; yinyipeng <yinyipeng1@huawei.com>; Palmer
> Dabbelt <palmer@dabbelt.com>; Wubin (H) <wu.wubin@huawei.com>;
> dengkai (A) <dengkai1@huawei.com>
> Subject: Re: [PATCH] target/riscv: raise exception to HS-mode at
> get_physical_address
>
> On Mon, Aug 24, 2020 at 1:43 AM Yifei Jiang <jiangyifei@huawei.com> wrote:
> >
> > VS-stage translation at get_physical_address needs to translate pte
> > address by G-stage translation. But the G-stage translation error can
> > not be distinguished from VS-stage translation error in
> > riscv_cpu_tlb_fill. On migration, destination needs to rebuild pte,
> > and this G-stage translation error must be handled by HS-mode. So
> > introduce TRANSLATE_STAGE2_FAIL so that riscv_cpu_tlb_fill could
> > distinguish and raise it to HS-mode.
> >
> > Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> > Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
>
> Thanks for the patch!
>
> Sorry for the delay here.
>
> > ---
> > target/riscv/cpu.h | 1 +
> > target/riscv/cpu_helper.c | 12 ++++++++++--
> > 2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index
> > a804a5d0ba..8b3b368d6a 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -85,6 +85,7 @@ enum {
> > #define TRANSLATE_FAIL 1
> > #define TRANSLATE_SUCCESS 0
> > #define MMU_USER_IDX 3
> > +#define TRANSLATE_G_STAGE_FAIL 4
> >
> > #define MAX_RISCV_PMPS (16)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index fd1d373b6f..1635b09c41 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -440,7 +440,10 @@ restart:
> > mmu_idx,
> false,
> > true);
> >
> > if (vbase_ret != TRANSLATE_SUCCESS) {
> > - return vbase_ret;
> > + env->guest_phys_fault_addr = (base |
> > + (addr &
> > +
> (TARGET_PAGE_SIZE -
> > + 1))) >> 2;
>
> Can we set guest_phys_fault_addr in riscv_cpu_tlb_fill() instead?
Hi Alistair,
It's not easy to do that. The key is that the wrong address(the `base` variable) is not visible to riscv_cpu_tlb_fill().
Because the wrong address may be from any level of PTE which can't be easily obtained by riscv_cpu_tlb_fill().
The alternative is to add an out parameter in get_physical_address(). But it is not either elegant.
What is your advice?
Best Regards,
Yifei
>
> > + return TRANSLATE_G_STAGE_FAIL;
> > }
> >
> > pte_addr = vbase + idx * ptesize; @@ -728,12 +731,17 @@
> > bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > ret = get_physical_address(env, &pa, &prot, address,
> access_type,
> > mmu_idx, true, true);
> >
> > + if (ret == TRANSLATE_G_STAGE_FAIL) {
> > + first_stage_error = false;
> > + access_type = MMU_DATA_LOAD;
> > + }
> > +
> > qemu_log_mask(CPU_LOG_MMU,
> > "%s 1st-stage address=%" VADDR_PRIx "
> ret %d physical "
> > TARGET_FMT_plx " prot %d\n",
> > __func__, address, ret, pa, prot);
> >
> > - if (ret != TRANSLATE_FAIL) {
> > + if (ret != TRANSLATE_FAIL && ret != TRANSLATE_G_STAGE_FAIL) {
>
> Otherwise this patch looks correct.
>
> Alistair
>
> > /* Second stage lookup */
> > im_address = pa;
> >
> > --
> > 2.19.1
> >
> >
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] target/riscv: raise exception to HS-mode at get_physical_address
2020-09-27 7:54 ` Jiangyifei
@ 2020-10-01 0:00 ` Alistair Francis
-1 siblings, 0 replies; 10+ messages in thread
From: Alistair Francis @ 2020-10-01 0:00 UTC (permalink / raw)
To: Jiangyifei
Cc: open list:RISC-V, Zhanghailiang, Sagar Karandikar,
Bastian Koppelmann, Zhangxiaofeng (F),
qemu-devel@nongnu.org Developers, Alistair Francis, yinyipeng,
Palmer Dabbelt, Wubin (H), dengkai (A)
On Sun, Sep 27, 2020 at 12:54 AM Jiangyifei <jiangyifei@huawei.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Alistair Francis [mailto:alistair23@gmail.com]
> > Sent: Saturday, September 26, 2020 6:24 AM
> > To: Jiangyifei <jiangyifei@huawei.com>
> > Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> > list:RISC-V <qemu-riscv@nongnu.org>; Zhanghailiang
> > <zhang.zhanghailiang@huawei.com>; Sagar Karandikar
> > <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> > <kbastian@mail.uni-paderborn.de>; Zhangxiaofeng (F)
> > <victor.zhangxiaofeng@huawei.com>; Alistair Francis
> > <Alistair.Francis@wdc.com>; yinyipeng <yinyipeng1@huawei.com>; Palmer
> > Dabbelt <palmer@dabbelt.com>; Wubin (H) <wu.wubin@huawei.com>;
> > dengkai (A) <dengkai1@huawei.com>
> > Subject: Re: [PATCH] target/riscv: raise exception to HS-mode at
> > get_physical_address
> >
> > On Mon, Aug 24, 2020 at 1:43 AM Yifei Jiang <jiangyifei@huawei.com> wrote:
> > >
> > > VS-stage translation at get_physical_address needs to translate pte
> > > address by G-stage translation. But the G-stage translation error can
> > > not be distinguished from VS-stage translation error in
> > > riscv_cpu_tlb_fill. On migration, destination needs to rebuild pte,
> > > and this G-stage translation error must be handled by HS-mode. So
> > > introduce TRANSLATE_STAGE2_FAIL so that riscv_cpu_tlb_fill could
> > > distinguish and raise it to HS-mode.
> > >
> > > Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> > > Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
> >
> > Thanks for the patch!
> >
> > Sorry for the delay here.
> >
> > > ---
> > > target/riscv/cpu.h | 1 +
> > > target/riscv/cpu_helper.c | 12 ++++++++++--
> > > 2 files changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index
> > > a804a5d0ba..8b3b368d6a 100644
> > > --- a/target/riscv/cpu.h
> > > +++ b/target/riscv/cpu.h
> > > @@ -85,6 +85,7 @@ enum {
> > > #define TRANSLATE_FAIL 1
> > > #define TRANSLATE_SUCCESS 0
> > > #define MMU_USER_IDX 3
> > > +#define TRANSLATE_G_STAGE_FAIL 4
> > >
> > > #define MAX_RISCV_PMPS (16)
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index fd1d373b6f..1635b09c41 100644
> > > --- a/target/riscv/cpu_helper.c
> > > +++ b/target/riscv/cpu_helper.c
> > > @@ -440,7 +440,10 @@ restart:
> > > mmu_idx,
> > false,
> > > true);
> > >
> > > if (vbase_ret != TRANSLATE_SUCCESS) {
> > > - return vbase_ret;
> > > + env->guest_phys_fault_addr = (base |
> > > + (addr &
> > > +
> > (TARGET_PAGE_SIZE -
> > > + 1))) >> 2;
> >
> > Can we set guest_phys_fault_addr in riscv_cpu_tlb_fill() instead?
>
> Hi Alistair,
>
> It's not easy to do that. The key is that the wrong address(the `base` variable) is not visible to riscv_cpu_tlb_fill().
> Because the wrong address may be from any level of PTE which can't be easily obtained by riscv_cpu_tlb_fill().
> The alternative is to add an out parameter in get_physical_address(). But it is not either elegant.
> What is your advice?
You are right. Good call.
In which case this looks like the right way to do this. Can you add a
small comment in riscv_cpu_tlb_fill() to indicate that
guest_phys_fault_addr has already been set when handling the error?
The TLB filling code is pretty complex so the more documentation the
better.
Alistair
>
> Best Regards,
> Yifei
>
> >
> > > + return TRANSLATE_G_STAGE_FAIL;
> > > }
> > >
> > > pte_addr = vbase + idx * ptesize; @@ -728,12 +731,17 @@
> > > bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > > ret = get_physical_address(env, &pa, &prot, address,
> > access_type,
> > > mmu_idx, true, true);
> > >
> > > + if (ret == TRANSLATE_G_STAGE_FAIL) {
> > > + first_stage_error = false;
> > > + access_type = MMU_DATA_LOAD;
> > > + }
> > > +
> > > qemu_log_mask(CPU_LOG_MMU,
> > > "%s 1st-stage address=%" VADDR_PRIx "
> > ret %d physical "
> > > TARGET_FMT_plx " prot %d\n",
> > > __func__, address, ret, pa, prot);
> > >
> > > - if (ret != TRANSLATE_FAIL) {
> > > + if (ret != TRANSLATE_FAIL && ret != TRANSLATE_G_STAGE_FAIL) {
> >
> > Otherwise this patch looks correct.
> >
> > Alistair
> >
> > > /* Second stage lookup */
> > > im_address = pa;
> > >
> > > --
> > > 2.19.1
> > >
> > >
> > >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] target/riscv: raise exception to HS-mode at get_physical_address
@ 2020-10-01 0:00 ` Alistair Francis
0 siblings, 0 replies; 10+ messages in thread
From: Alistair Francis @ 2020-10-01 0:00 UTC (permalink / raw)
To: Jiangyifei
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V,
Zhanghailiang, Sagar Karandikar, Bastian Koppelmann,
Zhangxiaofeng (F),
Alistair Francis, yinyipeng, Palmer Dabbelt, Wubin (H),
dengkai (A)
On Sun, Sep 27, 2020 at 12:54 AM Jiangyifei <jiangyifei@huawei.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Alistair Francis [mailto:alistair23@gmail.com]
> > Sent: Saturday, September 26, 2020 6:24 AM
> > To: Jiangyifei <jiangyifei@huawei.com>
> > Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> > list:RISC-V <qemu-riscv@nongnu.org>; Zhanghailiang
> > <zhang.zhanghailiang@huawei.com>; Sagar Karandikar
> > <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> > <kbastian@mail.uni-paderborn.de>; Zhangxiaofeng (F)
> > <victor.zhangxiaofeng@huawei.com>; Alistair Francis
> > <Alistair.Francis@wdc.com>; yinyipeng <yinyipeng1@huawei.com>; Palmer
> > Dabbelt <palmer@dabbelt.com>; Wubin (H) <wu.wubin@huawei.com>;
> > dengkai (A) <dengkai1@huawei.com>
> > Subject: Re: [PATCH] target/riscv: raise exception to HS-mode at
> > get_physical_address
> >
> > On Mon, Aug 24, 2020 at 1:43 AM Yifei Jiang <jiangyifei@huawei.com> wrote:
> > >
> > > VS-stage translation at get_physical_address needs to translate pte
> > > address by G-stage translation. But the G-stage translation error can
> > > not be distinguished from VS-stage translation error in
> > > riscv_cpu_tlb_fill. On migration, destination needs to rebuild pte,
> > > and this G-stage translation error must be handled by HS-mode. So
> > > introduce TRANSLATE_STAGE2_FAIL so that riscv_cpu_tlb_fill could
> > > distinguish and raise it to HS-mode.
> > >
> > > Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> > > Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
> >
> > Thanks for the patch!
> >
> > Sorry for the delay here.
> >
> > > ---
> > > target/riscv/cpu.h | 1 +
> > > target/riscv/cpu_helper.c | 12 ++++++++++--
> > > 2 files changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index
> > > a804a5d0ba..8b3b368d6a 100644
> > > --- a/target/riscv/cpu.h
> > > +++ b/target/riscv/cpu.h
> > > @@ -85,6 +85,7 @@ enum {
> > > #define TRANSLATE_FAIL 1
> > > #define TRANSLATE_SUCCESS 0
> > > #define MMU_USER_IDX 3
> > > +#define TRANSLATE_G_STAGE_FAIL 4
> > >
> > > #define MAX_RISCV_PMPS (16)
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index fd1d373b6f..1635b09c41 100644
> > > --- a/target/riscv/cpu_helper.c
> > > +++ b/target/riscv/cpu_helper.c
> > > @@ -440,7 +440,10 @@ restart:
> > > mmu_idx,
> > false,
> > > true);
> > >
> > > if (vbase_ret != TRANSLATE_SUCCESS) {
> > > - return vbase_ret;
> > > + env->guest_phys_fault_addr = (base |
> > > + (addr &
> > > +
> > (TARGET_PAGE_SIZE -
> > > + 1))) >> 2;
> >
> > Can we set guest_phys_fault_addr in riscv_cpu_tlb_fill() instead?
>
> Hi Alistair,
>
> It's not easy to do that. The key is that the wrong address(the `base` variable) is not visible to riscv_cpu_tlb_fill().
> Because the wrong address may be from any level of PTE which can't be easily obtained by riscv_cpu_tlb_fill().
> The alternative is to add an out parameter in get_physical_address(). But it is not either elegant.
> What is your advice?
You are right. Good call.
In which case this looks like the right way to do this. Can you add a
small comment in riscv_cpu_tlb_fill() to indicate that
guest_phys_fault_addr has already been set when handling the error?
The TLB filling code is pretty complex so the more documentation the
better.
Alistair
>
> Best Regards,
> Yifei
>
> >
> > > + return TRANSLATE_G_STAGE_FAIL;
> > > }
> > >
> > > pte_addr = vbase + idx * ptesize; @@ -728,12 +731,17 @@
> > > bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > > ret = get_physical_address(env, &pa, &prot, address,
> > access_type,
> > > mmu_idx, true, true);
> > >
> > > + if (ret == TRANSLATE_G_STAGE_FAIL) {
> > > + first_stage_error = false;
> > > + access_type = MMU_DATA_LOAD;
> > > + }
> > > +
> > > qemu_log_mask(CPU_LOG_MMU,
> > > "%s 1st-stage address=%" VADDR_PRIx "
> > ret %d physical "
> > > TARGET_FMT_plx " prot %d\n",
> > > __func__, address, ret, pa, prot);
> > >
> > > - if (ret != TRANSLATE_FAIL) {
> > > + if (ret != TRANSLATE_FAIL && ret != TRANSLATE_G_STAGE_FAIL) {
> >
> > Otherwise this patch looks correct.
> >
> > Alistair
> >
> > > /* Second stage lookup */
> > > im_address = pa;
> > >
> > > --
> > > 2.19.1
> > >
> > >
> > >
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] target/riscv: raise exception to HS-mode at get_physical_address
2020-10-01 0:00 ` Alistair Francis
@ 2020-10-09 8:03 ` Jiangyifei
-1 siblings, 0 replies; 10+ messages in thread
From: Jiangyifei @ 2020-10-09 8:03 UTC (permalink / raw)
To: Alistair Francis
Cc: open list:RISC-V, Zhanghailiang, Sagar Karandikar,
Bastian Koppelmann, Zhangxiaofeng (F),
qemu-devel@nongnu.org Developers, Alistair Francis, yinyipeng,
Palmer Dabbelt, Wubin (H), dengkai (A)
> -----Original Message-----
> From: Alistair Francis [mailto:alistair23@gmail.com]
> Sent: Thursday, October 1, 2020 8:00 AM
> To: Jiangyifei <jiangyifei@huawei.com>
> Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> list:RISC-V <qemu-riscv@nongnu.org>; Zhanghailiang
> <zhang.zhanghailiang@huawei.com>; Sagar Karandikar
> <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> <kbastian@mail.uni-paderborn.de>; Zhangxiaofeng (F)
> <victor.zhangxiaofeng@huawei.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; yinyipeng <yinyipeng1@huawei.com>; Palmer
> Dabbelt <palmer@dabbelt.com>; Wubin (H) <wu.wubin@huawei.com>;
> dengkai (A) <dengkai1@huawei.com>
> Subject: Re: [PATCH] target/riscv: raise exception to HS-mode at
> get_physical_address
>
> On Sun, Sep 27, 2020 at 12:54 AM Jiangyifei <jiangyifei@huawei.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Alistair Francis [mailto:alistair23@gmail.com]
> > > Sent: Saturday, September 26, 2020 6:24 AM
> > > To: Jiangyifei <jiangyifei@huawei.com>
> > > Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> > > list:RISC-V <qemu-riscv@nongnu.org>; Zhanghailiang
> > > <zhang.zhanghailiang@huawei.com>; Sagar Karandikar
> > > <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> > > <kbastian@mail.uni-paderborn.de>; Zhangxiaofeng (F)
> > > <victor.zhangxiaofeng@huawei.com>; Alistair Francis
> > > <Alistair.Francis@wdc.com>; yinyipeng <yinyipeng1@huawei.com>;
> > > Palmer Dabbelt <palmer@dabbelt.com>; Wubin (H)
> > > <wu.wubin@huawei.com>; dengkai (A) <dengkai1@huawei.com>
> > > Subject: Re: [PATCH] target/riscv: raise exception to HS-mode at
> > > get_physical_address
> > >
> > > On Mon, Aug 24, 2020 at 1:43 AM Yifei Jiang <jiangyifei@huawei.com>
> wrote:
> > > >
> > > > VS-stage translation at get_physical_address needs to translate
> > > > pte address by G-stage translation. But the G-stage translation
> > > > error can not be distinguished from VS-stage translation error in
> > > > riscv_cpu_tlb_fill. On migration, destination needs to rebuild
> > > > pte, and this G-stage translation error must be handled by
> > > > HS-mode. So introduce TRANSLATE_STAGE2_FAIL so that
> > > > riscv_cpu_tlb_fill could distinguish and raise it to HS-mode.
> > > >
> > > > Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> > > > Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
> > >
> > > Thanks for the patch!
> > >
> > > Sorry for the delay here.
> > >
> > > > ---
> > > > target/riscv/cpu.h | 1 +
> > > > target/riscv/cpu_helper.c | 12 ++++++++++--
> > > > 2 files changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index
> > > > a804a5d0ba..8b3b368d6a 100644
> > > > --- a/target/riscv/cpu.h
> > > > +++ b/target/riscv/cpu.h
> > > > @@ -85,6 +85,7 @@ enum {
> > > > #define TRANSLATE_FAIL 1
> > > > #define TRANSLATE_SUCCESS 0
> > > > #define MMU_USER_IDX 3
> > > > +#define TRANSLATE_G_STAGE_FAIL 4
> > > >
> > > > #define MAX_RISCV_PMPS (16)
> > > >
> > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > index fd1d373b6f..1635b09c41 100644
> > > > --- a/target/riscv/cpu_helper.c
> > > > +++ b/target/riscv/cpu_helper.c
> > > > @@ -440,7 +440,10 @@ restart:
> > > > mmu_idx,
> > > false,
> > > > true);
> > > >
> > > > if (vbase_ret != TRANSLATE_SUCCESS) {
> > > > - return vbase_ret;
> > > > + env->guest_phys_fault_addr = (base |
> > > > + (addr &
> > > > +
> > > (TARGET_PAGE_SIZE -
> > > > + 1))) >> 2;
> > >
> > > Can we set guest_phys_fault_addr in riscv_cpu_tlb_fill() instead?
> >
> > Hi Alistair,
> >
> > It's not easy to do that. The key is that the wrong address(the `base` variable)
> is not visible to riscv_cpu_tlb_fill().
> > Because the wrong address may be from any level of PTE which can't be
> easily obtained by riscv_cpu_tlb_fill().
> > The alternative is to add an out parameter in get_physical_address(). But it is
> not either elegant.
> > What is your advice?
>
> You are right. Good call.
>
> In which case this looks like the right way to do this. Can you add a small
> comment in riscv_cpu_tlb_fill() to indicate that guest_phys_fault_addr has
> already been set when handling the error?
> The TLB filling code is pretty complex so the more documentation the better.
>
> Alistair
>
I have already sent a new patch to add necessary comment. Please review the new patch.
Thanks,
Yifei
> >
> > Best Regards,
> > Yifei
> >
> > >
> > > > + return TRANSLATE_G_STAGE_FAIL;
> > > > }
> > > >
> > > > pte_addr = vbase + idx * ptesize; @@ -728,12 +731,17
> > > > @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > > > ret = get_physical_address(env, &pa, &prot, address,
> > > access_type,
> > > > mmu_idx, true, true);
> > > >
> > > > + if (ret == TRANSLATE_G_STAGE_FAIL) {
> > > > + first_stage_error = false;
> > > > + access_type = MMU_DATA_LOAD;
> > > > + }
> > > > +
> > > > qemu_log_mask(CPU_LOG_MMU,
> > > > "%s 1st-stage address=%" VADDR_PRIx "
> > > ret %d physical "
> > > > TARGET_FMT_plx " prot %d\n",
> > > > __func__, address, ret, pa, prot);
> > > >
> > > > - if (ret != TRANSLATE_FAIL) {
> > > > + if (ret != TRANSLATE_FAIL && ret !=
> > > > + TRANSLATE_G_STAGE_FAIL) {
> > >
> > > Otherwise this patch looks correct.
> > >
> > > Alistair
> > >
> > > > /* Second stage lookup */
> > > > im_address = pa;
> > > >
> > > > --
> > > > 2.19.1
> > > >
> > > >
> > > >
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] target/riscv: raise exception to HS-mode at get_physical_address
@ 2020-10-09 8:03 ` Jiangyifei
0 siblings, 0 replies; 10+ messages in thread
From: Jiangyifei @ 2020-10-09 8:03 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V,
Zhanghailiang, Sagar Karandikar, Bastian Koppelmann,
Zhangxiaofeng (F),
Alistair Francis, yinyipeng, Palmer Dabbelt, Wubin (H),
dengkai (A)
> -----Original Message-----
> From: Alistair Francis [mailto:alistair23@gmail.com]
> Sent: Thursday, October 1, 2020 8:00 AM
> To: Jiangyifei <jiangyifei@huawei.com>
> Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> list:RISC-V <qemu-riscv@nongnu.org>; Zhanghailiang
> <zhang.zhanghailiang@huawei.com>; Sagar Karandikar
> <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> <kbastian@mail.uni-paderborn.de>; Zhangxiaofeng (F)
> <victor.zhangxiaofeng@huawei.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; yinyipeng <yinyipeng1@huawei.com>; Palmer
> Dabbelt <palmer@dabbelt.com>; Wubin (H) <wu.wubin@huawei.com>;
> dengkai (A) <dengkai1@huawei.com>
> Subject: Re: [PATCH] target/riscv: raise exception to HS-mode at
> get_physical_address
>
> On Sun, Sep 27, 2020 at 12:54 AM Jiangyifei <jiangyifei@huawei.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Alistair Francis [mailto:alistair23@gmail.com]
> > > Sent: Saturday, September 26, 2020 6:24 AM
> > > To: Jiangyifei <jiangyifei@huawei.com>
> > > Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> > > list:RISC-V <qemu-riscv@nongnu.org>; Zhanghailiang
> > > <zhang.zhanghailiang@huawei.com>; Sagar Karandikar
> > > <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> > > <kbastian@mail.uni-paderborn.de>; Zhangxiaofeng (F)
> > > <victor.zhangxiaofeng@huawei.com>; Alistair Francis
> > > <Alistair.Francis@wdc.com>; yinyipeng <yinyipeng1@huawei.com>;
> > > Palmer Dabbelt <palmer@dabbelt.com>; Wubin (H)
> > > <wu.wubin@huawei.com>; dengkai (A) <dengkai1@huawei.com>
> > > Subject: Re: [PATCH] target/riscv: raise exception to HS-mode at
> > > get_physical_address
> > >
> > > On Mon, Aug 24, 2020 at 1:43 AM Yifei Jiang <jiangyifei@huawei.com>
> wrote:
> > > >
> > > > VS-stage translation at get_physical_address needs to translate
> > > > pte address by G-stage translation. But the G-stage translation
> > > > error can not be distinguished from VS-stage translation error in
> > > > riscv_cpu_tlb_fill. On migration, destination needs to rebuild
> > > > pte, and this G-stage translation error must be handled by
> > > > HS-mode. So introduce TRANSLATE_STAGE2_FAIL so that
> > > > riscv_cpu_tlb_fill could distinguish and raise it to HS-mode.
> > > >
> > > > Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> > > > Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
> > >
> > > Thanks for the patch!
> > >
> > > Sorry for the delay here.
> > >
> > > > ---
> > > > target/riscv/cpu.h | 1 +
> > > > target/riscv/cpu_helper.c | 12 ++++++++++--
> > > > 2 files changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index
> > > > a804a5d0ba..8b3b368d6a 100644
> > > > --- a/target/riscv/cpu.h
> > > > +++ b/target/riscv/cpu.h
> > > > @@ -85,6 +85,7 @@ enum {
> > > > #define TRANSLATE_FAIL 1
> > > > #define TRANSLATE_SUCCESS 0
> > > > #define MMU_USER_IDX 3
> > > > +#define TRANSLATE_G_STAGE_FAIL 4
> > > >
> > > > #define MAX_RISCV_PMPS (16)
> > > >
> > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > index fd1d373b6f..1635b09c41 100644
> > > > --- a/target/riscv/cpu_helper.c
> > > > +++ b/target/riscv/cpu_helper.c
> > > > @@ -440,7 +440,10 @@ restart:
> > > > mmu_idx,
> > > false,
> > > > true);
> > > >
> > > > if (vbase_ret != TRANSLATE_SUCCESS) {
> > > > - return vbase_ret;
> > > > + env->guest_phys_fault_addr = (base |
> > > > + (addr &
> > > > +
> > > (TARGET_PAGE_SIZE -
> > > > + 1))) >> 2;
> > >
> > > Can we set guest_phys_fault_addr in riscv_cpu_tlb_fill() instead?
> >
> > Hi Alistair,
> >
> > It's not easy to do that. The key is that the wrong address(the `base` variable)
> is not visible to riscv_cpu_tlb_fill().
> > Because the wrong address may be from any level of PTE which can't be
> easily obtained by riscv_cpu_tlb_fill().
> > The alternative is to add an out parameter in get_physical_address(). But it is
> not either elegant.
> > What is your advice?
>
> You are right. Good call.
>
> In which case this looks like the right way to do this. Can you add a small
> comment in riscv_cpu_tlb_fill() to indicate that guest_phys_fault_addr has
> already been set when handling the error?
> The TLB filling code is pretty complex so the more documentation the better.
>
> Alistair
>
I have already sent a new patch to add necessary comment. Please review the new patch.
Thanks,
Yifei
> >
> > Best Regards,
> > Yifei
> >
> > >
> > > > + return TRANSLATE_G_STAGE_FAIL;
> > > > }
> > > >
> > > > pte_addr = vbase + idx * ptesize; @@ -728,12 +731,17
> > > > @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > > > ret = get_physical_address(env, &pa, &prot, address,
> > > access_type,
> > > > mmu_idx, true, true);
> > > >
> > > > + if (ret == TRANSLATE_G_STAGE_FAIL) {
> > > > + first_stage_error = false;
> > > > + access_type = MMU_DATA_LOAD;
> > > > + }
> > > > +
> > > > qemu_log_mask(CPU_LOG_MMU,
> > > > "%s 1st-stage address=%" VADDR_PRIx "
> > > ret %d physical "
> > > > TARGET_FMT_plx " prot %d\n",
> > > > __func__, address, ret, pa, prot);
> > > >
> > > > - if (ret != TRANSLATE_FAIL) {
> > > > + if (ret != TRANSLATE_FAIL && ret !=
> > > > + TRANSLATE_G_STAGE_FAIL) {
> > >
> > > Otherwise this patch looks correct.
> > >
> > > Alistair
> > >
> > > > /* Second stage lookup */
> > > > im_address = pa;
> > > >
> > > > --
> > > > 2.19.1
> > > >
> > > >
> > > >
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-10-09 8:04 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-24 8:41 [PATCH] target/riscv: raise exception to HS-mode at get_physical_address Yifei Jiang
2020-08-24 8:41 ` Yifei Jiang
2020-09-25 22:24 ` Alistair Francis
2020-09-25 22:24 ` Alistair Francis
2020-09-27 7:54 ` Jiangyifei
2020-09-27 7:54 ` Jiangyifei
2020-10-01 0:00 ` Alistair Francis
2020-10-01 0:00 ` Alistair Francis
2020-10-09 8:03 ` Jiangyifei
2020-10-09 8:03 ` Jiangyifei
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.