Re: [PATCH] KVM: x86: Update the version number of SDM in comments

[Sean Christopherson <seanjc@google.com>]

On Thu, Jun 15, 2023, Yu Zhang wrote:
> On Thu, Jun 15, 2023 at 04:06:24PM +0800, Jun Miao wrote:
> > A little optimized update version number of SDM and corresponding
> > public date, making it more accurate to retrieve.
> > 
> > Signed-off-by: Jun Miao <jun.miao@intel.com>
> > ---
> >  arch/x86/kvm/lapic.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index d7639d126e6c..4c5493e08d2e 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -2260,7 +2260,7 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
> >  	/*
> >  	 * APIC register must be aligned on 128-bits boundary.
> >  	 * 32/64/128 bits registers must be accessed thru 32 bits.
> > -	 * Refer SDM 8.4.1
> 
> I would suggest just remove this line.

+1, and clean up the other part too?  The line about 32/64/128 bits registers is
also flawed; there are no 128-bit registers, just 256-bit registers.  I see no
reason to precisely call out the sizes.  And this would be a good opportunity to
call out that KVM allows smaller reads, but not smaller writes.

E.g. something like this?

	/*
	 * APIC registers must be aligned on 128-bit boundaries, and must be
	 * written using 32-bit stores regardless of the register size.  Note,
	 * KVM allows smaller 8-bit and 16-bit loads to be compatible with
	 * guest software (some microarchitectures support such loads, even
	 * though only 32-bit loads are architecturally guaranteed to work).
	 */

> And maybe, add "According to Intel SDM, " at the beginning of the comments.

Nah, not necessary.  It's implied that KVM is implementing architecturally behavior
unless otherwise stated.  And this isn't Intel specific code, e.g. the APM (hopefully)
says the same thing.