On 12/12/18 18:57, Vitaly Kuznetsov wrote:
> The TLFS structures are used for hypervisor-guest communication and must
> exactly meet the specification.
>
> Compilers can add alignment padding to structures or reorder struct members
> for randomization and optimization, which would break the hypervisor ABI.
>
> Mark the structures as packed to prevent this. 'struct hv_vp_assist_page'
> and 'struct hv_enlightened_vmcs' need to be properly padded to support the
> change.
>
> Suggested-by: Nadav Amit <nadav.amit@xxxxxxxxx>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> Acked-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Acked-by: Nadav Amit <nadav.amit@xxxxxxxxx>
> Reviewed-by: Michael Kelley <mikelley@xxxxxxxxxxxxx>
> ---
> - Changes since v3:
> - Properly pad 'struct hv_vp_assist_page' and 'struct hv_enlightened_vmcs'.
> - Add Michael's Reviewed-by tag.
>
> - This is a follow-up to my "[PATCH v2 0/4] x86/kvm/hyper-v: Implement
> Direct Mode for synthetic timers" series, as suggested by Thomas I'm
> routing it to KVM tree to avoid merge conflicts.
> ---
> arch/x86/include/asm/hyperv-tlfs.h | 57 ++++++++++++++++--------------
> 1 file changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index ebfed56976d2..64d2b1914d37 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -271,7 +271,7 @@ union hv_x64_msr_hypercall_contents {
> u64 enable:1;
> u64 reserved:11;
> u64 guest_physical_address:52;
> - };
> + } __packed;
> };
>
> /*
> @@ -283,7 +283,7 @@ struct ms_hyperv_tsc_page {
> volatile u64 tsc_scale;
> volatile s64 tsc_offset;
> u64 reserved2[509];
> -};
> +} __packed;
>
> /*
> * The guest OS needs to register the guest ID with the hypervisor.
> @@ -324,7 +324,7 @@ struct hv_reenlightenment_control {
> __u64 enabled:1;
> __u64 reserved2:15;
> __u64 target_vp:32;
> -};
> +} __packed;
>
> #define HV_X64_MSR_TSC_EMULATION_CONTROL 0x40000107
> #define HV_X64_MSR_TSC_EMULATION_STATUS 0x40000108
> @@ -332,12 +332,12 @@ struct hv_reenlightenment_control {
> struct hv_tsc_emulation_control {
> __u64 enabled:1;
> __u64 reserved:63;
> -};
> +} __packed;
>
> struct hv_tsc_emulation_status {
> __u64 inprogress:1;
> __u64 reserved:63;
> -};
> +} __packed;
>
> #define HV_X64_MSR_HYPERCALL_ENABLE 0x00000001
> #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT 12
> @@ -409,7 +409,7 @@ typedef struct _HV_REFERENCE_TSC_PAGE {
> __u32 res1;
> __u64 tsc_scale;
> __s64 tsc_offset;
> -} HV_REFERENCE_TSC_PAGE, *PHV_REFERENCE_TSC_PAGE;
> +} __packed HV_REFERENCE_TSC_PAGE, *PHV_REFERENCE_TSC_PAGE;
>
> /* Define the number of synthetic interrupt sources. */
> #define HV_SYNIC_SINT_COUNT (16)
> @@ -466,7 +466,7 @@ union hv_message_flags {
> struct {
> __u8 msg_pending:1;
> __u8 reserved:7;
> - };
> + } __packed;
> };
>
> /* Define port identifier type. */
> @@ -475,7 +475,7 @@ union hv_port_id {
> struct {
> __u32 id:24;
> __u32 reserved:8;
> - } u;
> + } __packed u;
> };
>
> /* Define synthetic interrupt controller message header. */
> @@ -488,7 +488,7 @@ struct hv_message_header {
> __u64 sender;
> union hv_port_id port;
> };
> -};
> +} __packed;
>
> /* Define synthetic interrupt controller message format. */
> struct hv_message {
> @@ -496,12 +496,12 @@ struct hv_message {
> union {
> __u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
> } u;
> -};
> +} __packed;
>
> /* Define the synthetic interrupt message page layout. */
> struct hv_message_page {
> struct hv_message sint_message[HV_SYNIC_SINT_COUNT];
> -};
> +} __packed;
>
> /* Define timer message payload structure. */
> struct hv_timer_message_payload {
> @@ -509,7 +509,7 @@ struct hv_timer_message_payload {
> __u32 reserved;
> __u64 expiration_time; /* When the timer expired */
> __u64 delivery_time; /* When the message was delivered */
> -};
> +} __packed;
>
> /* Define virtual processor assist page structure. */
> struct hv_vp_assist_page {
> @@ -518,8 +518,9 @@ struct hv_vp_assist_page {
> __u64 vtl_control[2];
> __u64 nested_enlightenments_control[2];
> __u32 enlighten_vmentry;
> + __u32 padding;
> __u64 current_nested_vmcs;
> -};
> +} __packed;
>
> struct hv_enlightened_vmcs {
> u32 revision_id;
> @@ -533,6 +534,8 @@ struct hv_enlightened_vmcs {
> u16 host_gs_selector;
> u16 host_tr_selector;
>
> + u16 padding16_1;
> +
> u64 host_ia32_pat;
> u64 host_ia32_efer;
>
> @@ -651,7 +654,7 @@ struct hv_enlightened_vmcs {
> u64 ept_pointer;
>
> u16 virtual_processor_id;
> - u16 padding16[3];
> + u16 padding16_2[3];
>
> u64 padding64_2[5];
> u64 guest_physical_address;
> @@ -693,7 +696,7 @@ struct hv_enlightened_vmcs {
> u32 nested_flush_hypercall:1;
> u32 msr_bitmap:1;
> u32 reserved:30;
> - } hv_enlightenments_control;
> + } __packed hv_enlightenments_control;
> u32 hv_vp_id;
>
> u64 hv_vm_id;
> @@ -703,7 +706,7 @@ struct hv_enlightened_vmcs {
> u64 padding64_5[7];
> u64 xss_exit_bitmap;
> u64 padding64_6[7];
> -};
> +} __packed;
>
> #define HV_VMX_ENLIGHTENED_CLEAN_FIELD_NONE 0
> #define HV_VMX_ENLIGHTENED_CLEAN_FIELD_IO_BITMAP BIT(0)
> @@ -744,7 +747,7 @@ union hv_stimer_config {
> u64 reserved_z0:3;
> u64 sintx:4;
> u64 reserved_z1:44;
> - };
> + } __packed;
> };
>
>
> @@ -759,7 +762,7 @@ union hv_synic_scontrol {
> struct {
> u64 enable:1;
> u64 reserved:63;
> - };
> + } __packed;
> };
>
> /* Define synthetic interrupt source. */
> @@ -771,7 +774,7 @@ union hv_synic_sint {
> u64 masked:1;
> u64 auto_eoi:1;
> u64 reserved2:46;
> - };
> + } __packed;
> };
>
> /* Define the format of the SIMP register */
> @@ -781,7 +784,7 @@ union hv_synic_simp {
> u64 simp_enabled:1;
> u64 preserved:11;
> u64 base_simp_gpa:52;
> - };
> + } __packed;
> };
>
> /* Define the format of the SIEFP register */
> @@ -791,34 +794,34 @@ union hv_synic_siefp {
> u64 siefp_enabled:1;
> u64 preserved:11;
> u64 base_siefp_gpa:52;
> - };
> + } __packed;
> };
>
> struct hv_vpset {
> u64 format;
> u64 valid_bank_mask;
> u64 bank_contents[];
> -};
> +} __packed;
>
> /* HvCallSendSyntheticClusterIpi hypercall */
> struct hv_send_ipi {
> u32 vector;
> u32 reserved;
> u64 cpu_mask;
> -};
> +} __packed;
>
> /* HvCallSendSyntheticClusterIpiEx hypercall */
> struct hv_send_ipi_ex {
> u32 vector;
> u32 reserved;
> struct hv_vpset vp_set;
> -};
> +} __packed;
>
> /* HvFlushGuestPhysicalAddressSpace hypercalls */
> struct hv_guest_mapping_flush {
> u64 address_space;
> u64 flags;
> -};
> +} __packed;
>
> /* HvFlushVirtualAddressSpace, HvFlushVirtualAddressList hypercalls */
> struct hv_tlb_flush {
> @@ -826,7 +829,7 @@ struct hv_tlb_flush {
> u64 flags;
> u64 processor_mask;
> u64 gva_list[];
> -};
> +} __packed;
>
> /* HvFlushVirtualAddressSpaceEx, HvFlushVirtualAddressListEx hypercalls */
> struct hv_tlb_flush_ex {
> @@ -834,6 +837,6 @@ struct hv_tlb_flush_ex {
> u64 flags;
> struct hv_vpset hv_vp_set;
> u64 gva_list[];
> -};
> +} __packed;
>
> #endif
>
Queued, thanks. I squashed the SynIC parts into "x86/hyper-v: move
synic/stimer control structures definitions to hyperv-tlfs.h" and kept
the rest separate.
Paolo
