[PATCH v2 01/10] i386/cpu: Extract a common fucntion to setup value of MSR_CORE_THREAD_COUNT

Xiaoyao Li posted 10 patches 1 year, 1 month ago
[PATCH v2 01/10] i386/cpu: Extract a common fucntion to setup value of MSR_CORE_THREAD_COUNT
Posted by Xiaoyao Li 1 year, 1 month ago
There are duplicated code to setup the value of MSR_CORE_THREAD_COUNT.
Extract a common function for it.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
Changes in v2:
- move the implementation of cpu_x86_get_msr_core_thread_count() to
  target/i386/cpu-sysemu.c;
---
 target/i386/cpu-sysemu.c             | 11 +++++++++++
 target/i386/cpu.h                    |  2 ++
 target/i386/hvf/x86_emu.c            |  3 +--
 target/i386/kvm/kvm.c                |  5 +----
 target/i386/tcg/sysemu/misc_helper.c |  3 +--
 5 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c
index 227ac021f62f..4e9df0bc0156 100644
--- a/target/i386/cpu-sysemu.c
+++ b/target/i386/cpu-sysemu.c
@@ -309,3 +309,14 @@ void x86_cpu_get_crash_info_qom(Object *obj, Visitor *v,
                                      errp);
     qapi_free_GuestPanicInformation(panic_info);
 }
+
+uint64_t cpu_x86_get_msr_core_thread_count(X86CPU *cpu)
+{
+    CPUState *cs = CPU(cpu);
+    uint64_t val;
+
+    val = cs->nr_threads * cs->nr_cores;  /* thread count, bits 15..0 */
+    val |= ((uint32_t)cs->nr_cores << 16); /* core count, bits 31..16 */
+
+    return val;
+}
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 4c239a6970fd..5e1beca94a62 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2390,6 +2390,8 @@ static inline void cpu_x86_load_seg_cache_sipi(X86CPU *cpu,
     cs->halted = 0;
 }
 
+uint64_t cpu_x86_get_msr_core_thread_count(X86CPU *cpu);
+
 int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
                             target_ulong *base, unsigned int *limit,
                             unsigned int *flags);
diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
index 015f760acb39..69c61c9c0737 100644
--- a/target/i386/hvf/x86_emu.c
+++ b/target/i386/hvf/x86_emu.c
@@ -765,8 +765,7 @@ void simulate_rdmsr(CPUX86State *env)
         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_x86_get_msr_core_thread_count(cpu);
         break;
     default:
         /* fprintf(stderr, "%s: unknown msr 0x%x\n", __func__, msr); */
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 8e17942c3ba1..18a1bd1297a4 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2602,10 +2602,7 @@ static bool kvm_rdmsr_core_thread_count(X86CPU *cpu,
                                         uint32_t msr,
                                         uint64_t *val)
 {
-    CPUState *cs = CPU(cpu);
-
-    *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_x86_get_msr_core_thread_count(cpu);
 
     return true;
 }
