Re: [Xen-devel] [PATCH 4/8] viridian: remove duplicate union types

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Roger Pau Monne
> Sent: 30 October 2018 16:40
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Jan Beulich
> <jbeulich@xxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH 4/8] viridian: remove duplicate union
> types
> 
> On Mon, Oct 29, 2018 at 06:02:07PM +0000, Paul Durrant wrote:
> > The 'viridian_vp_assist', 'viridian_hypercall_gpa' and
> > 'viridian_reference_tsc' union types are identical in layout. The layout
> > is also common throughout the specification [1].
> >
> > This patch declares a common 'viridian_page_msr' type and converts the
> rest
> > of the code to use that type for both the hypercall and VP assist pages.
> 
> I agree with the unification of the viridian_page_msr struct.
> 
> > Also, rename 'viridian_guest_os_id' to 'viridian_guest_os_id_msr' since
> it
> > also is a union representing an MSR value.
>                                ^ a
> 
> IMO (and I'm not that familiar with the code) adding those _msr
> prefixes here doesn't seem to add any value to the code, there's
> nothing MSR specific in those structs, apart from their size.
> 

There is actually... they are unions that represent the layout of an MSR, e.g 
See section 2.6 "Reporting the Guest OS Identity".

> Is there a plan to add a new structure like viridian_page_mmio or
> similar that requires such prefixes to be present?
> 

No, I'm not anticipating Windows communicating via an MMIO region, but I's 
still like the call out something that represents a bit-field encoding of an 
MSR.

  Paul

> Thanks, Roger.

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