Re: [Xen-devel] [PATCH 7/8] viridian: define type for the 'virtual VP assist page'

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

On Tue, Oct 30, 2018 at 05:11:30PM +0000, Paul Durrant wrote:
> > -----Original Message-----
> > From: Roger Pau Monne
> > Sent: 30 October 2018 17:09
> > 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 7/8] viridian: define type for the
> > 'virtual VP assist page'
> > 
> > On Mon, Oct 29, 2018 at 06:02:10PM +0000, Paul Durrant wrote:
> > > The specification [1] defines a type so we should use it, rather than
> > just
> > > OR-ing and AND-ing magic bits.
> > >
> > > No functional change.
> > >
> > > NOTE: The type defined in the specification does include an anonymous
> > >       sub-struct in the page type but, as we currently use only the
> > first
> > >       element, the struct declaration has been omitted.
> > >
> > > [1] https://github.com/MicrosoftDocs/Virtualization-
> > Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specif
> > ication%20v5.0C.pdf
> > >
> > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > > ---
> > > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > ---
> > >  xen/arch/x86/hvm/viridian/synic.c  | 52 +++++++++++++++++++++++--------
> > -------
> > >  xen/include/asm-x86/hvm/viridian.h |  2 +-
> > >  2 files changed, 33 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/xen/arch/x86/hvm/viridian/synic.c
> > b/xen/arch/x86/hvm/viridian/synic.c
> > > index 3f043f34ee..735c4c897c 100644
> > > --- a/xen/arch/x86/hvm/viridian/synic.c
> > > +++ b/xen/arch/x86/hvm/viridian/synic.c
> > > @@ -13,6 +13,18 @@
> > >  #include <asm/apic.h>
> > >  #include <asm/hvm/support.h>
> > >
> > > +typedef struct _HV_VIRTUAL_APIC_ASSIST
> > > +{
> > > +    uint32_t no_eoi:1;
> > 
> > Maybe bool:1 so you can use true/false?
> > 
> 
> No, I'm very specifically using a 32-bit bitfield to match what the spec. 
> says.

Right, but no_eoi is a single flag on that bitfield, unless I'm
missing something I think you could just use:

typedef union _HV_VIRTUAL_APIC_ASSIST
{
    struct {
        bool no_eoi:1;
    } fields;
    uint32_t raw;
} HV_VIRTUAL_APIC_ASSIST;

If you wish to access the raw value as a uint32_t while keeping access
to individual flags easy. This union also has the advantage that
adding new fields won't require you to adjust the size of the
reserved_zero field.

Thanks, Roger.

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