linux-kernel - Re: [RFC 28/33] x86/hyper-v: Introduce memory intercept message structure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4eda2ec5444cc151279d5b571a158a60a5406e1f.camel@redhat.com>
Date:   Tue, 28 Nov 2023 09:53:07 +0200
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Nicolas Saenz Julienne <nsaenz@...zon.com>, kvm@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, linux-hyperv@...r.kernel.org,
        pbonzini@...hat.com, seanjc@...gle.com, vkuznets@...hat.com,
        anelkz@...zon.com, graf@...zon.com, dwmw@...zon.co.uk,
        jgowans@...zon.com, corbert@....net, kys@...rosoft.com,
        haiyangz@...rosoft.com, decui@...rosoft.com, x86@...nel.org,
        linux-doc@...r.kernel.org
Subject: Re: [RFC 28/33] x86/hyper-v: Introduce memory intercept message
 structure

On Wed, 2023-11-08 at 11:18 +0000, Nicolas Saenz Julienne wrote:
> Introduce struct hv_memory_intercept_message, which is used when issuing
> memory intercepts to a Hyper-V VSM guest.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@...zon.com>
> ---
>  arch/x86/include/asm/hyperv-tlfs.h | 76 ++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
> 
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index af594aa65307..d3d74fde6da1 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -799,6 +799,82 @@ struct hv_get_vp_from_apic_id_in {
>  	u32 apic_ids[];
>  } __packed;
>  
> +
> +/* struct hv_intercept_header::access_type_mask */
> +#define HV_INTERCEPT_ACCESS_MASK_NONE    0
> +#define HV_INTERCEPT_ACCESS_MASK_READ    1
> +#define HV_INTERCEPT_ACCESS_MASK_WRITE   2
> +#define HV_INTERCEPT_ACCESS_MASK_EXECUTE 4
> +
> +/* struct hv_intercept_exception::cache_type */
> +#define HV_X64_CACHE_TYPE_UNCACHED       0
> +#define HV_X64_CACHE_TYPE_WRITECOMBINING 1
> +#define HV_X64_CACHE_TYPE_WRITETHROUGH   4
> +#define HV_X64_CACHE_TYPE_WRITEPROTECTED 5
> +#define HV_X64_CACHE_TYPE_WRITEBACK      6
> +
> +/* Intecept message header */
> +struct hv_intercept_header {
> +	__u32 vp_index;
> +	__u8 instruction_length;
> +#define HV_INTERCEPT_ACCESS_READ    0
> +#define HV_INTERCEPT_ACCESS_WRITE   1
> +#define HV_INTERCEPT_ACCESS_EXECUTE 2
> +	__u8 access_type_mask;
> +	union {
> +		__u16 as_u16;
> +		struct {
> +			__u16 cpl:2;
> +			__u16 cr0_pe:1;
> +			__u16 cr0_am:1;
> +			__u16 efer_lma:1;
> +			__u16 debug_active:1;
> +			__u16 interruption_pending:1;
> +			__u16 reserved:9;
> +		};
> +	} exec_state;
> +	struct hv_x64_segment_register cs;
> +	__u64 rip;
> +	__u64 rflags;
> +} __packed;


Although the struct/field names in the TLFS spec are terrible for obvious reasons,
we should still try to stick to them as much as possible to make one's life
less miserable when trying to find them in the spec.

It is also a good idea to mention from which part of the spec these fields
come (hint, it's not from VSM part).

Copying here the structs that I found in the spec:

typedef struct
{
	HV_VP_INDEX VpIndex;
	UINT8 InstructionLength;
	HV_INTERCEPT_ACCESS_TYPE_MASK InterceptAccessType;
	HV_X64_VP_EXECUTION_STATE ExecutionState;
	HV_X64_SEGMENT_REGISTER CsSegment;
	UINT64 Rip;
	UINT64 Rflags;
} HV_X64_INTERCEPT_MESSAGE_HEADER;


typedef struct
{
	UINT16 Cpl:2;
	UINT16 Cr0Pe:1;
	UINT16 Cr0Am:1;
	UINT16 EferLma:1;
	UINT16 DebugActive:1;
	UINT16 InterruptionPending:1;
	UINT16 Reserved:4;
	UINT16 Reserved:5;
} HV_X64_VP_EXECUTION_STATE;


For example 'access_type_mask' should be called intercept_access_type,
and so on.



> +
> +union hv_x64_memory_access_info {
> +	__u8 as_u8;
> +	struct {
> +		__u8 gva_valid:1;
> +		__u8 _reserved:7;
> +	};
> +};

typedef struct
{
	UINT8 GvaValid:1;
	UINT8 Reserved:7;

} HV_X64_MEMORY_ACCESS_INFO;

> +
> +struct hv_memory_intercept_message {
> +	struct hv_intercept_header header;
> +	__u32 cache_type;
> +	__u8 instruction_byte_count;

If I understand correctly this is the size of the following
'instruction_bytes' field?


> +	union hv_x64_memory_access_info memory_access_info;
> +	__u16 _reserved;
> +	__u64 gva;
> +	__u64 gpa;
> +	__u8 instruction_bytes[16];
> +	struct hv_x64_segment_register ds;
> +	struct hv_x64_segment_register ss;
> +	__u64 rax;
> +	__u64 rcx;
> +	__u64 rdx;
> +	__u64 rbx;
> +	__u64 rsp;
> +	__u64 rbp;
> +	__u64 rsi;
> +	__u64 rdi;
> +	__u64 r8;
> +	__u64 r9;
> +	__u64 r10;
> +	__u64 r11;
> +	__u64 r12;
> +	__u64 r13;
> +	__u64 r14;
> +	__u64 r15;
> +} __packed;

I can't seem to find this struct at all in the spec. If it was reverse-engineered,
then we must document everything that we know to help future readers of this code.


Best regards,
	Maxim Levitsky

> +
>  #include <asm-generic/hyperv-tlfs.h>
>  
>  #endif