[RFC PATCH 0/4] TDX KVM selftests

[RFC PATCH 0/4] TDX KVM selftests

[Erdem Aktas]

TDX stands for Trust Domain Extensions which isolates VMs from the
virtual-machine manager (VMM)/hypervisor and any other software on the
platform.

Intel has recently submitted a set of RFC patches for KVM support for
TDX and more information can be found on the latest TDX Support 
Patches: https://lkml.org/lkml/2021/7/2/558

Due to the nature of the confidential computing environment that TDX
provides, it is very difficult to verify/test the KVM support. TDX
requires UEFI and the guest kernel to be enlightened which are all under
development.

We are working on a set of selftests to close this gap and be able to
verify the KVM functionality to support TDX lifecycle and GHCI [1]
interface.

We are looking for any feedback on:
- Patch series itself
- Any suggestion on how we should approach testing TDX functionality.
Does selftests seems reasonable or should we switch to using KVM
unit tests. I would be happy to get some perspective on how KVM unit
tests can help us more.
- Any test case or scenario that we should add.
- Anything else I have not thought of yet.

Current patch series provide the following capabilities:

- Provide helper functions to create a TD (Trusted Domain) using the KVM
  ioctls
- Provide helper functions to create a guest image that can include any
  testing code
- Provide helper functions and wrapper functions to write testing code
  using GHCI interface
- Add a test case that verifies TDX life cycle 
- Add a test case that verifies TDX GHCI port IO 

TODOs:
- Use existing function to create page tables dynamically 
  (ie __virt_pg_map())
- Remove arbitrary defined magic numbers for data structure offsets
- Add TDVMCALL for error reporting
- Add additional test cases as some listed below
- Add #VE handlers to help testing more complicated test cases

Other test cases that we are planning to add:
(with credit to sagis@google.com)

VM call interface        Input                        Output                Result
GetTdVmCallInfo          R12=0                        None                VMCALL_SUCCESS
MapGPA                   Map private page (GPA.S=0)                       VMCALL_SUCCESS
MapGPA                   Map shared page (GPA.S=1)                        VMCALL_SUCCESS
MapGPA                   Map already private page as private              VMCALL_INVALID_OPERAND
MapGPA                   Map already shared page as shared                VMCALL_INVALID_OPERAND
GetQuote                        
ReportFatalError                        
SetupEventNotifyInterrupt   Valid interrupt value (32:255)                 VMCALL_SUCCESS
SetupEventNotifyInterrupt   Invalid value (>255)                          VMCALL_INVALID_OPERAND
Instruction.CPUID        R12(EAX)=1, R13(ECX)=0       EBX[8:15]=0x8        
                                                      EBX[16:23]=X        
                                                      EBX[24:31]=vcpu_id        
                                                      ECX[0]=1        
                                                      ECX[12]=Y        
Instruction.CPUID       R12(EAX)=1, R13(ECX)=4                            VMCALL_INVALID_OPERAND
VE.RequestMMIO                        
Instruction.HLT                                                           VMCALL_SUCCESS
Instruction.IO          Read/Write 1/2/4 bytes                            VMCALL_SUCCESS
Instruction.IO          Read/Write 3 bytes                                VMCALL_INVALID_OPERAND
Instruction.RDMSR       Accessible register           R11=msr_value       VMCALL_SUCCESS
                        Inaccessible register                             VMCALL_INVALID_OPERAND
Instruction.RDMSR       Accessible register                               VMCALL_SUCCESS
                        Inaccessible register                             VMCALL_INVALID_OPERAND
INSTRUCTION.PCONFIG                        

[1] Intel TDX Guest-Hypervisor Communication Interface
    https://software.intel.com/content/dam/develop/external/us/en/documents/intel-tdx-guest-hypervisor-communication-interface.pdf


Erdem Aktas (4):
  KVM: selftests: Add support for creating non-default type VMs
  KVM: selftest: Add helper functions to create TDX VMs
  KVM: selftest: Adding TDX life cycle test.
  KVM: selftest: Adding test case for TDX port IO

 tools/testing/selftests/kvm/Makefile          |   6 +-
 .../testing/selftests/kvm/include/kvm_util.h  |   1 +
 .../selftests/kvm/include/x86_64/processor.h  |   5 +
 tools/testing/selftests/kvm/lib/kvm_util.c    |  29 +-
 .../selftests/kvm/lib/x86_64/processor.c      |  23 ++
 tools/testing/selftests/kvm/lib/x86_64/tdx.h  | 220 ++++++++++++
 .../selftests/kvm/lib/x86_64/tdx_lib.c        | 314 ++++++++++++++++++
 .../selftests/kvm/x86_64/tdx_vm_tests.c       | 209 ++++++++++++
 8 files changed, 800 insertions(+), 7 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/lib/x86_64/tdx.h
 create mode 100644 tools/testing/selftests/kvm/lib/x86_64/tdx_lib.c
 create mode 100644 tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c

-- 
2.32.0.432.gabb21c7263-goog

[RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[Erdem Aktas]

Currently vm_create function only creates KVM_X86_LEGACY_VM type VMs.
Changing the vm_create function to accept type parameter to create
new VM types.

Signed-off-by: Erdem Aktas <erdemaktas@google.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Peter Gonda <pgonda@google.com>
Reviewed-by: Marc Orr <marcorr@google.com>
Reviewed-by: Sagi Shahar <sagis@google.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  |  1 +
 tools/testing/selftests/kvm/lib/kvm_util.c    | 29 +++++++++++++++++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index d53bfadd2..c63df42d6 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -88,6 +88,7 @@ int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id,
 void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
 
 struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
+struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm, int type);
 void kvm_vm_free(struct kvm_vm *vmp);
 void kvm_vm_restart(struct kvm_vm *vmp, int perm);
 void kvm_vm_release(struct kvm_vm *vmp);
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index e5fbf16f7..70caa3882 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -180,13 +180,36 @@ _Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct vm_guest_mode_params)
  * Return:
  *   Pointer to opaque structure that describes the created VM.
  *
- * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K).
+ * Wrapper VM Create function to create a VM with default type (0).
+ */
+struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
+{
+	return __vm_create(mode, phy_pages, perm, 0);
+}
+
+/*
+ * VM Create with a custom type
+ *
+ * Input Args:
+ *   mode - VM Mode (e.g. VM_MODE_P52V48_4K)
+ *   phy_pages - Physical memory pages
+ *   perm - permission
+ *   type - VM type
+ *
+ * Output Args: None
+ *
+ * Return:
+ *   Pointer to opaque structure that describes the created VM.
+ *
+ * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K) and the
+ * type specified in type (e.g. KVM_X86_LEGACY_VM, KVM_X86_TDX_VM ...).
  * When phy_pages is non-zero, a memory region of phy_pages physical pages
  * is created and mapped starting at guest physical address 0.  The file
  * descriptor to control the created VM is created with the permissions
  * given by perm (e.g. O_RDWR).
  */
-struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
+struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages,
+			    int perm, int type)
 {
 	struct kvm_vm *vm;
 
@@ -200,7 +223,7 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
 	INIT_LIST_HEAD(&vm->userspace_mem_regions);
 
 	vm->mode = mode;
-	vm->type = 0;
+	vm->type = type;
 
 	vm->pa_bits = vm_guest_mode_params[mode].pa_bits;
 	vm->va_bits = vm_guest_mode_params[mode].va_bits;
-- 
2.32.0.432.gabb21c7263-goog

Re: [RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[David Matlack]

On Mon, Jul 26, 2021 at 11:37:54AM -0700, Erdem Aktas wrote:
> Currently vm_create function only creates KVM_X86_LEGACY_VM type VMs.
> Changing the vm_create function to accept type parameter to create
> new VM types.
> 
> Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> Reviewed-by: Sean Christopherson <seanjc@google.com>
> Reviewed-by: Peter Gonda <pgonda@google.com>
> Reviewed-by: Marc Orr <marcorr@google.com>
> Reviewed-by: Sagi Shahar <sagis@google.com>

Reviewed-by: David Matlack <dmatlack@google.com>

(aside from the nit below)

> ---
>  .../testing/selftests/kvm/include/kvm_util.h  |  1 +
>  tools/testing/selftests/kvm/lib/kvm_util.c    | 29 +++++++++++++++++--
>  2 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index d53bfadd2..c63df42d6 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -88,6 +88,7 @@ int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id,
>  void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
>  
>  struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
> +struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm, int type);

nit: Consider using a more readable function name such as
vm_create_with_type().

>  void kvm_vm_free(struct kvm_vm *vmp);
>  void kvm_vm_restart(struct kvm_vm *vmp, int perm);
>  void kvm_vm_release(struct kvm_vm *vmp);
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index e5fbf16f7..70caa3882 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -180,13 +180,36 @@ _Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct vm_guest_mode_params)
>   * Return:
>   *   Pointer to opaque structure that describes the created VM.
>   *
> - * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K).
> + * Wrapper VM Create function to create a VM with default type (0).
> + */
> +struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
> +{
> +	return __vm_create(mode, phy_pages, perm, 0);
> +}
> +
> +/*
> + * VM Create with a custom type
> + *
> + * Input Args:
> + *   mode - VM Mode (e.g. VM_MODE_P52V48_4K)
> + *   phy_pages - Physical memory pages
> + *   perm - permission
> + *   type - VM type
> + *
> + * Output Args: None
> + *
> + * Return:
> + *   Pointer to opaque structure that describes the created VM.
> + *
> + * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K) and the
> + * type specified in type (e.g. KVM_X86_LEGACY_VM, KVM_X86_TDX_VM ...).
>   * When phy_pages is non-zero, a memory region of phy_pages physical pages
>   * is created and mapped starting at guest physical address 0.  The file
>   * descriptor to control the created VM is created with the permissions
>   * given by perm (e.g. O_RDWR).
>   */
> -struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
> +struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages,
> +			    int perm, int type)
>  {
>  	struct kvm_vm *vm;
>  
> @@ -200,7 +223,7 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
>  	INIT_LIST_HEAD(&vm->userspace_mem_regions);
>  
>  	vm->mode = mode;
> -	vm->type = 0;
> +	vm->type = type;
>  
>  	vm->pa_bits = vm_guest_mode_params[mode].pa_bits;
>  	vm->va_bits = vm_guest_mode_params[mode].va_bits;
> -- 
> 2.32.0.432.gabb21c7263-goog
> 

Re: [RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[Sean Christopherson]

On Mon, Jul 26, 2021, David Matlack wrote:
> On Mon, Jul 26, 2021 at 11:37:54AM -0700, Erdem Aktas wrote:
> > Currently vm_create function only creates KVM_X86_LEGACY_VM type VMs.
> > Changing the vm_create function to accept type parameter to create
> > new VM types.
> > 
> > Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> > Reviewed-by: Sean Christopherson <seanjc@google.com>

*-by tags should not be added unless explicitly provided.  IIRC, our internal
gerrit will convert +1 to Reviewed-by, but I don't think that's the case here.
This applies to all patches in this series.

See "Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:" in
Documentation/process/submitting-patches.rst for more info.

> > Reviewed-by: Peter Gonda <pgonda@google.com>
> > Reviewed-by: Marc Orr <marcorr@google.com>
> > Reviewed-by: Sagi Shahar <sagis@google.com>
> 
> Reviewed-by: David Matlack <dmatlack@google.com>
> 
> (aside from the nit below)
> 
> > ---
> >  .../testing/selftests/kvm/include/kvm_util.h  |  1 +
> >  tools/testing/selftests/kvm/lib/kvm_util.c    | 29 +++++++++++++++++--
> >  2 files changed, 27 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index d53bfadd2..c63df42d6 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -88,6 +88,7 @@ int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id,
> >  void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
> >  
> >  struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
> > +struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm, int type);
> 
> nit: Consider using a more readable function name such as
> vm_create_with_type().

Ha!  This is why I don't like doing internal reviews :-D

Erdem originally had vm_create_type(), I suggested __vm_create() as the double
underscore scheme is more common in the kernel for cases where there's a default
wrapper and an inner helper that implements the full API.

Convention aside, the argument againsts ...with_type() are that it doesn't scale,
e.g. if someone adds another parameter parameter for which vm_create() provides a
default, and it doesn't self-document the relationship between vm_create() and
the inner helper, e.g. by convention, based on names alone I know that vm_create()
likely is a wrapper around __vm_create().

Compare that with the existing

  vm_create_default_with_vcpus()
  vm_create_default()
  vm_create_with_vcpus()
  vm_create()

where the relationship between all the helpers is not immediately clear, and
vm_create_with_vcpus() is a misnomer because it does much more than call vm_create()
and instantiate vCPUs, e.g. it also instantiates the IRQ chip and loads the test
into guest memory.

Re: [RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[David Matlack]

On Tue, Jul 27, 2021 at 08:47:44PM +0000, Sean Christopherson wrote:
> On Mon, Jul 26, 2021, David Matlack wrote:
> > On Mon, Jul 26, 2021 at 11:37:54AM -0700, Erdem Aktas wrote:
> > > Currently vm_create function only creates KVM_X86_LEGACY_VM type VMs.
> > > Changing the vm_create function to accept type parameter to create
> > > new VM types.
> > > 
> > > Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> > > Reviewed-by: Sean Christopherson <seanjc@google.com>
> 
> *-by tags should not be added unless explicitly provided.  IIRC, our internal
> gerrit will convert +1 to Reviewed-by, but I don't think that's the case here.
> This applies to all patches in this series.
> 
> See "Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:" in
> Documentation/process/submitting-patches.rst for more info.
> 
> > > Reviewed-by: Peter Gonda <pgonda@google.com>
> > > Reviewed-by: Marc Orr <marcorr@google.com>
> > > Reviewed-by: Sagi Shahar <sagis@google.com>
> > 
> > Reviewed-by: David Matlack <dmatlack@google.com>
> > 
> > (aside from the nit below)
> > 
> > > ---
> > >  .../testing/selftests/kvm/include/kvm_util.h  |  1 +
> > >  tools/testing/selftests/kvm/lib/kvm_util.c    | 29 +++++++++++++++++--
> > >  2 files changed, 27 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > > index d53bfadd2..c63df42d6 100644
> > > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > > @@ -88,6 +88,7 @@ int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id,
> > >  void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
> > >  
> > >  struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
> > > +struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm, int type);
> > 
> > nit: Consider using a more readable function name such as
> > vm_create_with_type().
> 
> Ha!  This is why I don't like doing internal reviews :-D

+1 :)

