| Message ID | CAE2XoE84K6vdQ23upRa1MaCNWSycUGKja9DrTpVCQ4bdY7bZuQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | What's the correct way to implement rfi and related instruction. | expand |
On 1/7/21 8:14 PM, 罗勇刚(Yonggang Luo) wrote: > This is the first patch,: > It's store MSR bits differntly for different rfi instructions: > [Qemu-devel] [PATCH] target-ppc: fix RFI by clearing some bits of MSR > https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html <https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html> > Comes from target-ppc: fix RFI by clearing some bits of MSR > SHA-1: c3d420ead1aee9fcfd12be11cbdf6b1620134773 > target-ppc/op_helper.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > ``` > diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c > index 8f2ee986bb..3c3aa60bc3 100644 > --- a/target-ppc/op_helper.c > +++ b/target-ppc/op_helper.c > @@ -1646,20 +1646,20 @@ static inline void do_rfi(target_ulong nip, target_ulong msr, > void helper_rfi (void) > { > do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1], > - ~((target_ulong)0x0), 1); > + ~((target_ulong)0x783F0000), 1); > } > > #if defined(TARGET_PPC64) > void helper_rfid (void) > { > do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1], > - ~((target_ulong)0x0), 0); > + ~((target_ulong)0x783F0000), 0); > } > > void helper_hrfid (void) > { > do_rfi(env->spr[SPR_HSRR0], env->spr[SPR_HSRR1], > - ~((target_ulong)0x0), 0); > + ~((target_ulong)0x783F0000), 0); > } > #endif > #endif > ``` > > This is the second patch,: > it's remove the parameter `target_ulong msrm, int keep_msrh` > Comes from ppc: Fix rfi/rfid/hrfi/... emulation > SHA-1: a2e71b28e832346409efc795ecd1f0a2bcb705a3 > ``` > target-ppc/excp_helper.c | 51 +++++++++++++++++++----------------------------- > 1 file changed, 20 insertions(+), 31 deletions(-) > > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > index 30e960e30b..aa0b63f4b0 100644 > --- a/target-ppc/excp_helper.c > +++ b/target-ppc/excp_helper.c > @@ -922,25 +922,20 @@ void helper_store_msr(CPUPPCState *env, target_ulong val) > } > } > > -static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr, > - target_ulong msrm, int keep_msrh) > +static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) > { > CPUState *cs = CPU(ppc_env_get_cpu(env)); > > + /* MSR:POW cannot be set by any form of rfi */ > + msr &= ~(1ULL << MSR_POW); > + > #if defined(TARGET_PPC64) > - if (msr_is_64bit(env, msr)) { > - nip = (uint64_t)nip; > - msr &= (uint64_t)msrm; > - } else { > + /* Switching to 32-bit ? Crop the nip */ > + if (!msr_is_64bit(env, msr)) { > nip = (uint32_t)nip; > - msr = (uint32_t)(msr & msrm); > - if (keep_msrh) { > - msr |= env->msr & ~((uint64_t)0xFFFFFFFF); > - } > } > #else > nip = (uint32_t)nip; > - msr &= (uint32_t)msrm; > #endif > /* XXX: beware: this is false if VLE is supported */ > env->nip = nip & ~((target_ulong)0x00000003); > @@ -959,26 +954,24 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr, > > void helper_rfi(CPUPPCState *env) > { > - if (env->excp_model == POWERPC_EXCP_BOOKE) { > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > - ~((target_ulong)0), 0); > - } else { > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > - ~((target_ulong)0x783F0000), 1); > - } > + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful); > } > > +#define MSR_BOOK3S_MASK > #if defined(TARGET_PPC64) > void helper_rfid(CPUPPCState *env) > { > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > - ~((target_ulong)0x783F0000), 0); > + /* The architeture defines a number of rules for which bits > + * can change but in practice, we handle this in hreg_store_msr() > + * which will be called by do_rfi(), so there is no need to filter > + * here > + */ > + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1]); > } > > void helper_hrfid(CPUPPCState *env) > { > - do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1], > - ~((target_ulong)0x783F0000), 0); > + do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]); > } > #endif > > @@ -986,28 +979,24 @@ void helper_hrfid(CPUPPCState *env) > /* Embedded PowerPC specific helpers */ > void helper_40x_rfci(CPUPPCState *env) > { > - do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3], > - ~((target_ulong)0xFFFF0000), 0); > + do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3]); > } > > void helper_rfci(CPUPPCState *env) > { > - do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1], > - ~((target_ulong)0), 0); > + do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1]); > } > > void helper_rfdi(CPUPPCState *env) > { > /* FIXME: choose CSRR1 or DSRR1 based on cpu type */ > - do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1], > - ~((target_ulong)0), 0); > + do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1]); > } > > void helper_rfmci(CPUPPCState *env) > { > /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */ > - do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1], > - ~((target_ulong)0), 0); > + do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]); > } > #endif > > @@ -1045,7 +1034,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1, target_ulong arg2, > > void helper_rfsvc(CPUPPCState *env) > { > - do_rfi(env, env->lr, env->ctr, 0x0000FFFF, 0); > + do_rfi(env, env->lr, env->ctr & 0x0000FFFF); > } > > /* Embedded.Processor Control */ > ``` > > And of cause, the second patch fixes some problem, but also cause new problem, > how to implement these instruction properly? What are the new problems ? C. > > > > -- > 此致 > 礼 > 罗勇刚 > Yours > sincerely, > Yonggang Luo
On Fri, Jan 8, 2021 at 5:54 AM Cédric Le Goater <clg@kaod.org> wrote: > > On 1/7/21 8:14 PM, 罗勇刚(Yonggang Luo) wrote: > > This is the first patch,: > > It's store MSR bits differntly for different rfi instructions: > > [Qemu-devel] [PATCH] target-ppc: fix RFI by clearing some bits of MSR > > https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html < https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html> > > Comes from target-ppc: fix RFI by clearing some bits of MSR > > SHA-1: c3d420ead1aee9fcfd12be11cbdf6b1620134773 > > target-ppc/op_helper.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > ``` > > diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c > > index 8f2ee986bb..3c3aa60bc3 100644 > > --- a/target-ppc/op_helper.c > > +++ b/target-ppc/op_helper.c > > @@ -1646,20 +1646,20 @@ static inline void do_rfi(target_ulong nip, target_ulong msr, > > void helper_rfi (void) > > { > > do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1], > > - ~((target_ulong)0x0), 1); > > + ~((target_ulong)0x783F0000), 1); > > } > > > > #if defined(TARGET_PPC64) > > void helper_rfid (void) > > { > > do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1], > > - ~((target_ulong)0x0), 0); > > + ~((target_ulong)0x783F0000), 0); > > } > > > > void helper_hrfid (void) > > { > > do_rfi(env->spr[SPR_HSRR0], env->spr[SPR_HSRR1], > > - ~((target_ulong)0x0), 0); > > + ~((target_ulong)0x783F0000), 0); > > } > > #endif > > #endif > > ``` > > > > This is the second patch,: > > it's remove the parameter `target_ulong msrm, int keep_msrh` > > Comes from ppc: Fix rfi/rfid/hrfi/... emulation > > SHA-1: a2e71b28e832346409efc795ecd1f0a2bcb705a3 > > ``` > > target-ppc/excp_helper.c | 51 +++++++++++++++++++----------------------------- > > 1 file changed, 20 insertions(+), 31 deletions(-) > > > > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > > index 30e960e30b..aa0b63f4b0 100644 > > --- a/target-ppc/excp_helper.c > > +++ b/target-ppc/excp_helper.c > > @@ -922,25 +922,20 @@ void helper_store_msr(CPUPPCState *env, target_ulong val) > > } > > } > > > > -static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr, > > - target_ulong msrm, int keep_msrh) > > +static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) > > { > > CPUState *cs = CPU(ppc_env_get_cpu(env)); > > > > + /* MSR:POW cannot be set by any form of rfi */ > > + msr &= ~(1ULL << MSR_POW); > > + > > #if defined(TARGET_PPC64) > > - if (msr_is_64bit(env, msr)) { > > - nip = (uint64_t)nip; > > - msr &= (uint64_t)msrm; > > - } else { > > + /* Switching to 32-bit ? Crop the nip */ > > + if (!msr_is_64bit(env, msr)) { > > nip = (uint32_t)nip; > > - msr = (uint32_t)(msr & msrm); > > - if (keep_msrh) { > > - msr |= env->msr & ~((uint64_t)0xFFFFFFFF); > > - } > > } > > #else > > nip = (uint32_t)nip; > > - msr &= (uint32_t)msrm; > > #endif > > /* XXX: beware: this is false if VLE is supported */ > > env->nip = nip & ~((target_ulong)0x00000003); > > @@ -959,26 +954,24 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr, > > > > void helper_rfi(CPUPPCState *env) > > { > > - if (env->excp_model == POWERPC_EXCP_BOOKE) { > > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > > - ~((target_ulong)0), 0); > > - } else { > > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > > - ~((target_ulong)0x783F0000), 1); > > - } > > + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful); > > } > > > > +#define MSR_BOOK3S_MASK > > #if defined(TARGET_PPC64) > > void helper_rfid(CPUPPCState *env) > > { > > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > > - ~((target_ulong)0x783F0000), 0); > > + /* The architeture defines a number of rules for which bits > > + * can change but in practice, we handle this in hreg_store_msr() > > + * which will be called by do_rfi(), so there is no need to filter > > + * here > > + */ > > + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1]); > > } > > > > void helper_hrfid(CPUPPCState *env) > > { > > - do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1], > > - ~((target_ulong)0x783F0000), 0); > > + do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]); > > } > > #endif > > > > @@ -986,28 +979,24 @@ void helper_hrfid(CPUPPCState *env) > > /* Embedded PowerPC specific helpers */ > > void helper_40x_rfci(CPUPPCState *env) > > { > > - do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3], > > - ~((target_ulong)0xFFFF0000), 0); > > + do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3]); > > } > > > > void helper_rfci(CPUPPCState *env) > > { > > - do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1], > > - ~((target_ulong)0), 0); > > + do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1]); > > } > > > > void helper_rfdi(CPUPPCState *env) > > { > > /* FIXME: choose CSRR1 or DSRR1 based on cpu type */ > > - do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1], > > - ~((target_ulong)0), 0); > > + do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1]); > > } > > > > void helper_rfmci(CPUPPCState *env) > > { > > /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */ > > - do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1], > > - ~((target_ulong)0), 0); > > + do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]); > > } > > #endif > > > > @@ -1045,7 +1034,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1, target_ulong arg2, > > > > void helper_rfsvc(CPUPPCState *env) > > { > > - do_rfi(env, env->lr, env->ctr, 0x0000FFFF, 0); > > + do_rfi(env, env->lr, env->ctr & 0x0000FFFF); > > } > > > > /* Embedded.Processor Control */ > > ``` > > > > And of cause, the second patch fixes some problem, but also cause new problem, > > how to implement these instruction properly? > > What are the new problems ? Before this patch, VxWorks can working, but after this, VxWorks can not boot anymore. > > C. > > > > > > > > > -- > > 此致 > > 礼 > > 罗勇刚 > > Yours > > sincerely, > > Yonggang Luo > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
On 1/8/21 5:21 AM, 罗勇刚(Yonggang Luo) wrote: > > > On Fri, Jan 8, 2021 at 5:54 AM Cédric Le Goater <clg@kaod.org <mailto:clg@kaod.org>> wrote: >> >> On 1/7/21 8:14 PM, 罗勇刚(Yonggang Luo) wrote: >> > This is the first patch,: >> > It's store MSR bits differntly for different rfi instructions: >> > [Qemu-devel] [PATCH] target-ppc: fix RFI by clearing some bits of MSR >> > https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html <https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html> <https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html <https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html>> >> > Comes from target-ppc: fix RFI by clearing some bits of MSR >> > SHA-1: c3d420ead1aee9fcfd12be11cbdf6b1620134773 >> > target-ppc/op_helper.c | 6 +++--- >> > 1 file changed, 3 insertions(+), 3 deletions(-) >> > ``` >> > diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c >> > index 8f2ee986bb..3c3aa60bc3 100644 >> > --- a/target-ppc/op_helper.c >> > +++ b/target-ppc/op_helper.c >> > @@ -1646,20 +1646,20 @@ static inline void do_rfi(target_ulong nip, target_ulong msr, >> > void helper_rfi (void) >> > { >> > do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1], >> > - ~((target_ulong)0x0), 1); >> > + ~((target_ulong)0x783F0000), 1); >> > } >> > >> > #if defined(TARGET_PPC64) >> > void helper_rfid (void) >> > { >> > do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1], >> > - ~((target_ulong)0x0), 0); >> > + ~((target_ulong)0x783F0000), 0); >> > } >> > >> > void helper_hrfid (void) >> > { >> > do_rfi(env->spr[SPR_HSRR0], env->spr[SPR_HSRR1], >> > - ~((target_ulong)0x0), 0); >> > + ~((target_ulong)0x783F0000), 0); >> > } >> > #endif >> > #endif >> > ``` >> > >> > This is the second patch,: >> > it's remove the parameter `target_ulong msrm, int keep_msrh` >> > Comes from ppc: Fix rfi/rfid/hrfi/... emulation >> > SHA-1: a2e71b28e832346409efc795ecd1f0a2bcb705a3 >> > ``` >> > target-ppc/excp_helper.c | 51 +++++++++++++++++++----------------------------- >> > 1 file changed, 20 insertions(+), 31 deletions(-) >> > >> > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c >> > index 30e960e30b..aa0b63f4b0 100644 >> > --- a/target-ppc/excp_helper.c >> > +++ b/target-ppc/excp_helper.c >> > @@ -922,25 +922,20 @@ void helper_store_msr(CPUPPCState *env, target_ulong val) >> > } >> > } >> > >> > -static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr, >> > - target_ulong msrm, int keep_msrh) >> > +static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) >> > { >> > CPUState *cs = CPU(ppc_env_get_cpu(env)); >> > >> > + /* MSR:POW cannot be set by any form of rfi */ >> > + msr &= ~(1ULL << MSR_POW); >> > + >> > #if defined(TARGET_PPC64) >> > - if (msr_is_64bit(env, msr)) { >> > - nip = (uint64_t)nip; >> > - msr &= (uint64_t)msrm; >> > - } else { >> > + /* Switching to 32-bit ? Crop the nip */ >> > + if (!msr_is_64bit(env, msr)) { >> > nip = (uint32_t)nip; >> > - msr = (uint32_t)(msr & msrm); >> > - if (keep_msrh) { >> > - msr |= env->msr & ~((uint64_t)0xFFFFFFFF); >> > - } >> > } >> > #else >> > nip = (uint32_t)nip; >> > - msr &= (uint32_t)msrm; >> > #endif >> > /* XXX: beware: this is false if VLE is supported */ >> > env->nip = nip & ~((target_ulong)0x00000003); >> > @@ -959,26 +954,24 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr, >> > >> > void helper_rfi(CPUPPCState *env) >> > { >> > - if (env->excp_model == POWERPC_EXCP_BOOKE) { >> > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], >> > - ~((target_ulong)0), 0); >> > - } else { >> > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], >> > - ~((target_ulong)0x783F0000), 1); >> > - } >> > + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful); >> > } >> > >> > +#define MSR_BOOK3S_MASK >> > #if defined(TARGET_PPC64) >> > void helper_rfid(CPUPPCState *env) >> > { >> > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], >> > - ~((target_ulong)0x783F0000), 0); >> > + /* The architeture defines a number of rules for which bits >> > + * can change but in practice, we handle this in hreg_store_msr() >> > + * which will be called by do_rfi(), so there is no need to filter >> > + * here >> > + */ >> > + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1]); >> > } >> > >> > void helper_hrfid(CPUPPCState *env) >> > { >> > - do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1], >> > - ~((target_ulong)0x783F0000), 0); >> > + do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]); >> > } >> > #endif >> > >> > @@ -986,28 +979,24 @@ void helper_hrfid(CPUPPCState *env) >> > /* Embedded PowerPC specific helpers */ >> > void helper_40x_rfci(CPUPPCState *env) >> > { >> > - do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3], >> > - ~((target_ulong)0xFFFF0000), 0); >> > + do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3]); >> > } >> > >> > void helper_rfci(CPUPPCState *env) >> > { >> > - do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1], >> > - ~((target_ulong)0), 0); >> > + do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1]); >> > } >> > >> > void helper_rfdi(CPUPPCState *env) >> > { >> > /* FIXME: choose CSRR1 or DSRR1 based on cpu type */ >> > - do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1], >> > - ~((target_ulong)0), 0); >> > + do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1]); >> > } >> > >> > void helper_rfmci(CPUPPCState *env) >> > { >> > /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */ >> > - do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1], >> > - ~((target_ulong)0), 0); >> > + do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]); >> > } >> > #endif >> > >> > @@ -1045,7 +1034,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1, target_ulong arg2, >> > >> > void helper_rfsvc(CPUPPCState *env) >> > { >> > - do_rfi(env, env->lr, env->ctr, 0x0000FFFF, 0); >> > + do_rfi(env, env->lr, env->ctr & 0x0000FFFF); >> > } >> > >> > /* Embedded.Processor Control */ >> > ``` >> > >> > And of cause, the second patch fixes some problem, but also cause new problem, >> > how to implement these instruction properly? >> >> What are the new problems ? > > > Before this patch, VxWorks can working, but after this, VxWorks can not boot anymore. I suppose you did a bisect to reach this patch. Which QEMU machine is impacted ? Which CPU ? What are the symptoms ? Did you try to run with -d exec or -d in_asm to identify the exact instruction ? From there, you could try to revert partially the patch above to fix the problem. Thanks, C.
On Fri, Jan 8, 2021 at 2:02 AM Cédric Le Goater <clg@kaod.org> wrote: > > On 1/8/21 5:21 AM, 罗勇刚(Yonggang Luo) wrote: > > > > > > On Fri, Jan 8, 2021 at 5:54 AM Cédric Le Goater <clg@kaod.org <mailto: clg@kaod.org>> wrote: > >> > >> On 1/7/21 8:14 PM, 罗勇刚(Yonggang Luo) wrote: > >> > This is the first patch,: > >> > It's store MSR bits differntly for different rfi instructions: > >> > [Qemu-devel] [PATCH] target-ppc: fix RFI by clearing some bits of MSR > >> > https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html < https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html> < https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html < https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html>> > >> > Comes from target-ppc: fix RFI by clearing some bits of MSR > >> > SHA-1: c3d420ead1aee9fcfd12be11cbdf6b1620134773 > >> > target-ppc/op_helper.c | 6 +++--- > >> > 1 file changed, 3 insertions(+), 3 deletions(-) > >> > ``` > >> > diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c > >> > index 8f2ee986bb..3c3aa60bc3 100644 > >> > --- a/target-ppc/op_helper.c > >> > +++ b/target-ppc/op_helper.c > >> > @@ -1646,20 +1646,20 @@ static inline void do_rfi(target_ulong nip, target_ulong msr, > >> > void helper_rfi (void) > >> > { > >> > do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1], > >> > - ~((target_ulong)0x0), 1); > >> > + ~((target_ulong)0x783F0000), 1); > >> > } > >> > > >> > #if defined(TARGET_PPC64) > >> > void helper_rfid (void) > >> > { > >> > do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1], > >> > - ~((target_ulong)0x0), 0); > >> > + ~((target_ulong)0x783F0000), 0); > >> > } > >> > > >> > void helper_hrfid (void) > >> > { > >> > do_rfi(env->spr[SPR_HSRR0], env->spr[SPR_HSRR1], > >> > - ~((target_ulong)0x0), 0); > >> > + ~((target_ulong)0x783F0000), 0); > >> > } > >> > #endif > >> > #endif > >> > ``` > >> > > >> > This is the second patch,: > >> > it's remove the parameter `target_ulong msrm, int keep_msrh` > >> > Comes from ppc: Fix rfi/rfid/hrfi/... emulation > >> > SHA-1: a2e71b28e832346409efc795ecd1f0a2bcb705a3 > >> > ``` > >> > target-ppc/excp_helper.c | 51 +++++++++++++++++++----------------------------- > >> > 1 file changed, 20 insertions(+), 31 deletions(-) > >> > > >> > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > >> > index 30e960e30b..aa0b63f4b0 100644 > >> > --- a/target-ppc/excp_helper.c > >> > +++ b/target-ppc/excp_helper.c > >> > @@ -922,25 +922,20 @@ void helper_store_msr(CPUPPCState *env, target_ulong val) > >> > } > >> > } > >> > > >> > -static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr, > >> > - target_ulong msrm, int keep_msrh) > >> > +static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) > >> > { > >> > CPUState *cs = CPU(ppc_env_get_cpu(env)); > >> > > >> > + /* MSR:POW cannot be set by any form of rfi */ > >> > + msr &= ~(1ULL << MSR_POW); > >> > + > >> > #if defined(TARGET_PPC64) > >> > - if (msr_is_64bit(env, msr)) { > >> > - nip = (uint64_t)nip; > >> > - msr &= (uint64_t)msrm; > >> > - } else { > >> > + /* Switching to 32-bit ? Crop the nip */ > >> > + if (!msr_is_64bit(env, msr)) { > >> > nip = (uint32_t)nip; > >> > - msr = (uint32_t)(msr & msrm); > >> > - if (keep_msrh) { > >> > - msr |= env->msr & ~((uint64_t)0xFFFFFFFF); > >> > - } > >> > } > >> > #else > >> > nip = (uint32_t)nip; > >> > - msr &= (uint32_t)msrm; > >> > #endif > >> > /* XXX: beware: this is false if VLE is supported */ > >> > env->nip = nip & ~((target_ulong)0x00000003); > >> > @@ -959,26 +954,24 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr, > >> > > >> > void helper_rfi(CPUPPCState *env) > >> > { > >> > - if (env->excp_model == POWERPC_EXCP_BOOKE) { > >> > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > >> > - ~((target_ulong)0), 0); > >> > - } else { > >> > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > >> > - ~((target_ulong)0x783F0000), 1); > >> > - } > >> > + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful); > >> > } > >> > > >> > +#define MSR_BOOK3S_MASK > >> > #if defined(TARGET_PPC64) > >> > void helper_rfid(CPUPPCState *env) > >> > { > >> > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > >> > - ~((target_ulong)0x783F0000), 0); > >> > + /* The architeture defines a number of rules for which bits > >> > + * can change but in practice, we handle this in hreg_store_msr() > >> > + * which will be called by do_rfi(), so there is no need to filter > >> > + * here > >> > + */ > >> > + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1]); > >> > } > >> > > >> > void helper_hrfid(CPUPPCState *env) > >> > { > >> > - do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1], > >> > - ~((target_ulong)0x783F0000), 0); > >> > + do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]); > >> > } > >> > #endif > >> > > >> > @@ -986,28 +979,24 @@ void helper_hrfid(CPUPPCState *env) > >> > /* Embedded PowerPC specific helpers */ > >> > void helper_40x_rfci(CPUPPCState *env) > >> > { > >> > - do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3], > >> > - ~((target_ulong)0xFFFF0000), 0); > >> > + do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3]); > >> > } > >> > > >> > void helper_rfci(CPUPPCState *env) > >> > { > >> > - do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1], > >> > - ~((target_ulong)0), 0); > >> > + do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1]); > >> > } > >> > > >> > void helper_rfdi(CPUPPCState *env) > >> > { > >> > /* FIXME: choose CSRR1 or DSRR1 based on cpu type */ > >> > - do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1], > >> > - ~((target_ulong)0), 0); > >> > + do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1]); > >> > } > >> > > >> > void helper_rfmci(CPUPPCState *env) > >> > { > >> > /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */ > >> > - do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1], > >> > - ~((target_ulong)0), 0); > >> > + do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]); > >> > } > >> > #endif > >> > > >> > @@ -1045,7 +1034,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1, target_ulong arg2, > >> > > >> > void helper_rfsvc(CPUPPCState *env) > >> > { > >> > - do_rfi(env, env->lr, env->ctr, 0x0000FFFF, 0); > >> > + do_rfi(env, env->lr, env->ctr & 0x0000FFFF); > >> > } > >> > > >> > /* Embedded.Processor Control */ > >> > ``` > >> > > >> > And of cause, the second patch fixes some problem, but also cause new problem, > >> > how to implement these instruction properly? > >> > >> What are the new problems ? > > > > > > Before this patch, VxWorks can working, but after this, VxWorks can not boot anymore. > > I suppose you did a bisect to reach this patch. > > Which QEMU machine is impacted ? Which CPU ? What are the symptoms ? > > Did you try to run with -d exec or -d in_asm to identify the exact > instruction ? > > From there, you could try to revert partially the patch above to > fix the problem. > > Thanks, > > C. > > > QEMU 5.2.x, an e300 based machine ppc603 are impacted. Here is my fix, narrowed down to MSR_TGPR and MSR_ILE ``` From 42ce41671f1e6c4dd44e6fb481bbda9df09320bd Mon Sep 17 00:00:00 2001 From: Yonggang Luo <luoyonggang@gmail.com> Date: Sun, 10 Jan 2021 00:08:00 -0800 Subject: [PATCH] ppc: Fix rfi/rfid/hrfi/... emulation again This revert part mask bits for ppc603/ppc4x that disabled in a2e71b28e832346409efc795ecd1f0a2bcb705a3. Remove redundant macro MSR_BOOK3S_MASK. Fixes boot VxWorks on e300 Signed-off-by: Yonggang Luo <luoyonggang@gmail.com> --- target/ppc/excp_helper.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 1c48b9fdf6..df70c5a4e8 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -1156,8 +1156,10 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) { CPUState *cs = env_cpu(env); - /* MSR:POW cannot be set by any form of rfi */ + /* MSR:POW,TGPR,ILE cannot be set by any form of rfi */ msr &= ~(1ULL << MSR_POW); + msr &= ~(1ULL << MSR_TGPR); + msr &= ~(1ULL << MSR_ILE); #if defined(TARGET_PPC64) /* Switching to 32-bit ? Crop the nip */ @@ -1190,7 +1192,6 @@ void helper_rfi(CPUPPCState *env) do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful); } -#define MSR_BOOK3S_MASK #if defined(TARGET_PPC64) void helper_rfid(CPUPPCState *env) {
> QEMU 5.2.x, an e300 based machine ppc603 are impacted. > Here is my fix, narrowed down to MSR_TGPR and MSR_ILE > ``` > From 42ce41671f1e6c4dd44e6fb481bbda9df09320bd Mon Sep 17 00:00:00 2001 > From: Yonggang Luo <luoyonggang@gmail.com <mailto:luoyonggang@gmail.com>> > Date: Sun, 10 Jan 2021 00:08:00 -0800 > Subject: [PATCH] ppc: Fix rfi/rfid/hrfi/... emulation again > > This revert part mask bits for ppc603/ppc4x that disabled in a2e71b28e832346409efc795ecd1f0a2bcb705a3. > Remove redundant macro MSR_BOOK3S_MASK. > Fixes boot VxWorks on e300 > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com <mailto:luoyonggang@gmail.com>> > --- > target/ppc/excp_helper.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index 1c48b9fdf6..df70c5a4e8 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -1156,8 +1156,10 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) > { > CPUState *cs = env_cpu(env); > > - /* MSR:POW cannot be set by any form of rfi */ > + /* MSR:POW,TGPR,ILE cannot be set by any form of rfi */ > msr &= ~(1ULL << MSR_POW); > + msr &= ~(1ULL << MSR_TGPR); Indeed. The e300 user manual says that TGPR is cleared by rfi. We should add a per-cpu family mask and not a global setting. > + msr &= ~(1ULL << MSR_ILE); that's curious. I am still trying to understand that part. May be this is due to the lack of HID2 modeling which contains a "True little-endian" bit. Is your image Little endian ? C. > > #if defined(TARGET_PPC64) > /* Switching to 32-bit ? Crop the nip */ > @@ -1190,7 +1192,6 @@ void helper_rfi(CPUPPCState *env) > do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful); > } > > -#define MSR_BOOK3S_MASK > #if defined(TARGET_PPC64) > void helper_rfid(CPUPPCState *env) > { > -- > 2.29.2.windows.3 > > ``` > > -- > 此致 > 礼 > 罗勇刚 > Yours > sincerely, > Yonggang Luo
On Tue, Jan 12, 2021 at 5:23 PM Cédric Le Goater <clg@kaod.org> wrote: > > > QEMU 5.2.x, an e300 based machine ppc603 are impacted. > > Here is my fix, narrowed down to MSR_TGPR and MSR_ILE > > ``` > > From 42ce41671f1e6c4dd44e6fb481bbda9df09320bd Mon Sep 17 00:00:00 2001 > > From: Yonggang Luo <luoyonggang@gmail.com <mailto:luoyonggang@gmail.com >> > > Date: Sun, 10 Jan 2021 00:08:00 -0800 > > Subject: [PATCH] ppc: Fix rfi/rfid/hrfi/... emulation again > > > > This revert part mask bits for ppc603/ppc4x that disabled in a2e71b28e832346409efc795ecd1f0a2bcb705a3. > > Remove redundant macro MSR_BOOK3S_MASK. > > Fixes boot VxWorks on e300 > > > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com <mailto: luoyonggang@gmail.com>> > > --- > > target/ppc/excp_helper.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > > index 1c48b9fdf6..df70c5a4e8 100644 > > --- a/target/ppc/excp_helper.c > > +++ b/target/ppc/excp_helper.c > > @@ -1156,8 +1156,10 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) > > { > > CPUState *cs = env_cpu(env); > > > > - /* MSR:POW cannot be set by any form of rfi */ > > + /* MSR:POW,TGPR,ILE cannot be set by any form of rfi */ > > msr &= ~(1ULL << MSR_POW); > > + msr &= ~(1ULL << MSR_TGPR); > > Indeed. The e300 user manual says that TGPR is cleared by rfi. We should > add a per-cpu family mask and not a global setting. Refer to https://www.nxp.com/docs/en/reference-manual/e300coreRM.pdf `Table 2-4. MSR Bit Settings` ``` Temporary GPR remapping (implementation-specific) 0 Normal operation 1 TGPR mode. GPR0–GPR3 are remapped to TGPR0–TGPR3 for use by TLB miss routines. The contents of GPR0–GPR3 remain unchanged while MSR[TGPR] = 1. Attempts to use GPR4–GPR31 with MSR[TGPR] = 1 yield undefined results. Temporarily replaces TGPR0–TGPR3 with GPR0–GPR3 for use by TLB miss routines. The TGPR bit is set when either an instruction TLB miss, data read miss, or data write miss interrupt is taken. The TGPR bit is cleared by an rfi instruction. ``` > > > + msr &= ~(1ULL << MSR_ILE); > > that's curious. I am still trying to understand that part. May be this is > due to the lack of HID2 modeling which contains a "True little-endian" bit. Don't understand this part, I am running VxWorks 6.9 on MPC8349EA https://www.nxp.com/docs/en/reference-manual/MPC8349EARM.pdf Didn't got any idea about why MSR_ILE are set > > Is your image Little endian ? > Big Endian vxworks image. > C. > > > > > #if defined(TARGET_PPC64) > > /* Switching to 32-bit ? Crop the nip */ > > @@ -1190,7 +1192,6 @@ void helper_rfi(CPUPPCState *env) > > do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful); > > } > > > > -#define MSR_BOOK3S_MASK > > #if defined(TARGET_PPC64) > > void helper_rfid(CPUPPCState *env) > > { > > -- > > 2.29.2.windows.3 > > > > ``` > > > > -- > > 此致 > > 礼 > > 罗勇刚 > > Yours > > sincerely, > > Yonggang Luo > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
On Tue, Jan 12, 2021 at 1:23 AM Cédric Le Goater <clg@kaod.org> wrote: > > > QEMU 5.2.x, an e300 based machine ppc603 are impacted. > > Here is my fix, narrowed down to MSR_TGPR and MSR_ILE > > ``` > > From 42ce41671f1e6c4dd44e6fb481bbda9df09320bd Mon Sep 17 00:00:00 2001 > > From: Yonggang Luo <luoyonggang@gmail.com <mailto:luoyonggang@gmail.com >> > > Date: Sun, 10 Jan 2021 00:08:00 -0800 > > Subject: [PATCH] ppc: Fix rfi/rfid/hrfi/... emulation again > > > > This revert part mask bits for ppc603/ppc4x that disabled in a2e71b28e832346409efc795ecd1f0a2bcb705a3. > > Remove redundant macro MSR_BOOK3S_MASK. > > Fixes boot VxWorks on e300 > > > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com <mailto: luoyonggang@gmail.com>> > > --- > > target/ppc/excp_helper.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > > index 1c48b9fdf6..df70c5a4e8 100644 > > --- a/target/ppc/excp_helper.c > > +++ b/target/ppc/excp_helper.c > > @@ -1156,8 +1156,10 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) > > { > > CPUState *cs = env_cpu(env); > > > > - /* MSR:POW cannot be set by any form of rfi */ > > + /* MSR:POW,TGPR,ILE cannot be set by any form of rfi */ > > msr &= ~(1ULL << MSR_POW); > > + msr &= ~(1ULL << MSR_TGPR); > > Indeed. The e300 user manual says that TGPR is cleared by rfi. We should > add a per-cpu family mask and not a global setting. > > > + msr &= ~(1ULL << MSR_ILE); > > that's curious. I am still trying to understand that part. May be this is > due to the lack of HID2 modeling which contains a "True little-endian" bit. > > Is your image Little endian ? Hi, According to ` Table 2-4. MSR Bit Settings (continued) ` in https://www.nxp.com/docs/en/reference-manual/e300coreRM.pdf ``` Interrupt little-endian mode. When an interrupt occurs, this bit is copied into MSR[LE] to select the endian mode for the context established by the interrupt. ``` Does this means MSR[LE] = MSR[LE] > > C. > > > > > #if defined(TARGET_PPC64) > > /* Switching to 32-bit ? Crop the nip */ > > @@ -1190,7 +1192,6 @@ void helper_rfi(CPUPPCState *env) > > do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful); > > } > > > > -#define MSR_BOOK3S_MASK > > #if defined(TARGET_PPC64) > > void helper_rfid(CPUPPCState *env) > > { > > -- > > 2.29.2.windows.3 > > > > ``` > > > > -- > > 此致 > > 礼 > > 罗勇刚 > > Yours > > sincerely, > > Yonggang Luo > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
On 1/12/21 2:52 PM, 罗勇刚(Yonggang Luo) wrote: > > > On Tue, Jan 12, 2021 at 5:23 PM Cédric Le Goater <clg@kaod.org <mailto:clg@kaod.org>> wrote: >> >> > QEMU 5.2.x, an e300 based machine ppc603 are impacted. >> > Here is my fix, narrowed down to MSR_TGPR and MSR_ILE >> > ``` >> > From 42ce41671f1e6c4dd44e6fb481bbda9df09320bd Mon Sep 17 00:00:00 2001 >> > From: Yonggang Luo <luoyonggang@gmail.com <mailto:luoyonggang@gmail.com> <mailto:luoyonggang@gmail.com <mailto:luoyonggang@gmail.com>>> >> > Date: Sun, 10 Jan 2021 00:08:00 -0800 >> > Subject: [PATCH] ppc: Fix rfi/rfid/hrfi/... emulation again >> > >> > This revert part mask bits for ppc603/ppc4x that disabled in a2e71b28e832346409efc795ecd1f0a2bcb705a3. >> > Remove redundant macro MSR_BOOK3S_MASK. >> > Fixes boot VxWorks on e300 >> > >> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com <mailto:luoyonggang@gmail.com> <mailto:luoyonggang@gmail.com <mailto:luoyonggang@gmail.com>>> >> > --- >> > target/ppc/excp_helper.c | 5 +++-- >> > 1 file changed, 3 insertions(+), 2 deletions(-) >> > >> > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c >> > index 1c48b9fdf6..df70c5a4e8 100644 >> > --- a/target/ppc/excp_helper.c >> > +++ b/target/ppc/excp_helper.c >> > @@ -1156,8 +1156,10 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) >> > { >> > CPUState *cs = env_cpu(env); >> > >> > - /* MSR:POW cannot be set by any form of rfi */ >> > + /* MSR:POW,TGPR,ILE cannot be set by any form of rfi */ >> > msr &= ~(1ULL << MSR_POW); >> > + msr &= ~(1ULL << MSR_TGPR); >> >> Indeed. The e300 user manual says that TGPR is cleared by rfi. We should >> add a per-cpu family mask and not a global setting. > Refer to https://www.nxp.com/docs/en/reference-manual/e300coreRM.pdf <https://www.nxp.com/docs/en/reference-manual/e300coreRM.pdf> > > `Table 2-4. MSR Bit Settings` > > ``` > Temporary GPR remapping (implementation-specific) 0 Normal operation 1 TGPR mode. GPR0–GPR3 are remapped to TGPR0–TGPR3 for use by TLB miss routines. The contents of GPR0–GPR3 remain unchanged while MSR[TGPR] = 1. Attempts to use GPR4–GPR31 with MSR[TGPR] = 1 yield undefined results. Temporarily replaces TGPR0–TGPR3 with GPR0–GPR3 for use by TLB miss routines. The TGPR bit is set when either an instruction TLB miss, data read miss, or data write miss interrupt is taken. The TGPR bit is cleared by an rfi instruction. > ``` > >> >> > + msr &= ~(1ULL << MSR_ILE); >> >> that's curious. I am still trying to understand that part. May be this is >> due to the lack of HID2 modeling which contains a "True little-endian" bit. > > Don't understand this part, I am running VxWorks 6.9 on MPC8349EA > https://www.nxp.com/docs/en/reference-manual/MPC8349EARM.pdf <https://www.nxp.com/docs/en/reference-manual/MPC8349EARM.pdf> > > Didn't got any idea about why MSR_ILE are set > >> >> Is your image Little endian ? >> > Big Endian vxworks image. Can you share the image ? and the QEMU command line ? Thanks, C.
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c index 8f2ee986bb..3c3aa60bc3 100644 --- a/target-ppc/op_helper.c +++ b/target-ppc/op_helper.c @@ -1646,20 +1646,20 @@ static inline void do_rfi(target_ulong nip, target_ulong msr, void helper_rfi (void) { do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1], - ~((target_ulong)0x0), 1); + ~((target_ulong)0x783F0000), 1); } #if defined(TARGET_PPC64) void helper_rfid (void) { do_rfi(env->spr[SPR_SRR0], env->spr[SPR_SRR1], - ~((target_ulong)0x0), 0); + ~((target_ulong)0x783F0000), 0); } void helper_hrfid (void) { do_rfi(env->spr[SPR_HSRR0], env->spr[SPR_HSRR1], - ~((target_ulong)0x0), 0); + ~((target_ulong)0x783F0000), 0); } #endif #endif ``` This is the second patch,: it's remove the parameter `target_ulong msrm, int keep_msrh` Comes from ppc: Fix rfi/rfid/hrfi/... emulation SHA-1: a2e71b28e832346409efc795ecd1f0a2bcb705a3 ``` target-ppc/excp_helper.c | 51 +++++++++++++++++++----------------------------- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c index 30e960e30b..aa0b63f4b0 100644 --- a/target-ppc/excp_helper.c +++ b/target-ppc/excp_helper.c @@ -922,25 +922,20 @@ void helper_store_msr(CPUPPCState *env, target_ulong val) } } -static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr, - target_ulong msrm, int keep_msrh) +static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) { CPUState *cs = CPU(ppc_env_get_cpu(env)); + /* MSR:POW cannot be set by any form of rfi */ + msr &= ~(1ULL << MSR_POW); + #if defined(TARGET_PPC64) - if (msr_is_64bit(env, msr)) { - nip = (uint64_t)nip; - msr &= (uint64_t)msrm; - } else { + /* Switching to 32-bit ? Crop the nip */ + if (!msr_is_64bit(env, msr)) { nip = (uint32_t)nip; - msr = (uint32_t)(msr & msrm); - if (keep_msrh) { - msr |= env->msr & ~((uint64_t)0xFFFFFFFF); - } } #else nip = (uint32_t)nip; - msr &= (uint32_t)msrm;
This is the first patch,: It's store MSR bits differntly for different rfi instructions: [Qemu-devel] [PATCH] target-ppc: fix RFI by clearing some bits of MSR https://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02999.html Comes from target-ppc: fix RFI by clearing some bits of MSR SHA-1: c3d420ead1aee9fcfd12be11cbdf6b1620134773 target-ppc/op_helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) ``` #endif /* XXX: beware: this is false if VLE is supported */ env->nip = nip & ~((target_ulong)0x00000003); @@ -959,26 +954,24 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr, void helper_rfi(CPUPPCState *env) { - if (env->excp_model == POWERPC_EXCP_BOOKE) { - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], - ~((target_ulong)0), 0); - } else { - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], - ~((target_ulong)0x783F0000), 1); - } + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful); } +#define MSR_BOOK3S_MASK #if defined(TARGET_PPC64) void helper_rfid(CPUPPCState *env) { - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], - ~((target_ulong)0x783F0000), 0); + /* The architeture defines a number of rules for which bits + * can change but in practice, we handle this in hreg_store_msr() + * which will be called by do_rfi(), so there is no need to filter + * here + */ + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1]); } void helper_hrfid(CPUPPCState *env) { - do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1], - ~((target_ulong)0x783F0000), 0); + do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]); } #endif @@ -986,28 +979,24 @@ void helper_hrfid(CPUPPCState *env) /* Embedded PowerPC specific helpers */ void helper_40x_rfci(CPUPPCState *env) { - do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3], - ~((target_ulong)0xFFFF0000), 0); + do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3]); } void helper_rfci(CPUPPCState *env) { - do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1], - ~((target_ulong)0), 0); + do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1]); } void helper_rfdi(CPUPPCState *env) { /* FIXME: choose CSRR1 or DSRR1 based on cpu type */ - do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1], - ~((target_ulong)0), 0); + do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1]); } void helper_rfmci(CPUPPCState *env) { /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */ - do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1], - ~((target_ulong)0), 0); + do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]); } #endif @@ -1045,7 +1034,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1, target_ulong arg2, void helper_rfsvc(CPUPPCState *env) { - do_rfi(env, env->lr, env->ctr, 0x0000FFFF, 0); + do_rfi(env, env->lr, env->ctr & 0x0000FFFF); } /* Embedded.Processor Control */ ``` And of cause, the second patch fixes some problem, but also cause new problem, how to implement these instruction properly? -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo