Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
target/riscv/cpu_helper.c | 39 ++++++++++++++++++++++++++++++---------
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 098873c83e..9aa6906acd 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -318,10 +318,19 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
*
* Adapted from Spike's mmu_t::translate and mmu_t::walk
*
+ * @env: CPURISCVState
+ * @physical: This will be set to the calculated physical address
+ * @prot: The returned protection attributes
+ * @addr: The virtual address to be translated
+ * @access_type: The type of MMU access
+ * @mmu_idx: Indicates current privilege level
+ * @first_stage: Are we in first stage translation?
+ * Second stage is used for hypervisor guest translation
*/
static int get_physical_address(CPURISCVState *env, hwaddr *physical,
int *prot, target_ulong addr,
- int access_type, int mmu_idx)
+ int access_type, int mmu_idx,
+ bool first_stage)
{
/* NOTE: the env->pc value visible here will not be
* correct, but the value visible to the exception handler
@@ -518,13 +527,23 @@ restart:
}
static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
- MMUAccessType access_type, bool pmp_violation)
+ MMUAccessType access_type, bool pmp_violation,
+ bool first_stage)
{
CPUState *cs = env_cpu(env);
- int page_fault_exceptions =
- (env->priv_ver >= PRIV_VERSION_1_10_0) &&
- get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
- !pmp_violation;
+ int page_fault_exceptions;
+ if (first_stage) {
+ page_fault_exceptions =
+ (env->priv_ver >= PRIV_VERSION_1_10_0) &&
+ get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
+ !pmp_violation;
+ riscv_cpu_set_force_hs_excep(env, CLEAR_HS_EXCEP);
+ } else {
+ page_fault_exceptions =
+ get_field(env->hgatp, HGATP_MODE) != VM_1_10_MBARE &&
+ !pmp_violation;
+ riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
+ }
switch (access_type) {
case MMU_INST_FETCH:
cs->exception_index = page_fault_exceptions ?
@@ -551,7 +570,8 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
int prot;
int mmu_idx = cpu_mmu_index(&cpu->env, false);
- if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0, mmu_idx)) {
+ if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0, mmu_idx,
+ true)) {
return -1;
}
return phys_addr;
@@ -613,7 +633,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
__func__, address, access_type, mmu_idx);
- ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx);
+ ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx,
+ true);
if (mode == PRV_M && access_type != MMU_INST_FETCH) {
if (get_field(*env->mstatus, MSTATUS_MPRV)) {
@@ -640,7 +661,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
} else if (probe) {
return false;
} else {
- raise_mmu_exception(env, address, access_type, pmp_violation);
+ raise_mmu_exception(env, address, access_type, pmp_violation, true);
riscv_raise_exception(env, cs->exception_index, retaddr);
}
#else
--
2.22.0
On Fri, 23 Aug 2019 16:38:47 PDT (-0700), Alistair Francis wrote:
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> target/riscv/cpu_helper.c | 39 ++++++++++++++++++++++++++++++---------
> 1 file changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 098873c83e..9aa6906acd 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -318,10 +318,19 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
> *
> * Adapted from Spike's mmu_t::translate and mmu_t::walk
> *
> + * @env: CPURISCVState
> + * @physical: This will be set to the calculated physical address
> + * @prot: The returned protection attributes
> + * @addr: The virtual address to be translated
> + * @access_type: The type of MMU access
> + * @mmu_idx: Indicates current privilege level
> + * @first_stage: Are we in first stage translation?
> + * Second stage is used for hypervisor guest translation
> */
> static int get_physical_address(CPURISCVState *env, hwaddr *physical,
> int *prot, target_ulong addr,
> - int access_type, int mmu_idx)
> + int access_type, int mmu_idx,
> + bool first_stage)
> {
> /* NOTE: the env->pc value visible here will not be
> * correct, but the value visible to the exception handler
> @@ -518,13 +527,23 @@ restart:
> }
>
> static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
> - MMUAccessType access_type, bool pmp_violation)
> + MMUAccessType access_type, bool pmp_violation,
> + bool first_stage)
> {
> CPUState *cs = env_cpu(env);
> - int page_fault_exceptions =
> - (env->priv_ver >= PRIV_VERSION_1_10_0) &&
> - get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
> - !pmp_violation;
> + int page_fault_exceptions;
> + if (first_stage) {
> + page_fault_exceptions =
> + (env->priv_ver >= PRIV_VERSION_1_10_0) &&
> + get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
> + !pmp_violation;
> + riscv_cpu_set_force_hs_excep(env, CLEAR_HS_EXCEP);
It might just be email, but the indentation looks wrong here.
> + } else {
> + page_fault_exceptions =
> + get_field(env->hgatp, HGATP_MODE) != VM_1_10_MBARE &&
> + !pmp_violation;
> + riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
> + }
> switch (access_type) {
> case MMU_INST_FETCH:
> cs->exception_index = page_fault_exceptions ?
> @@ -551,7 +570,8 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> int prot;
> int mmu_idx = cpu_mmu_index(&cpu->env, false);
>
> - if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0, mmu_idx)) {
> + if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0, mmu_idx,
> + true)) {
> return -1;
> }
> return phys_addr;
> @@ -613,7 +633,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
> __func__, address, access_type, mmu_idx);
>
> - ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx);
> + ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx,
> + true);
>
> if (mode == PRV_M && access_type != MMU_INST_FETCH) {
> if (get_field(*env->mstatus, MSTATUS_MPRV)) {
> @@ -640,7 +661,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> } else if (probe) {
> return false;
> } else {
> - raise_mmu_exception(env, address, access_type, pmp_violation);
> + raise_mmu_exception(env, address, access_type, pmp_violation, true);
> riscv_raise_exception(env, cs->exception_index, retaddr);
> }
> #else
I don't think it makes sense to split off these two (23 and 24, that add the
argument) out from the implementation.
On Thu, Oct 3, 2019 at 8:53 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Fri, 23 Aug 2019 16:38:47 PDT (-0700), Alistair Francis wrote:
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> > target/riscv/cpu_helper.c | 39 ++++++++++++++++++++++++++++++---------
> > 1 file changed, 30 insertions(+), 9 deletions(-)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 098873c83e..9aa6906acd 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -318,10 +318,19 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
> > *
> > * Adapted from Spike's mmu_t::translate and mmu_t::walk
> > *
> > + * @env: CPURISCVState
> > + * @physical: This will be set to the calculated physical address
> > + * @prot: The returned protection attributes
> > + * @addr: The virtual address to be translated
> > + * @access_type: The type of MMU access
> > + * @mmu_idx: Indicates current privilege level
> > + * @first_stage: Are we in first stage translation?
> > + * Second stage is used for hypervisor guest translation
> > */
> > static int get_physical_address(CPURISCVState *env, hwaddr *physical,
> > int *prot, target_ulong addr,
> > - int access_type, int mmu_idx)
> > + int access_type, int mmu_idx,
> > + bool first_stage)
> > {
> > /* NOTE: the env->pc value visible here will not be
> > * correct, but the value visible to the exception handler
> > @@ -518,13 +527,23 @@ restart:
> > }
> >
> > static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
> > - MMUAccessType access_type, bool pmp_violation)
> > + MMUAccessType access_type, bool pmp_violation,
> > + bool first_stage)
> > {
> > CPUState *cs = env_cpu(env);
> > - int page_fault_exceptions =
> > - (env->priv_ver >= PRIV_VERSION_1_10_0) &&
> > - get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
> > - !pmp_violation;
> > + int page_fault_exceptions;
> > + if (first_stage) {
> > + page_fault_exceptions =
> > + (env->priv_ver >= PRIV_VERSION_1_10_0) &&
> > + get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
> > + !pmp_violation;
> > + riscv_cpu_set_force_hs_excep(env, CLEAR_HS_EXCEP);
>
> It might just be email, but the indentation looks wrong here.
Yep, fixed.
>
> > + } else {
> > + page_fault_exceptions =
> > + get_field(env->hgatp, HGATP_MODE) != VM_1_10_MBARE &&
> > + !pmp_violation;
> > + riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
> > + }
> > switch (access_type) {
> > case MMU_INST_FETCH:
> > cs->exception_index = page_fault_exceptions ?
> > @@ -551,7 +570,8 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> > int prot;
> > int mmu_idx = cpu_mmu_index(&cpu->env, false);
> >
> > - if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0, mmu_idx)) {
> > + if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0, mmu_idx,
> > + true)) {
> > return -1;
> > }
> > return phys_addr;
> > @@ -613,7 +633,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
> > __func__, address, access_type, mmu_idx);
> >
> > - ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx);
> > + ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx,
> > + true);
> >
> > if (mode == PRV_M && access_type != MMU_INST_FETCH) {
> > if (get_field(*env->mstatus, MSTATUS_MPRV)) {
> > @@ -640,7 +661,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > } else if (probe) {
> > return false;
> > } else {
> > - raise_mmu_exception(env, address, access_type, pmp_violation);
> > + raise_mmu_exception(env, address, access_type, pmp_violation, true);
> > riscv_raise_exception(env, cs->exception_index, retaddr);
> > }
> > #else
>
> I don't think it makes sense to split off these two (23 and 24, that add the
> argument) out from the implementation.
The goal was just to make it easier to review. If you want them
combined I can easily combine them.
Alistair
On Mon, 07 Oct 2019 11:05:33 PDT (-0700), alistair23@gmail.com wrote:
> On Thu, Oct 3, 2019 at 8:53 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>>
>> On Fri, 23 Aug 2019 16:38:47 PDT (-0700), Alistair Francis wrote:
>> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> > ---
>> > target/riscv/cpu_helper.c | 39 ++++++++++++++++++++++++++++++---------
>> > 1 file changed, 30 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> > index 098873c83e..9aa6906acd 100644
>> > --- a/target/riscv/cpu_helper.c
>> > +++ b/target/riscv/cpu_helper.c
>> > @@ -318,10 +318,19 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
>> > *
>> > * Adapted from Spike's mmu_t::translate and mmu_t::walk
>> > *
>> > + * @env: CPURISCVState
>> > + * @physical: This will be set to the calculated physical address
>> > + * @prot: The returned protection attributes
>> > + * @addr: The virtual address to be translated
>> > + * @access_type: The type of MMU access
>> > + * @mmu_idx: Indicates current privilege level
>> > + * @first_stage: Are we in first stage translation?
>> > + * Second stage is used for hypervisor guest translation
>> > */
>> > static int get_physical_address(CPURISCVState *env, hwaddr *physical,
>> > int *prot, target_ulong addr,
>> > - int access_type, int mmu_idx)
>> > + int access_type, int mmu_idx,
>> > + bool first_stage)
>> > {
>> > /* NOTE: the env->pc value visible here will not be
>> > * correct, but the value visible to the exception handler
>> > @@ -518,13 +527,23 @@ restart:
>> > }
>> >
>> > static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
>> > - MMUAccessType access_type, bool pmp_violation)
>> > + MMUAccessType access_type, bool pmp_violation,
>> > + bool first_stage)
>> > {
>> > CPUState *cs = env_cpu(env);
>> > - int page_fault_exceptions =
>> > - (env->priv_ver >= PRIV_VERSION_1_10_0) &&
>> > - get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
>> > - !pmp_violation;
>> > + int page_fault_exceptions;
>> > + if (first_stage) {
>> > + page_fault_exceptions =
>> > + (env->priv_ver >= PRIV_VERSION_1_10_0) &&
>> > + get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
>> > + !pmp_violation;
>> > + riscv_cpu_set_force_hs_excep(env, CLEAR_HS_EXCEP);
>>
>> It might just be email, but the indentation looks wrong here.
>
> Yep, fixed.
>
>>
>> > + } else {
>> > + page_fault_exceptions =
>> > + get_field(env->hgatp, HGATP_MODE) != VM_1_10_MBARE &&
>> > + !pmp_violation;
>> > + riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
>> > + }
>> > switch (access_type) {
>> > case MMU_INST_FETCH:
>> > cs->exception_index = page_fault_exceptions ?
>> > @@ -551,7 +570,8 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
>> > int prot;
>> > int mmu_idx = cpu_mmu_index(&cpu->env, false);
>> >
>> > - if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0, mmu_idx)) {
>> > + if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0, mmu_idx,
>> > + true)) {
>> > return -1;
>> > }
>> > return phys_addr;
>> > @@ -613,7 +633,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>> > qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
>> > __func__, address, access_type, mmu_idx);
>> >
>> > - ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx);
>> > + ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx,
>> > + true);
>> >
>> > if (mode == PRV_M && access_type != MMU_INST_FETCH) {
>> > if (get_field(*env->mstatus, MSTATUS_MPRV)) {
>> > @@ -640,7 +661,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>> > } else if (probe) {
>> > return false;
>> > } else {
>> > - raise_mmu_exception(env, address, access_type, pmp_violation);
>> > + raise_mmu_exception(env, address, access_type, pmp_violation, true);
>> > riscv_raise_exception(env, cs->exception_index, retaddr);
>> > }
>> > #else
>>
>> I don't think it makes sense to split off these two (23 and 24, that add the
>> argument) out from the implementation.
>
> The goal was just to make it easier to review. If you want them
> combined I can easily combine them.
It's making it harder to read on my end :)
On Wed, Oct 16, 2019 at 12:02 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Mon, 07 Oct 2019 11:05:33 PDT (-0700), alistair23@gmail.com wrote:
> > On Thu, Oct 3, 2019 at 8:53 AM Palmer Dabbelt <palmer@sifive.com> wrote:
> >>
> >> On Fri, 23 Aug 2019 16:38:47 PDT (-0700), Alistair Francis wrote:
> >> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> >> > ---
> >> > target/riscv/cpu_helper.c | 39 ++++++++++++++++++++++++++++++---------
> >> > 1 file changed, 30 insertions(+), 9 deletions(-)
> >> >
> >> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> >> > index 098873c83e..9aa6906acd 100644
> >> > --- a/target/riscv/cpu_helper.c
> >> > +++ b/target/riscv/cpu_helper.c
> >> > @@ -318,10 +318,19 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
> >> > *
> >> > * Adapted from Spike's mmu_t::translate and mmu_t::walk
> >> > *
> >> > + * @env: CPURISCVState
> >> > + * @physical: This will be set to the calculated physical address
> >> > + * @prot: The returned protection attributes
> >> > + * @addr: The virtual address to be translated
> >> > + * @access_type: The type of MMU access
> >> > + * @mmu_idx: Indicates current privilege level
> >> > + * @first_stage: Are we in first stage translation?
> >> > + * Second stage is used for hypervisor guest translation
> >> > */
> >> > static int get_physical_address(CPURISCVState *env, hwaddr *physical,
> >> > int *prot, target_ulong addr,
> >> > - int access_type, int mmu_idx)
> >> > + int access_type, int mmu_idx,
> >> > + bool first_stage)
> >> > {
> >> > /* NOTE: the env->pc value visible here will not be
> >> > * correct, but the value visible to the exception handler
> >> > @@ -518,13 +527,23 @@ restart:
> >> > }
> >> >
> >> > static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
> >> > - MMUAccessType access_type, bool pmp_violation)
> >> > + MMUAccessType access_type, bool pmp_violation,
> >> > + bool first_stage)
> >> > {
> >> > CPUState *cs = env_cpu(env);
> >> > - int page_fault_exceptions =
> >> > - (env->priv_ver >= PRIV_VERSION_1_10_0) &&
> >> > - get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
> >> > - !pmp_violation;
> >> > + int page_fault_exceptions;
> >> > + if (first_stage) {
> >> > + page_fault_exceptions =
> >> > + (env->priv_ver >= PRIV_VERSION_1_10_0) &&
> >> > + get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
> >> > + !pmp_violation;
> >> > + riscv_cpu_set_force_hs_excep(env, CLEAR_HS_EXCEP);
> >>
> >> It might just be email, but the indentation looks wrong here.
> >
> > Yep, fixed.
> >
> >>
> >> > + } else {
> >> > + page_fault_exceptions =
> >> > + get_field(env->hgatp, HGATP_MODE) != VM_1_10_MBARE &&
> >> > + !pmp_violation;
> >> > + riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
> >> > + }
> >> > switch (access_type) {
> >> > case MMU_INST_FETCH:
> >> > cs->exception_index = page_fault_exceptions ?
> >> > @@ -551,7 +570,8 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> >> > int prot;
> >> > int mmu_idx = cpu_mmu_index(&cpu->env, false);
> >> >
> >> > - if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0, mmu_idx)) {
> >> > + if (get_physical_address(&cpu->env, &phys_addr, &prot, addr, 0, mmu_idx,
> >> > + true)) {
> >> > return -1;
> >> > }
> >> > return phys_addr;
> >> > @@ -613,7 +633,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> >> > qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
> >> > __func__, address, access_type, mmu_idx);
> >> >
> >> > - ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx);
> >> > + ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx,
> >> > + true);
> >> >
> >> > if (mode == PRV_M && access_type != MMU_INST_FETCH) {
> >> > if (get_field(*env->mstatus, MSTATUS_MPRV)) {
> >> > @@ -640,7 +661,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> >> > } else if (probe) {
> >> > return false;
> >> > } else {
> >> > - raise_mmu_exception(env, address, access_type, pmp_violation);
> >> > + raise_mmu_exception(env, address, access_type, pmp_violation, true);
> >> > riscv_raise_exception(env, cs->exception_index, retaddr);
> >> > }
> >> > #else
> >>
> >> I don't think it makes sense to split off these two (23 and 24, that add the
> >> argument) out from the implementation.
> >
> > The goal was just to make it easier to review. If you want them
> > combined I can easily combine them.
>
> It's making it harder to read on my end :)
Ha ok. I have squashed all three.
Alistair
© 2016 - 2026 Red Hat, Inc.