> 
> Erdem originally had vm_create_type(), I suggested __vm_create() as the double
> underscore scheme is more common in the kernel for cases where there's a default
> wrapper and an inner helper that implements the full API.
> 
> Convention aside, the argument againsts ...with_type() are that it doesn't scale,
> e.g. if someone adds another parameter parameter for which vm_create() provides a
> default, and it doesn't self-document the relationship between vm_create() and
> the inner helper, e.g. by convention, based on names alone I know that vm_create()
> likely is a wrapper around __vm_create().

True, although with __vm_create() is not solving the scalability
problem, it's just preventing scaling altogether (you can only have 1
wrapper function, vm_create). So if any caller wants to override one of
the defaults they have to override all of them.

I agree with you though in this case: __vm_create() is a better choice
(especially given the existence of vm_create_with_vcpus).

A better option than both (but would involve more work) would be to
create an options struct with all optional arguments. Unfortunately C
makes working with options structs a bit clumsy. But it's something to
consider as the number of options passed to __vm_create increases.

For example:

struct vm_options {
        enum vm_guest_mode mode;
        uint64_t phy_pages;
        int perm;
        int type;
};

struct kvm_vm *vm_create(const struct vm_options *options)
{
        ...
}

static const struct vm_options default_vm_options = {
  .mode = VM_MODE_DEFAULT,
  .phy_pages = DEFAULT_GUEST_PHY_PAGES,
  .perm = O_RDWR,
  .type = DEFAULT_VM_TYPE,
};

/* Create a VM with default options. */
vm = create_vm(&default_vm_options);

/* Create a VM with TDX enabled. */
struct vm_options options = default_vm_options;
options.type = VM_TYPE_TDX;
vm = create_vm(&options);

(I'm sure I ham-fisted the const stuff but you get the idea.)

I'm toying with introducing an options struct to perf_test_util as well
so this is very top of mind.

> 
> Compare that with the existing
> 
>   vm_create_default_with_vcpus()
>   vm_create_default()
>   vm_create_with_vcpus()
>   vm_create()
> 
> where the relationship between all the helpers is not immediately clear, and
> vm_create_with_vcpus() is a misnomer because it does much more than call vm_create()
> and instantiate vCPUs, e.g. it also instantiates the IRQ chip and loads the test
> into guest memory.

Re: [RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[Andrew Jones]

On Wed, Jul 28, 2021 at 04:07:19PM +0000, David Matlack wrote:
> On Tue, Jul 27, 2021 at 08:47:44PM +0000, Sean Christopherson wrote:
> > On Mon, Jul 26, 2021, David Matlack wrote:
> > > On Mon, Jul 26, 2021 at 11:37:54AM -0700, Erdem Aktas wrote:
> > > > Currently vm_create function only creates KVM_X86_LEGACY_VM type VMs.
> > > > Changing the vm_create function to accept type parameter to create
> > > > new VM types.
> > > > 
> > > > Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> > > > Reviewed-by: Sean Christopherson <seanjc@google.com>
> > 
> > *-by tags should not be added unless explicitly provided.  IIRC, our internal
> > gerrit will convert +1 to Reviewed-by, but I don't think that's the case here.
> > This applies to all patches in this series.
> > 
> > See "Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:" in
> > Documentation/process/submitting-patches.rst for more info.
> > 
> > > > Reviewed-by: Peter Gonda <pgonda@google.com>
> > > > Reviewed-by: Marc Orr <marcorr@google.com>
> > > > Reviewed-by: Sagi Shahar <sagis@google.com>
> > > 
> > > Reviewed-by: David Matlack <dmatlack@google.com>
> > > 
> > > (aside from the nit below)
> > > 
> > > > ---
> > > >  .../testing/selftests/kvm/include/kvm_util.h  |  1 +
> > > >  tools/testing/selftests/kvm/lib/kvm_util.c    | 29 +++++++++++++++++--
> > > >  2 files changed, 27 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > > > index d53bfadd2..c63df42d6 100644
> > > > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > > > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > > > @@ -88,6 +88,7 @@ int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id,
> > > >  void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
> > > >  
> > > >  struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
> > > > +struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm, int type);
> > > 
> > > nit: Consider using a more readable function name such as
> > > vm_create_with_type().
> > 
> > Ha!  This is why I don't like doing internal reviews :-D
> 
> +1 :)
> 
> > 
> > Erdem originally had vm_create_type(), I suggested __vm_create() as the double
> > underscore scheme is more common in the kernel for cases where there's a default
> > wrapper and an inner helper that implements the full API.
> > 
> > Convention aside, the argument againsts ...with_type() are that it doesn't scale,
> > e.g. if someone adds another parameter parameter for which vm_create() provides a
> > default, and it doesn't self-document the relationship between vm_create() and
> > the inner helper, e.g. by convention, based on names alone I know that vm_create()
> > likely is a wrapper around __vm_create().
> 
> True, although with __vm_create() is not solving the scalability
> problem, it's just preventing scaling altogether (you can only have 1
> wrapper function, vm_create). So if any caller wants to override one of
> the defaults they have to override all of them.
> 
> I agree with you though in this case: __vm_create() is a better choice
> (especially given the existence of vm_create_with_vcpus).
> 
> A better option than both (but would involve more work) would be to
> create an options struct with all optional arguments. Unfortunately C
> makes working with options structs a bit clumsy. But it's something to
> consider as the number of options passed to __vm_create increases.
> 
> For example:
> 
> struct vm_options {
>         enum vm_guest_mode mode;
>         uint64_t phy_pages;
>         int perm;
>         int type;
> };
> 
> struct kvm_vm *vm_create(const struct vm_options *options)
> {
>         ...
> }
> 
> static const struct vm_options default_vm_options = {
>   .mode = VM_MODE_DEFAULT,
>   .phy_pages = DEFAULT_GUEST_PHY_PAGES,
>   .perm = O_RDWR,
>   .type = DEFAULT_VM_TYPE,
> };
> 
> /* Create a VM with default options. */
> vm = create_vm(&default_vm_options);
> 
> /* Create a VM with TDX enabled. */
> struct vm_options options = default_vm_options;
> options.type = VM_TYPE_TDX;
> vm = create_vm(&options);

I like this.

Thanks,
drew

Re: [RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[Xiaoyao Li]

On 7/27/2021 2:37 AM, Erdem Aktas wrote:
> Currently vm_create function only creates KVM_X86_LEGACY_VM type VMs.
> Changing the vm_create function to accept type parameter to create
> new VM types.
> 
> Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> Reviewed-by: Sean Christopherson <seanjc@google.com>
> Reviewed-by: Peter Gonda <pgonda@google.com>
> Reviewed-by: Marc Orr <marcorr@google.com>
> Reviewed-by: Sagi Shahar <sagis@google.com>
> ---
>   .../testing/selftests/kvm/include/kvm_util.h  |  1 +
>   tools/testing/selftests/kvm/lib/kvm_util.c    | 29 +++++++++++++++++--
>   2 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index d53bfadd2..c63df42d6 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -88,6 +88,7 @@ int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id,
>   void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
>   
>   struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
> +struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm, int type);
>   void kvm_vm_free(struct kvm_vm *vmp);
>   void kvm_vm_restart(struct kvm_vm *vmp, int perm);
>   void kvm_vm_release(struct kvm_vm *vmp);
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index e5fbf16f7..70caa3882 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -180,13 +180,36 @@ _Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct vm_guest_mode_params)
>    * Return:
>    *   Pointer to opaque structure that describes the created VM.
>    *
> - * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K).
> + * Wrapper VM Create function to create a VM with default type (0).

