[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v2 4/7] target/i386/cpu: Avoid mixing signed and unsigned in prop
From: |
Markus Armbruster |
Subject: |
[PATCH v2 4/7] target/i386/cpu: Avoid mixing signed and unsigned in property setters |
Date: |
Thu, 10 Oct 2024 17:01:41 +0200 |
Properties "family", "model", and "stepping" are visited as signed
integers. They are backed by bits in CPUX86State member
@cpuid_version. The code to extract and insert these bits mixes
signed and unsigned. Not actually wrong, but avoiding such mixing is
good practice.
Visit them as unsigned integers instead.
This adds a few mildly ugly cast in arguments of error_setg(). The
next commit will get rid of them again.
Property "tsc-frequency" is also visited as signed integer. The value
ultimately flows into the kernel, where it is 31 bits unsigned. The
QEMU code freely mixes int, uint32_t, int64_t. I elect not to attempt
draining this swamp today.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
target/i386/cpu.c | 45 +++++++++++++++++++++------------------------
1 file changed, 21 insertions(+), 24 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ff227a8c5c..4f8fa60432 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5433,13 +5433,13 @@ static void x86_cpuid_version_get_family(Object *obj,
Visitor *v,
{
X86CPU *cpu = X86_CPU(obj);
CPUX86State *env = &cpu->env;
- int64_t value;
+ uint64_t value;
value = (env->cpuid_version >> 8) & 0xf;
if (value == 0xf) {
value += (env->cpuid_version >> 20) & 0xff;
}
- visit_type_int(v, name, &value, errp);
+ visit_type_uint64(v, name, &value, errp);
}
static void x86_cpuid_version_set_family(Object *obj, Visitor *v,
@@ -5448,16 +5448,15 @@ static void x86_cpuid_version_set_family(Object *obj,
Visitor *v,
{
X86CPU *cpu = X86_CPU(obj);
CPUX86State *env = &cpu->env;
- const int64_t min = 0;
- const int64_t max = 0xff + 0xf;
- int64_t value;
+ const uint64_t max = 0xff + 0xf;
+ uint64_t value;
- if (!visit_type_int(v, name, &value, errp)) {
+ if (!visit_type_uint64(v, name, &value, errp)) {
return;
}
- if (value < min || value > max) {
+ if (value > max) {
error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
- name ? name : "null", value, min, max);
+ name ? name : "null", value, (int64_t)0, (int64_t)max);
return;
}
@@ -5475,11 +5474,11 @@ static void x86_cpuid_version_get_model(Object *obj,
Visitor *v,
{
X86CPU *cpu = X86_CPU(obj);
CPUX86State *env = &cpu->env;
- int64_t value;
+ uint64_t value;
value = (env->cpuid_version >> 4) & 0xf;
value |= ((env->cpuid_version >> 16) & 0xf) << 4;
- visit_type_int(v, name, &value, errp);
+ visit_type_uint64(v, name, &value, errp);
}
static void x86_cpuid_version_set_model(Object *obj, Visitor *v,
@@ -5488,16 +5487,15 @@ static void x86_cpuid_version_set_model(Object *obj,
Visitor *v,
{
X86CPU *cpu = X86_CPU(obj);
CPUX86State *env = &cpu->env;
- const int64_t min = 0;
- const int64_t max = 0xff;
- int64_t value;
+ const uint64_t max = 0xff;
+ uint64_t value;
- if (!visit_type_int(v, name, &value, errp)) {
+ if (!visit_type_uint64(v, name, &value, errp)) {
return;
}
- if (value < min || value > max) {
+ if (value > max) {
error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
- name ? name : "null", value, min, max);
+ name ? name : "null", value, (int64_t)0, (int64_t)max);
return;
}
@@ -5511,10 +5509,10 @@ static void x86_cpuid_version_get_stepping(Object *obj,
Visitor *v,
{
X86CPU *cpu = X86_CPU(obj);
CPUX86State *env = &cpu->env;
- int64_t value;
+ uint64_t value;
value = env->cpuid_version & 0xf;
- visit_type_int(v, name, &value, errp);
+ visit_type_uint64(v, name, &value, errp);
}
static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v,
@@ -5523,16 +5521,15 @@ static void x86_cpuid_version_set_stepping(Object *obj,
Visitor *v,
{
X86CPU *cpu = X86_CPU(obj);
CPUX86State *env = &cpu->env;
- const int64_t min = 0;
- const int64_t max = 0xf;
- int64_t value;
+ const uint64_t max = 0xf;
+ uint64_t value;
- if (!visit_type_int(v, name, &value, errp)) {
+ if (!visit_type_uint64(v, name, &value, errp)) {
return;
}
- if (value < min || value > max) {
+ if (value > max) {
error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
- name ? name : "null", value, min, max);
+ name ? name : "null", value, (int64_t)0, (int64_t)max);
return;
}
--
2.46.0
[PATCH v2 6/7] hw/intc/openpic: Improve errors for out of bounds property values, Markus Armbruster, 2024/10/10