On 12/05/2016 01:41 AM, Jin Guojie wrote:
> ------------------ Original ------------------
> From: "Richard Henderson";<r...@twiddle.net>;
> Send time: Thursday, Dec 1, 2016 11:52 PM
> To: "Jin Guojie"<jinguo...@loongson.cn>; "qemu-devel"<qemu-devel@nongnu.org>;
> Cc: "Aurelien Jarno"<aurel...@aurel32.net>; "James
> Hogan"<james.ho...@imgtec.com>;
> Subject: Re: [PATCH v5 00/10] tcg mips64 and mips r6 improvements
>
> Thanks a lot for Richard's careful review.
> I have some different opinions for discussion:
>
>> @@ -1233,7 +1235,8 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg
>> base,
>> - mask = (target_ulong)TARGET_PAGE_MASK | ((1 << a_bits) - 1);
>> + mask = TARGET_PAGE_MASK | ((1 << a_bits) - 1);
>> You need the target_ulong cast here for mips64.
>> See patch ebb90a005da67147245cd38fb04a965a87a961b7.
>
> Since mask is defined as a C type "target_ulong" at the beginning of the
> function,
> I guess an implicit type cast should be already done by the compiler.
> So your change is functionally the same with v5 patch.
> To test my idea, I wrote a small case and compiled it on an x86_64 host:
>
> main()
> {
> int a_bits = 2;
> int page_mask = 0xfffff000; /* X86 4KB page*/
> unsigned int mask = page_mask | ((1 << a_bits) - 1);
> unsigned long m = mask;
> printf("mask=%lx\n", m);
> }
>
> $ gcc a.c
> $ file a.out
> a.out: Mach-O 64-bit executable x86_64
> $ ./a.out
> mask=fffff003
You misunderstand. For a 64-bit guest, the result we're looking for is
0xfffffffffffff003.
(1) the type of a_bits is unsigned int, not int,
(2) which means the expression "page_mask | ((1 << a_bits) - 1)"
becomes unsigned int,
(3) which means that the assignment to mask gets truncated.
> The output is exactly an unsigned 32-bit quantity.
> I didn't see a wrong result generated.
> Could you teach me where is my mistake?
For a 64-bit guest we need a 64-bit quantity. For a 32-bit guest... we need to
match the load that we issue from cmp_off. If use use LWU, then we need an
unsigned 32-bit quantity; if we use LW, then we need a signed 32-bit quantity.
r~