Can we pass KVM_X86_LEGACY_VM (whatever name when it's upstreamed) 
instead of 0?

> + */
> +struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
> +{
> +	return __vm_create(mode, phy_pages, perm, 0);
> +}
> +

Re: [RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[Maxim Levitsky]

On Wed, 2021-08-04 at 14:09 +0800, Xiaoyao Li wrote:
> On 7/27/2021 2:37 AM, Erdem Aktas wrote:
> > Currently vm_create function only creates KVM_X86_LEGACY_VM type VMs.
> > Changing the vm_create function to accept type parameter to create
> > new VM types.
> > 
> > Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> > Reviewed-by: Sean Christopherson <seanjc@google.com>
> > Reviewed-by: Peter Gonda <pgonda@google.com>
> > Reviewed-by: Marc Orr <marcorr@google.com>
> > Reviewed-by: Sagi Shahar <sagis@google.com>
> > ---
> >   .../testing/selftests/kvm/include/kvm_util.h  |  1 +
> >   tools/testing/selftests/kvm/lib/kvm_util.c    | 29 +++++++++++++++++--
> >   2 files changed, 27 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index d53bfadd2..c63df42d6 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -88,6 +88,7 @@ int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id,
> >   void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
> >   
> >   struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
> > +struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm, int type);
> >   void kvm_vm_free(struct kvm_vm *vmp);
> >   void kvm_vm_restart(struct kvm_vm *vmp, int perm);
> >   void kvm_vm_release(struct kvm_vm *vmp);
> > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > index e5fbf16f7..70caa3882 100644
> > --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> > @@ -180,13 +180,36 @@ _Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct vm_guest_mode_params)
> >    * Return:
> >    *   Pointer to opaque structure that describes the created VM.
> >    *
> > - * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K).
> > + * Wrapper VM Create function to create a VM with default type (0).
> 
> Can we pass KVM_X86_LEGACY_VM (whatever name when it's upstreamed) 
> instead of 0?

To be honest I would prefer this to be called something like KVM_X86_STANDARD_VM,
or something.

I don't think that normal unencrypted virtualization is already legacy, even if TDX
docs claim that.

Just my personal opinion.

Best regards,
	Maxim Levitsky

> 
> > + */
> > +struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
> > +{
> > +	return __vm_create(mode, phy_pages, perm, 0);
> > +}
> > +
> 
> 

Re: [RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[Xiaoyao Li]

On 8/4/2021 10:24 PM, Maxim Levitsky wrote:
> On Wed, 2021-08-04 at 14:09 +0800, Xiaoyao Li wrote:
>> On 7/27/2021 2:37 AM, Erdem Aktas wrote:
>>> Currently vm_create function only creates KVM_X86_LEGACY_VM type VMs.
>>> Changing the vm_create function to accept type parameter to create
>>> new VM types.
>>>
>>> Signed-off-by: Erdem Aktas <erdemaktas@google.com>
>>> Reviewed-by: Sean Christopherson <seanjc@google.com>
>>> Reviewed-by: Peter Gonda <pgonda@google.com>
>>> Reviewed-by: Marc Orr <marcorr@google.com>
>>> Reviewed-by: Sagi Shahar <sagis@google.com>
>>> ---
>>>    .../testing/selftests/kvm/include/kvm_util.h  |  1 +
>>>    tools/testing/selftests/kvm/lib/kvm_util.c    | 29 +++++++++++++++++--
>>>    2 files changed, 27 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
>>> index d53bfadd2..c63df42d6 100644
>>> --- a/tools/testing/selftests/kvm/include/kvm_util.h
>>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
>>> @@ -88,6 +88,7 @@ int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id,
>>>    void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
>>>    
>>>    struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
>>> +struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm, int type);
>>>    void kvm_vm_free(struct kvm_vm *vmp);
>>>    void kvm_vm_restart(struct kvm_vm *vmp, int perm);
>>>    void kvm_vm_release(struct kvm_vm *vmp);
>>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
>>> index e5fbf16f7..70caa3882 100644
>>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
>>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
>>> @@ -180,13 +180,36 @@ _Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct vm_guest_mode_params)
>>>     * Return:
>>>     *   Pointer to opaque structure that describes the created VM.
>>>     *
>>> - * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K).
>>> + * Wrapper VM Create function to create a VM with default type (0).
>>
>> Can we pass KVM_X86_LEGACY_VM (whatever name when it's upstreamed)
>> instead of 0?
> 
> To be honest I would prefer this to be called something like KVM_X86_STANDARD_VM,
> or something.
> 
> I don't think that normal unencrypted virtualization is already legacy, even if TDX
> docs claim that.

I'm not proposing to use this specific name introduced in TDX RFC 
series, but proposing to use the name defined in KVM in the future 
instead of hard-coded 0.

Yes, KVM_X86_STANDARD_VM or KVM_X86_NORMAL_VM (proposed by Paolo) is 
better than KVM_X86_LEGACY_VM.

> Just my personal opinion.
> 
> Best regards,
> 	Maxim Levitsky
> 
>>
>>> + */
>>> +struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
>>> +{
>>> +	return __vm_create(mode, phy_pages, perm, 0);
>>> +}
>>> +
>>
>>
> 
> 

Re: [RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[Maxim Levitsky]

On Wed, 2021-08-04 at 22:42 +0800, Xiaoyao Li wrote:
> On 8/4/2021 10:24 PM, Maxim Levitsky wrote:
> > On Wed, 2021-08-04 at 14:09 +0800, Xiaoyao Li wrote:
> > > On 7/27/2021 2:37 AM, Erdem Aktas wrote:
> > > > Currently vm_create function only creates KVM_X86_LEGACY_VM type VMs.
> > > > Changing the vm_create function to accept type parameter to create
> > > > new VM types.
> > > > 
> > > > Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> > > > Reviewed-by: Sean Christopherson <seanjc@google.com>
> > > > Reviewed-by: Peter Gonda <pgonda@google.com>
> > > > Reviewed-by: Marc Orr <marcorr@google.com>
> > > > Reviewed-by: Sagi Shahar <sagis@google.com>
> > > > ---
> > > >    .../testing/selftests/kvm/include/kvm_util.h  |  1 +
> > > >    tools/testing/selftests/kvm/lib/kvm_util.c    | 29 +++++++++++++++++--
> > > >    2 files changed, 27 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > > > index d53bfadd2..c63df42d6 100644
> > > > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > > > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > > > @@ -88,6 +88,7 @@ int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id,
> > > >    void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
> > > >    
> > > >    struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
> > > > +struct kvm_vm *__vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm, int type);
> > > >    void kvm_vm_free(struct kvm_vm *vmp);
> > > >    void kvm_vm_restart(struct kvm_vm *vmp, int perm);
> > > >    void kvm_vm_release(struct kvm_vm *vmp);
> > > > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > > > index e5fbf16f7..70caa3882 100644
> > > > --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> > > > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> > > > @@ -180,13 +180,36 @@ _Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct vm_guest_mode_params)
> > > >     * Return:
> > > >     *   Pointer to opaque structure that describes the created VM.
> > > >     *
> > > > - * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K).
> > > > + * Wrapper VM Create function to create a VM with default type (0).
> > > 
> > > Can we pass KVM_X86_LEGACY_VM (whatever name when it's upstreamed)
> > > instead of 0?
> > 
> > To be honest I would prefer this to be called something like KVM_X86_STANDARD_VM,
> > or something.
> > 
> > I don't think that normal unencrypted virtualization is already legacy, even if TDX
> > docs claim that.
> 
> I'm not proposing to use this specific name introduced in TDX RFC 
> series, but proposing to use the name defined in KVM in the future 
> instead of hard-coded 0.
> 
> Yes, KVM_X86_STANDARD_VM or KVM_X86_NORMAL_VM (proposed by Paolo) is 
> better than KVM_X86_LEGACY_VM.

KVM_X86_NORMAL_VM is a very good name IMHO as well.
Thanks!

Best regards,
	Maxim Levitsky

> 
> > Just my personal opinion.
> > 
> > Best regards,
> > 	Maxim Levitsky
> > 
> > > > + */
> > > > +struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
> > > > +{
> > > > +	return __vm_create(mode, phy_pages, perm, 0);
> > > > +}
> > > > +

Re: [RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[Erdem Aktas]

Thank you all for all that great feedback! I will include them in my v2.

On Wed, Aug 4, 2021 at 7:46 AM Maxim Levitsky <mlevitsk@redhat.com> wrote:
>
> > > > Can we pass KVM_X86_LEGACY_VM (whatever name when it's upstreamed)
> > > > instead of 0?
> > >
I was originally thinking of doing this but Sean has suggested that we
should use 0 to make it  arch-agnostic for creating default VMs.
+Sean Christopherson : What do you think?

>
> KVM_X86_NORMAL_VM is a very good name IMHO as well.
> Thanks!

Sounds good to me.

> For example:
>
> struct vm_options {
>         enum vm_guest_mode mode;
>         uint64_t phy_pages;
>         int perm;
>         int type;
> };
>
> struct kvm_vm *vm_create(const struct vm_options *options)
> {
>         ...
> }
>

I liked this idea, I will see if I can include it in my v2.

Thank you so much again.
-Erdem

Re: [RFC PATCH 1/4] KVM: selftests: Add support for creating non-default type VMs

[Sean Christopherson]

On Wed, Aug 04, 2021, Erdem Aktas wrote:
> Thank you all for all that great feedback! I will include them in my v2.
> 
> On Wed, Aug 4, 2021 at 7:46 AM Maxim Levitsky <mlevitsk@redhat.com> wrote:
> >
> > > > > Can we pass KVM_X86_LEGACY_VM (whatever name when it's upstreamed)
> > > > > instead of 0?
> > > >
> I was originally thinking of doing this but Sean has suggested that we
> should use 0 to make it  arch-agnostic for creating default VMs.
> +Sean Christopherson : What do you think?

I hate passing '0', but KVM_X86_LEGACY_VM is worse because it's nonsensical for
other architectures.
 
> >
> > KVM_X86_NORMAL_VM is a very good name IMHO as well.

But that implies protected guests are abnormal!  And KVM_X86_STANDARD_VM would
imply protected guests are sub-standard!  I'm only half-joking, e.g. if we get
to the point where the majority of guests being run are protected guests, then
!protected guests are no longer the "standard".

Looking at other architectures, I think the least awful option is a generic
KVM_VM_TYPE_AUTO, or maybe KVM_VM_TYPE_DEFAULT.  That aligns with how '0' is used
by PPC, MIPS, and arm64[*], and would work for x86 as well without implying what's
normal/standard.

[*] arm64 uses the type to specify the IPA width (I'm not even sure what that is),
    but thankfully interprets '0' as a default.

[RFC PATCH 2/4] KVM: selftest: Add helper functions to create TDX VMs

[Erdem Aktas]

TDX requires additional IOCTLs to initialize VM and  vCPUs, to add
private memory and to finalize the VM memory. Also additional utility
functions are provided to create a guest image that will include test code.

TDX enabled VM's memory is encrypted and cannot be modified or observed
by the VMM. We need to create a guest image that includes the testing
code.

When TDX is enabled, vCPUs will enter guest mode with 32 bit mode with
paging disabled. TDX requires the CPU to run on long mode with paging
enabled. The guest image should have transition code from 32 bit to 64
bit, enable paging and run the testing code. There has to be predifined
offset values for each data structure that will be used by the guest code.
The guest image layout is as following:

| Page Tables | GDTR | GDT | Stack | Testing Code | Transition Boot Code |

Guest image will be loaded to the bottom of the first 4GB of the memory.

Signed-off-by: Erdem Aktas <erdemaktas@google.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Peter Gonda <pgonda@google.com>
Reviewed-by: Marc Orr <marcorr@google.com>
Reviewed-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/Makefile          |   5 +-
 .../selftests/kvm/include/x86_64/processor.h  |   5 +
 .../selftests/kvm/lib/x86_64/processor.c      |  23 ++
 tools/testing/selftests/kvm/lib/x86_64/tdx.h  |  89 +++++
 .../selftests/kvm/lib/x86_64/tdx_lib.c        | 314 ++++++++++++++++++
 5 files changed, 432 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/lib/x86_64/tdx.h
 create mode 100644 tools/testing/selftests/kvm/lib/x86_64/tdx_lib.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index b634926e2..d84c09b5e 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -34,7 +34,7 @@ ifeq ($(ARCH),s390)
 endif
 
 LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/rbtree.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c lib/perf_test_util.c
-LIBKVM_x86_64 = lib/x86_64/apic.c lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
+LIBKVM_x86_64 = lib/x86_64/apic.c lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S /lib/x86_64/tdx_lib.c
 LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c lib/aarch64/handlers.S
 LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c
 
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index f5cb6fba6..62b26b50f 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -422,6 +422,11 @@ struct kvm_cpuid2 *kvm_get_supported_hv_cpuid(void);
 void vcpu_set_hv_cpuid(struct kvm_vm *vm, uint32_t vcpuid);
 struct kvm_cpuid2 *vcpu_get_supported_hv_cpuid(struct kvm_vm *vm, uint32_t vcpuid);
 
+void vm_vcpu_add_tdx(struct kvm_vm *vm, uint32_t vcpuid);
+
+#define __stringify_1(x) #x
+#define __stringify(x)  __stringify_1(x)
+
 /*
  * Basic CPU control in CR0
  */
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index a8906e60a..d97d40fa9 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -8,6 +8,7 @@
 #include "test_util.h"
 #include "kvm_util.h"
 #include "../kvm_util_internal.h"
+#include "tdx.h"
 #include "processor.h"
 
 #ifndef NUM_INTERRUPTS
@@ -578,6 +579,28 @@ static void vcpu_setup(struct kvm_vm *vm, int vcpuid, int pgd_memslot, int gdt_m
 	vcpu_sregs_set(vm, vcpuid, &sregs);
 }
 
+/*
+ * Adds a vCPU to a TD (Trusted Domain) with minimum  defaults. It will not set
+ * up any general purpose registers as they will be initialized by the TDX. In
+ * TDX, vCPUs RIP is set to 0xFFFFFFF0. See Intel TDX EAS Section "Initial State
+ * of Guest GPRs" for more information on vCPUs initial register values when
+ * entering the TD first time.
+ *
+ * Input Args:
+ *   vm - Virtual Machine
+ *   vcpuid - The id of the VCPU to add to the VM.
+ */
+void vm_vcpu_add_tdx(struct kvm_vm *vm, uint32_t vcpuid)
+{
+	struct kvm_mp_state mp_state;
+
+	vm_vcpu_add(vm, vcpuid);
+	initialize_td_vcpu(vm, vcpuid);
+
+	mp_state.mp_state = 0;
+	vcpu_set_mp_state(vm, vcpuid, &mp_state);
+}
+
 void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
 {
 	struct kvm_mp_state mp_state;
diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx.h b/tools/testing/selftests/kvm/lib/x86_64/tdx.h
new file mode 100644
index 000000000..6e3e8384e
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86_64/tdx.h
@@ -0,0 +1,89 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef KVM_LIB_TDX_H_
+#define KVM_LIB_TDX_H_
+
+#include <kvm_util.h>
+#include "../kvm_util_internal.h"
+
+/*
+ * Max page size for the guest image.
+ */
+#define TDX_GUEST_MAX_NR_PAGES 10000
+#define PAGE_SIZE 4096
+
+/*
+ * Page Table Address used when paging is enabled.
+ */
+#define TDX_GUEST_PT_FIXED_ADDR (0xFFFFFFFF -\
+				 (TDX_GUEST_MAX_NR_PAGES * PAGE_SIZE) + 1)
+
+/*
+ * Max Page Table Size
+ * To map 4GB memory region with 2MB pages, there needs to be 1 page for PML4,
+ * 1 Page for PDPT, 4 pages for PD. Reserving 6 pages for PT.
+ */
+#define TDX_GUEST_NR_PT_PAGES (1 + 1 + 4)
+
+/*
+ * Predefined GDTR values.
+ */
+#define TDX_GUEST_GDTR_ADDR (TDX_GUEST_PT_FIXED_ADDR + (TDX_GUEST_NR_PT_PAGES *\
+							PAGE_SIZE))
+#define TDX_GUEST_GDTR_BASE (TDX_GUEST_GDTR_ADDR + PAGE_SIZE)
+#define TDX_GUEST_LINEAR_CODE64_SEL 0x38
+
+#define TDX_GUEST_STACK_NR_PAGES (3)
+#define TDX_GUEST_STACK_BASE (TDX_GUEST_GDTR_BASE + (TDX_GUEST_STACK_NR_PAGES *\
+						     PAGE_SIZE) - 1)
+/*
+ * Reserving some pages to copy the test code. This is an arbitrary number for
+ * now to simplify to guest image layout calculation.
+ * TODO: calculate the guest code dynamcially.
+ */
+#define TDX_GUEST_CODE_ENTRY (TDX_GUEST_GDTR_BASE + (TDX_GUEST_STACK_NR_PAGES *\
+						     PAGE_SIZE))
+
+#define KVM_MAX_CPUID_ENTRIES 256
+
+/*
+ * TODO: Move page attributes to processor.h file.
+ */
+#define _PAGE_PRESENT       (1UL<<0)       /* is present */
+#define _PAGE_RW            (1UL<<1)       /* writeable */
+#define _PAGE_PS            (1UL<<7)       /* page size bit*/
+
+#define TDX_TEST_PORT 0x33
+
+#define GDT_ENTRY(flags, base, limit)				\
+		((((base)  & 0xff000000ULL) << (56-24)) |	\
+		 (((flags) & 0x0000f0ffULL) << 40) |		\
+		 (((limit) & 0x000f0000ULL) << (48-16)) |	\
+		 (((base)  & 0x00ffffffULL) << 16) |		\
+		 (((limit) & 0x0000ffffULL)))
+
+struct tdx_cpuid_data {
+	struct kvm_cpuid2 cpuid;
+	struct kvm_cpuid_entry2 entries[KVM_MAX_CPUID_ENTRIES];
+};
+
+struct __packed tdx_gdtr {
+	uint16_t limit;
+	uint32_t base;
+};
+
+struct page_table {
+	uint64_t  pml4[512];
+	uint64_t  pdpt[512];
+	uint64_t  pd[4][512];
+};
+
+void add_td_memory(struct kvm_vm *vm, void *source_page,
+		   uint64_t gpa, int size);
+void finalize_td_memory(struct kvm_vm *vm);
+void initialize_td(struct kvm_vm *vm);
+void initialize_td_vcpu(struct kvm_vm *vm, uint32_t vcpuid);
+void prepare_source_image(struct kvm_vm *vm, void *guest_code,
+			  size_t guest_code_size,
+			  uint64_t guest_code_signature);
+
+#endif  // KVM_LIB_TDX_H_
diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx_lib.c b/tools/testing/selftests/kvm/lib/x86_64/tdx_lib.c
new file mode 100644
index 000000000..03f0c3af8
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86_64/tdx_lib.c
@@ -0,0 +1,314 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "tdx.h"
+#include <stdlib.h>
+#include <malloc.h>
+#include "processor.h"
+
+char *tdx_cmd_str[] = {
+	"KVM_TDX_CAPABILITIES",
+	"KVM_TDX_INIT_VM",
+	"KVM_TDX_INIT_VCPU",
+	"KVM_TDX_INIT_MEM_REGION",
+	"KVM_TDX_FINALIZE_VM"
+};
+
+#define TDX_MAX_CMD_STR (ARRAY_SIZE(tdx_cmd_str))
+#define EIGHT_INT3_INSTRUCTIONS 0xCCCCCCCCCCCCCCCC
+
+static void tdx_ioctl(int fd, int ioctl_no, uint32_t metadata, void *data)
+{
+	struct kvm_tdx_cmd tdx_cmd;
+	int r;
+
+	TEST_ASSERT(ioctl_no < TDX_MAX_CMD_STR, "Unknown TDX CMD : %d\n",
+		    ioctl_no);
+
+	memset(&tdx_cmd, 0x0, sizeof(tdx_cmd));
+	tdx_cmd.id = ioctl_no;
+	tdx_cmd.metadata = metadata;
+	tdx_cmd.data = (uint64_t)data;
+	r = ioctl(fd, KVM_MEMORY_ENCRYPT_OP, &tdx_cmd);
+	TEST_ASSERT(r == 0, "%s failed: %d  %d", tdx_cmd_str[ioctl_no], r,
+		    errno);
+}
+
+/*
+ * Initialize a VM as a TD.
+ *
+ */
+void initialize_td(struct kvm_vm *vm)
+{
+	int ret, i;
+	struct tdx_cpuid_data cpuid_data;
+	int rc;
+
+	/* No guest VMM controlled cpuid information yet. */
+	struct kvm_tdx_init_vm init_vm;
+
+	rc = kvm_check_cap(KVM_CAP_X2APIC_API);
+	TEST_ASSERT(rc, "TDX: KVM_CAP_X2APIC_API is not supported!");
+	rc = kvm_check_cap(KVM_CAP_SPLIT_IRQCHIP);
+	TEST_ASSERT(rc, "TDX: KVM_CAP_SPLIT_IRQCHIP is not supported!");
+
+	struct kvm_enable_cap cap = {
+		.cap = KVM_CAP_X2APIC_API,
+		.args[0] = KVM_X2APIC_API_USE_32BIT_IDS |
+				KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK
+	};
+	vm_enable_cap(vm, &cap);
+	struct kvm_enable_cap cap2 = {
+		.cap = KVM_CAP_SPLIT_IRQCHIP,
+		.args[0] = 24
+	};
+	vm_enable_cap(vm, &cap2);
+
+	/* Allocate and setup memoryfor the td guest. */
+	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
+				    TDX_GUEST_PT_FIXED_ADDR,
+				    0, TDX_GUEST_MAX_NR_PAGES, 0);
+
+	memset(&init_vm, 0, sizeof(init_vm));
+	memset(&cpuid_data, 0, sizeof(cpuid_data));
+	cpuid_data.cpuid.nent = KVM_MAX_CPUID_ENTRIES;
+	ret = ioctl(vm->kvm_fd, KVM_GET_SUPPORTED_CPUID, &cpuid_data);
+	TEST_ASSERT(ret == 0, "KVM_GET_SUPPORTED_CPUID failed %d %d\n",
+		    ret, errno);
+	for (i = 0; i < KVM_MAX_CPUID_ENTRIES; i++) {
+		struct kvm_cpuid_entry2 *e = &cpuid_data.entries[i];
+
+		/* Setting max VA and PA bits to 48. This will make sure that
+		 * TDX module will use 4 level SEPT structures.
+		 */
+		if (e->function == 0x80000008 && (e->index == 0)) {
+			e->eax = 0x3030;
+			break;
+		}
+	}
+	init_vm.max_vcpus = 1;
+	init_vm.attributes = 0;
+	init_vm.cpuid = (uint64_t)&cpuid_data;
+	tdx_ioctl(vm->fd, KVM_TDX_INIT_VM, 0, &init_vm);
+}
+
+
+void initialize_td_vcpu(struct kvm_vm *vm, uint32_t vcpuid)
+{
+	struct vcpu *vcpu = vcpu_find(vm, vcpuid);
+
+	TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid);
+	tdx_ioctl(vcpu->fd, KVM_TDX_INIT_VCPU, 0, NULL);
+}
+
+void add_td_memory(struct kvm_vm *vm, void *source_pages,
+		   uint64_t gpa, int size)
+{
+	struct kvm_tdx_init_mem_region mem_region = {
+		.source_addr = (uint64_t)source_pages,
+		.gpa = gpa,
+		.nr_pages = size / PAGE_SIZE,
+	};
+	uint32_t metadata = KVM_TDX_MEASURE_MEMORY_REGION;
+
+	TEST_ASSERT((mem_region.nr_pages > 0) &&
+		   ((mem_region.nr_pages * PAGE_SIZE) == size),
+		   "Cannot add partial pages to the guest memory.\n");
+	TEST_ASSERT(((uint64_t)source_pages & (PAGE_SIZE - 1)) == 0,
+		    "Source memory buffer is not page aligned\n");
+	tdx_ioctl(vm->fd, KVM_TDX_INIT_MEM_REGION, metadata, &mem_region);
+}
+
+void finalize_td_memory(struct kvm_vm *vm)
+{
+	tdx_ioctl(vm->fd, KVM_TDX_FINALIZE_VM, 0, NULL);
+}
+
+void build_gdtr_table(void *gdtr_target, void *gdt_target)
+{
+	uint64_t gdt_table[] = {
+		GDT_ENTRY(0, 0, 0),              // NULL_SEL
+		GDT_ENTRY(0xc093, 0, 0xfffff),   // LINEAR_DATA32_SEL
+		GDT_ENTRY(0xc09b, 0, 0xfffff),   // LINEAR_CODE32_SEL
+		GDT_ENTRY(0, 0, 0),              // NULL_SEL
+		GDT_ENTRY(0, 0, 0),              // NULL_SEL
+		GDT_ENTRY(0, 0, 0),              // NULL_SEL
+		GDT_ENTRY(0, 0, 0),              // NULL_SEL
+		GDT_ENTRY(0xa09b, 0, 0xfffff)    // LINEAR_CODE64_SEL
+	};
+
+	struct tdx_gdtr gdtr;
+
+	gdtr.limit = sizeof(gdt_table) - 1;
+	gdtr.base = TDX_GUEST_GDTR_BASE;
+
+	memcpy(gdt_target, gdt_table, sizeof(gdt_table));
+	memcpy(gdtr_target, &gdtr, sizeof(gdtr));
+
+}
+
+
+/*
+ * Constructing 1:1 mapping for the lowest 4GB address space using 2MB pages
+ * which will be used by the TDX guest when paging is enabled.
+ * TODO: use virt_pg_map() functions to dynamically allocate the page tables.
+ */
+void build_page_tables(void *pt_target, uint64_t  pml4_base_address)
+{
+	uint64_t i;
+	struct page_table *pt;
+
+	pt = malloc(sizeof(struct page_table));
+	TEST_ASSERT(pt != NULL, "Could not allocate memory for page tables!\n");
+	memset((void *) &(pt->pml4[0]), 0, sizeof(pt->pml4));
+	memset((void *) &(pt->pdpt[0]), 0, sizeof(pt->pdpt));
+	for (i = 0; i < 4; i++)
+		memset((void *) &(pt->pd[i][0]), 0, sizeof(pt->pd[i]));
+
+	pt->pml4[0] = (pml4_base_address + PAGE_SIZE) |
+		      _PAGE_PRESENT | _PAGE_RW;
+	for (i = 0; i < 4; i++)
+		pt->pdpt[i] = (pml4_base_address + (i + 2) * PAGE_SIZE) |
+				_PAGE_PRESENT | _PAGE_RW;
+
+	uint64_t *pde = &(pt->pd[0][0]);
+
+	for (i = 0; i < sizeof(pt->pd) / sizeof(pt->pd[0][0]); i++, pde++)
+		*pde = (i << 21) | _PAGE_PRESENT | _PAGE_RW | _PAGE_PS;
+	memcpy(pt_target, pt, 6 * PAGE_SIZE);
+}
+
+static void
+__attribute__((__flatten__, section("guest_boot_section"))) guest_boot(void)
+{
+	asm volatile(" .code32\n\t;"
+		     "main_32:\n\t;"
+		     "	cli\n\t;"
+		     "	movl $" __stringify(TDX_GUEST_STACK_BASE) ", %%esp\n\t;"
+		     "	movl $" __stringify(TDX_GUEST_GDTR_ADDR) ", %%eax\n\t;"
+		     "	lgdt (%%eax)\n\t;"
+		     "	movl $0x660, %%eax\n\t;"
+		     "	movl %%eax, %%cr4\n\t;"
+		     "	movl $" __stringify(TDX_GUEST_PT_FIXED_ADDR) ", %%eax\n\t;"
+		     "	movl %%eax, %%cr3\n\t;"
+		     "	movl $0x80000023, %%eax\n\t;"
+		     "	movl %%eax, %%cr0\n\t;"
+		     "	ljmp $" __stringify(TDX_GUEST_LINEAR_CODE64_SEL)
+		     ", $" __stringify(TDX_GUEST_CODE_ENTRY) "\n\t;"
+		     /*
+		      * This is where the CPU will start running.
+		      * Do not remove any int3 instruction below.
+		      */
+		     "reset_vector:\n\t;"
+		     "	jmp main_32\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     "	int3\n\t;"
+		     ".code64\n\t"
+		     :::"rax");
+}
+
+extern char *__start_guest_boot_section;
+extern char *__stop_guest_boot_section;
+#define GUEST_BOOT_SIZE ((uint64_t)&__stop_guest_boot_section -\
+			(uint64_t)&__start_guest_boot_section)
+
+/*
+ * Copies the guest code to the guest image. If signature value is not 0, it
+ * will verify that the guest code ends with the signature provided. We might
+ * need to check the signature to prevent compiler to add additional instruction
+ * to the end of the guest code which might create problems in some cases ie
+ * when copying code for resetvector.
+ */
+void copy_guest_code(void *target, void *guest_function, size_t code_size,
+		     uint64_t signature)
+{
+	uint64_t *end;
+
+	TEST_ASSERT((target != NULL) && (guest_function != NULL) &&
+		    (code_size > 0), "Invalid inputs to copy guest code\n");
+	if (signature) {
+		while (code_size >= sizeof(signature)) {
+			end = guest_function + code_size - sizeof(signature);
+			if (*end == signature)
+				break;
+			/* Trimming the unwanted code at the end added by
+			 * compiler. We need to add nop instruction to the
+			 * begginning of the buffer to make sure that the guest
+			 * code is aligned from the bottom and top as expected
+			 * based on the original code size. This is important
+			 * for reset vector which is copied to the bottom of
+			 * the first 4GB memory.
+			 */
+			code_size--;
+			*(unsigned char *)target = 0x90;
+			target++;
+		}
+		TEST_ASSERT(code_size >= sizeof(signature),
+			    "Guest code does not end with the signature: %lx\n"
+			    , signature);
+	}
+
+	memcpy(target, guest_function, code_size);
+}
+
+void prepare_source_image(struct kvm_vm *vm, void *guest_code,
+			  size_t guest_code_size, uint64_t guest_code_signature)
+{
+	void *source_mem, *pt_address, *code_address, *gdtr_address,
+	     *gdt_address, *guest_code_base;
+	int number_of_pages;
+
+	number_of_pages = (GUEST_BOOT_SIZE + guest_code_size) / PAGE_SIZE + 1 +
+			TDX_GUEST_NR_PT_PAGES + TDX_GUEST_STACK_NR_PAGES;
+	TEST_ASSERT(number_of_pages < TDX_GUEST_MAX_NR_PAGES,
+		    "Initial image does not fit to the memory");
+
+	source_mem = memalign(PAGE_SIZE,
+				   (TDX_GUEST_MAX_NR_PAGES * PAGE_SIZE));
+	TEST_ASSERT(source_mem != NULL,
+		    "Could not allocate memory for guest image\n");
+
+	pt_address = source_mem;
+	gdtr_address = source_mem + (TDX_GUEST_NR_PT_PAGES * PAGE_SIZE);
+	gdt_address = gdtr_address + PAGE_SIZE;
+	code_address = source_mem + (TDX_GUEST_MAX_NR_PAGES * PAGE_SIZE) -
+			GUEST_BOOT_SIZE;
+	guest_code_base =  gdt_address + (TDX_GUEST_STACK_NR_PAGES *
+					  PAGE_SIZE);
+
+	build_page_tables(pt_address, TDX_GUEST_PT_FIXED_ADDR);
+	build_gdtr_table(gdtr_address, gdt_address);
+
+	/* reset vector code should end with int3 instructions.
+	 * The unused bytes at the reset vector with int3 to trigger triple
+	 * fault shutdown if the guest manages to get into the unused code.
+	 * Using the last 8 int3 instruction as a signature to find the function
+	 * end offset for guest boot code that includes the instructions for
+	 * reset vector.
+	 * TODO: Using signature to find the exact size is a little strange but
+	 * compiler might add additional bytes to the end of the function which
+	 * makes it hard to calculate the offset addresses correctly.
+	 * Alternatively, we can construct the jmp instruction for the reset
+	 * vector manually to prevent any offset mismatch when copying the
+	 * compiler generated code.
+	 */
+	copy_guest_code(code_address, guest_boot, GUEST_BOOT_SIZE,
+			EIGHT_INT3_INSTRUCTIONS);
+	if (guest_code)
+		copy_guest_code(guest_code_base, guest_code, guest_code_size,
+				guest_code_signature);
+
+	add_td_memory(vm, source_mem, TDX_GUEST_PT_FIXED_ADDR,
+		      (TDX_GUEST_MAX_NR_PAGES * PAGE_SIZE));
+	free(source_mem);
+}
-- 
2.32.0.432.gabb21c7263-goog

