xen/arch/x86/apic.c | 2 +- xen/arch/x86/cpu/intel.c | 2 +- xen/arch/x86/cpu/mcheck/mce.c | 9 ++++----- xen/arch/x86/cpu/mcheck/vmce.c | 2 +- xen/arch/x86/e820.c | 6 +++--- xen/arch/x86/hvm/vmx/vmcs.c | 4 ++-- xen/arch/x86/io_apic.c | 2 +- xen/common/page_alloc.c | 4 ++-- xen/common/perfc.c | 8 ++++---- 9 files changed, 19 insertions(+), 20 deletions(-)
Coverity complains about it being invalid. It turns out that it is a GCC-ism
to treat ll and L equivalently. C99 only permits L to mean long double.
Convert all uses of L in to alternatives, either l, ll or PRI.64 depending on
the operand type. This in turn removes some unnecessary casts which look to
predate us having correct PRI* constants.
No functional change.
Coverity-IDs: 1464224, 1464228, 1464248, 1464261, 1464272, 1464281, 1464287, 1464290, 1494437
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
I'm disappointed at having to use %ll for __fix_to_virt() in apic.c and
io_apic.c. The expression ends up ULL because of the GB(64) in VMAP_VIRT_END,
but can't really be changed without breaking 32bit builds of Xen.
One option might be to turn __fix_to_virt() into a proper function, but
there's a lot of that infrastructure which should be dedup'd and not left to
each arch to copy.
---
xen/arch/x86/apic.c | 2 +-
xen/arch/x86/cpu/intel.c | 2 +-
xen/arch/x86/cpu/mcheck/mce.c | 9 ++++-----
xen/arch/x86/cpu/mcheck/vmce.c | 2 +-
xen/arch/x86/e820.c | 6 +++---
xen/arch/x86/hvm/vmx/vmcs.c | 4 ++--
xen/arch/x86/io_apic.c | 2 +-
xen/common/page_alloc.c | 4 ++--
xen/common/perfc.c | 8 ++++----
9 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 6567af685a1b..406ecbaab53e 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -938,7 +938,7 @@ void __init init_apic_mappings(void)
apic_phys = mp_lapic_addr;
set_fixmap_nocache(FIX_APIC_BASE, apic_phys);
- apic_printk(APIC_VERBOSE, "mapped APIC to %08Lx (%08lx)\n", APIC_BASE,
+ apic_printk(APIC_VERBOSE, "mapped APIC to %08llx (%08lx)\n", APIC_BASE,
apic_phys);
__next:
diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index af56e57bd8ab..29690843fa4a 100644
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -441,7 +441,7 @@ static void intel_log_freq(const struct cpuinfo_x86 *c)
unsigned long long val = ecx;
val *= ebx;
- printk("CPU%u: TSC: %u Hz * %u / %u = %Lu Hz\n",
+ printk("CPU%u: TSC: %u Hz * %u / %u = %llu Hz\n",
smp_processor_id(), ecx, ebx, eax, val / eax);
}
else if ( ecx | eax | ebx )
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 32c1b2756b90..c2b5199f8bcc 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1183,8 +1183,8 @@ static bool x86_mc_msrinject_verify(struct xen_mc_msrinject *mci)
if ( reason != NULL )
{
- printk("HV MSR INJECT ERROR: MSR %#Lx %s\n",
- (unsigned long long)mci->mcinj_msr[i].reg, reason);
+ printk("HV MSR INJECT ERROR: MSR %#"PRIx64" %s\n",
+ mci->mcinj_msr[i].reg, reason);
errs++;
}
}
@@ -1228,11 +1228,10 @@ static void cf_check x86_mc_msrinject(void *data)
for ( i = 0, msr = &mci->mcinj_msr[0]; i < mci->mcinj_count; i++, msr++ )
{
- printk("HV MSR INJECT (%s) target %u actual %u MSR %#Lx <-- %#Lx\n",
+ printk("HV MSR INJECT (%s) target %u actual %u MSR %#"PRIx64" <-- %#"PRIx64"\n",
intpose ? "interpose" : "hardware",
mci->mcinj_cpunr, smp_processor_id(),
- (unsigned long long)msr->reg,
- (unsigned long long)msr->value);
+ msr->reg, msr->value);
if ( intpose )
intpose_add(mci->mcinj_cpunr, msr->reg, msr->value);
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index 5abdf4cb5fd5..dce67a3e1b2c 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -71,7 +71,7 @@ int vmce_restore_vcpu(struct vcpu *v, const struct hvm_vmce_vcpu *ctxt)
if ( ctxt->caps & ~guest_mcg_cap & ~MCG_CAP_COUNT & ~MCG_CTL_P )
{
printk(XENLOG_G_ERR
- "%s restore: unsupported MCA capabilities %#"PRIx64" for %pv (supported: %#Lx)\n",
+ "%s restore: unsupported MCA capabilities %#"PRIx64" for %pv (supported: %#llx)\n",
is_hvm_vcpu(v) ? "HVM" : "PV", ctxt->caps,
v, guest_mcg_cap & ~MCG_CAP_COUNT);
return -EINVAL;
diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
index 6a3ce7e0a07f..bfe373e03427 100644
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -94,9 +94,9 @@ void __init print_e820_memory_map(const struct e820entry *map,
unsigned int i;
for (i = 0; i < entries; i++) {
- printk(" [%016Lx, %016Lx] ",
- (unsigned long long)(map[i].addr),
- (unsigned long long)(map[i].addr + map[i].size) - 1);
+ printk(" [%016"PRIx64", %016"PRIx64"] ",
+ map[i].addr,
+ map[i].addr + map[i].size - 1);
switch (map[i].type) {
case E820_RAM:
printk("(usable)\n");
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 9b6dc51f36ab..28d91c3857d5 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -517,7 +517,7 @@ static int vmx_init_vmcs_config(bool bsp)
if ( (vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32)) >
PAGE_SIZE )
{
- printk("VMX: CPU%d VMCS size is too big (%Lu bytes)\n",
+ printk("VMX: CPU%d VMCS size is too big (%llu bytes)\n",
smp_processor_id(),
vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32));
return -EINVAL;
@@ -564,7 +564,7 @@ static int vmx_init_vmcs_config(bool bsp)
if ( (vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32)) !=
((vmx_basic_msr & VMX_BASIC_VMCS_SIZE_MASK) >> 32) )
{
- printk("VMX: CPU%d unexpected VMCS size %Lu\n",
+ printk("VMX: CPU%d unexpected VMCS size %llu\n",
smp_processor_id(),
vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32));
mismatch = 1;
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index d2a313c4ac72..ef076bfaf3f5 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2597,7 +2597,7 @@ static void __init ioapic_init_mappings(void)
}
set_fixmap_nocache(idx, ioapic_phys);
- apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08Lx (%08lx)\n",
+ apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08llx (%08lx)\n",
__fix_to_virt(idx), ioapic_phys);
if ( bad_ioapic_register(i) )
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 33c8c917d984..3ea5c1b38b4a 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2395,7 +2395,7 @@ int assign_pages(
if ( unlikely(nr > d->max_pages - tot_pages) )
{
- gprintk(XENLOG_INFO, "Over-allocation for %pd: %Lu > %u\n",
+ gprintk(XENLOG_INFO, "Over-allocation for %pd: %llu > %u\n",
d, tot_pages + 0ULL + nr, d->max_pages);
rc = -E2BIG;
goto out;
@@ -2407,7 +2407,7 @@ int assign_pages(
if ( unlikely(d->tot_pages + nr < nr) )
{
gprintk(XENLOG_INFO,
- "Excess allocation for %pd: %Lu (%u extra)\n",
+ "Excess allocation for %pd: %llu (%u extra)\n",
d, d->tot_pages + 0ULL + nr, d->extra_pages);
if ( pg[0].count_info & PGC_extra )
d->extra_pages -= nr;
diff --git a/xen/common/perfc.c b/xen/common/perfc.c
index 80480aa7766d..3efb3a4262ef 100644
--- a/xen/common/perfc.c
+++ b/xen/common/perfc.c
@@ -49,7 +49,7 @@ void cf_check perfc_printall(unsigned char key)
sum += per_cpu(perfcounters, cpu)[j];
if ( perfc_info[i].type == TYPE_S_SINGLE )
sum = (perfc_t) sum;
- printk("TOTAL[%12Lu]", sum);
+ printk("TOTAL[%12llu]", sum);
if ( sum )
{
k = 0;
@@ -74,7 +74,7 @@ void cf_check perfc_printall(unsigned char key)
}
if ( perfc_info[i].type == TYPE_S_ARRAY )
sum = (perfc_t) sum;
- printk("TOTAL[%12Lu]", sum);
+ printk("TOTAL[%12llu]", sum);
if (sum)
{
#ifdef CONFIG_PERF_ARRAYS
@@ -87,7 +87,7 @@ void cf_check perfc_printall(unsigned char key)
sum = (perfc_t) sum;
if ( (k % 4) == 0 )
printk("\n%16s", "");
- printk(" ARR%02u[%10Lu]", k, sum);
+ printk(" ARR%02u[%10llu]", k, sum);
}
#else
k = 0;
@@ -103,7 +103,7 @@ void cf_check perfc_printall(unsigned char key)
sum = (perfc_t) sum;
if ( k > 0 && (k % 4) == 0 )
printk("\n%53s", "");
- printk(" CPU%02u[%10Lu]", cpu, sum);
+ printk(" CPU%02u[%10llu]", cpu, sum);
++k;
}
#endif
--
2.39.2
On 23.07.2024 19:41, Andrew Cooper wrote:
> Coverity complains about it being invalid. It turns out that it is a GCC-ism
> to treat ll and L equivalently. C99 only permits L to mean long double.
>
> Convert all uses of L in to alternatives, either l, ll or PRI.64 depending on
> the operand type. This in turn removes some unnecessary casts which look to
> predate us having correct PRI* constants.
I'm certainly okay with switching to PRI.64 where appropriate. Switching to ll,
however, means longer string literals for no really good reason. We use all
sorts of GCC / GNU extensions; I don't see why we shouldn't be permitted to
also use this one. It's Coverity that wants to cope, imo.
Or, if we really meant to no longer use L, support for it should then imo also
be purged from vsnprintf().
> I'm disappointed at having to use %ll for __fix_to_virt() in apic.c and
> io_apic.c. The expression ends up ULL because of the GB(64) in VMAP_VIRT_END,
> but can't really be changed without breaking 32bit builds of Xen.
>
> One option might be to turn __fix_to_virt() into a proper function, but
> there's a lot of that infrastructure which should be dedup'd and not left to
> each arch to copy.
Maybe it doesn't need us going that far, as ...
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -938,7 +938,7 @@ void __init init_apic_mappings(void)
> apic_phys = mp_lapic_addr;
>
> set_fixmap_nocache(FIX_APIC_BASE, apic_phys);
> - apic_printk(APIC_VERBOSE, "mapped APIC to %08Lx (%08lx)\n", APIC_BASE,
> + apic_printk(APIC_VERBOSE, "mapped APIC to %08llx (%08lx)\n", APIC_BASE,
> apic_phys);
... I wonder why we use __fix_to_virt() for APIC_BASE in the first place.
Using fix_to_virt() would look to be more logical, as all users cast to
a pointer anyway. Then it could simply be %p here.
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -441,7 +441,7 @@ static void intel_log_freq(const struct cpuinfo_x86 *c)
> unsigned long long val = ecx;
>
> val *= ebx;
> - printk("CPU%u: TSC: %u Hz * %u / %u = %Lu Hz\n",
> + printk("CPU%u: TSC: %u Hz * %u / %u = %llu Hz\n",
> smp_processor_id(), ecx, ebx, eax, val / eax);
> }
Maybe change val to be uint64_t instead? That's against what ./CODING_STYLE
calls for, but would be for a reason (to be able to use PRIu64) here.
> --- a/xen/arch/x86/cpu/mcheck/vmce.c
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> @@ -71,7 +71,7 @@ int vmce_restore_vcpu(struct vcpu *v, const struct hvm_vmce_vcpu *ctxt)
> if ( ctxt->caps & ~guest_mcg_cap & ~MCG_CAP_COUNT & ~MCG_CTL_P )
> {
> printk(XENLOG_G_ERR
> - "%s restore: unsupported MCA capabilities %#"PRIx64" for %pv (supported: %#Lx)\n",
> + "%s restore: unsupported MCA capabilities %#"PRIx64" for %pv (supported: %#llx)\n",
> is_hvm_vcpu(v) ? "HVM" : "PV", ctxt->caps,
> v, guest_mcg_cap & ~MCG_CAP_COUNT);
guest_mcg_cap is unsigned long and MCG_CAP_COUNT could as well use UL instead
of ULL, couldn't it?
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -517,7 +517,7 @@ static int vmx_init_vmcs_config(bool bsp)
> if ( (vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32)) >
> PAGE_SIZE )
> {
> - printk("VMX: CPU%d VMCS size is too big (%Lu bytes)\n",
> + printk("VMX: CPU%d VMCS size is too big (%llu bytes)\n",
> smp_processor_id(),
> vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32));
> return -EINVAL;
> @@ -564,7 +564,7 @@ static int vmx_init_vmcs_config(bool bsp)
> if ( (vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32)) !=
> ((vmx_basic_msr & VMX_BASIC_VMCS_SIZE_MASK) >> 32) )
> {
> - printk("VMX: CPU%d unexpected VMCS size %Lu\n",
> + printk("VMX: CPU%d unexpected VMCS size %llu\n",
> smp_processor_id(),
> vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32));
> mismatch = 1;
Same here for VMX_BASIC_VMCS_SIZE_MASK. We leverage not doing 32-bit builds
anymore in exactly this way elsewhere.
> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -2597,7 +2597,7 @@ static void __init ioapic_init_mappings(void)
> }
>
> set_fixmap_nocache(idx, ioapic_phys);
> - apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08Lx (%08lx)\n",
> + apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08llx (%08lx)\n",
> __fix_to_virt(idx), ioapic_phys);
Like above, switch to using fix_to_virt() and %p?
Jan
On 24/07/2024 8:34 am, Jan Beulich wrote:
> On 23.07.2024 19:41, Andrew Cooper wrote:
>> Coverity complains about it being invalid. It turns out that it is a GCC-ism
>> to treat ll and L equivalently. C99 only permits L to mean long double.
>>
>> Convert all uses of L in to alternatives, either l, ll or PRI.64 depending on
>> the operand type. This in turn removes some unnecessary casts which look to
>> predate us having correct PRI* constants.
> I'm certainly okay with switching to PRI.64 where appropriate. Switching to ll,
> however, means longer string literals for no really good reason. We use all
> sorts of GCC / GNU extensions; I don't see why we shouldn't be permitted to
> also use this one. It's Coverity that wants to cope, imo.
I'm about as unfussed with ll as I am over size_t. The differences
presented here are not interesting.
> Or, if we really meant to no longer use L, support for it should then imo also
> be purged from vsnprintf().
>
>> I'm disappointed at having to use %ll for __fix_to_virt() in apic.c and
>> io_apic.c. The expression ends up ULL because of the GB(64) in VMAP_VIRT_END,
>> but can't really be changed without breaking 32bit builds of Xen.
>>
>> One option might be to turn __fix_to_virt() into a proper function, but
>> there's a lot of that infrastructure which should be dedup'd and not left to
>> each arch to copy.
> Maybe it doesn't need us going that far, as ...
>
>> --- a/xen/arch/x86/apic.c
>> +++ b/xen/arch/x86/apic.c
>> @@ -938,7 +938,7 @@ void __init init_apic_mappings(void)
>> apic_phys = mp_lapic_addr;
>>
>> set_fixmap_nocache(FIX_APIC_BASE, apic_phys);
>> - apic_printk(APIC_VERBOSE, "mapped APIC to %08Lx (%08lx)\n", APIC_BASE,
>> + apic_printk(APIC_VERBOSE, "mapped APIC to %08llx (%08lx)\n", APIC_BASE,
>> apic_phys);
> ... I wonder why we use __fix_to_virt() for APIC_BASE in the first place.
> Using fix_to_virt() would look to be more logical, as all users cast to
> a pointer anyway. Then it could simply be %p here.
That could work. Lets see how it ends up looking.
>
>> --- a/xen/arch/x86/cpu/intel.c
>> +++ b/xen/arch/x86/cpu/intel.c
>> @@ -441,7 +441,7 @@ static void intel_log_freq(const struct cpuinfo_x86 *c)
>> unsigned long long val = ecx;
>>
>> val *= ebx;
>> - printk("CPU%u: TSC: %u Hz * %u / %u = %Lu Hz\n",
>> + printk("CPU%u: TSC: %u Hz * %u / %u = %llu Hz\n",
>> smp_processor_id(), ecx, ebx, eax, val / eax);
>> }
> Maybe change val to be uint64_t instead? That's against what ./CODING_STYLE
> calls for, but would be for a reason (to be able to use PRIu64) here.
Can do.
>
>> --- a/xen/arch/x86/cpu/mcheck/vmce.c
>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
>> @@ -71,7 +71,7 @@ int vmce_restore_vcpu(struct vcpu *v, const struct hvm_vmce_vcpu *ctxt)
>> if ( ctxt->caps & ~guest_mcg_cap & ~MCG_CAP_COUNT & ~MCG_CTL_P )
>> {
>> printk(XENLOG_G_ERR
>> - "%s restore: unsupported MCA capabilities %#"PRIx64" for %pv (supported: %#Lx)\n",
>> + "%s restore: unsupported MCA capabilities %#"PRIx64" for %pv (supported: %#llx)\n",
>> is_hvm_vcpu(v) ? "HVM" : "PV", ctxt->caps,
>> v, guest_mcg_cap & ~MCG_CAP_COUNT);
> guest_mcg_cap is unsigned long and MCG_CAP_COUNT could as well use UL instead
> of ULL, couldn't it?
Well, like ...
>
>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>> @@ -517,7 +517,7 @@ static int vmx_init_vmcs_config(bool bsp)
>> if ( (vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32)) >
>> PAGE_SIZE )
>> {
>> - printk("VMX: CPU%d VMCS size is too big (%Lu bytes)\n",
>> + printk("VMX: CPU%d VMCS size is too big (%llu bytes)\n",
>> smp_processor_id(),
>> vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32));
>> return -EINVAL;
>> @@ -564,7 +564,7 @@ static int vmx_init_vmcs_config(bool bsp)
>> if ( (vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32)) !=
>> ((vmx_basic_msr & VMX_BASIC_VMCS_SIZE_MASK) >> 32) )
>> {
>> - printk("VMX: CPU%d unexpected VMCS size %Lu\n",
>> + printk("VMX: CPU%d unexpected VMCS size %llu\n",
>> smp_processor_id(),
>> vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32));
>> mismatch = 1;
> Same here for VMX_BASIC_VMCS_SIZE_MASK. We leverage not doing 32-bit builds
> anymore in exactly this way elsewhere.
... this, it is about 32bit builds.
For better or worse, the msr-index cleanup says to use ULL, and this was
so it could be shared into 32bit codebases. (In this case, I was
thinking HVMLoader and misc bits of userspace.)
~Andrew
On 24.07.2024 12:30, Andrew Cooper wrote:
> On 24/07/2024 8:34 am, Jan Beulich wrote:
>> On 23.07.2024 19:41, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/cpu/mcheck/vmce.c
>>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
>>> @@ -71,7 +71,7 @@ int vmce_restore_vcpu(struct vcpu *v, const struct hvm_vmce_vcpu *ctxt)
>>> if ( ctxt->caps & ~guest_mcg_cap & ~MCG_CAP_COUNT & ~MCG_CTL_P )
>>> {
>>> printk(XENLOG_G_ERR
>>> - "%s restore: unsupported MCA capabilities %#"PRIx64" for %pv (supported: %#Lx)\n",
>>> + "%s restore: unsupported MCA capabilities %#"PRIx64" for %pv (supported: %#llx)\n",
>>> is_hvm_vcpu(v) ? "HVM" : "PV", ctxt->caps,
>>> v, guest_mcg_cap & ~MCG_CAP_COUNT);
>> guest_mcg_cap is unsigned long and MCG_CAP_COUNT could as well use UL instead
>> of ULL, couldn't it?
>
> Well, like ...
>
>>
>>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>>> @@ -517,7 +517,7 @@ static int vmx_init_vmcs_config(bool bsp)
>>> if ( (vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32)) >
>>> PAGE_SIZE )
>>> {
>>> - printk("VMX: CPU%d VMCS size is too big (%Lu bytes)\n",
>>> + printk("VMX: CPU%d VMCS size is too big (%llu bytes)\n",
>>> smp_processor_id(),
>>> vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32));
>>> return -EINVAL;
>>> @@ -564,7 +564,7 @@ static int vmx_init_vmcs_config(bool bsp)
>>> if ( (vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32)) !=
>>> ((vmx_basic_msr & VMX_BASIC_VMCS_SIZE_MASK) >> 32) )
>>> {
>>> - printk("VMX: CPU%d unexpected VMCS size %Lu\n",
>>> + printk("VMX: CPU%d unexpected VMCS size %llu\n",
>>> smp_processor_id(),
>>> vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32));
>>> mismatch = 1;
>> Same here for VMX_BASIC_VMCS_SIZE_MASK. We leverage not doing 32-bit builds
>> anymore in exactly this way elsewhere.
>
> ... this, it is about 32bit builds.
I don't think this is relevant for the VMCS constants?
> For better or worse, the msr-index cleanup says to use ULL, and this was
> so it could be shared into 32bit codebases. (In this case, I was
> thinking HVMLoader and misc bits of userspace.)
Hmm, yes. Let me mention though that right when starting that cleanup, you
introduced two outliers - MSR_CTC_{THREAD,CORE}_MASK. The latter was later
changed to have a U suffix, in part due to both Stefano and me not paying
enough attention when reviewing. So while I can see the goal, I'm wondering
whether we shouldn't take the abstraction there yet a step further and
arrange for UL suffixes in 64-bit builds, but for ULL in 32-bit ones:
#ifdef __i386__
# define _MC(x) _AC(x, ULL)
#else
# define _MC(x) _AC(x, UL)
#endif
Thoughts?
Jan
On Tue, 23 Jul 2024, Andrew Cooper wrote: > Coverity complains about it being invalid. It turns out that it is a GCC-ism > to treat ll and L equivalently. C99 only permits L to mean long double. > > Convert all uses of L in to alternatives, either l, ll or PRI.64 depending on > the operand type. This in turn removes some unnecessary casts which look to > predate us having correct PRI* constants. > > No functional change. > > Coverity-IDs: 1464224, 1464228, 1464248, 1464261, 1464272, 1464281, 1464287, 1464290, 1494437 > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
© 2016 - 2024 Red Hat, Inc.