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

[Sean Christopherson <seanjc@google.com>]

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.

> "the GDT limit should always be one less
> +	 * than an integral multiple of eight"
> +	 */
> +	dt->limit = getpagesize() - 1;
>  }
>  
>  static void kvm_setup_tss_64bit(struct kvm_vm *vm, struct kvm_segment *segp,
> -- 
> 2.39.0.314.g84b9a713c41-goog
>