[RFC PATCH 3/4] KVM: selftest: Adding TDX life cycle test.

[Erdem Aktas]

Adding a test to verify TDX lifecycle by creating a TD and running a
dummy TDVMCALL<INSTRUCTION.IO> inside it.

Signed-off-by: Erdem Aktas <erdemaktas@google.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Peter Gonda <pgonda@google.com>
Reviewed-by: Marc Orr <marcorr@google.com>
Reviewed-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/Makefile          |   1 +
 tools/testing/selftests/kvm/lib/x86_64/tdx.h  | 131 ++++++++++++++++++
 .../selftests/kvm/x86_64/tdx_vm_tests.c       | 102 ++++++++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index d84c09b5e..259be634c 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -72,6 +72,7 @@ TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
 TEST_GEN_PROGS_x86_64 += memslot_modification_stress_test
 TEST_GEN_PROGS_x86_64 += set_memory_region_test
 TEST_GEN_PROGS_x86_64 += steal_time
 TEST_GEN_PROGS_x86_64 += kvm_binary_stats_test
+TEST_GEN_PROGS_x86_64 += x86_64/tdx_vm_tests

 TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
 TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx.h b/tools/testing/selftests/kvm/lib/x86_64/tdx.h
index 6e3e8384e..395be3c81 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/tdx.h
+++ b/tools/testing/selftests/kvm/lib/x86_64/tdx.h
@@ -86,4 +86,135 @@ void prepare_source_image(struct kvm_vm *vm, void *guest_code,
 			  size_t guest_code_size,
 			  uint64_t guest_code_signature);
 
