Dear Jamie,
Thank you very much for comment.
Jamie Lokier wrote:
>TAKEDA, toshiya wrote:
>> @@ -940,7 +966,15 @@ void cpu_x86_set_a20(CPUX86State *env, int a20_state)
>> /* when a20 is changed, all the MMU mappings are invalid, so
>> we must flush everything */
>> tlb_flush(env, 1);
>> - env->a20_mask = ~(1 << 20) | (a20_state << 20);
>> + if (env->private_features & PRIVATE_FEATURE_PC98_A20MASK) {
>> + if (a20_state) {
>> + env->a20_mask = ~0x0;
>> + } else {
>> + env->a20_mask = 0xfffff;
>> + }
>> + } else {
>> + env->a20_mask = ~(1 << 20) | (a20_state << 20);
>> + }
>> }
>> }
>
>It seems strange to mix different styles in that way. How about this,
>which I think makes the PC98 vs. PC difference clearer too:
>
> if (a20_state) {
> env->a20_mask = ~0x0;
> } else if (env->private_features & PRIVATE_FEATURE_PC98_A20MASK) {
> env->a20_mask = (1 << 20) - 1; /* A20 masks all bits >= 20. */
> } else {
> env->a20_mask = ~(1 << 20); /* A20 masks only bit 20. */
> }
I agree.
I will repost new patch later.
>(When I say clearer, it's not _that_ obvious that these or the
>original expressions store the right thing into "uint64
>env->a20_mask", given these expressions are all of type "int", but, in
>fact, they do owing to C type promotion rules. Same for the save/load
>state).
Well, I hope I dont deal with this item now in pc98 patches...
Thanks,
TAKEDA, toshiya
>
>-- Jamie
>
>
>
