On Thu, Feb 01, 2024 at 01:09:11PM +0100, Jan Beulich wrote:
> On 01.02.2024 12:50, Roger Pau Monné wrote:
> > On Thu, Jan 11, 2024 at 10:00:10AM +0100, Jan Beulich wrote:
> >> @@ -503,6 +538,9 @@ static int vmx_init_vmcs_config(bool bsp
> >> "Secondary Exec Control",
> >> vmx_secondary_exec_control, _vmx_secondary_exec_control);
> >> mismatch |= cap_check(
> >> + "Tertiary Exec Control",
> >> + vmx_tertiary_exec_control, _vmx_tertiary_exec_control);
> >
> > I know it's done to match the surrounding style, but couldn't you move
> > the name parameter one line up, and then limit the call to two lines?
> >
> > (I don't think it will compromise readability).
>
> You mean like this:
>
> mismatch |= cap_check("Tertiary Exec Control",
> vmx_tertiary_exec_control, _vmx_tertiary_exec_control);
>
> ? No, I view this as a mix of two possible styles. If the string literal
> was moved up, the other legitimate style would only be
>
> mismatch |= cap_check("Tertiary Exec Control",
> vmx_tertiary_exec_control,
> _vmx_tertiary_exec_control);
>
> aiui (again extending over 3 lines). Yet none of this is written down
> anywhere.
>
> But anyway - consistency with surrounding code trumps here, I think.
I was hoping it could still fit on 2 lines, but if you need 3 never
mind then.
> >> @@ -2068,10 +2111,12 @@ void vmcs_dump_vcpu(struct vcpu *v)
> >> vmr(HOST_PERF_GLOBAL_CTRL));
> >>
> >> printk("*** Control State ***\n");
> >> - printk("PinBased=%08x CPUBased=%08x SecondaryExec=%08x\n",
> >> + printk("PinBased=%08x CPUBased=%08x\n",
> >> vmr32(PIN_BASED_VM_EXEC_CONTROL),
> >> - vmr32(CPU_BASED_VM_EXEC_CONTROL),
> >> - vmr32(SECONDARY_VM_EXEC_CONTROL));
> >> + vmr32(CPU_BASED_VM_EXEC_CONTROL));
> >> + printk("SecondaryExec=%08x TertiaryExec=%08lx\n",
> >
> > For consistency, shouldn't TertiaryExec use 016 instead of 08 (as it's
> > a 64bit filed).
>
> Perhaps, assuming we'll gets bits 32 and populated sooner or later.
> However, I view 16-digit literal numbers as hard to read, so I'd be
> inclined to insert a separator (e.g. an underscore) between the low
> and high halves. Thoughts?
Works for me.
Thanks, Roger.
