Re: [PATCH] x86/vm_event: transfer nested p2m base info

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 03/01/2021 18:41, Tamas K Lengyel wrote:
> Required to introspect events originating from nested VMs.
>
> Signed-off-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/monitor.c    | 32 ++++++++++++++++++++++++++++++--
>  xen/include/public/vm_event.h |  7 ++++++-
>  2 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
> index e4a09964a0..eb4afe81b3 100644
> --- a/xen/arch/x86/hvm/monitor.c
> +++ b/xen/arch/x86/hvm/monitor.c
> @@ -26,6 +26,7 @@
>  #include <xen/mem_access.h>
>  #include <xen/monitor.h>
>  #include <asm/hvm/monitor.h>
> +#include <asm/hvm/nestedhvm.h>
>  #include <asm/altp2m.h>
>  #include <asm/monitor.h>
>  #include <asm/p2m.h>
> @@ -33,6 +34,15 @@
>  #include <asm/vm_event.h>
>  #include <public/vm_event.h>
>  
> +static inline void set_npt_base(struct vcpu *curr, vm_event_request_t *req)

No need for inline here.  Can fix on commit.

> diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
> index fdd3ad8a30..8415bc7618 100644
> --- a/xen/include/public/vm_event.h
> +++ b/xen/include/public/vm_event.h
> @@ -208,6 +212,7 @@ struct vm_event_regs_x86 {
>      uint64_t msr_star;
>      uint64_t msr_lstar;
>      uint64_t gdtr_base;
> +    uint64_t npt_base;

This needs enough description to actually use it correctly.

/* Guest physical address.  On Intel hardware, this is the EPT_POINTER
field from the L1 hypervisors VMCS, including all architecturally
defined metadata. */

Except, its not.  nvmx_vcpu_eptp_base() masks out the lower metadata, so
the walk length is missing, and the introspection agent can't
distinguish between 4 and 5 level EPT.  Same on the AMD side (except it
could be any paging mode, including 2 and 3 level).

Furthermore, (and more in reference to your pagewalk patch), it might be
necessary to know whether EPT A/D is enabled for the agent to do the
correct thing when getting a gla-not-valid fault.

~Andrew