+/*
+ * Generic TDCALL function that can be used to communicate with TDX module or
+ * VMM.
+ * Input operands: rax, rbx, rcx, rdx, r8-r15, rbp, rsi, rdi
+ * Output operands: rax, r8-r15, rbx, rdx, rdi, rsi
+ * rcx is actually a bitmap to tell TDX module which register values will be
+ * exposed to the VMM.
+ * XMM0-XMM15 registers can be used as input operands but the current
+ * implementation does not support it yet.
+ */
+static inline void tdcall(struct kvm_regs *regs)
+{
+	asm volatile (
+			"mov %13, %%rax;\n\t"
+			"mov %14, %%rbx;\n\t"
+			"mov %15, %%rcx;\n\t"
+			"mov %16, %%rdx;\n\t"
+			"mov %17, %%r8;\n\t"
+			"mov %18, %%r9;\n\t"
+			"mov %19, %%r10;\n\t"
+			"mov %20, %%r11;\n\t"
+			"mov %21, %%r12;\n\t"
+			"mov %22, %%r13;\n\t"
+			"mov %23, %%r14;\n\t"
+			"mov %24, %%r15;\n\t"
+			"mov %25, %%rbp;\n\t"
+			"mov %26, %%rsi;\n\t"
+			"mov %27, %%rdi;\n\t"
+			".byte 0x66, 0x0F, 0x01, 0xCC;\n\t"
+			"mov %%rax, %0;\n\t"
+			"mov %%rbx, %1;\n\t"
+			"mov %%rdx, %2;\n\t"
+			"mov %%r8, %3;\n\t"
+			"mov %%r9, %4;\n\t"
+			"mov %%r10, %5;\n\t"
+			"mov %%r11, %6;\n\t"
+			"mov %%r12, %7;\n\t"
+			"mov %%r13, %8;\n\t"
+			"mov %%r14, %9;\n\t"
+			"mov %%r15, %10;\n\t"
+			"mov %%rsi, %11;\n\t"
+			"mov %%rdi, %12;\n\t"
+			: "=m" (regs->rax), "=m" (regs->rbx), "=m" (regs->rdx),
+			"=m" (regs->r8), "=m" (regs->r9), "=m" (regs->r10),
+			"=m" (regs->r11), "=m" (regs->r12), "=m" (regs->r13),
+			"=m" (regs->r14), "=m" (regs->r15), "=m" (regs->rsi),
+			"=m" (regs->rdi)
+			: "m" (regs->rax), "m" (regs->rbx), "m" (regs->rcx),
+			"m" (regs->rdx), "m" (regs->r8), "m" (regs->r9),
+			"m" (regs->r10), "m" (regs->r11), "m" (regs->r12),
+			"m" (regs->r13), "m" (regs->r14), "m" (regs->r15),
+			"m" (regs->rbp), "m" (regs->rsi), "m" (regs->rdi)
+			: "rax", "rbx", "rcx", "rdx", "r8", "r9", "r10", "r11",
+			"r12", "r13", "r14", "r15", "rbp", "rsi", "rdi");
+}
+
+
+/*
+ * Do a TDVMCALL IO request
+ *
+ * Input Args:
+ *  port - IO port to do read/write
+ *  size - Number of bytes to read/write. 1=1byte, 2=2bytes, 4=4bytes.
+ *  write - 1=IO write 0=IO read
+ *  data - pointer for the data to write
+ *
+ * Output Args:
+ *  data - pointer for data to be read
+ *
+ * Return:
+ *   On success, return 0. For Invalid-IO-Port error, returns -1.
+ *
+ * Does an IO operation using the following tdvmcall interface.
+ *
+ * TDG.VP.VMCALL<Instruction.IO>-Input Operands
+ * R11 30 for IO
+ *
+ * R12 Size of access. 1=1byte, 2=2bytes, 4=4bytes.
+ * R13 Direction. 0=Read, 1=Write.
+ * R14 Port number
+ * R15 Data to write, if R13 is 1.
+ *
+ * TDG.VP.VMCALL<Instruction.IO>-Output Operands
+ * R10 TDG.VP.VMCALL-return code.
+ * R11 Data to read, if R13 is 0.
+ *
+ * TDG.VP.VMCALL<Instruction.IO>-Status Codes
+ * Error Code Value Description
+ * TDG.VP.VMCALL_SUCCESS 0x0 TDG.VP.VMCALL is successful
+ * TDG.VP.VMCALL_INVALID_OPERAND 0x80000000 00000000 Invalid-IO-Port access
+ */
+static inline int tdvmcall_io(uint64_t port, uint64_t size,
+			      uint64_t write, uint64_t *data)
+{
+	struct kvm_regs regs;
+
+	memset(&regs, 0, sizeof(regs));
+	regs.r11 = 30;
+	regs.r12 = size;
+	regs.r13 = write;
+	regs.r14 = port;
+	if (write)
+		regs.r15 = *data;
+	/* TODO: update the bitmap register with only the relavent registers */
+	regs.rcx = 0xFC00;
+	tdcall(&regs);
+	if (!write)
+		*data = regs.r11;
+	return regs.r10;
+}
+
+
+#define TDX_FUNCTION_SIZE(name) ((uint64_t)&__stop_sec_ ## name -\
+			   (uint64_t)&__start_sec_ ## name) \
+
+#define TDX_GUEST_FUNCTION__(name, section_name) \
+extern char *__start_sec_ ## name ; \
+extern char *__stop_sec_ ## name ; \
+static void \
+__attribute__((__flatten__, section(section_name))) name(void *arg)
+
+
+#define STRINGIFY2(x) #x
+#define STRINGIFY(x) STRINGIFY2(x)
+#define CONCAT2(a, b) a##b
+#define CONCAT(a, b) CONCAT2(a, b)
+
+
+#define TDX_GUEST_FUNCTION(name) \
+TDX_GUEST_FUNCTION__(name, STRINGIFY(CONCAT(sec_, name)))
+
 #endif  // KVM_LIB_TDX_H_
diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
new file mode 100644
index 000000000..da7ea67b1
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <fcntl.h>
+#include <limits.h>
+#include <kvm_util.h>
+#include "../lib/kvm_util_internal.h"
+#include "../lib/x86_64/tdx.h"
+#include <linux/kvm.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <test_util.h>
+#include <unistd.h>
+#include <processor.h>
+#include <time.h>
+#include <sys/mman.h>
+#include<sys/wait.h>
+
+/*
+ * There might be multiple tests we are running and if one test fails, it will
+ * prevent the subsequent tests to run due to how tests are failing with
+ * TEST_ASSERT function. The run_in_new_process function will run a test in a
+ * new process context and wait for it to finish or fail to prevent TEST_ASSERT
+ * to kill the main testing process.
+ */
+void run_in_new_process(void (*func)(void))
+{
+	if (fork() == 0) {
+		func();
+		exit(0);
+	}
+	wait(NULL);
+}
+
+/*
+ * Verify that the TDX  is supported by the KVM.
+ */
+bool is_tdx_enabled(void)
+{
+	return !!(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_TDX_VM));
+}
+
+/*
+ * Do a dummy io exit to verify that the TD has been initialized correctly and
+ * guest can run some code inside.
+ */
+TDX_GUEST_FUNCTION(guest_dummy_exit)
+{
+	uint64_t data;
+
+	data = 0xAB;
+	tdvmcall_io(TDX_TEST_PORT, 1, 1, &data);
+}
+
+/*
+ * TD lifecycle test will create a TD which runs a dumy IO exit to verify that
+ * the guest TD has been created correctly.
+ */
+void  verify_td_lifecycle(void)
+{
+	struct kvm_vm *vm;
+	struct kvm_run *run;
+
+	printf("Verifying TD lifecycle:\n");
+	/* Create a TD VM with no memory.*/
+	vm = __vm_create(VM_MODE_DEFAULT, 0, O_RDWR, KVM_X86_TDX_VM);
+
+	/* Allocate TD guest memory and initialize the TD.*/
+	initialize_td(vm);
+
+	/* Initialize the TD vcpu and copy the test code to the guest memory.*/
+	vm_vcpu_add_tdx(vm, 0);
+
+	/* Setup and initialize VM memory */
+	prepare_source_image(vm, guest_dummy_exit,
+			     TDX_FUNCTION_SIZE(guest_dummy_exit), 0);
+	finalize_td_memory(vm);
+
+	run = vcpu_state(vm, 0);
+	vcpu_run(vm, 0);
+	TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
+		    "Got exit_reason other than KVM_EXIT_IO: %u (%s)\n",
+		    run->exit_reason,
+		    exit_reason_str(run->exit_reason));
+
+	kvm_vm_free(vm);
+	printf("\t ... PASSED\n");
+}
+int main(int argc, char **argv)
+{
+	if (!is_tdx_enabled()) {
+		printf("TDX is not supported by the KVM\n"
+		       "Skipping the TDX tests.\n");
+		return 0;
+	}
+
+	run_in_new_process(&verify_td_lifecycle);
+
+	return 0;
+}
-- 
2.32.0.432.gabb21c7263-goog

Re: [RFC PATCH 3/4] KVM: selftest: Adding TDX life cycle test.

[David Matlack]

