Re: [PATCH] xen/printk: Avoid the use of L as a length modifier

[Jan Beulich <jbeulich@xxxxxxxx>]

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