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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

> 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.

Jan