Re: [PATCH] x86/svm: Move svm_domain structure to svm.h

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 30/05/2025 9:53 am, Teddy Astie wrote:
> struct svm_domain was in vmcb.h which is meant for VMCB specific operations 
> and
> constants, move it to svm.h where it belongs.
>
> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
> ---
>  xen/arch/x86/include/asm/hvm/domain.h   |  1 +
>  xen/arch/x86/include/asm/hvm/svm/svm.h  | 11 +++++++++++
>  xen/arch/x86/include/asm/hvm/svm/vmcb.h | 11 -----------
>  3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/hvm/domain.h 
> b/xen/arch/x86/include/asm/hvm/domain.h
> index 333501d5f2..2608bcfad2 100644
> --- a/xen/arch/x86/include/asm/hvm/domain.h
> +++ b/xen/arch/x86/include/asm/hvm/domain.h
> @@ -16,6 +16,7 @@
>  #include <asm/hvm/io.h>
>  #include <asm/hvm/vmx/vmcs.h>
>  #include <asm/hvm/svm/vmcb.h>
> +#include <asm/hvm/svm/svm.h>
>  

I agree the headers aren't laid out well, but this isn't great either.

You're now including svm.h in ~all translation units, because
~everything includes sched.h and sched.h includes these.

In some copious free time, what we need to do is split $foo-types.h out
of current headers so we can avoid including most of these headers in
most TUs in Xen.  But that's a huge effort.

>  #ifdef CONFIG_MEM_SHARING
>  struct mem_sharing_domain
> diff --git a/xen/arch/x86/include/asm/hvm/svm/svm.h 
> b/xen/arch/x86/include/asm/hvm/svm/svm.h
> index 4eeeb25da9..32f6e48e30 100644
> --- a/xen/arch/x86/include/asm/hvm/svm/svm.h
> +++ b/xen/arch/x86/include/asm/hvm/svm/svm.h
> @@ -21,6 +21,17 @@ bool svm_load_segs(unsigned int ldt_ents, unsigned long 
> ldt_base,
>                     unsigned long fs_base, unsigned long gs_base,
>                     unsigned long gs_shadow);
>  
> +struct svm_domain {
> +    /* OSVW MSRs */
> +    union {
> +        uint64_t raw[2];
> +        struct {
> +            uint64_t length;
> +            uint64_t status;
> +        };
> +    } osvw;
> +};
> +

Honestly, I'm tempted to just drop OSVW.

It's a legacy AMD facility which predates the Zen/EPYC days, which isn't
even available to guests (because it was broken when I came to do some
remedial fixes, and there's still been no work to put it into suitably
into the migrate stream).

Right now, I'm pretty confident that guests will uniformly get #GP
accessing the MSRs, and there's no way to configure visibility.

~Andrew