Message ID | 1422289059-6849-1-git-send-email-leon.alrae@imgtec.com |
---|---|
State | New |
Headers | show |
On Mon, 26 Jan 2015, Leon Alrae wrote: > Signed-off-by: Leon Alrae <leon.alrae@imgtec.com> > --- Enthusiastically: Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org> However... > diff --git a/target-mips/translate.c b/target-mips/translate.c > index 635192c..77d89be 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -4947,7 +4947,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel) > #if defined(TARGET_MIPS64) > if (ctx->rxi) { > TCGv tmp = tcg_temp_new(); > - tcg_gen_andi_tl(tmp, arg, (3ull << 62)); > + tcg_gen_andi_tl(tmp, arg, (3ull << CP0EnLo_XI)); > tcg_gen_shri_tl(tmp, tmp, 32); ... don't we need to do: tcg_gen_andi_tl(arg, arg, ~(3ull << CP0EnLo_XI)); here and for EntryLo1 as well (for LPA-enabled processors)? > tcg_gen_or_tl(arg, arg, tmp); > tcg_temp_free(tmp); And do we want to have CP0C3_LPA set in the few templates that do in the first place? AFAICT we don't really implement LPA so this bit will confuse software. Of course implementing it would be another option, not very complicated AFAICS, and if we can track the requirement to update MFC0 at that time, then the clean-up I mentioned above can be deferred until then. Maciej
On 28/01/2015 23:11, Maciej W. Rozycki wrote: >> @@ -4947,7 +4947,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel) >> #if defined(TARGET_MIPS64) >> if (ctx->rxi) { >> TCGv tmp = tcg_temp_new(); >> - tcg_gen_andi_tl(tmp, arg, (3ull << 62)); >> + tcg_gen_andi_tl(tmp, arg, (3ull << CP0EnLo_XI)); >> tcg_gen_shri_tl(tmp, tmp, 32); > > ... don't we need to do: > > tcg_gen_andi_tl(arg, arg, ~(3ull << CP0EnLo_XI)); > > here and for EntryLo1 as well (for LPA-enabled processors)? Yes, this would be required if we supported LPA - good spot. > >> tcg_gen_or_tl(arg, arg, tmp); >> tcg_temp_free(tmp); > > And do we want to have CP0C3_LPA set in the few templates that do in the > first place? AFAICT we don't really implement LPA so this bit will > confuse software. Of course implementing it would be another option, not > very complicated AFAICS, and if we can track the requirement to update > MFC0 at that time, then the clean-up I mentioned above can be deferred > until then. In general I don't think it's a good idea to indicate presence of a feature in a CPU config if it isn't implemented at all in QEMU -- as you said, it will confuse software. As far as LPA goes I've got already an implementation of it (and XPA as well) which I haven't submitted upstream yet. I'll make sure it contains the change you suggested. Thanks, Leon
On Thu, 29 Jan 2015, Leon Alrae wrote: > > And do we want to have CP0C3_LPA set in the few templates that do in the > > first place? AFAICT we don't really implement LPA so this bit will > > confuse software. Of course implementing it would be another option, not > > very complicated AFAICS, and if we can track the requirement to update > > MFC0 at that time, then the clean-up I mentioned above can be deferred > > until then. > > In general I don't think it's a good idea to indicate presence of a > feature in a CPU config if it isn't implemented at all in QEMU -- as you > said, it will confuse software. As far as LPA goes I've got already an > implementation of it (and XPA as well) which I haven't submitted > upstream yet. I'll make sure it contains the change you suggested. Great, I'll leave it to you to sort out then. Thanks! Maciej
diff --git a/target-mips/translate.c b/target-mips/translate.c index 635192c..77d89be 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -4947,7 +4947,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel) #if defined(TARGET_MIPS64) if (ctx->rxi) { TCGv tmp = tcg_temp_new(); - tcg_gen_andi_tl(tmp, arg, (3ull << 62)); + tcg_gen_andi_tl(tmp, arg, (3ull << CP0EnLo_XI)); tcg_gen_shri_tl(tmp, tmp, 32); tcg_gen_or_tl(arg, arg, tmp); tcg_temp_free(tmp); @@ -5002,7 +5002,7 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel) #if defined(TARGET_MIPS64) if (ctx->rxi) { TCGv tmp = tcg_temp_new(); - tcg_gen_andi_tl(tmp, arg, (3ull << 62)); + tcg_gen_andi_tl(tmp, arg, (3ull << CP0EnLo_XI)); tcg_gen_shri_tl(tmp, tmp, 32); tcg_gen_or_tl(arg, arg, tmp); tcg_temp_free(tmp);
Signed-off-by: Leon Alrae <leon.alrae@imgtec.com> --- target-mips/translate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)