xen/arch/x86/hvm/viridian/synic.c | 17 ++++++---------- xen/arch/x86/include/asm/guest/hyperv-tlfs.h | 21 ++++++++++---------- 2 files changed, 17 insertions(+), 21 deletions(-)
Instead of open-coding the struct type in
viridian_synic_deliver_timer_msg(). Additionally expand the union field
in hv_message struct to contain the timer payload, this way the memcpy in
viridian_synic_deliver_timer_msg() can be replaced with a struct assign.
No functional change intended.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
xen/arch/x86/hvm/viridian/synic.c | 17 ++++++----------
xen/arch/x86/include/asm/guest/hyperv-tlfs.h | 21 ++++++++++----------
2 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/xen/arch/x86/hvm/viridian/synic.c b/xen/arch/x86/hvm/viridian/synic.c
index e6cba7548f1b..6d7b6bd0eda2 100644
--- a/xen/arch/x86/hvm/viridian/synic.c
+++ b/xen/arch/x86/hvm/viridian/synic.c
@@ -327,15 +327,10 @@ bool viridian_synic_deliver_timer_msg(struct vcpu *v, unsigned int sintx,
struct viridian_vcpu *vv = v->arch.hvm.viridian;
const union hv_synic_sint *vs = &vv->sint[sintx];
struct hv_message *msg = vv->simp.ptr;
- struct {
- uint32_t TimerIndex;
- uint32_t Reserved;
- uint64_t ExpirationTime;
- uint64_t DeliveryTime;
- } payload = {
- .TimerIndex = index,
- .ExpirationTime = expiration,
- .DeliveryTime = delivery,
+ const struct hv_timer_message_payload payload = {
+ .timer_index = index,
+ .expiration_time = expiration,
+ .delivery_time = delivery,
};
/* Don't assume SIM page to be mapped. */
@@ -359,8 +354,8 @@ bool viridian_synic_deliver_timer_msg(struct vcpu *v, unsigned int sintx,
msg->header.message_flags.msg_pending = 0;
msg->header.payload_size = sizeof(payload);
- BUILD_BUG_ON(sizeof(payload) > sizeof(msg->u.payload));
- memcpy(msg->u.payload, &payload, sizeof(payload));
+ BUILD_BUG_ON(sizeof(msg->payload.timer) > sizeof(msg->payload.raw));
+ msg->payload.timer = payload;
if ( !vs->masked && vlapic_enabled(vcpu_vlapic(v)) )
vlapic_set_irq(vcpu_vlapic(v), vs->vector, 0);
diff --git a/xen/arch/x86/include/asm/guest/hyperv-tlfs.h b/xen/arch/x86/include/asm/guest/hyperv-tlfs.h
index 79cfc90dd8ec..18da73c74e03 100644
--- a/xen/arch/x86/include/asm/guest/hyperv-tlfs.h
+++ b/xen/arch/x86/include/asm/guest/hyperv-tlfs.h
@@ -527,12 +527,21 @@ struct hv_message_header {
};
};
+/* Define timer message payload structure. */
+struct hv_timer_message_payload {
+ uint32_t timer_index;
+ uint32_t reserved;
+ uint64_t expiration_time; /* When the timer expired */
+ uint64_t delivery_time; /* When the message was delivered */
+};
+
/* Define synthetic interrupt controller message format. */
struct hv_message {
struct hv_message_header header;
union {
- uint64_t payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
- } u;
+ struct hv_timer_message_payload timer;
+ uint64_t raw[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
+ } payload;
};
/* Define the synthetic interrupt message page layout. */
@@ -540,14 +549,6 @@ struct hv_message_page {
struct hv_message sint_message[HV_SYNIC_SINT_COUNT];
};
-/* Define timer message payload structure. */
-struct hv_timer_message_payload {
- uint32_t timer_index;
- uint32_t reserved;
- uint64_t expiration_time; /* When the timer expired */
- uint64_t delivery_time; /* When the message was delivered */
-};
-
struct hv_nested_enlightenments_control {
struct {
uint32_t directhypercall:1;
--
2.51.0
On 13/11/2025 5:24 pm, Roger Pau Monne wrote:
> diff --git a/xen/arch/x86/hvm/viridian/synic.c b/xen/arch/x86/hvm/viridian/synic.c
> index e6cba7548f1b..6d7b6bd0eda2 100644
> --- a/xen/arch/x86/hvm/viridian/synic.c
> +++ b/xen/arch/x86/hvm/viridian/synic.c
> @@ -327,15 +327,10 @@ bool viridian_synic_deliver_timer_msg(struct vcpu *v, unsigned int sintx,
> struct viridian_vcpu *vv = v->arch.hvm.viridian;
> const union hv_synic_sint *vs = &vv->sint[sintx];
> struct hv_message *msg = vv->simp.ptr;
> - struct {
> - uint32_t TimerIndex;
> - uint32_t Reserved;
> - uint64_t ExpirationTime;
> - uint64_t DeliveryTime;
> - } payload = {
> - .TimerIndex = index,
> - .ExpirationTime = expiration,
> - .DeliveryTime = delivery,
> + const struct hv_timer_message_payload payload = {
> + .timer_index = index,
> + .expiration_time = expiration,
> + .delivery_time = delivery,
Align these = for readability?
> };
>
> /* Don't assume SIM page to be mapped. */
> @@ -359,8 +354,8 @@ bool viridian_synic_deliver_timer_msg(struct vcpu *v, unsigned int sintx,
> msg->header.message_flags.msg_pending = 0;
> msg->header.payload_size = sizeof(payload);
>
> - BUILD_BUG_ON(sizeof(payload) > sizeof(msg->u.payload));
> - memcpy(msg->u.payload, &payload, sizeof(payload));
> + BUILD_BUG_ON(sizeof(msg->payload.timer) > sizeof(msg->payload.raw));
This BUILD_BUG_ON() was only needed because of the memcpy() between
different types. With structure assignment, the compiler will tell you
if the type mismatches.
Therefore, it's safe to drop.
Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
On Thu, Nov 13, 2025 at 05:52:47PM +0000, Andrew Cooper wrote:
> On 13/11/2025 5:24 pm, Roger Pau Monne wrote:
> > diff --git a/xen/arch/x86/hvm/viridian/synic.c b/xen/arch/x86/hvm/viridian/synic.c
> > index e6cba7548f1b..6d7b6bd0eda2 100644
> > --- a/xen/arch/x86/hvm/viridian/synic.c
> > +++ b/xen/arch/x86/hvm/viridian/synic.c
> > @@ -327,15 +327,10 @@ bool viridian_synic_deliver_timer_msg(struct vcpu *v, unsigned int sintx,
> > struct viridian_vcpu *vv = v->arch.hvm.viridian;
> > const union hv_synic_sint *vs = &vv->sint[sintx];
> > struct hv_message *msg = vv->simp.ptr;
> > - struct {
> > - uint32_t TimerIndex;
> > - uint32_t Reserved;
> > - uint64_t ExpirationTime;
> > - uint64_t DeliveryTime;
> > - } payload = {
> > - .TimerIndex = index,
> > - .ExpirationTime = expiration,
> > - .DeliveryTime = delivery,
> > + const struct hv_timer_message_payload payload = {
> > + .timer_index = index,
> > + .expiration_time = expiration,
> > + .delivery_time = delivery,
>
> Align these = for readability?
Sure, can do.
> > };
> >
> > /* Don't assume SIM page to be mapped. */
> > @@ -359,8 +354,8 @@ bool viridian_synic_deliver_timer_msg(struct vcpu *v, unsigned int sintx,
> > msg->header.message_flags.msg_pending = 0;
> > msg->header.payload_size = sizeof(payload);
> >
> > - BUILD_BUG_ON(sizeof(payload) > sizeof(msg->u.payload));
> > - memcpy(msg->u.payload, &payload, sizeof(payload));
> > + BUILD_BUG_ON(sizeof(msg->payload.timer) > sizeof(msg->payload.raw));
>
> This BUILD_BUG_ON() was only needed because of the memcpy() between
> different types. With structure assignment, the compiler will tell you
> if the type mismatches.
I've keep it to ensure the size of the hv_timer_message_payload
doesn't exceed the maximum payload size (240 bytes), as
msg->payload.raw is the maximum payload size defined by the standard.
> Therefore, it's safe to drop.
>
> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Let me know if you are fine with keeping the BUILD_BUG_ON() given the
justification above, as that would be my preference.
Thanks, Roger.
© 2016 - 2025 Red Hat, Inc.