Re: [Xen-devel] [PATCH v2 9/9] viridian: introduce struct viridian_page

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.10.18 at 13:43, <paul.durrant@xxxxxxxxxx> wrote:
> The 'vp_assist' page is currently an example of a guest page which needs to
> be kept mapped throughout the life-time of a guest, but there are other
> such examples in the specifiction [1]. This patch therefore introduces a
> generic 'viridian_page' type and converts the current vp_assist/apic_assist
> related code to use it. Subsequent patches implementing other enlightments
> can then also make use of it.

This sounds generic (as in "not synic specific), yet ...

> --- a/xen/arch/x86/hvm/viridian/synic.c
> +++ b/xen/arch/x86/hvm/viridian/synic.c
> @@ -37,14 +37,13 @@ static void dump_vp_assist(const struct vcpu *v)
>             v, (unsigned long)va->fields.pfn);
>  }
>  
> -static void initialize_vp_assist(struct vcpu *v)
> +static void initialize_guest_page(struct vcpu *v, struct viridian_page *vp)
>  {
>      struct domain *d = v->domain;
> -    unsigned long gmfn = v->arch.hvm.viridian.vp_assist.msr.fields.pfn;
> +    unsigned long gmfn = vp->msr.fields.pfn;
>      struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
> -    HV_VP_ASSIST_PAGE *ptr;
>  
> -    ASSERT(!v->arch.hvm.viridian.vp_assist.ptr);
> +    ASSERT(!vp->ptr);
>  
>      if ( !page )
>          goto fail;

... you retain the implementation here, when it would now perhaps
more logically live in viridian.c (again). Is this intentional?

> @@ -221,9 +218,9 @@ void viridian_synic_load_vcpu_ctxt(
>  {
>      v->arch.hvm.viridian.vp_assist.msr.raw = ctxt->vp_assist_msr;
>      if ( v->arch.hvm.viridian.vp_assist.msr.fields.enabled )
> -        initialize_vp_assist(v);
> +        initialize_guest_page(v, &v->arch.hvm.viridian.vp_assist);
>  
> -    v->arch.hvm.viridian.vp_assist.pending = !!ctxt->vp_assist_pending;
> +    v->arch.hvm.viridian.apic_assist_pending = !!ctxt->apic_assist_pending;

No need for !! anymore with ...

> --- a/xen/include/asm-x86/hvm/viridian.h
> +++ b/xen/include/asm-x86/hvm/viridian.h
> @@ -88,13 +88,16 @@ union viridian_page_msr
>      } fields;
>  };
>  
> +struct viridian_page
> +{
> +    union viridian_page_msr msr;
> +    void *ptr;
> +};
> +
>  struct viridian_vcpu
>  {
> -    struct {
> -        union viridian_page_msr msr;
> -        void *ptr;
> -        bool pending;
> -    } vp_assist;
> +    struct viridian_page vp_assist;
> +    bool apic_assist_pending;

... this being (and having been) bool.

Jan



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