[RFC PATCH] target/i386: terminate a tcg block on gen_RDMSR

[RFC PATCH] target/i386: terminate a tcg block on gen_RDMSR

[Alex Bennée]

End the block the same way that gen_WRMSR does so an IO access under
icount. While this fixes this particular case I suspect there are more
IO like accesses the x86 TCG front-end should take care off.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4779
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/i386/tcg/emit.c.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index ed56edf8a3a..911bce5f539 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -3783,6 +3783,7 @@ static void gen_RDMSR(DisasContext *s, X86DecodedInsn *decode)
     gen_update_cc_op(s);
     gen_update_eip_cur(s);
     gen_helper_rdmsr(tcg_env);
+    s->base.is_jmp = DISAS_EOB_NEXT;
 }
 #else
 #define gen_RDMSR gen_unreachable
-- 
2.47.3

Re: [RFC PATCH] target/i386: terminate a tcg block on gen_RDMSR

[Richard Henderson]

On 10/29/26 12:17, Alex Bennée wrote:
> End the block the same way that gen_WRMSR does so an IO access under
> icount. While this fixes this particular case I suspect there are more
> IO like accesses the x86 TCG front-end should take care off.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4779
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   target/i386/tcg/emit.c.inc | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
> index ed56edf8a3a..911bce5f539 100644
> --- a/target/i386/tcg/emit.c.inc
> +++ b/target/i386/tcg/emit.c.inc
> @@ -3783,6 +3783,7 @@ static void gen_RDMSR(DisasContext *s, X86DecodedInsn *decode)
>       gen_update_cc_op(s);
>       gen_update_eip_cur(s);
>       gen_helper_rdmsr(tcg_env);
> +    s->base.is_jmp = DISAS_EOB_NEXT;
>   }
>   #else
>   #define gen_RDMSR gen_unreachable

You've done nothing to update state.  I think you want

     translator_io_start(&s->base);

which will go though DISAS_TOO_MANY.


r~

Re: [RFC PATCH] target/i386: terminate a tcg block on gen_RDMSR

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 10/29/26 12:17, Alex Bennée wrote:
>> End the block the same way that gen_WRMSR does so an IO access under
>> icount. While this fixes this particular case I suspect there are more
>> IO like accesses the x86 TCG front-end should take care off.
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4779
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   target/i386/tcg/emit.c.inc | 1 +
>>   1 file changed, 1 insertion(+)
>> diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
>> index ed56edf8a3a..911bce5f539 100644
>> --- a/target/i386/tcg/emit.c.inc
>> +++ b/target/i386/tcg/emit.c.inc
>> @@ -3783,6 +3783,7 @@ static void gen_RDMSR(DisasContext *s, X86DecodedInsn *decode)
>>       gen_update_cc_op(s);
>>       gen_update_eip_cur(s);
>>       gen_helper_rdmsr(tcg_env);
>> +    s->base.is_jmp = DISAS_EOB_NEXT;
>>   }
>>   #else
>>   #define gen_RDMSR gen_unreachable
>
> You've done nothing to update state.  I think you want
>
>     translator_io_start(&s->base);
>
> which will go though DISAS_TOO_MANY.

Should WRMSR do the same? If not why is it different?

>
>
> r~

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH] target/i386: terminate a tcg block on gen_RDMSR

[Richard Henderson]

On 10/30/26 07:54, Alex Bennée wrote:
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 10/29/26 12:17, Alex Bennée wrote:
>>> End the block the same way that gen_WRMSR does so an IO access under
>>> icount. While this fixes this particular case I suspect there are more
>>> IO like accesses the x86 TCG front-end should take care off.
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4779
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>    target/i386/tcg/emit.c.inc | 1 +
>>>    1 file changed, 1 insertion(+)
>>> diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
>>> index ed56edf8a3a..911bce5f539 100644
>>> --- a/target/i386/tcg/emit.c.inc
>>> +++ b/target/i386/tcg/emit.c.inc
>>> @@ -3783,6 +3783,7 @@ static void gen_RDMSR(DisasContext *s, X86DecodedInsn *decode)
>>>        gen_update_cc_op(s);
>>>        gen_update_eip_cur(s);
>>>        gen_helper_rdmsr(tcg_env);
>>> +    s->base.is_jmp = DISAS_EOB_NEXT;
>>>    }
>>>    #else
>>>    #define gen_RDMSR gen_unreachable
>>
>> You've done nothing to update state.  I think you want
>>
>>      translator_io_start(&s->base);
>>
>> which will go though DISAS_TOO_MANY.
> 
> Should WRMSR do the same? If not why is it different?
Oops, mis-read that.  Just above is updating.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~