Re: [Qemu-devel] [PATCH v1 2/7] drivers/hv: Move struct hv_message into UAPI Hyper-V x86 header

[KY Srinivasan]

> -----Original Message-----
> From: Paolo Bonzini [mailto:address@hidden On Behalf Of Paolo
> Bonzini
> Sent: Friday, November 27, 2015 1:35 AM
> To: Andrey Smetanin <address@hidden>; address@hidden
> Cc: Gleb Natapov <address@hidden>; KY Srinivasan <address@hidden>;
> Haiyang Zhang <address@hidden>; Vitaly Kuznetsov
> <address@hidden>; Roman Kagan <address@hidden>; Denis V.
> Lunev <address@hidden>; address@hidden
> Subject: Re: [PATCH v1 2/7] drivers/hv: Move struct hv_message into UAPI
> Hyper-V x86 header
> 
> 
> 
> On 25/11/2015 16:20, Andrey Smetanin wrote:
> > This struct is required for Hyper-V SynIC timers implementation inside KVM
> > and for upcoming Hyper-V VMBus support by userspace(QEMU). So place
> it into
> > Hyper-V UAPI header.
> >
> > Signed-off-by: Andrey Smetanin <address@hidden>
> > Reviewed-by: Roman Kagan <address@hidden>
Acked-by: K. Y. Srinivasan <address@hidden>

> > CC: Gleb Natapov <address@hidden>
> > CC: Paolo Bonzini <address@hidden>
> > CC: "K. Y. Srinivasan" <address@hidden>
> > CC: Haiyang Zhang <address@hidden>
> > CC: Vitaly Kuznetsov <address@hidden>
> > CC: Roman Kagan <address@hidden>
> > CC: Denis V. Lunev <address@hidden>
> > CC: address@hidden
> > ---
> >  arch/x86/include/uapi/asm/hyperv.h | 91
> ++++++++++++++++++++++++++++++++++++++
> >  drivers/hv/hyperv_vmbus.h          | 91 
> > --------------------------------------
> >  2 files changed, 91 insertions(+), 91 deletions(-)
> >
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h
> b/arch/x86/include/uapi/asm/hyperv.h
> > index 07981f0..e86d77e 100644
> > --- a/arch/x86/include/uapi/asm/hyperv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -271,4 +271,95 @@ typedef struct _HV_REFERENCE_TSC_PAGE {
> >
> >  #define HV_SYNIC_STIMER_COUNT              (4)
> >
> > +/* Define synthetic interrupt controller message constants. */
> > +#define HV_MESSAGE_SIZE                    (256)
> > +#define HV_MESSAGE_PAYLOAD_BYTE_COUNT      (240)
> > +#define HV_MESSAGE_PAYLOAD_QWORD_COUNT     (30)
> > +
> > +/* Define hypervisor message types. */
> > +enum hv_message_type {
> > +   HVMSG_NONE                      = 0x00000000,
> > +
> > +   /* Memory access messages. */
> > +   HVMSG_UNMAPPED_GPA              = 0x80000000,
> > +   HVMSG_GPA_INTERCEPT             = 0x80000001,
> > +
> > +   /* Timer notification messages. */
> > +   HVMSG_TIMER_EXPIRED                     = 0x80000010,
> > +
> > +   /* Error messages. */
> > +   HVMSG_INVALID_VP_REGISTER_VALUE = 0x80000020,
> > +   HVMSG_UNRECOVERABLE_EXCEPTION   = 0x80000021,
> > +   HVMSG_UNSUPPORTED_FEATURE               = 0x80000022,
> > +
> > +   /* Trace buffer complete messages. */
> > +   HVMSG_EVENTLOG_BUFFERCOMPLETE   = 0x80000040,
> > +
> > +   /* Platform-specific processor intercept messages. */
> > +   HVMSG_X64_IOPORT_INTERCEPT              = 0x80010000,
> > +   HVMSG_X64_MSR_INTERCEPT         = 0x80010001,
> > +   HVMSG_X64_CPUID_INTERCEPT               = 0x80010002,
> > +   HVMSG_X64_EXCEPTION_INTERCEPT   = 0x80010003,
> > +   HVMSG_X64_APIC_EOI                      = 0x80010004,
> > +   HVMSG_X64_LEGACY_FP_ERROR               = 0x80010005
> > +};
> > +
> > +/* Define synthetic interrupt controller message flags. */
> > +union hv_message_flags {
> > +   __u8 asu8;
> > +   struct {
> > +           __u8 msg_pending:1;
> > +           __u8 reserved:7;
> > +   };
> > +};
> > +
> > +/* Define port identifier type. */
> > +union hv_port_id {
> > +   __u32 asu32;
> > +   struct {
> > +           __u32 id:24;
> > +           __u32 reserved:8;
> > +   } u;
> > +};
> > +
> > +/* Define port type. */
> > +enum hv_port_type {
> > +   HVPORT_MSG      = 1,
> > +   HVPORT_EVENT            = 2,
> > +   HVPORT_MONITOR  = 3
> > +};
> > +
> > +/* Define synthetic interrupt controller message header. */
> > +struct hv_message_header {
> > +   enum hv_message_type message_type;
> 
> Do not declare this as an enum, declare it as __u32 to make the size
> portable.  It can be a patch on top.
> 
> KY, can you ack these two patches?
> 
> Paolo
> 
> > +   __u8 payload_size;
> > +   union hv_message_flags message_flags;
> > +   __u8 reserved[2];
> > +   union {
> > +           __u64 sender;
> > +           union hv_port_id port;
> > +   };
> > +};
> > +
> > +/* Define timer message payload structure. */
> > +struct hv_timer_message_payload {
> > +   __u32 timer_index;
> > +   __u32 reserved;
> > +   __u64 expiration_time;  /* When the timer expired */
> > +   __u64 delivery_time;    /* When the message was delivered */
> > +};
> > +
> > +/* Define synthetic interrupt controller message format. */
> > +struct hv_message {
> > +   struct hv_message_header header;
> > +   union {
> > +           __u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
> > +   } u;
> > +};
> > +
> > +/* Define the synthetic interrupt message page layout. */
> > +struct hv_message_page {
> > +   struct hv_message sint_message[HV_SYNIC_SINT_COUNT];
> > +};
> > +
> >  #endif
> > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> > index 46e23d1..d22230c 100644
> > --- a/drivers/hv/hyperv_vmbus.h
> > +++ b/drivers/hv/hyperv_vmbus.h
> > @@ -63,10 +63,6 @@ enum hv_cpuid_function {
> >  /* Define version of the synthetic interrupt controller. */
> >  #define HV_SYNIC_VERSION           (1)
> >
> > -/* Define synthetic interrupt controller message constants. */
> > -#define HV_MESSAGE_SIZE                    (256)
> > -#define HV_MESSAGE_PAYLOAD_BYTE_COUNT      (240)
> > -#define HV_MESSAGE_PAYLOAD_QWORD_COUNT     (30)
> >  #define HV_ANY_VP                  (0xFFFFFFFF)
> >
> >  /* Define synthetic interrupt controller flag constants. */
> > @@ -74,53 +70,9 @@ enum hv_cpuid_function {
> >  #define HV_EVENT_FLAGS_BYTE_COUNT  (256)
> >  #define HV_EVENT_FLAGS_DWORD_COUNT (256 / sizeof(u32))
> >
> > -/* Define hypervisor message types. */
> > -enum hv_message_type {
> > -   HVMSG_NONE                      = 0x00000000,
> > -
> > -   /* Memory access messages. */
> > -   HVMSG_UNMAPPED_GPA              = 0x80000000,
> > -   HVMSG_GPA_INTERCEPT             = 0x80000001,
> > -
> > -   /* Timer notification messages. */
> > -   HVMSG_TIMER_EXPIRED                     = 0x80000010,
> > -
> > -   /* Error messages. */
> > -   HVMSG_INVALID_VP_REGISTER_VALUE = 0x80000020,
> > -   HVMSG_UNRECOVERABLE_EXCEPTION   = 0x80000021,
> > -   HVMSG_UNSUPPORTED_FEATURE               = 0x80000022,
> > -
> > -   /* Trace buffer complete messages. */
> > -   HVMSG_EVENTLOG_BUFFERCOMPLETE   = 0x80000040,
> > -
> > -   /* Platform-specific processor intercept messages. */
> > -   HVMSG_X64_IOPORT_INTERCEPT              = 0x80010000,
> > -   HVMSG_X64_MSR_INTERCEPT         = 0x80010001,
> > -   HVMSG_X64_CPUID_INTERCEPT               = 0x80010002,
> > -   HVMSG_X64_EXCEPTION_INTERCEPT   = 0x80010003,
> > -   HVMSG_X64_APIC_EOI                      = 0x80010004,
> > -   HVMSG_X64_LEGACY_FP_ERROR               = 0x80010005
> > -};
> > -
> >  /* Define invalid partition identifier. */
> >  #define HV_PARTITION_ID_INVALID            ((u64)0x0)
> >
> > -/* Define port identifier type. */
> > -union hv_port_id {
> > -   u32 asu32;
> > -   struct {
> > -           u32 id:24;
> > -           u32 reserved:8;
> > -   } u ;
> > -};
> > -
> > -/* Define port type. */
> > -enum hv_port_type {
> > -   HVPORT_MSG      = 1,
> > -   HVPORT_EVENT            = 2,
> > -   HVPORT_MONITOR  = 3
> > -};
> > -
> >  /* Define port information structure. */
> >  struct hv_port_info {
> >     enum hv_port_type port_type;
> > @@ -161,27 +113,6 @@ struct hv_connection_info {
> >     };
> >  };
> >
> > -/* Define synthetic interrupt controller message flags. */
> > -union hv_message_flags {
> > -   u8 asu8;
> > -   struct {
> > -           u8 msg_pending:1;
> > -           u8 reserved:7;
> > -   };
> > -};
> > -
> > -/* Define synthetic interrupt controller message header. */
> > -struct hv_message_header {
> > -   enum hv_message_type message_type;
> > -   u8 payload_size;
> > -   union hv_message_flags message_flags;
> > -   u8 reserved[2];
> > -   union {
> > -           u64 sender;
> > -           union hv_port_id port;
> > -   };
> > -};
> > -
> >  /*
> >   * Timer configuration register.
> >   */
> > @@ -198,31 +129,9 @@ union hv_timer_config {
> >     };
> >  };
> >
> > -
> > -/* Define timer message payload structure. */
> > -struct hv_timer_message_payload {
> > -   u32 timer_index;
> > -   u32 reserved;
> > -   u64 expiration_time;    /* When the timer expired */
> > -   u64 delivery_time;      /* When the message was delivered */
> > -};
> > -
> > -/* Define synthetic interrupt controller message format. */
> > -struct hv_message {
> > -   struct hv_message_header header;
> > -   union {
> > -           u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
> > -   } u ;
> > -};
> > -
> >  /* Define the number of message buffers associated with each port. */
> >  #define HV_PORT_MESSAGE_BUFFER_COUNT       (16)
> >
> > -/* Define the synthetic interrupt message page layout. */
> > -struct hv_message_page {
> > -   struct hv_message sint_message[HV_SYNIC_SINT_COUNT];
> > -};
> > -
> >  /* Define the synthetic interrupt controller event flags format. */
> >  union hv_synic_event_flags {
> >     u8 flags8[HV_EVENT_FLAGS_BYTE_COUNT];
> >