The check of cpu->phys_bits to be in range between
[32, TARGET_PHYS_ADDR_SPACE_BITS] in host_cpu_realizefn()
is duplicated with check in x86_cpu_realizefn().

Since the ckeck in x86_cpu_realizefn() is called later and can cover all
the x86 cases. Remove the one in host_cpu_realizefn().

Opportunistically adjust cpu->phys_bits directly in
host_cpu_adjust_phys_bits(), which matches more with the function name.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes in v2:
 - make host_cpu_adjust_phys_bits() return void and adjust
   cpu->phys_bits inside it.
---
 target/i386/host-cpu.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
index 8b8bf5afeccf..03b9d1b169a5 100644
--- a/target/i386/host-cpu.c
+++ b/target/i386/host-cpu.c
@@ -42,7 +42,7 @@ static uint32_t host_cpu_phys_bits(void)
     return host_phys_bits;
 }
 
-static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
+static void host_cpu_adjust_phys_bits(X86CPU *cpu)
 {
     uint32_t host_phys_bits = host_cpu_phys_bits();
     uint32_t phys_bits = cpu->phys_bits;
@@ -66,7 +66,7 @@ static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
         }
     }
 
-    return phys_bits;
+    cpu->phys_bits = phys_bits;
 }
 
 bool host_cpu_realizefn(CPUState *cs, Error **errp)
@@ -75,17 +75,7 @@ bool host_cpu_realizefn(CPUState *cs, Error **errp)
     CPUX86State *env = &cpu->env;
 
     if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
-        uint32_t phys_bits = host_cpu_adjust_phys_bits(cpu);
-
-        if (phys_bits &&
-            (phys_bits > TARGET_PHYS_ADDR_SPACE_BITS ||
-             phys_bits < 32)) {
-            error_setg(errp, "phys-bits should be between 32 and %u "
-                       " (but is %u)",
-                       TARGET_PHYS_ADDR_SPACE_BITS, phys_bits);
-            return false;
-        }
-        cpu->phys_bits = phys_bits;
+        host_cpu_adjust_phys_bits(cpu);
     }
     return true;
 }
-- 
2.34.1

Queued, thanks.

Paolo

Ping.

Paolo, this is a simple clean up.

On 9/29/2024 4:57 PM, Xiaoyao Li wrote:
> The check of cpu->phys_bits to be in range between
> [32, TARGET_PHYS_ADDR_SPACE_BITS] in host_cpu_realizefn()
> is duplicated with check in x86_cpu_realizefn().
> 
> Since the ckeck in x86_cpu_realizefn() is called later and can cover all
> the x86 cases. Remove the one in host_cpu_realizefn().
> 
> Opportunistically adjust cpu->phys_bits directly in
> host_cpu_adjust_phys_bits(), which matches more with the function name.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Changes in v2:
>   - make host_cpu_adjust_phys_bits() return void and adjust
>     cpu->phys_bits inside it.
> ---
>   target/i386/host-cpu.c | 16 +++-------------
>   1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
> index 8b8bf5afeccf..03b9d1b169a5 100644
> --- a/target/i386/host-cpu.c
> +++ b/target/i386/host-cpu.c
> @@ -42,7 +42,7 @@ static uint32_t host_cpu_phys_bits(void)
>       return host_phys_bits;
>   }
>   
> -static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
> +static void host_cpu_adjust_phys_bits(X86CPU *cpu)
>   {
>       uint32_t host_phys_bits = host_cpu_phys_bits();
>       uint32_t phys_bits = cpu->phys_bits;
> @@ -66,7 +66,7 @@ static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
>           }
>       }
>   
> -    return phys_bits;
> +    cpu->phys_bits = phys_bits;
>   }
>   
>   bool host_cpu_realizefn(CPUState *cs, Error **errp)
> @@ -75,17 +75,7 @@ bool host_cpu_realizefn(CPUState *cs, Error **errp)
>       CPUX86State *env = &cpu->env;
>   
>       if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
> -        uint32_t phys_bits = host_cpu_adjust_phys_bits(cpu);
> -
> -        if (phys_bits &&
> -            (phys_bits > TARGET_PHYS_ADDR_SPACE_BITS ||
> -             phys_bits < 32)) {
> -            error_setg(errp, "phys-bits should be between 32 and %u "
> -                       " (but is %u)",
> -                       TARGET_PHYS_ADDR_SPACE_BITS, phys_bits);
> -            return false;
> -        }
> -        cpu->phys_bits = phys_bits;
> +        host_cpu_adjust_phys_bits(cpu);
>       }
>       return true;
>   }