Re: PKEY syscall number for selftest? (was: [PATCH 4/7] KVM: TDX: restore host xsave state when exit from the guest TD)

[Sean Christopherson <seanjc@google.com>]

Ping, I'm guessing this fell through the holiday cracks.

On Fri, Dec 20, 2024, Sean Christopherson wrote:
> Switching topics, dropped everyone else except the list.
> 
> On Fri, Dec 20, 2024, Sean Christopherson wrote:
> >  arch/x86/kvm/x86.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 4320647bd78a..9d5cece9260b 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1186,7 +1186,7 @@ void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu)
> >  	    vcpu->arch.pkru != vcpu->arch.host_pkru &&
> >  	    ((vcpu->arch.xcr0 & XFEATURE_MASK_PKRU) ||
> >  	     kvm_is_cr4_bit_set(vcpu, X86_CR4_PKE)))
> > -		write_pkru(vcpu->arch.pkru);
> > +		wrpkru(vcpu->arch.pkru);
> >  }
> >  EXPORT_SYMBOL_GPL(kvm_load_guest_xsave_state);
> >  
> > @@ -1200,7 +1200,7 @@ void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu)
> >  	     kvm_is_cr4_bit_set(vcpu, X86_CR4_PKE))) {
> >  		vcpu->arch.pkru = rdpkru();
> >  		if (vcpu->arch.pkru != vcpu->arch.host_pkru)
> > -			write_pkru(vcpu->arch.host_pkru);
> > +			wrpkru(vcpu->arch.host_pkru);
> >  	}
> >  
> >  	if (kvm_is_cr4_bit_set(vcpu, X86_CR4_OSXSAVE)) {
> > 
> > base-commit: 13e98294d7cec978e31138d16824f50556a62d17
> > -- 
> 
> I tried to test this by running the mm/protection_keys selftest in a VM, but it
> gives what are effectively false passes on x86-64 due to the selftest picking up
> the generic syscall numbers, e.g. 289 for SYS_pkey_alloc, instead of the x86-64
> numbers.
> 
> I was able to get the test to run by hacking tools/testing/selftests/mm/pkey-x86.h
> to shove in the right numbers, but I can't imagine that's the intended behavior.
> 
> If I omit the #undefs from pkey-x86.h, it shows that the test is grabbing the
> definitions from the generic usr/include/asm-generic/unistd.h header.
> 
> Am I doing something stupid?
> 
> Regardless of whether this is PEBKAC or working as intended, on x86, the test
> should ideally assert that "ospke" support in /proc/cpuinfo is consistent with
> the result of sys_pkey_alloc(), e.g. so that an failure to allocate a pkey on a
> system that work is reported as an error, not a pass.
> 
> --
> diff --git a/tools/testing/selftests/mm/pkey-x86.h b/tools/testing/selftests/mm/pkey-x86.h
> index ac91777c8917..ccc3552e6b77 100644
> --- a/tools/testing/selftests/mm/pkey-x86.h
> +++ b/tools/testing/selftests/mm/pkey-x86.h
> @@ -3,6 +3,10 @@
>  #ifndef _PKEYS_X86_H
>  #define _PKEYS_X86_H
>  
> +#define __NR_pkey_mprotect     329
> +#define __NR_pkey_alloc                330
> +#define __NR_pkey_free         331
> +
>  #ifdef __i386__
>  
>  #define REG_IP_IDX             REG_EIP
> --
> 
> Yields:
> 
> $ ARCH=x86_64 make protection_keys_64
> gcc -Wall -I /home/sean/go/src/kernel.org/linux/tools/testing/selftests/../../..  -isystem /home/sean/go/src/kernel.org/linux/tools/testing/selftests/../../../usr/include -isystem /home/sean/go/src/kernel.org/linux/tools/testing/selftests/../../../tools/include/uapi -no-pie -D_GNU_SOURCE=  -m64 -mxsave  protection_keys.c vm_util.c thp_settings.c -lrt -lpthread -lm -lrt -ldl -o /home/sean/go/src/kernel.org/linux/tools/testing/selftests/mm/protection_keys_64
> In file included from pkey-helpers.h:102:0,
>                  from protection_keys.c:49:
> pkey-x86.h:6:0: warning: "__NR_pkey_mprotect" redefined
>  #define __NR_pkey_mprotect 329
>  
> In file included from protection_keys.c:45:0:
> /home/sean/go/src/kernel.org/linux/usr/include/asm-generic/unistd.h:693:0: note: this is the location of the previous definition
>  #define __NR_pkey_mprotect 288
>  
> In file included from pkey-helpers.h:102:0,
>                  from protection_keys.c:49:
> pkey-x86.h:7:0: warning: "__NR_pkey_alloc" redefined
>  #define __NR_pkey_alloc  330
>  
> In file included from protection_keys.c:45:0:
> /home/sean/go/src/kernel.org/linux/usr/include/asm-generic/unistd.h:695:0: note: this is the location of the previous definition
>  #define __NR_pkey_alloc 289
>  
> In file included from pkey-helpers.h:102:0,
>                  from protection_keys.c:49:
> pkey-x86.h:8:0: warning: "__NR_pkey_free" redefined
>  #define __NR_pkey_free  331
>  
> In file included from protection_keys.c:45:0:
> /home/sean/go/src/kernel.org/linux/usr/include/asm-generic/unistd.h:697:0: note: this is the location of the previous definition
>  #define __NR_pkey_free 290
>  
>