On Mon, Jul 26, 2021 at 11:37:56AM -0700, Erdem Aktas wrote:
> Adding a test to verify TDX lifecycle by creating a TD and running a
> dummy TDVMCALL<INSTRUCTION.IO> inside it.
> 
> Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> Reviewed-by: Sean Christopherson <seanjc@google.com>
> Reviewed-by: Peter Gonda <pgonda@google.com>
> Reviewed-by: Marc Orr <marcorr@google.com>
> Reviewed-by: Sagi Shahar <sagis@google.com>
> ---
>  tools/testing/selftests/kvm/Makefile          |   1 +
>  tools/testing/selftests/kvm/lib/x86_64/tdx.h  | 131 ++++++++++++++++++
>  .../selftests/kvm/x86_64/tdx_vm_tests.c       | 102 ++++++++++++++
>  3 files changed, 234 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> 
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index d84c09b5e..259be634c 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -72,6 +72,7 @@ TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
>  TEST_GEN_PROGS_x86_64 += memslot_modification_stress_test
>  TEST_GEN_PROGS_x86_64 += set_memory_region_test
>  TEST_GEN_PROGS_x86_64 += steal_time
>  TEST_GEN_PROGS_x86_64 += kvm_binary_stats_test
> +TEST_GEN_PROGS_x86_64 += x86_64/tdx_vm_tests
> 
>  TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
>  TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx.h b/tools/testing/selftests/kvm/lib/x86_64/tdx.h
> index 6e3e8384e..395be3c81 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/tdx.h
> +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx.h
> @@ -86,4 +86,135 @@ void prepare_source_image(struct kvm_vm *vm, void *guest_code,
>  			  size_t guest_code_size,
>  			  uint64_t guest_code_signature);
>  
> +/*
> + * Generic TDCALL function that can be used to communicate with TDX module or
> + * VMM.
> + * Input operands: rax, rbx, rcx, rdx, r8-r15, rbp, rsi, rdi
> + * Output operands: rax, r8-r15, rbx, rdx, rdi, rsi
> + * rcx is actually a bitmap to tell TDX module which register values will be
> + * exposed to the VMM.
> + * XMM0-XMM15 registers can be used as input operands but the current
> + * implementation does not support it yet.
> + */
> +static inline void tdcall(struct kvm_regs *regs)
> +{
> +	asm volatile (
> +			"mov %13, %%rax;\n\t"
> +			"mov %14, %%rbx;\n\t"
> +			"mov %15, %%rcx;\n\t"
> +			"mov %16, %%rdx;\n\t"
> +			"mov %17, %%r8;\n\t"
> +			"mov %18, %%r9;\n\t"
> +			"mov %19, %%r10;\n\t"
> +			"mov %20, %%r11;\n\t"
> +			"mov %21, %%r12;\n\t"
> +			"mov %22, %%r13;\n\t"
> +			"mov %23, %%r14;\n\t"
> +			"mov %24, %%r15;\n\t"
> +			"mov %25, %%rbp;\n\t"
> +			"mov %26, %%rsi;\n\t"
> +			"mov %27, %%rdi;\n\t"
> +			".byte 0x66, 0x0F, 0x01, 0xCC;\n\t"
> +			"mov %%rax, %0;\n\t"
> +			"mov %%rbx, %1;\n\t"
> +			"mov %%rdx, %2;\n\t"
> +			"mov %%r8, %3;\n\t"
> +			"mov %%r9, %4;\n\t"
> +			"mov %%r10, %5;\n\t"
> +			"mov %%r11, %6;\n\t"
> +			"mov %%r12, %7;\n\t"
> +			"mov %%r13, %8;\n\t"
> +			"mov %%r14, %9;\n\t"
> +			"mov %%r15, %10;\n\t"
> +			"mov %%rsi, %11;\n\t"
> +			"mov %%rdi, %12;\n\t"
> +			: "=m" (regs->rax), "=m" (regs->rbx), "=m" (regs->rdx),
> +			"=m" (regs->r8), "=m" (regs->r9), "=m" (regs->r10),
> +			"=m" (regs->r11), "=m" (regs->r12), "=m" (regs->r13),
> +			"=m" (regs->r14), "=m" (regs->r15), "=m" (regs->rsi),
> +			"=m" (regs->rdi)
> +			: "m" (regs->rax), "m" (regs->rbx), "m" (regs->rcx),
> +			"m" (regs->rdx), "m" (regs->r8), "m" (regs->r9),
> +			"m" (regs->r10), "m" (regs->r11), "m" (regs->r12),
> +			"m" (regs->r13), "m" (regs->r14), "m" (regs->r15),
> +			"m" (regs->rbp), "m" (regs->rsi), "m" (regs->rdi)
> +			: "rax", "rbx", "rcx", "rdx", "r8", "r9", "r10", "r11",
> +			"r12", "r13", "r14", "r15", "rbp", "rsi", "rdi");
> +}
> +
> +
> +/*
> + * Do a TDVMCALL IO request
> + *
> + * Input Args:
> + *  port - IO port to do read/write
> + *  size - Number of bytes to read/write. 1=1byte, 2=2bytes, 4=4bytes.
> + *  write - 1=IO write 0=IO read
> + *  data - pointer for the data to write
> + *
> + * Output Args:
> + *  data - pointer for data to be read
> + *
> + * Return:
> + *   On success, return 0. For Invalid-IO-Port error, returns -1.
> + *
> + * Does an IO operation using the following tdvmcall interface.
> + *
> + * TDG.VP.VMCALL<Instruction.IO>-Input Operands
> + * R11 30 for IO
> + *
> + * R12 Size of access. 1=1byte, 2=2bytes, 4=4bytes.
> + * R13 Direction. 0=Read, 1=Write.
> + * R14 Port number
> + * R15 Data to write, if R13 is 1.
> + *
> + * TDG.VP.VMCALL<Instruction.IO>-Output Operands
> + * R10 TDG.VP.VMCALL-return code.
> + * R11 Data to read, if R13 is 0.
> + *
> + * TDG.VP.VMCALL<Instruction.IO>-Status Codes
> + * Error Code Value Description
> + * TDG.VP.VMCALL_SUCCESS 0x0 TDG.VP.VMCALL is successful
> + * TDG.VP.VMCALL_INVALID_OPERAND 0x80000000 00000000 Invalid-IO-Port access
> + */
> +static inline int tdvmcall_io(uint64_t port, uint64_t size,
> +			      uint64_t write, uint64_t *data)
> +{
> +	struct kvm_regs regs;
> +
> +	memset(&regs, 0, sizeof(regs));
> +	regs.r11 = 30;
> +	regs.r12 = size;
> +	regs.r13 = write;
> +	regs.r14 = port;
> +	if (write)
> +		regs.r15 = *data;
> +	/* TODO: update the bitmap register with only the relavent registers */
> +	regs.rcx = 0xFC00;
> +	tdcall(&regs);
> +	if (!write)
> +		*data = regs.r11;
> +	return regs.r10;
> +}
> +
> +
> +#define TDX_FUNCTION_SIZE(name) ((uint64_t)&__stop_sec_ ## name -\
> +			   (uint64_t)&__start_sec_ ## name) \
> +
> +#define TDX_GUEST_FUNCTION__(name, section_name) \
> +extern char *__start_sec_ ## name ; \
> +extern char *__stop_sec_ ## name ; \
> +static void \
> +__attribute__((__flatten__, section(section_name))) name(void *arg)
> +
> +
> +#define STRINGIFY2(x) #x
> +#define STRINGIFY(x) STRINGIFY2(x)
> +#define CONCAT2(a, b) a##b
> +#define CONCAT(a, b) CONCAT2(a, b)
> +
> +
> +#define TDX_GUEST_FUNCTION(name) \
> +TDX_GUEST_FUNCTION__(name, STRINGIFY(CONCAT(sec_, name)))
> +
>  #endif  // KVM_LIB_TDX_H_
> diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> new file mode 100644
> index 000000000..da7ea67b1
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <fcntl.h>
> +#include <limits.h>
> +#include <kvm_util.h>
> +#include "../lib/kvm_util_internal.h"
> +#include "../lib/x86_64/tdx.h"
> +#include <linux/kvm.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <sys/types.h>
> +#include <test_util.h>
> +#include <unistd.h>
> +#include <processor.h>
> +#include <time.h>
> +#include <sys/mman.h>
> +#include<sys/wait.h>
> +
> +/*
> + * There might be multiple tests we are running and if one test fails, it will
> + * prevent the subsequent tests to run due to how tests are failing with
> + * TEST_ASSERT function. The run_in_new_process function will run a test in a
> + * new process context and wait for it to finish or fail to prevent TEST_ASSERT
> + * to kill the main testing process.
> + */
> +void run_in_new_process(void (*func)(void))
> +{
> +	if (fork() == 0) {
> +		func();
> +		exit(0);
> +	}
> +	wait(NULL);
> +}
> +
> +/*
> + * Verify that the TDX  is supported by the KVM.
> + */
> +bool is_tdx_enabled(void)
> +{
> +	return !!(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_TDX_VM));
> +}
> +
> +/*
> + * Do a dummy io exit to verify that the TD has been initialized correctly and
> + * guest can run some code inside.
> + */
> +TDX_GUEST_FUNCTION(guest_dummy_exit)
> +{
> +	uint64_t data;
> +
> +	data = 0xAB;
> +	tdvmcall_io(TDX_TEST_PORT, 1, 1, &data);
> +}
> +
> +/*
> + * TD lifecycle test will create a TD which runs a dumy IO exit to verify that
> + * the guest TD has been created correctly.
> + */
> +void  verify_td_lifecycle(void)
> +{
> +	struct kvm_vm *vm;
> +	struct kvm_run *run;
> +
> +	printf("Verifying TD lifecycle:\n");
> +	/* Create a TD VM with no memory.*/
> +	vm = __vm_create(VM_MODE_DEFAULT, 0, O_RDWR, KVM_X86_TDX_VM);
> +
> +	/* Allocate TD guest memory and initialize the TD.*/
> +	initialize_td(vm);
> +
> +	/* Initialize the TD vcpu and copy the test code to the guest memory.*/
> +	vm_vcpu_add_tdx(vm, 0);
> +
> +	/* Setup and initialize VM memory */
> +	prepare_source_image(vm, guest_dummy_exit,
> +			     TDX_FUNCTION_SIZE(guest_dummy_exit), 0);
> +	finalize_td_memory(vm);
> +
> +	run = vcpu_state(vm, 0);
> +	vcpu_run(vm, 0);
> +	TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
> +		    "Got exit_reason other than KVM_EXIT_IO: %u (%s)\n",
> +		    run->exit_reason,
> +		    exit_reason_str(run->exit_reason));
> +
> +	kvm_vm_free(vm);
> +	printf("\t ... PASSED\n");
> +}
> +int main(int argc, char **argv)
> +{
> +	if (!is_tdx_enabled()) {
> +		printf("TDX is not supported by the KVM\n"
> +		       "Skipping the TDX tests.\n");

Use print_skip() to emit the standard "skipping test" message.

> +		return 0;

Exit with KSFT_SKIP so that test runners know that this test was skipped
(rather than running and passing).

> +	}
> +
> +	run_in_new_process(&verify_td_lifecycle);
> +
> +	return 0;
> +}
> -- 
> 2.32.0.432.gabb21c7263-goog
> 

[RFC PATCH 4/4] KVM: selftest: Adding test case for TDX port IO

[Erdem Aktas]

Verifies TDVMCALL<INSTRUCTION.IO> READ and WRITE operations.

Signed-off-by: Erdem Aktas <erdemaktas@google.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Peter Gonda <pgonda@google.com>
Reviewed-by: Marc Orr <marcorr@google.com>
Reviewed-by: Sagi Shahar <sagis@google.com>
---
 .../selftests/kvm/x86_64/tdx_vm_tests.c       | 107 ++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
