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

[Leon Alrae <leon.alrae@imgtec.com>]

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