Re: [Qemu-devel] [PATCH] target-mips: use CP0EnLo_XI instead of magic number

2015-01-29 Thread Leon Alrae 
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

Re: [Qemu-devel] [PATCH] target-mips: use CP0EnLo_XI instead of magic number

2015-01-29 Thread Maciej W. Rozycki 
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

Re: [Qemu-devel] [PATCH] target-mips: use CP0EnLo_XI instead of magic number

2015-01-28 Thread Maciej W. Rozycki 
On Mon, 26 Jan 2015, Leon Alrae wrote:

 Signed-off-by: Leon Alrae leon.al...@imgtec.com
 ---

 Enthusiastically:

Reviewed-by: Maciej W. Rozycki ma...@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

[Qemu-devel] [PATCH] target-mips: use CP0EnLo_XI instead of magic number

2015-01-26 Thread Leon Alrae 
Signed-off-by: Leon Alrae leon.al...@imgtec.com
---
 target-mips/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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);
-- 
2.1.0