Re: [Xen-devel] [PATCH v2 5/9] viridian: separate interrupt related enlightenment implementations...

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.10.18 at 13:43, <paul.durrant@xxxxxxxxxx> wrote:
> --- /dev/null
> +++ b/xen/arch/x86/hvm/viridian/synic.c
> @@ -0,0 +1,225 @@
> +/**************************************************************************
> *
> + * synic.c
> + *
> + * An implementation of some interrupt related Viridian enlightenments.
> + * See Microsoft's Hypervisor Top Level Functional Specification.
> + * for more information.
> + */
> +
> +#include <xen/domain_page.h>
> +#include <xen/hypercall.h>
> +#include <xen/sched.h>
> +#include <xen/version.h>
> +
> +#include <asm/apic.h>
> +#include <asm/hvm/support.h>
> +
> +static void dump_vp_assist(const struct vcpu *v)

With the name change to synic, is the _vp_ infix here and in a few
other places retained intentionally?

> --- a/xen/include/asm-x86/hvm/viridian.h
> +++ b/xen/include/asm-x86/hvm/viridian.h
> @@ -9,6 +9,74 @@
>  #ifndef __ASM_X86_HVM_VIRIDIAN_H__
>  #define __ASM_X86_HVM_VIRIDIAN_H__
>  
> +#include <asm/hvm/save.h>
> +
> +/* Viridian MSR numbers. */
> +#define HV_X64_MSR_GUEST_OS_ID                   0x40000000

Are these needed outside of xen/arch/x86/hvm/viridian/ ? If not,
please introduce a local header there rather than polluting
every CU's name space. Other definitions and declaration local
to the Viridian implementation should then go there too, rather
than here.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel