[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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),
-#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




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.