diff --git a/target/i386/tcg/sysemu/misc_helper.c b/target/i386/tcg/sysemu/misc_helper.c
index 094aa56a20d1..ff7b201b44d8 100644
--- a/target/i386/tcg/sysemu/misc_helper.c
+++ b/target/i386/tcg/sysemu/misc_helper.c
@@ -468,8 +468,7 @@ void helper_rdmsr(CPUX86State *env)
         val = x86_cpu->ucode_rev;
         break;
     case MSR_CORE_THREAD_COUNT: {
-        CPUState *cs = CPU(x86_cpu);
-        val = (cs->nr_threads * cs->nr_cores) | (cs->nr_cores << 16);
+        val = cpu_x86_get_msr_core_thread_count(x86_cpu);
         break;
     }
     case MSR_APIC_START ... MSR_APIC_END: {
-- 
2.34.1
Re: [PATCH v2 01/10] i386/cpu: Extract a common fucntion to setup value of MSR_CORE_THREAD_COUNT
Posted by Philippe Mathieu-Daudé 1 year, 1 month ago
On 19/12/24 12:01, Xiaoyao Li wrote:
> There are duplicated code to setup the value of MSR_CORE_THREAD_COUNT.
> Extract a common function for it.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> Changes in v2:
> - move the implementation of cpu_x86_get_msr_core_thread_count() to
>    target/i386/cpu-sysemu.c;
> ---
>   target/i386/cpu-sysemu.c             | 11 +++++++++++
>   target/i386/cpu.h                    |  2 ++
>   target/i386/hvf/x86_emu.c            |  3 +--
>   target/i386/kvm/kvm.c                |  5 +----
>   target/i386/tcg/sysemu/misc_helper.c |  3 +--
>   5 files changed, 16 insertions(+), 8 deletions(-)


> +uint64_t cpu_x86_get_msr_core_thread_count(X86CPU *cpu)
> +{
> +    CPUState *cs = CPU(cpu);
> +    uint64_t val;
> +
> +    val = cs->nr_threads * cs->nr_cores;  /* thread count, bits 15..0 */
> +    val |= ((uint32_t)cs->nr_cores << 16); /* core count, bits 31..16 */
> +
> +    return val;

Alternatively:

        return deposit64(cs->nr_threads * cs->nr_cores, 16, 16,
                         cs->nr_cores);

> +}
Re: [PATCH v2 01/10] i386/cpu: Extract a common fucntion to setup value of MSR_CORE_THREAD_COUNT
Posted by Philippe Mathieu-Daudé 1 year, 1 month ago
On 28/12/24 18:37, Philippe Mathieu-Daudé wrote:
> On 19/12/24 12:01, Xiaoyao Li wrote:
>> There are duplicated code to setup the value of MSR_CORE_THREAD_COUNT.
>> Extract a common function for it.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>> Changes in v2:
>> - move the implementation of cpu_x86_get_msr_core_thread_count() to
>>    target/i386/cpu-sysemu.c;
>> ---
>>   target/i386/cpu-sysemu.c             | 11 +++++++++++
>>   target/i386/cpu.h                    |  2 ++
>>   target/i386/hvf/x86_emu.c            |  3 +--
>>   target/i386/kvm/kvm.c                |  5 +----
>>   target/i386/tcg/sysemu/misc_helper.c |  3 +--
>>   5 files changed, 16 insertions(+), 8 deletions(-)
> 
> 
>> +uint64_t cpu_x86_get_msr_core_thread_count(X86CPU *cpu)
>> +{
>> +    CPUState *cs = CPU(cpu);
>> +    uint64_t val;
>> +
>> +    val = cs->nr_threads * cs->nr_cores;  /* thread count, bits 15..0 */
>> +    val |= ((uint32_t)cs->nr_cores << 16); /* core count, bits 31..16 */
>> +
>> +    return val;
> 
> Alternatively:
> 
>         return deposit64(cs->nr_threads * cs->nr_cores, 16, 16,
>                          cs->nr_cores);
> 
>> +}
> 

Typo "function" in patch subject.

Re: [PATCH v2 01/10] i386/cpu: Extract a common fucntion to setup value of MSR_CORE_THREAD_COUNT
Posted by Xiaoyao Li 1 year, 1 month ago
On 1/3/2025 4:52 PM, Philippe Mathieu-Daudé wrote:
> On 28/12/24 18:37, Philippe Mathieu-Daudé wrote:
>> On 19/12/24 12:01, Xiaoyao Li wrote:
>>> There are duplicated code to setup the value of MSR_CORE_THREAD_COUNT.
>>> Extract a common function for it.
>>>
>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>>> ---
>>> Changes in v2:
>>> - move the implementation of cpu_x86_get_msr_core_thread_count() to
>>>    target/i386/cpu-sysemu.c;
>>> ---
>>>   target/i386/cpu-sysemu.c             | 11 +++++++++++
>>>   target/i386/cpu.h                    |  2 ++
>>>   target/i386/hvf/x86_emu.c            |  3 +--
>>>   target/i386/kvm/kvm.c                |  5 +----
>>>   target/i386/tcg/sysemu/misc_helper.c |  3 +--
>>>   5 files changed, 16 insertions(+), 8 deletions(-)
>>
>>
>>> +uint64_t cpu_x86_get_msr_core_thread_count(X86CPU *cpu)
>>> +{
>>> +    CPUState *cs = CPU(cpu);
>>> +    uint64_t val;
>>> +
>>> +    val = cs->nr_threads * cs->nr_cores;  /* thread count, bits 
>>> 15..0 */
>>> +    val |= ((uint32_t)cs->nr_cores << 16); /* core count, bits 
>>> 31..16 */
>>> +
>>> +    return val;
>>
>> Alternatively:
>>
>>         return deposit64(cs->nr_threads * cs->nr_cores, 16, 16,
>>                          cs->nr_cores);

I would rather keep it as-is as suguested by Philippe[1].

(deposit64() is really a new thing for i386)

[1] 
https://lore.kernel.org/qemu-devel/20241210173524.48e203a3@imammedo.users.ipa.redhat.com/

>>> +}
>>
> 
> Typo "function" in patch subject.

thanks for catching it!


Re: [PATCH v2 01/10] i386/cpu: Extract a common fucntion to setup value of MSR_CORE_THREAD_COUNT
Posted by Xiaoyao Li 1 year, 1 month ago
On 1/7/2025 12:31 PM, Xiaoyao Li wrote:
> On 1/3/2025 4:52 PM, Philippe Mathieu-Daudé wrote:
>> On 28/12/24 18:37, Philippe Mathieu-Daudé wrote:
>>> On 19/12/24 12:01, Xiaoyao Li wrote:
>>>> There are duplicated code to setup the value of MSR_CORE_THREAD_COUNT.
>>>> Extract a common function for it.
>>>>
>>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>>>> ---
>>>> Changes in v2:
>>>> - move the implementation of cpu_x86_get_msr_core_thread_count() to
>>>>    target/i386/cpu-sysemu.c;
>>>> ---
>>>>   target/i386/cpu-sysemu.c             | 11 +++++++++++
>>>>   target/i386/cpu.h                    |  2 ++
>>>>   target/i386/hvf/x86_emu.c            |  3 +--
>>>>   target/i386/kvm/kvm.c                |  5 +----
>>>>   target/i386/tcg/sysemu/misc_helper.c |  3 +--
>>>>   5 files changed, 16 insertions(+), 8 deletions(-)
>>>
>>>
>>>> +uint64_t cpu_x86_get_msr_core_thread_count(X86CPU *cpu)
>>>> +{
>>>> +    CPUState *cs = CPU(cpu);
>>>> +    uint64_t val;
>>>> +
>>>> +    val = cs->nr_threads * cs->nr_cores;  /* thread count, bits 
>>>> 15..0 */
>>>> +    val |= ((uint32_t)cs->nr_cores << 16); /* core count, bits 
>>>> 31..16 */
>>>> +
>>>> +    return val;
>>>
>>> Alternatively:
>>>
>>>         return deposit64(cs->nr_threads * cs->nr_cores, 16, 16,
>>>                          cs->nr_cores);
> 
> I would rather keep it as-is as suguested by Philippe[1].

Sorry for being wrong with the name. (awkward)

s/Philippe/Igor

> (deposit64() is really a new thing for i386)
> 
> [1] https://lore.kernel.org/qemu- 
> devel/20241210173524.48e203a3@imammedo.users.ipa.redhat.com/
> 
>>>> +}
>>>
>>
>> Typo "function" in patch subject.
> 
> thanks for catching it!
> 
> 


Re: [PATCH v2 01/10] i386/cpu: Extract a common fucntion to setup value of MSR_CORE_THREAD_COUNT
Posted by Philippe Mathieu-Daudé 1 year, 1 month ago
On 7/1/25 05:34, Xiaoyao Li wrote:
> On 1/7/2025 12:31 PM, Xiaoyao Li wrote:
>> On 1/3/2025 4:52 PM, Philippe Mathieu-Daudé wrote:
>>> On 28/12/24 18:37, Philippe Mathieu-Daudé wrote:
>>>> On 19/12/24 12:01, Xiaoyao Li wrote:
>>>>> There are duplicated code to setup the value of MSR_CORE_THREAD_COUNT.
>>>>> Extract a common function for it.
>>>>>
>>>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>>>>> ---
>>>>> Changes in v2:
>>>>> - move the implementation of cpu_x86_get_msr_core_thread_count() to
>>>>>    target/i386/cpu-sysemu.c;
>>>>> ---
>>>>>   target/i386/cpu-sysemu.c             | 11 +++++++++++
>>>>>   target/i386/cpu.h                    |  2 ++
>>>>>   target/i386/hvf/x86_emu.c            |  3 +--
>>>>>   target/i386/kvm/kvm.c                |  5 +----
>>>>>   target/i386/tcg/sysemu/misc_helper.c |  3 +--
>>>>>   5 files changed, 16 insertions(+), 8 deletions(-)
>>>>
>>>>
>>>>> +uint64_t cpu_x86_get_msr_core_thread_count(X86CPU *cpu)
>>>>> +{
>>>>> +    CPUState *cs = CPU(cpu);
>>>>> +    uint64_t val;
>>>>> +
>>>>> +    val = cs->nr_threads * cs->nr_cores;  /* thread count, bits 
>>>>> 15..0 */
>>>>> +    val |= ((uint32_t)cs->nr_cores << 16); /* core count, bits 
>>>>> 31..16 */
>>>>> +
>>>>> +    return val;
>>>>
>>>> Alternatively:
>>>>
>>>>         return deposit64(cs->nr_threads * cs->nr_cores, 16, 16,
>>>>                          cs->nr_cores);
>>
>> I would rather keep it as-is as suguested by Philippe[1].
> 
> Sorry for being wrong with the name. (awkward)
> 
> s/Philippe/Igor
> 
>> (deposit64() is really a new thing for i386)
>>
>> [1] https://lore.kernel.org/qemu- 
>> devel/20241210173524.48e203a3@imammedo.users.ipa.redhat.com/

Then use deposit64() in another patch on top?

>>>>> +}
>>>>
>>>
>>> Typo "function" in patch subject.
>>
>> thanks for catching it!
>>
>>
> 


Re: [PATCH v2 01/10] i386/cpu: Extract a common fucntion to setup value of MSR_CORE_THREAD_COUNT
Posted by Zhao Liu 1 year, 1 month ago
On Thu, Dec 19, 2024 at 06:01:16AM -0500, Xiaoyao Li wrote:
> Date: Thu, 19 Dec 2024 06:01:16 -0500
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: [PATCH v2 01/10] i386/cpu: Extract a common fucntion to setup
>  value of MSR_CORE_THREAD_COUNT
> X-Mailer: git-send-email 2.34.1
> 
> There are duplicated code to setup the value of MSR_CORE_THREAD_COUNT.
> Extract a common function for it.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> Changes in v2:
> - move the implementation of cpu_x86_get_msr_core_thread_count() to
>   target/i386/cpu-sysemu.c;
> ---
>  target/i386/cpu-sysemu.c             | 11 +++++++++++
>  target/i386/cpu.h                    |  2 ++
>  target/i386/hvf/x86_emu.c            |  3 +--
>  target/i386/kvm/kvm.c                |  5 +----
>  target/i386/tcg/sysemu/misc_helper.c |  3 +--
>  5 files changed, 16 insertions(+), 8 deletions(-)
> 

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