Re: [PATCH v3] KVM: VMX: do not disable interception for MSR_IA32_SPEC_CTRL on eIBRS

[Sean Christopherson <seanjc@xxxxxxxxxx>]

On Wed, May 25, 2022, Jon Kohler wrote:
> > On May 25, 2022, at 1:04 PM, Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > the code.  Again, it's all about whether eIBRS is advertised to the guest.  With
> > some other minor tweaking to wrangle the comment to 80 chars...
> 
> RE 80 chars - quick question (and forgive the silly question here), but how are you
> counting that? I’ve got my editor cutting at 79 cols, where tab size is accounted
> for as 4 cols, so the longest line on my side for this patch is 72-73 or so.

Tabs are 8 cols in the kernel.  FWIW, your patch was totally fine with respect to
wrapping, my comment was purely to give the heads up that I arbitrarily tweaked
other bits of the comment to adjust for my suggested reword.

> These also pass the checkpatch.pl script as well, so I just want to make sure
> going forward I’m formatting them appropriately.

For future reference, checkpatch.pl only yells if a line length exceeds 100 chars.
The 80 char limit is a soft limit, with 100 chars being a mostly-firm limit.
checkpatch used to yell at 80, but was changed because too many people were
interpreting 80 chars as a hard limit and blindly wrapping code to make checkpatch
happy at the cost of yielding less readable code.

Whether or not to run over the soft limit is definitely subjective, just try to
use common sense.  E.g. overflowing by 1-2 chars, not a big deal, especially if
the statement would otherwise fit on a single line, i.e. doesn't already wrap.

A decent example is the SGX MSRs, which allows the SGX_LC_ENABLED check to run
over a little, but wraps the data update.  The reasoning is that

		if (!msr_info->host_initiated &&
		    (!guest_cpuid_has(vcpu, X86_FEATURE_SGX_LC) ||
		    ((vmx->msr_ia32_feature_control & FEAT_CTL_LOCKED) &&
		    !(vmx->msr_ia32_feature_control & FEAT_CTL_SGX_LC_ENABLED))))
			return 1;

is much more readable than 

		if (!msr_info->host_initiated &&
		    (!guest_cpuid_has(vcpu, X86_FEATURE_SGX_LC) ||
		    ((vmx->msr_ia32_feature_control & FEAT_CTL_LOCKED) &&
		    !(vmx->msr_ia32_feature_control &
		      FEAT_CTL_SGX_LC_ENABLED))))
			return 1;

but this

		vmx->msr_ia32_sgxlepubkeyhash
			[msr_index - MSR_IA32_SGXLEPUBKEYHASH0] = data;

is only isn't _that_ much worse than running the line way out, and ~93 chars gets
to be a bit too long.

		vmx->msr_ia32_sgxlepubkeyhash[msr_index - MSR_IA32_SGXLEPUBKEYHASH0] = data;