index da7ea67b1..7b0b4b378 100644
--- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
+++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
@@ -88,6 +88,112 @@ void  verify_td_lifecycle(void)
 	kvm_vm_free(vm);
 	printf("\t ... PASSED\n");
 }
+
+/*
+ * Verifies IO functionality by writing a |value| to a predefined port.
+ * Verifies that the read value is |value| + 1 from the same port.
+ * If all the tests are passed then write a value to port TDX_TEST_PORT
+ */
+TDX_GUEST_FUNCTION(guest_io_exit)
+{
+	uint64_t data_out, data_in, delta;
+
+	data_out = 0xAB;
+	tdvmcall_io(TDX_TEST_PORT, 1, 1, &data_out);
+	tdvmcall_io(TDX_TEST_PORT, 1, 0, &data_in);
+	delta = data_in - data_out - 1;
+	tdvmcall_io(TDX_TEST_PORT, 1, 1, &delta);
+}
+
+void  verify_td_ioexit(void)
+{
+	struct kvm_vm *vm;
+	struct kvm_run *run;
+	uint32_t port_data;
+
+	printf("Verifying TD IO Exit:\n");
+	/* Create a TD VM with no memory.*/
+	vm = __vm_create(VM_MODE_DEFAULT, 0, O_RDWR, KVM_X86_TDX_VM);
+
+	/* Allocate TD guest memory and initialize the TD.*/
+	initialize_td(vm);
+
+	/* Initialize the TD vcpu and copy the test code to the guest memory.*/
+	vm_vcpu_add_tdx(vm, 0);
+
+	/* Setup and initialize VM memory */
+	prepare_source_image(vm, guest_io_exit,
+			     TDX_FUNCTION_SIZE(guest_io_exit), 0);
+	finalize_td_memory(vm);
+
+	run = vcpu_state(vm, 0);
+
+	/* Wait for guest to do a IO write */
+	vcpu_run(vm, 0);
+	TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
+		    "Got exit_reason other than KVM_EXIT_IO: %u (%s)\n",
+		    run->exit_reason,
+		    exit_reason_str(run->exit_reason));
+
+	TEST_ASSERT((run->exit_reason == KVM_EXIT_IO)
+		    && (run->io.port == TDX_TEST_PORT)
+		    && (run->io.size == 1)
+		    && (run->io.direction == 1),
+		    "Got an unexpected IO exit values: %u (%s) %d %d %d\n",
+		    run->exit_reason,
+		    exit_reason_str(run->exit_reason),
+		    run->io.port, run->io.size, run->io.direction);
+	port_data = *(uint8_t *)((void *)run + run->io.data_offset);
+
+	printf("\t ... IO WRITE: OK\n");
+	/*
+	 * Wait for the guest to do a IO read. Provide the previos written data
+	 * + 1 back to the guest
+	 */
+	vcpu_run(vm, 0);
+	TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
+		    "Got exit_reason other than KVM_EXIT_IO: %u (%s)\n",
+		    run->exit_reason,
+		    exit_reason_str(run->exit_reason));
+
+	TEST_ASSERT(run->exit_reason == KVM_EXIT_IO &&
+		    run->io.port == TDX_TEST_PORT &&
+		    run->io.size == 1 &&
+		    run->io.direction == 0,
+		    "Got an unexpected IO exit values: %u (%s) %d %d %d\n",
+		    run->exit_reason,
+		    exit_reason_str(run->exit_reason),
+		    run->io.port, run->io.size, run->io.direction);
+	*(uint8_t *)((void *)run + run->io.data_offset) = port_data + 1;
+
+	printf("\t ... IO READ: OK\n");
+	/*
+	 * Wait for the guest to do a IO write to the TDX_TEST_PORT with the
+	 * value of 0. Any value other than 0 means, the guest has not able to
+	 * read/write values correctly.
+	 */
+	vcpu_run(vm, 0);
+	TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
+		    "KVM_EXIT_IO is expected but got an exit_reason: %u (%s)\n",
+		    run->exit_reason,
+		    exit_reason_str(run->exit_reason));
+
+	TEST_ASSERT(run->exit_reason == KVM_EXIT_IO &&
+		    run->io.port == TDX_TEST_PORT &&
+		    run->io.size == 1 &&
+		    run->io.direction == 1 &&
+		    *(uint32_t *)((void *)run + run->io.data_offset) == 0,
+		    "Got an unexpected IO exit values: %u (%s) %d %d %d %d\n",
+		    run->exit_reason,
+		    exit_reason_str(run->exit_reason),
+		    run->io.port, run->io.size, run->io.direction,
+		    *(uint32_t *)((void *)run + run->io.data_offset));
+
+	printf("\t ... IO verify read/write values: OK\n");
+	kvm_vm_free(vm);
+	printf("\t ... PASSED\n");
+}
+
 int main(int argc, char **argv)
 {
 	if (!is_tdx_enabled()) {
@@ -97,6 +203,7 @@ int main(int argc, char **argv)
 	}
 
 	run_in_new_process(&verify_td_lifecycle);
+	run_in_new_process(&verify_td_ioexit);
 
 	return 0;
 }
-- 
2.32.0.432.gabb21c7263-goog

RE: [RFC PATCH 0/4] TDX KVM selftests

[Duan, Zhenzhong]

> -----Original Message-----
> From: Erdem Aktas <erdemaktas@google.com>
> Sent: Tuesday, July 27, 2021 2:38 AM
> To: linux-kselftest@vger.kernel.org
> Cc: erdemaktas@google.com; Paolo Bonzini <pbonzini@redhat.com>; Shuah
> Khan <shuah@kernel.org>; Andrew Jones <drjones@redhat.com>; Ben
> Gardon <bgardon@google.com>; Peter Xu <peterx@redhat.com>; Sean
> Christopherson <seanjc@google.com>; Christian Borntraeger
> <borntraeger@de.ibm.com>; Eric Auger <eric.auger@redhat.com>;
> Emanuele Giuseppe Esposito <eesposit@redhat.com>; Ricardo Koller
> <ricarkol@google.com>; Duan, Zhenzhong <zhenzhong.duan@intel.com>;
> Aaron Lewis <aaronlewis@google.com>; Jim Mattson
> <jmattson@google.com>; Oliver Upton <oupton@google.com>; Vitaly
> Kuznetsov <vkuznets@redhat.com>; Peter Shier <pshier@google.com>; Axel
> Rasmussen <axelrasmussen@google.com>; Yanan Wang
> <wangyanan55@huawei.com>; Maciej S. Szmigiero
> <maciej.szmigiero@oracle.com>; David Matlack <dmatlack@google.com>;
> Like Xu <like.xu@linux.intel.com>; open list <linux-kernel@vger.kernel.org>;
> open list:KERNEL VIRTUAL MACHINE (KVM) <kvm@vger.kernel.org>
> Subject: [RFC PATCH 0/4] TDX KVM selftests
> 
> TDX stands for Trust Domain Extensions which isolates VMs from the virtual-
> machine manager (VMM)/hypervisor and any other software on the
> platform.
> 
> Intel has recently submitted a set of RFC patches for KVM support for TDX
> and more information can be found on the latest TDX Support
> Patches: https://lkml.org/lkml/2021/7/2/558
> 
> Due to the nature of the confidential computing environment that TDX
> provides, it is very difficult to verify/test the KVM support. TDX requires UEFI
> and the guest kernel to be enlightened which are all under development.
> 
> We are working on a set of selftests to close this gap and be able to verify the
> KVM functionality to support TDX lifecycle and GHCI [1] interface.
> 
> We are looking for any feedback on:
> - Patch series itself
> - Any suggestion on how we should approach testing TDX functionality.
> Does selftests seems reasonable or should we switch to using KVM unit tests.
> I would be happy to get some perspective on how KVM unit tests can help us
> more.
> - Any test case or scenario that we should add.
> - Anything else I have not thought of yet.
> 
> Current patch series provide the following capabilities:
> 
> - Provide helper functions to create a TD (Trusted Domain) using the KVM
>   ioctls
> - Provide helper functions to create a guest image that can include any
>   testing code
> - Provide helper functions and wrapper functions to write testing code
>   using GHCI interface
> - Add a test case that verifies TDX life cycle
> - Add a test case that verifies TDX GHCI port IO
> 
> TODOs:
> - Use existing function to create page tables dynamically
>   (ie __virt_pg_map())
> - Remove arbitrary defined magic numbers for data structure offsets
> - Add TDVMCALL for error reporting
> - Add additional test cases as some listed below
> - Add #VE handlers to help testing more complicated test cases
> 
> Other test cases that we are planning to add:
> (with credit to sagis@google.com)
> 
> VM call interface        Input                        Output                Result
> GetTdVmCallInfo          R12=0                        None                VMCALL_SUCCESS
> MapGPA                   Map private page (GPA.S=0)
> VMCALL_SUCCESS
> MapGPA                   Map shared page (GPA.S=1)
> VMCALL_SUCCESS
> MapGPA                   Map already private page as private
> VMCALL_INVALID_OPERAND
> MapGPA                   Map already shared page as shared
> VMCALL_INVALID_OPERAND
> GetQuote
> ReportFatalError
> SetupEventNotifyInterrupt   Valid interrupt value (32:255)
> VMCALL_SUCCESS
> SetupEventNotifyInterrupt   Invalid value (>255)
> VMCALL_INVALID_OPERAND
> Instruction.CPUID        R12(EAX)=1, R13(ECX)=0       EBX[8:15]=0x8
>                                                       EBX[16:23]=X
>                                                       EBX[24:31]=vcpu_id
>                                                       ECX[0]=1
>                                                       ECX[12]=Y
> Instruction.CPUID       R12(EAX)=1, R13(ECX)=4
> VMCALL_INVALID_OPERAND
> VE.RequestMMIO
> Instruction.HLT                                                           VMCALL_SUCCESS
> Instruction.IO          Read/Write 1/2/4 bytes                            VMCALL_SUCCESS
> Instruction.IO          Read/Write 3 bytes
> VMCALL_INVALID_OPERAND
> Instruction.RDMSR       Accessible register           R11=msr_value
> VMCALL_SUCCESS
>                         Inaccessible register
> VMCALL_INVALID_OPERAND
> Instruction.RDMSR       Accessible register                               VMCALL_SUCCESS
>                         Inaccessible register
> VMCALL_INVALID_OPERAND
> INSTRUCTION.PCONFIG
> 
> [1] Intel TDX Guest-Hypervisor Communication Interface
> 
> https://software.intel.com/content/dam/develop/external/us/en/document
> s/intel-tdx-guest-hypervisor-communication-interface.pdf
> 
> 
> Erdem Aktas (4):
>   KVM: selftests: Add support for creating non-default type VMs
>   KVM: selftest: Add helper functions to create TDX VMs
In tools/testing/selftests/kvm/Makefile, '/lib/x86_64/tdx_lib.c' should be changed to 'lib/x86_64/tdx_lib.c'
After that, build and test passes.

# ./tdx_vm_tests
Verifying TD lifecycle:
Verifying TD IO Exit:
         ... IO WRITE: OK
         ... IO READ: OK
         ... IO verify read/write values: OK

Tested-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Regards
Zhenzhong