Re: [PATCH 1/2] KVM: selftests: Fix initialization of GDT limit

[Sean Christopherson <seanjc@google.com>]

On Wed, Jan 18, 2023, Sean Christopherson wrote:
> On Sat, Jan 14, 2023, Ackerley Tng wrote:
> > Subtract 1 to initialize GDT limit according to spec.
> 
> Nit, make changelogs standalone, i.e. don't make me read the code just to
> understand the changelog.  "Subtract 1" is meaningless without seeing the
> existing code.  The changelog doesn't need to be a play-by-play, e.g. describing
> the change as "inclusive vs. exclusive" is also fine, the important thing is that
> readers can gain a basic understanding of the change without needing to read code.
> 
> > Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> > ---
> >  tools/testing/selftests/kvm/lib/x86_64/processor.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> > index acfa1d01e7df..33ca7f5232a4 100644
> > --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
> > +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> > @@ -502,7 +502,12 @@ static void kvm_setup_gdt(struct kvm_vm *vm, struct kvm_dtable *dt)
> >  		vm->gdt = __vm_vaddr_alloc_page(vm, MEM_REGION_DATA);
> >  
> >  	dt->base = vm->gdt;
> > -	dt->limit = getpagesize();
> > +
> > +	/*
> > +	 * Intel SDM Volume 3, 3.5.1:
> 
> As a general rule, especially in code comments, never reference manual sections
> by their index/numbers, there's a 99% chance the comment will be stale within a
> few years.
> 
> Quoting manuals is ok, because if the quote because stale then that in and of
> itself is an interesting datapoint.  If referencing a specific section is the
> easiest way to convey something, then use then name of the section, as that's less
> likely to be arbitrarily change.
> 
> In this case, I'd just omit the comment entirely.  We have to assume readers have
> a minimum level of knowledge, and IMO this is firmly below (above?) the threshold.

No need to post a v2, I'll fold this patch into a larger cleanup of the descriptor
table code.

Thanks!