[PATCH 0/6] target: Use env_archcpu() instead of ARCH_CPU(env_cpu(env))

[PATCH 0/6] target: Use env_archcpu() instead of ARCH_CPU(env_cpu(env))

[Philippe Mathieu-Daudé]

When we have a CPUArchState* pointer we can directly get the
corresponding ArchCPU* pointer with env_archcpu(). No need to
use QOM cast which is slower due when building with
--enable-qom-cast-debug.

This series replace ARCH_CPU(env_cpu(env)) by env_archcpu(env)
when possible.

Philippe Mathieu-Daudé (6):
  target/ppc: Use env_archcpu() in helper_book3s_msgsndp()
  target/riscv: Use env_archcpu() in [check_]nanbox()
  target/s390x: Use env_archcpu() in handle_diag_308()
  target/xtensa: Use env_archcpu() in update_c[compare|count]()
  target/i386/hvf: Use x86_cpu in simulate_[rdmsr|wrmsr]()
  target/i386: Use env_archcpu() in simulate_[rdmsr/wrmsr]()

 target/i386/hvf/x86_emu.h |  4 ++--
 target/riscv/internals.h  |  8 ++++----
 target/i386/hvf/hvf.c     |  4 ++--
 target/i386/hvf/x86_emu.c | 25 ++++++++++++-------------
 target/ppc/excp_helper.c  |  2 +-
 target/s390x/diag.c       |  2 +-
 target/xtensa/op_helper.c |  4 ++--
 7 files changed, 24 insertions(+), 25 deletions(-)

-- 
2.41.0

[PATCH 1/6] target/ppc: Use env_archcpu() in helper_book3s_msgsndp()

[Philippe Mathieu-Daudé]

