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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Oct 30, 2018 at 05:03:54PM +0000, Paul Durrant wrote:
> > -----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.

OK, that's fine for me.

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks, Roger.

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