[PATCH] i386/cpu: Drop the check of phys_bits in host_cpu_realizefn()

Xiaoyao Li posted 1 patch 4 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240704111231.2881016-1-xiaoyao.li@intel.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
target/i386/host-cpu.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
[PATCH] i386/cpu: Drop the check of phys_bits in host_cpu_realizefn()
Posted by Xiaoyao Li 4 months, 3 weeks ago
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
teh x86 case. Remove the one in host_cpu_realizefn().

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/host-cpu.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
index 8b8bf5afeccf..b109c1a2221f 100644
--- a/target/i386/host-cpu.c
+++ b/target/i386/host-cpu.c
@@ -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;
+        cpu->phys_bits = host_cpu_adjust_phys_bits(cpu);
     }
     return true;
 }
-- 
2.34.1
Re: [PATCH] i386/cpu: Drop the check of phys_bits in host_cpu_realizefn()
Posted by Zhao Liu 4 months, 2 weeks ago
Hi Xiaoyao,

On Thu, Jul 04, 2024 at 07:12:31AM -0400, Xiaoyao Li wrote:
> Date: Thu,  4 Jul 2024 07:12:31 -0400
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: [PATCH] i386/cpu: Drop the check of phys_bits in
>  host_cpu_realizefn()
> X-Mailer: git-send-email 2.34.1
> 
> 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
> teh x86 case. Remove the one in host_cpu_realizefn().
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  target/i386/host-cpu.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
> index 8b8bf5afeccf..b109c1a2221f 100644
> --- a/target/i386/host-cpu.c
> +++ b/target/i386/host-cpu.c
> @@ -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;
> +        cpu->phys_bits = host_cpu_adjust_phys_bits(cpu);

Just nit:

cpu->phys_bits can be adjusted directly in host_cpu_adjust_phys_bits(),
and no need to return it out again.

Otherwise,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Re: [PATCH] i386/cpu: Drop the check of phys_bits in host_cpu_realizefn()
Posted by Igor Mammedov 4 months, 3 weeks ago
On Thu,  4 Jul 2024 07:12:31 -0400
Xiaoyao Li <xiaoyao.li@intel.com> 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
> teh x86 case. Remove the one in host_cpu_realizefn().
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  target/i386/host-cpu.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
> index 8b8bf5afeccf..b109c1a2221f 100644
> --- a/target/i386/host-cpu.c
> +++ b/target/i386/host-cpu.c
> @@ -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;
> +        cpu->phys_bits = host_cpu_adjust_phys_bits(cpu);
>      }
>      return true;
>  }