Re: [PATCH] x86/vhpet: Fix type size in timer_int_route_valid

[Eslam Elnikety <elnikety@xxxxxxxxxx>]

On 28.07.20 19:51, Jan Beulich wrote:
> On 28.07.2020 11:26, Andrew Cooper wrote:
> > Does this work?
> >
> > diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
> > index ca94e8b453..638f6174de 100644
> > --- a/xen/arch/x86/hvm/hpet.c
> > +++ b/xen/arch/x86/hvm/hpet.c
> > @@ -62,8 +62,7 @@
> >   #define timer_int_route(h, n)    MASK_EXTR(timer_config(h, n),
> > HPET_TN_ROUTE)
> > -#define timer_int_route_cap(h, n) \
> > -    MASK_EXTR(timer_config(h, n), HPET_TN_INT_ROUTE_CAP)
> > +#define timer_int_route_cap(h, n) (h)->hpet.timers[(n)].route
> Seeing that this is likely the route taken here, and hence to avoid
> an extra round trip, two remarks: Here I see no need for the
> parentheses inside the square brackets.
Will take of this in v2.

> > diff --git a/xen/include/asm-x86/hvm/vpt.h 
> > b/xen/include/asm-x86/hvm/vpt.h
> > index f0e0eaec83..a41fc443cc 100644
> > --- a/xen/include/asm-x86/hvm/vpt.h
> > +++ b/xen/include/asm-x86/hvm/vpt.h
> > @@ -73,7 +73,13 @@ struct hpet_registers {
> >       uint64_t isr;               /* interrupt status reg */
> >       uint64_t mc64;              /* main counter */
> >       struct {                    /* timers */
> > -        uint64_t config;        /* configuration/cap */
> > +        union {
> > +            uint64_t config;    /* configuration/cap */
> > +            struct {
> > +                uint32_t _;
> > +                uint32_t route;
> > +            };
> > +        };
> So long as there are no static initializers for this construct
> that would then suffer the old-gcc problem, this is of course a
> fine arrangement to make.
I have to admit that I have no clue what the "old-gcc" problem is. I am 
curious, and I would appreciate pointers to figure out if/how to 
resolve. Is that an old, existing problem? Or a problem that was present 
in older versions of gcc? If the latter, is that a gcc version that we 
still care about? Thanks, Jan.
-- Eslam

> Jan