When CPUArchState* is available (here CPUPPCState*), we
can use the fast env_archcpu() macro to get ArchCPU* (here
PowerPCCPU*). The QOM cast POWERPC_CPU() macro will be
slower when building with --enable-qom-cast-debug.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/excp_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 7926114d5c..a42743a3e0 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -3136,7 +3136,7 @@ void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb)
 void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb)
 {
     CPUState *cs = env_cpu(env);
-    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    PowerPCCPU *cpu = env_archcpu(env);
     CPUState *ccs;
     uint32_t nr_threads = cs->nr_threads;
     int ttir = rb & PPC_BITMASK(57, 63);
-- 
2.41.0

Re: [PATCH 1/6] target/ppc: Use env_archcpu() in helper_book3s_msgsndp()

[Daniel Henrique Barboza]

On 10/9/23 08:02, Philippe Mathieu-Daudé wrote:
> When CPUArchState* is available (here CPUPPCState*), we
> can use the fast env_archcpu() macro to get ArchCPU* (here
> PowerPCCPU*). The QOM cast POWERPC_CPU() macro will be
> slower when building with --enable-qom-cast-debug.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/ppc/excp_helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 7926114d5c..a42743a3e0 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -3136,7 +3136,7 @@ void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb)
>   void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb)
>   {
>       CPUState *cs = env_cpu(env);
> -    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    PowerPCCPU *cpu = env_archcpu(env);


Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>       CPUState *ccs;
>       uint32_t nr_threads = cs->nr_threads;
>       int ttir = rb & PPC_BITMASK(57, 63);

Re: [PATCH 1/6] target/ppc: Use env_archcpu() in helper_book3s_msgsndp()

[Alistair Francis]

On Mon, Oct 9, 2023 at 9:03 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> When CPUArchState* is available (here CPUPPCState*), we
> can use the fast env_archcpu() macro to get ArchCPU* (here
> PowerPCCPU*). The QOM cast POWERPC_CPU() macro will be
> slower when building with --enable-qom-cast-debug.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/ppc/excp_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 7926114d5c..a42743a3e0 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -3136,7 +3136,7 @@ void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb)
>  void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb)
>  {
>      CPUState *cs = env_cpu(env);
> -    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    PowerPCCPU *cpu = env_archcpu(env);
>      CPUState *ccs;
>      uint32_t nr_threads = cs->nr_threads;
>      int ttir = rb & PPC_BITMASK(57, 63);
> --
> 2.41.0
>
>

[PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[Philippe Mathieu-Daudé]

When CPUArchState* is available (here CPURISCVState*), we
can use the fast env_archcpu() macro to get ArchCPU* (here
RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
when building with --enable-qom-cast-debug.

Inspired-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/riscv/internals.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index b5f823c7ec..8239ae83cc 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -87,7 +87,7 @@ enum {
 static inline uint64_t nanbox_s(CPURISCVState *env, float32 f)
 {
     /* the value is sign-extended instead of NaN-boxing for zfinx */
-    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+    if (env_archcpu(env)->cfg.ext_zfinx) {
         return (int32_t)f;
     } else {
         return f | MAKE_64BIT_MASK(32, 32);
@@ -97,7 +97,7 @@ static inline uint64_t nanbox_s(CPURISCVState *env, float32 f)
 static inline float32 check_nanbox_s(CPURISCVState *env, uint64_t f)
 {
     /* Disable NaN-boxing check when enable zfinx */
-    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+    if (env_archcpu(env)->cfg.ext_zfinx) {
         return (uint32_t)f;
     }
 
@@ -113,7 +113,7 @@ static inline float32 check_nanbox_s(CPURISCVState *env, uint64_t f)
 static inline uint64_t nanbox_h(CPURISCVState *env, float16 f)
 {
     /* the value is sign-extended instead of NaN-boxing for zfinx */
-    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+    if (env_archcpu(env)->cfg.ext_zfinx) {
         return (int16_t)f;
     } else {
         return f | MAKE_64BIT_MASK(16, 48);
@@ -123,7 +123,7 @@ static inline uint64_t nanbox_h(CPURISCVState *env, float16 f)
 static inline float16 check_nanbox_h(CPURISCVState *env, uint64_t f)
 {
     /* Disable nanbox check when enable zfinx */
-    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+    if (env_archcpu(env)->cfg.ext_zfinx) {
         return (uint16_t)f;
     }
 
-- 
2.41.0

Re: [PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[Daniel Henrique Barboza]

On 10/9/23 08:02, Philippe Mathieu-Daudé wrote:
> When CPUArchState* is available (here CPURISCVState*), we
> can use the fast env_archcpu() macro to get ArchCPU* (here
> RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
> when building with --enable-qom-cast-debug.
> 
> Inspired-by: Richard W.M. Jones <rjones@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>


>   target/riscv/internals.h | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index b5f823c7ec..8239ae83cc 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -87,7 +87,7 @@ enum {
>   static inline uint64_t nanbox_s(CPURISCVState *env, float32 f)
>   {
>       /* the value is sign-extended instead of NaN-boxing for zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>           return (int32_t)f;
>       } else {
>           return f | MAKE_64BIT_MASK(32, 32);
> @@ -97,7 +97,7 @@ static inline uint64_t nanbox_s(CPURISCVState *env, float32 f)
>   static inline float32 check_nanbox_s(CPURISCVState *env, uint64_t f)
>   {
>       /* Disable NaN-boxing check when enable zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>           return (uint32_t)f;
>       }
>   
> @@ -113,7 +113,7 @@ static inline float32 check_nanbox_s(CPURISCVState *env, uint64_t f)
>   static inline uint64_t nanbox_h(CPURISCVState *env, float16 f)
>   {
>       /* the value is sign-extended instead of NaN-boxing for zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>           return (int16_t)f;
>       } else {
>           return f | MAKE_64BIT_MASK(16, 48);
> @@ -123,7 +123,7 @@ static inline uint64_t nanbox_h(CPURISCVState *env, float16 f)
>   static inline float16 check_nanbox_h(CPURISCVState *env, uint64_t f)
>   {
>       /* Disable nanbox check when enable zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>           return (uint16_t)f;
>       }
>   

Re: [PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[LIU Zhiwei]

On 2023/10/9 19:02, Philippe Mathieu-Daudé wrote:
> When CPUArchState* is available (here CPURISCVState*), we
> can use the fast env_archcpu() macro to get ArchCPU* (here
> RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
> when building with --enable-qom-cast-debug.
>
> Inspired-by: Richard W.M. Jones <rjones@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

By the way, does the community has the plan to support heterogeneous 
architecture cpus in one soc?
If so, maybe we have to do this qom cast somewhere.

Zhiwei

> ---
>   target/riscv/internals.h | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index b5f823c7ec..8239ae83cc 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -87,7 +87,7 @@ enum {
>   static inline uint64_t nanbox_s(CPURISCVState *env, float32 f)
>   {
>       /* the value is sign-extended instead of NaN-boxing for zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>           return (int32_t)f;
>       } else {
>           return f | MAKE_64BIT_MASK(32, 32);
> @@ -97,7 +97,7 @@ static inline uint64_t nanbox_s(CPURISCVState *env, float32 f)
>   static inline float32 check_nanbox_s(CPURISCVState *env, uint64_t f)
>   {
>       /* Disable NaN-boxing check when enable zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>           return (uint32_t)f;
>       }
>   
> @@ -113,7 +113,7 @@ static inline float32 check_nanbox_s(CPURISCVState *env, uint64_t f)
>   static inline uint64_t nanbox_h(CPURISCVState *env, float16 f)
>   {
>       /* the value is sign-extended instead of NaN-boxing for zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>           return (int16_t)f;
>       } else {
>           return f | MAKE_64BIT_MASK(16, 48);
> @@ -123,7 +123,7 @@ static inline uint64_t nanbox_h(CPURISCVState *env, float16 f)
>   static inline float16 check_nanbox_h(CPURISCVState *env, uint64_t f)
>   {
>       /* Disable nanbox check when enable zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>           return (uint16_t)f;
>       }
>   

Re: [PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[Richard Henderson]

On 10/9/23 05:42, LIU Zhiwei wrote:
> 
> On 2023/10/9 19:02, Philippe Mathieu-Daudé wrote:
>> When CPUArchState* is available (here CPURISCVState*), we
>> can use the fast env_archcpu() macro to get ArchCPU* (here
>> RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
>> when building with --enable-qom-cast-debug.
>>
>> Inspired-by: Richard W.M. Jones <rjones@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> 
> By the way, does the community has the plan to support heterogeneous architecture cpus in 
> one soc?

Yes.

> If so, maybe we have to do this qom cast somewhere.

No, I don't think so.  Or at least not in these places.


r~

Re: [PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[LIU Zhiwei]

On 2023/10/11 1:04, Richard Henderson wrote:
> On 10/9/23 05:42, LIU Zhiwei wrote:
>>
>> On 2023/10/9 19:02, Philippe Mathieu-Daudé wrote:
>>> When CPUArchState* is available (here CPURISCVState*), we
>>> can use the fast env_archcpu() macro to get ArchCPU* (here
>>> RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
>>> when building with --enable-qom-cast-debug.
>>>
>>> Inspired-by: Richard W.M. Jones <rjones@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>>
>> By the way, does the community has the plan to support heterogeneous 
>> architecture cpus in one soc?
>
> Yes.

Hi Richard,

It's a good news. Thanks.

>
>> If so, maybe we have to do this qom cast somewhere.
>
> No, I don't think so.  Or at least not in these places.

Yes.  Perhaps, we should remove all RISCV_CPU macros using after the qom 
objects realized.

Do you think we should remove the RISCV_CPU using in 
riscv_cpu_exec_interrupt? Although it  is not so hot. I think there is 
no reason to use it there.
Except this, there are many other places in hw/ and target/riscv using 
the RISCV_CPU macro.

If we know whether we should remove the RISCV_CPU macro use in these 
places, we can do it in another patch.

Thanks,
Zhiwei

>
>
> r~

Re: [PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[Philippe Mathieu-Daudé]

On 11/10/23 05:25, LIU Zhiwei wrote:
> 
> On 2023/10/11 1:04, Richard Henderson wrote:
>> On 10/9/23 05:42, LIU Zhiwei wrote:
>>>
>>> On 2023/10/9 19:02, Philippe Mathieu-Daudé wrote:
>>>> When CPUArchState* is available (here CPURISCVState*), we
>>>> can use the fast env_archcpu() macro to get ArchCPU* (here
>>>> RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
>>>> when building with --enable-qom-cast-debug.


>>> If so, maybe we have to do this qom cast somewhere.
>>
>> No, I don't think so.  Or at least not in these places.
> 
> Yes.  Perhaps, we should remove all RISCV_CPU macros using after the qom 
> objects realized.
> 
> Do you think we should remove the RISCV_CPU using in 
> riscv_cpu_exec_interrupt? Although it  is not so hot. I think there is 
> no reason to use it there.

I have some note in my TODO to check replacing CPUState by ArchCPU in
TCGCPUOps (like the cpu_exec_interrupt handler you mentioned). However
I'm running out of time, so feel free to try it.

Using ArchCPU avoids the cast in target code.

> Except this, there are many other places in hw/ and target/riscv using 
> the RISCV_CPU macro.

If a method is exposed as API, we need to check the type. After that
for internal calls this is pointless.

> If we know whether we should remove the RISCV_CPU macro use in these 
> places, we can do it in another patch.
> 
> Thanks,
> Zhiwei
> 
>>
>>
>> r~

Re: [PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[LIU Zhiwei]

On 2023/10/11 13:31, Philippe Mathieu-Daudé wrote:
> On 11/10/23 05:25, LIU Zhiwei wrote:
>>
>> On 2023/10/11 1:04, Richard Henderson wrote:
>>> On 10/9/23 05:42, LIU Zhiwei wrote:
>>>>
>>>> On 2023/10/9 19:02, Philippe Mathieu-Daudé wrote:
>>>>> When CPUArchState* is available (here CPURISCVState*), we
>>>>> can use the fast env_archcpu() macro to get ArchCPU* (here
>>>>> RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
>>>>> when building with --enable-qom-cast-debug.
>
>
>>>> If so, maybe we have to do this qom cast somewhere.
>>>
>>> No, I don't think so.  Or at least not in these places.
>>
>> Yes.  Perhaps, we should remove all RISCV_CPU macros using after the 
>> qom objects realized.
>>
>> Do you think we should remove the RISCV_CPU using in 
>> riscv_cpu_exec_interrupt? Although it  is not so hot. I think there 
>> is no reason to use it there.
>
> I have some note in my TODO to check replacing CPUState by ArchCPU in
> TCGCPUOps (like the cpu_exec_interrupt handler you mentioned). 

IMHO, this will make it harder for heterogeneous SOC support. ArchCPU is 
not a target agnostic struct.

I must miss something.

> However
> I'm running out of time, so feel free to try it.
I'd like to have a try if it will not block the heterogeneous SOC support.
>
> Using ArchCPU avoids the cast in target code.
Yes
>
>> Except this, there are many other places in hw/ and target/riscv 
>> using the RISCV_CPU macro.
>
> If a method is exposed as API, we need to check the type. After that
> for internal calls this is pointless.

Make sense. Thanks.

Zhiwei

>
>> If we know whether we should remove the RISCV_CPU macro use in these 
>> places, we can do it in another patch.
>>
>> Thanks,
>> Zhiwei
>>
>>>
>>>
>>> r~

Re: [PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[Richard Henderson]

On 10/11/23 22:59, LIU Zhiwei wrote:
> 
> On 2023/10/11 13:31, Philippe Mathieu-Daudé wrote:
>> On 11/10/23 05:25, LIU Zhiwei wrote:
>>>
>>> On 2023/10/11 1:04, Richard Henderson wrote:
>>>> On 10/9/23 05:42, LIU Zhiwei wrote:
>>>>>
>>>>> On 2023/10/9 19:02, Philippe Mathieu-Daudé wrote:
>>>>>> When CPUArchState* is available (here CPURISCVState*), we
>>>>>> can use the fast env_archcpu() macro to get ArchCPU* (here
>>>>>> RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
>>>>>> when building with --enable-qom-cast-debug.
>>
>>
>>>>> If so, maybe we have to do this qom cast somewhere.
>>>>
>>>> No, I don't think so.  Or at least not in these places.
>>>
>>> Yes.  Perhaps, we should remove all RISCV_CPU macros using after the qom objects realized.
>>>
>>> Do you think we should remove the RISCV_CPU using in riscv_cpu_exec_interrupt? Although 
>>> it  is not so hot. I think there is no reason to use it there.
>>
>> I have some note in my TODO to check replacing CPUState by ArchCPU in
>> TCGCPUOps (like the cpu_exec_interrupt handler you mentioned). 
> 
> IMHO, this will make it harder for heterogeneous SOC support. ArchCPU is not a target 
> agnostic struct.

ArchCPU is a target-agnostic typedef of a structure with no visible definition.
C is perfectly happy to manipulate pointers to such structures.

Whether it is worthwhile to adjust interfaces from CPUState to ArchCPU, I don't know.


r~

Re: [PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[LIU Zhiwei]

On 2023/10/13 0:06, Richard Henderson wrote:
> On 10/11/23 22:59, LIU Zhiwei wrote:
>>
>> On 2023/10/11 13:31, Philippe Mathieu-Daudé wrote:
>>> On 11/10/23 05:25, LIU Zhiwei wrote:
>>>>
>>>> On 2023/10/11 1:04, Richard Henderson wrote:
>>>>> On 10/9/23 05:42, LIU Zhiwei wrote:
>>>>>>
>>>>>> On 2023/10/9 19:02, Philippe Mathieu-Daudé wrote:
>>>>>>> When CPUArchState* is available (here CPURISCVState*), we
>>>>>>> can use the fast env_archcpu() macro to get ArchCPU* (here
>>>>>>> RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
>>>>>>> when building with --enable-qom-cast-debug.
>>>
>>>
>>>>>> If so, maybe we have to do this qom cast somewhere.
>>>>>
>>>>> No, I don't think so.  Or at least not in these places.
>>>>
>>>> Yes.  Perhaps, we should remove all RISCV_CPU macros using after 
>>>> the qom objects realized.
>>>>
>>>> Do you think we should remove the RISCV_CPU using in 
>>>> riscv_cpu_exec_interrupt? Although it  is not so hot. I think there 
>>>> is no reason to use it there.
>>>
>>> I have some note in my TODO to check replacing CPUState by ArchCPU in
>>> TCGCPUOps (like the cpu_exec_interrupt handler you mentioned). 
>>
>> IMHO, this will make it harder for heterogeneous SOC support. ArchCPU 
>> is not a target agnostic struct.
>
> ArchCPU is a target-agnostic typedef of a structure with no visible 
> definition.
> C is perfectly happy to manipulate pointers to such structures.
>
Get it. Thanks
> Whether it is worthwhile to adjust interfaces from CPUState to 
> ArchCPU, I don't know.

OK. Let's just wait for a good reason to do that.

Zhiwei

>
>
> r~

Re: [PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[Richard W.M. Jones]

On Mon, Oct 09, 2023 at 01:02:35PM +0200, Philippe Mathieu-Daudé wrote:
> When CPUArchState* is available (here CPURISCVState*), we
> can use the fast env_archcpu() macro to get ArchCPU* (here
> RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
> when building with --enable-qom-cast-debug.
> 
> Inspired-by: Richard W.M. Jones <rjones@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/riscv/internals.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index b5f823c7ec..8239ae83cc 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -87,7 +87,7 @@ enum {
>  static inline uint64_t nanbox_s(CPURISCVState *env, float32 f)
>  {
>      /* the value is sign-extended instead of NaN-boxing for zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>          return (int32_t)f;
>      } else {
>          return f | MAKE_64BIT_MASK(32, 32);
> @@ -97,7 +97,7 @@ static inline uint64_t nanbox_s(CPURISCVState *env, float32 f)
>  static inline float32 check_nanbox_s(CPURISCVState *env, uint64_t f)
>  {
>      /* Disable NaN-boxing check when enable zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>          return (uint32_t)f;
>      }
>  
> @@ -113,7 +113,7 @@ static inline float32 check_nanbox_s(CPURISCVState *env, uint64_t f)
>  static inline uint64_t nanbox_h(CPURISCVState *env, float16 f)
>  {
>      /* the value is sign-extended instead of NaN-boxing for zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>          return (int16_t)f;
>      } else {
>          return f | MAKE_64BIT_MASK(16, 48);
> @@ -123,7 +123,7 @@ static inline uint64_t nanbox_h(CPURISCVState *env, float16 f)
>  static inline float16 check_nanbox_h(CPURISCVState *env, uint64_t f)
>  {
>      /* Disable nanbox check when enable zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>          return (uint16_t)f;
>      }

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Re: [PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[Alistair Francis]

On Mon, Oct 9, 2023 at 9:03 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> When CPUArchState* is available (here CPURISCVState*), we
> can use the fast env_archcpu() macro to get ArchCPU* (here
> RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
> when building with --enable-qom-cast-debug.
>
> Inspired-by: Richard W.M. Jones <rjones@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/internals.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index b5f823c7ec..8239ae83cc 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -87,7 +87,7 @@ enum {
>  static inline uint64_t nanbox_s(CPURISCVState *env, float32 f)
>  {
>      /* the value is sign-extended instead of NaN-boxing for zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>          return (int32_t)f;
>      } else {
>          return f | MAKE_64BIT_MASK(32, 32);
> @@ -97,7 +97,7 @@ static inline uint64_t nanbox_s(CPURISCVState *env, float32 f)
>  static inline float32 check_nanbox_s(CPURISCVState *env, uint64_t f)
>  {
>      /* Disable NaN-boxing check when enable zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>          return (uint32_t)f;
>      }
>
> @@ -113,7 +113,7 @@ static inline float32 check_nanbox_s(CPURISCVState *env, uint64_t f)
>  static inline uint64_t nanbox_h(CPURISCVState *env, float16 f)
>  {
>      /* the value is sign-extended instead of NaN-boxing for zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>          return (int16_t)f;
>      } else {
>          return f | MAKE_64BIT_MASK(16, 48);
> @@ -123,7 +123,7 @@ static inline uint64_t nanbox_h(CPURISCVState *env, float16 f)
>  static inline float16 check_nanbox_h(CPURISCVState *env, uint64_t f)
>  {
>      /* Disable nanbox check when enable zfinx */
> -    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> +    if (env_archcpu(env)->cfg.ext_zfinx) {
>          return (uint16_t)f;
>      }
>
> --
> 2.41.0
>
>

[PATCH 3/6] target/s390x: Use env_archcpu() in handle_diag_308()

[Philippe Mathieu-Daudé]

When CPUArchState* is available (here CPUS390XState*), we
can use the fast env_archcpu() macro to get ArchCPU* (here
S390CPU*). The QOM cast S390_CPU() macro will be slower when
building with --enable-qom-cast-debug.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/s390x/diag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index 8ce18e08f3..27ffd48576 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -77,7 +77,7 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
 {
     bool valid;
     CPUState *cs = env_cpu(env);
-    S390CPU *cpu = S390_CPU(cs);
+    S390CPU *cpu = env_archcpu(env);
     uint64_t addr =  env->regs[r1];
     uint64_t subcode = env->regs[r3];
     IplParameterBlock *iplb;
-- 
2.41.0

Re: [PATCH 3/6] target/s390x: Use env_archcpu() in handle_diag_308()

[Alistair Francis]

On Mon, Oct 9, 2023 at 9:04 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> When CPUArchState* is available (here CPUS390XState*), we
> can use the fast env_archcpu() macro to get ArchCPU* (here
> S390CPU*). The QOM cast S390_CPU() macro will be slower when
> building with --enable-qom-cast-debug.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/s390x/diag.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index 8ce18e08f3..27ffd48576 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -77,7 +77,7 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
>  {
>      bool valid;
>      CPUState *cs = env_cpu(env);
> -    S390CPU *cpu = S390_CPU(cs);
> +    S390CPU *cpu = env_archcpu(env);
>      uint64_t addr =  env->regs[r1];
>      uint64_t subcode = env->regs[r3];
>      IplParameterBlock *iplb;
> --
> 2.41.0
>
>

[PATCH 4/6] target/xtensa: Use env_archcpu() in update_c[compare|count]()

[Philippe Mathieu-Daudé]

When CPUArchState* is available (here CPUXtensaState*), we
can use the fast env_archcpu() macro to get ArchCPU* (here
XtensaCPU*). The QOM cast XTENSA_CPU() macro will be slower
when building with --enable-qom-cast-debug.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/xtensa/op_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/xtensa/op_helper.c b/target/xtensa/op_helper.c
index 7bb8cd6726..496754ba57 100644
--- a/target/xtensa/op_helper.c
+++ b/target/xtensa/op_helper.c
@@ -37,7 +37,7 @@
 
 void HELPER(update_ccount)(CPUXtensaState *env)
 {
-    XtensaCPU *cpu = XTENSA_CPU(env_cpu(env));
+    XtensaCPU *cpu = env_archcpu(env);
     uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
     env->ccount_time = now;
@@ -58,7 +58,7 @@ void HELPER(wsr_ccount)(CPUXtensaState *env, uint32_t v)
 
 void HELPER(update_ccompare)(CPUXtensaState *env, uint32_t i)
 {
-    XtensaCPU *cpu = XTENSA_CPU(env_cpu(env));
+    XtensaCPU *cpu = env_archcpu(env);
     uint64_t dcc;
 
     qatomic_and(&env->sregs[INTSET],
-- 
2.41.0

Re: [PATCH 4/6] target/xtensa: Use env_archcpu() in update_c[compare|count]()

[Alistair Francis]

On Mon, Oct 9, 2023 at 9:03 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> When CPUArchState* is available (here CPUXtensaState*), we
> can use the fast env_archcpu() macro to get ArchCPU* (here
> XtensaCPU*). The QOM cast XTENSA_CPU() macro will be slower
> when building with --enable-qom-cast-debug.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/xtensa/op_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/xtensa/op_helper.c b/target/xtensa/op_helper.c
> index 7bb8cd6726..496754ba57 100644
> --- a/target/xtensa/op_helper.c
> +++ b/target/xtensa/op_helper.c
> @@ -37,7 +37,7 @@
>
>  void HELPER(update_ccount)(CPUXtensaState *env)
>  {
> -    XtensaCPU *cpu = XTENSA_CPU(env_cpu(env));
> +    XtensaCPU *cpu = env_archcpu(env);
>      uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>
>      env->ccount_time = now;
> @@ -58,7 +58,7 @@ void HELPER(wsr_ccount)(CPUXtensaState *env, uint32_t v)
>
>  void HELPER(update_ccompare)(CPUXtensaState *env, uint32_t i)
>  {
> -    XtensaCPU *cpu = XTENSA_CPU(env_cpu(env));
> +    XtensaCPU *cpu = env_archcpu(env);
>      uint64_t dcc;
>
>      qatomic_and(&env->sregs[INTSET],
> --
> 2.41.0
>
>

[PATCH 5/6] target/i386/hvf: Use x86_cpu in simulate_[rdmsr|wrmsr]()

[Philippe Mathieu-Daudé]

We already have 'x86_cpu = X86_CPU(cpu)'. Use the variable
instead of doing another QOM cast with X86_CPU().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/i386/hvf/x86_emu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
index ccda568478..af1f205ecf 100644
--- a/target/i386/hvf/x86_emu.c
+++ b/target/i386/hvf/x86_emu.c
@@ -676,7 +676,7 @@ void simulate_rdmsr(struct CPUState *cpu)
         val = rdtscp() + rvmcs(cpu->accel->fd, VMCS_TSC_OFFSET);
         break;
     case MSR_IA32_APICBASE:
-        val = cpu_get_apic_base(X86_CPU(cpu)->apic_state);
+        val = cpu_get_apic_base(x86_cpu->apic_state);
         break;
     case MSR_IA32_UCODE_REV:
         val = x86_cpu->ucode_rev;
@@ -776,7 +776,7 @@ void simulate_wrmsr(struct CPUState *cpu)
     case MSR_IA32_TSC:
         break;
     case MSR_IA32_APICBASE:
-        cpu_set_apic_base(X86_CPU(cpu)->apic_state, data);
+        cpu_set_apic_base(x86_cpu->apic_state, data);
         break;
     case MSR_FSBASE:
         wvmcs(cpu->accel->fd, VMCS_GUEST_FS_BASE, data);
-- 
2.41.0

Re: [PATCH 5/6] target/i386/hvf: Use x86_cpu in simulate_[rdmsr|wrmsr]()

[Roman Bolshakov]

On Mon, Oct 09, 2023 at 01:02:38PM +0200, Philippe Mathieu-Daudé wrote:
> We already have 'x86_cpu = X86_CPU(cpu)'. Use the variable
> instead of doing another QOM cast with X86_CPU().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Roman Bolshakov <roman@roolebo.dev>

--
Regards,
Roman

Re: [PATCH 5/6] target/i386/hvf: Use x86_cpu in simulate_[rdmsr|wrmsr]()

[Zhao Liu]

On Mon, Oct 09, 2023 at 01:02:38PM +0200, Philippe Mathieu-Daudé wrote:
> Date: Mon,  9 Oct 2023 13:02:38 +0200
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH 5/6] target/i386/hvf: Use x86_cpu in
>  simulate_[rdmsr|wrmsr]()
> X-Mailer: git-send-email 2.41.0
> 
> We already have 'x86_cpu = X86_CPU(cpu)'. Use the variable
> instead of doing another QOM cast with X86_CPU().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/i386/hvf/x86_emu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

> 
> diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
> index ccda568478..af1f205ecf 100644
> --- a/target/i386/hvf/x86_emu.c
> +++ b/target/i386/hvf/x86_emu.c
> @@ -676,7 +676,7 @@ void simulate_rdmsr(struct CPUState *cpu)
>          val = rdtscp() + rvmcs(cpu->accel->fd, VMCS_TSC_OFFSET);
>          break;
>      case MSR_IA32_APICBASE:
> -        val = cpu_get_apic_base(X86_CPU(cpu)->apic_state);
> +        val = cpu_get_apic_base(x86_cpu->apic_state);
>          break;
>      case MSR_IA32_UCODE_REV:
>          val = x86_cpu->ucode_rev;
> @@ -776,7 +776,7 @@ void simulate_wrmsr(struct CPUState *cpu)
>      case MSR_IA32_TSC:
>          break;
>      case MSR_IA32_APICBASE:
> -        cpu_set_apic_base(X86_CPU(cpu)->apic_state, data);
> +        cpu_set_apic_base(x86_cpu->apic_state, data);
>          break;
>      case MSR_FSBASE:
>          wvmcs(cpu->accel->fd, VMCS_GUEST_FS_BASE, data);
> -- 
> 2.41.0
> 
> 
> 

[PATCH 6/6] target/i386: Use env_archcpu() in simulate_[rdmsr/wrmsr]()

[Philippe Mathieu-Daudé]

When CPUArchState* is available (here CPUX86State*), we can
use the fast env_archcpu() macro to get ArchCPU* (here X86CPU*).
The QOM cast X86_CPU() macro will be slower when building with
--enable-qom-cast-debug.

Pass CPUX86State* as argument to simulate_rdmsr / simulate_wrmsr
instead of a CPUState* to avoid an extra cast.

simulate_rdmsr/simulate_rdmsr(CPUX86State

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC: Not even build-tested.
---
 target/i386/hvf/x86_emu.h |  4 ++--
 target/i386/hvf/hvf.c     |  4 ++--
 target/i386/hvf/x86_emu.c | 21 ++++++++++-----------
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/target/i386/hvf/x86_emu.h b/target/i386/hvf/x86_emu.h
index 640da90b30..4b846ba80e 100644
--- a/target/i386/hvf/x86_emu.h
+++ b/target/i386/hvf/x86_emu.h
@@ -29,8 +29,8 @@ bool exec_instruction(CPUX86State *env, struct x86_decode *ins);
 void load_regs(struct CPUState *cpu);
 void store_regs(struct CPUState *cpu);
 
-void simulate_rdmsr(struct CPUState *cpu);
-void simulate_wrmsr(struct CPUState *cpu);
+void simulate_rdmsr(CPUX86State *env);
+void simulate_wrmsr(CPUX86State *env);
 
 target_ulong read_reg(CPUX86State *env, int reg, int size);
 void write_reg(CPUX86State *env, int reg, target_ulong val, int size);
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index cb2cd0b02f..20b9ca3ef5 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -591,9 +591,9 @@ int hvf_vcpu_exec(CPUState *cpu)
         {
             load_regs(cpu);
             if (exit_reason == EXIT_REASON_RDMSR) {
-                simulate_rdmsr(cpu);
+                simulate_rdmsr(env);
             } else {
-                simulate_wrmsr(cpu);
+                simulate_wrmsr(env);
             }
             env->eip += ins_len;
             store_regs(cpu);
diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
index af1f205ecf..b1f8a685d1 100644
--- a/target/i386/hvf/x86_emu.c
+++ b/target/i386/hvf/x86_emu.c
@@ -663,11 +663,10 @@ static void exec_lods(CPUX86State *env, struct x86_decode *decode)
     env->eip += decode->len;
 }
 
-void simulate_rdmsr(struct CPUState *cpu)
+void simulate_rdmsr(CPUX86State *env)
 {
-    X86CPU *x86_cpu = X86_CPU(cpu);
-    CPUX86State *env = &x86_cpu->env;
-    CPUState *cs = env_cpu(env);
+    X86CPU *x86_cpu = env_archcpu(env);
+    CPUState *cpu = env_cpu(env);
     uint32_t msr = ECX(env);
     uint64_t val = 0;
 
@@ -746,8 +745,8 @@ void simulate_rdmsr(struct CPUState *cpu)
         val = env->mtrr_deftype;
         break;
     case MSR_CORE_THREAD_COUNT:
-        val = cs->nr_threads * cs->nr_cores; /* thread count, bits 15..0 */
-        val |= ((uint32_t)cs->nr_cores << 16); /* core count, bits 31..16 */
+        val = cpu->nr_threads * cpu->nr_cores;  /* thread count, bits 15..0 */
+        val |= ((uint32_t)cpu->nr_cores << 16); /* core count, bits 31..16 */
         break;
     default:
         /* fprintf(stderr, "%s: unknown msr 0x%x\n", __func__, msr); */
@@ -761,14 +760,14 @@ void simulate_rdmsr(struct CPUState *cpu)
 
 static void exec_rdmsr(CPUX86State *env, struct x86_decode *decode)
 {
-    simulate_rdmsr(env_cpu(env));
+    simulate_rdmsr(env);
     env->eip += decode->len;
 }
 
-void simulate_wrmsr(struct CPUState *cpu)
+void simulate_wrmsr(CPUX86State *env)
 {
-    X86CPU *x86_cpu = X86_CPU(cpu);
-    CPUX86State *env = &x86_cpu->env;
+    X86CPU *x86_cpu = env_archcpu(env);
+    CPUState *cpu = env_cpu(env);
     uint32_t msr = ECX(env);
     uint64_t data = ((uint64_t)EDX(env) << 32) | EAX(env);
 
@@ -856,7 +855,7 @@ void simulate_wrmsr(struct CPUState *cpu)
 
 static void exec_wrmsr(CPUX86State *env, struct x86_decode *decode)
 {
-    simulate_wrmsr(env_cpu(env));
+    simulate_wrmsr(env);
     env->eip += decode->len;
 }
 
-- 
2.41.0

Re: [PATCH 6/6] target/i386: Use env_archcpu() in simulate_[rdmsr/wrmsr]()

[Roman Bolshakov]

On Mon, Oct 09, 2023 at 01:02:39PM +0200, Philippe Mathieu-Daudé wrote:
> 
> simulate_rdmsr/simulate_rdmsr(CPUX86State
> 

Hi Philippe, the above is likely a typo. The subject may also be changed to
target/i386/hvf.

> RFC: Not even build-tested.
>

The patch compiles and works fine. Besides the nits,

Reviewed-by: Roman Bolshakov <roman@roolebo.dev>
Tested-by: Roman Bolshakov <roman@roolebo.dev>

Thanks,
Roman

Re: [PATCH 6/6] target/i386: Use env_archcpu() in simulate_[rdmsr/wrmsr]()

[Zhao Liu]

On Mon, Oct 09, 2023 at 01:02:39PM +0200, Philippe Mathieu-Daudé wrote:
> Date: Mon,  9 Oct 2023 13:02:39 +0200
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH 6/6] target/i386: Use env_archcpu() in
>  simulate_[rdmsr/wrmsr]()
> X-Mailer: git-send-email 2.41.0
> 
> When CPUArchState* is available (here CPUX86State*), we can
> use the fast env_archcpu() macro to get ArchCPU* (here X86CPU*).
> The QOM cast X86_CPU() macro will be slower when building with
> --enable-qom-cast-debug.
> 
> Pass CPUX86State* as argument to simulate_rdmsr / simulate_wrmsr
> instead of a CPUState* to avoid an extra cast.
> 
> simulate_rdmsr/simulate_rdmsr(CPUX86State
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> RFC: Not even build-tested.
> ---
>  target/i386/hvf/x86_emu.h |  4 ++--
>  target/i386/hvf/hvf.c     |  4 ++--
>  target/i386/hvf/x86_emu.c | 21 ++++++++++-----------
>  3 files changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/target/i386/hvf/x86_emu.h b/target/i386/hvf/x86_emu.h
> index 640da90b30..4b846ba80e 100644
> --- a/target/i386/hvf/x86_emu.h
> +++ b/target/i386/hvf/x86_emu.h
> @@ -29,8 +29,8 @@ bool exec_instruction(CPUX86State *env, struct x86_decode *ins);
>  void load_regs(struct CPUState *cpu);
>  void store_regs(struct CPUState *cpu);
>  
> -void simulate_rdmsr(struct CPUState *cpu);
> -void simulate_wrmsr(struct CPUState *cpu);
> +void simulate_rdmsr(CPUX86State *env);
> +void simulate_wrmsr(CPUX86State *env);
>  
>  target_ulong read_reg(CPUX86State *env, int reg, int size);
>  void write_reg(CPUX86State *env, int reg, target_ulong val, int size);
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index cb2cd0b02f..20b9ca3ef5 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -591,9 +591,9 @@ int hvf_vcpu_exec(CPUState *cpu)
>          {
>              load_regs(cpu);
>              if (exit_reason == EXIT_REASON_RDMSR) {
> -                simulate_rdmsr(cpu);
> +                simulate_rdmsr(env);
>              } else {
> -                simulate_wrmsr(cpu);
> +                simulate_wrmsr(env);
>              }
>              env->eip += ins_len;
>              store_regs(cpu);
> diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
> index af1f205ecf..b1f8a685d1 100644
> --- a/target/i386/hvf/x86_emu.c
> +++ b/target/i386/hvf/x86_emu.c
> @@ -663,11 +663,10 @@ static void exec_lods(CPUX86State *env, struct x86_decode *decode)
>      env->eip += decode->len;
>  }
>  
> -void simulate_rdmsr(struct CPUState *cpu)
> +void simulate_rdmsr(CPUX86State *env)
>  {
> -    X86CPU *x86_cpu = X86_CPU(cpu);
> -    CPUX86State *env = &x86_cpu->env;
> -    CPUState *cs = env_cpu(env);
> +    X86CPU *x86_cpu = env_archcpu(env);
> +    CPUState *cpu = env_cpu(env);

I find these names are confusing since in other i386 file
(target/i386/cpu.c), the "X86CPU" is called "cpu", and the "CPUState" is
called "cs".

Regarding this naming, it may be worthy of cleanup to unify the naming
for i386. ;-)

>      uint32_t msr = ECX(env);
>      uint64_t val = 0;
>  
> @@ -746,8 +745,8 @@ void simulate_rdmsr(struct CPUState *cpu)
>          val = env->mtrr_deftype;
>          break;
>      case MSR_CORE_THREAD_COUNT:
> -        val = cs->nr_threads * cs->nr_cores; /* thread count, bits 15..0 */
> -        val |= ((uint32_t)cs->nr_cores << 16); /* core count, bits 31..16 */
> +        val = cpu->nr_threads * cpu->nr_cores;  /* thread count, bits 15..0 */
> +        val |= ((uint32_t)cpu->nr_cores << 16); /* core count, bits 31..16 */
>          break;
>      default:
>          /* fprintf(stderr, "%s: unknown msr 0x%x\n", __func__, msr); */
> @@ -761,14 +760,14 @@ void simulate_rdmsr(struct CPUState *cpu)
>  
>  static void exec_rdmsr(CPUX86State *env, struct x86_decode *decode)
>  {
> -    simulate_rdmsr(env_cpu(env));
> +    simulate_rdmsr(env);
>      env->eip += decode->len;
>  }
>  
> -void simulate_wrmsr(struct CPUState *cpu)
> +void simulate_wrmsr(CPUX86State *env)
>  {
> -    X86CPU *x86_cpu = X86_CPU(cpu);
> -    CPUX86State *env = &x86_cpu->env;
> +    X86CPU *x86_cpu = env_archcpu(env);
> +    CPUState *cpu = env_cpu(env);
>      uint32_t msr = ECX(env);
>      uint64_t data = ((uint64_t)EDX(env) << 32) | EAX(env);
>  
> @@ -856,7 +855,7 @@ void simulate_wrmsr(struct CPUState *cpu)
>  
>  static void exec_wrmsr(CPUX86State *env, struct x86_decode *decode)
>  {
> -    simulate_wrmsr(env_cpu(env));
> +    simulate_wrmsr(env);
>      env->eip += decode->len;
>  }

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

-Zhao

>  
> -- 
> 2.41.0
> 
> 
> 

Re: [PATCH 0/6] target: Use env_archcpu() instead of ARCH_CPU(env_cpu(env))

[Richard Henderson]

On 10/9/23 04:02, Philippe Mathieu-Daudé wrote:
> When we have a CPUArchState* pointer we can directly get the
> corresponding ArchCPU* pointer with env_archcpu(). No need to
> use QOM cast which is slower due when building with
> --enable-qom-cast-debug.
> 
> This series replace ARCH_CPU(env_cpu(env)) by env_archcpu(env)
> when possible.
> 
> Philippe Mathieu-Daudé (6):
>    target/ppc: Use env_archcpu() in helper_book3s_msgsndp()
>    target/riscv: Use env_archcpu() in [check_]nanbox()
>    target/s390x: Use env_archcpu() in handle_diag_308()
>    target/xtensa: Use env_archcpu() in update_c[compare|count]()
>    target/i386/hvf: Use x86_cpu in simulate_[rdmsr|wrmsr]()
>    target/i386: Use env_archcpu() in simulate_[rdmsr/wrmsr]()

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


r~

Re: [PATCH 0/6] target: Use env_archcpu() instead of ARCH_CPU(env_cpu(env))

[Philippe Mathieu-Daudé]

On 9/10/23 13:02, Philippe Mathieu-Daudé wrote:

> Philippe Mathieu-Daudé (6):
>    target/ppc: Use env_archcpu() in helper_book3s_msgsndp()
>    target/riscv: Use env_archcpu() in [check_]nanbox()
>    target/s390x: Use env_archcpu() in handle_diag_308()
>    target/xtensa: Use env_archcpu() in update_c[compare|count]()
>    target/i386/hvf: Use x86_cpu in simulate_[rdmsr|wrmsr]()
>    target/i386: Use env_archcpu() in simulate_[rdmsr/wrmsr]()

Series queued.