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

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 05 November 2018 12:51
> To: Paul Durrant 
> Cc: Andrew Cooper ; Wei Liu
> ; xen-devel 
> Subject: Re: [PATCH v2 5/9] viridian: separate interrupt related
> enlightenment implementations...
> 
> >>> On 31.10.18 at 13:43,  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 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +
> > +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?

Yes, it's meant to dump after programming the VP assist MSR, so I think the 
name is still correct.

> 
> > --- 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 
> > +
> > +/* Viridian MSR numbers. */
> > +#define HV_X64_MSR_GUEST_OS_ID   0x4000
> 
> 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.

Ok. They are only needed under that sub-dir so I'll add the extra header.

  Paul

> 
> Jan
> 


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

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

[Wei Liu]

On Wed, Oct 31, 2018 at 12:43:31PM +, Paul Durrant wrote:
> ...into new 'synic' module.
> 
> The SynIC (synthetic interrupt controller) is specified [1] to be a super-
> set of a virtualized LAPIC, and its definition encompasses all
> enlightenments related to virtual interrupt control.
> 
> This patch reduces the size of the main viridian source module by giving
> these enlightenments their own module. This is done in anticipation of
> implementation of more such enlightenments and a desire not to further
> lengthen then main source module when this work is done.
> 
> Whilst moving the code:
> 
> - Fix various style issues.
> - Move the MSR definitions into the header (since they are now needed in
>   more than one source module).
> 
> [1] 
> https://github.com/MicrosoftDocs/Virtualization-Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specification%20v5.0C.pdf
> 
> Signed-off-by: Paul Durrant 

Reviewed-by: Wei Liu 

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