Re: [PATCH v2] x86/hyper-v: Mark TLFS structures packed

From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Nadav Amit <nadav.amit@gmail.com>
Cc: "kvm list" <kvm@vger.kernel.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"Roman Kagan" <rkagan@virtuozzo.com>,
	"K. Y. Srinivasan" <kys@microsoft.com>,
	"Haiyang Zhang" <haiyangz@microsoft.com>,
	"Stephen Hemminger" <sthemmin@microsoft.com>,
	x86@kernel.org,
	"Michael Kelley (EOSG)" <Michael.H.Kelley@microsoft.com>,
	"Thomas Gleixner" <tglx@linutronix.de>
Subject: Re: [PATCH v2] x86/hyper-v: Mark TLFS structures packed
Date: Mon, 03 Dec 2018 00:35:35 +0100	[thread overview]
Message-ID: <87k1krh588.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <DC5975C2-6517-4D20-9DC9-574C5439C426@gmail.com>

Nadav Amit <nadav.amit@gmail.com> writes:

[skip]

>
> Having said that, something else is sort of strange in the TLFS definitions,
> I think (I really know little about this whole protocol). Look at the
> following definitions from hyperv-tlfs.h:
>
>> struct hv_vpset {
>>         u64 format;
>>         u64 valid_bank_mask;
>>         u64 bank_contents[];
>> };
>> 
>> struct hv_tlb_flush_ex {
>>         u64 address_space;
>>         u64 flags;
>>         struct hv_vpset hv_vp_set;
>>         u64 gva_list[];
>> };
>
> It seems you have two flexible array members at the end of hv_tlb_flush_ex.
> This causes bank_contents[x] and gva_list[x] to overlap. So unless they have
> the same meaning, this asks for trouble IMHO.
>

This is weird but intentional :-) We're just following Hyper-V spec
here.

E.g. HvFlushVirtualAddressListEx hypercall has the following input ABI:

[Fixed len head][[Fixed len VP set spec]Var len VP set][Var len addr List]

"Fixed len VP set spec" defines the true length of "Var len VP set" and
"Address List" starts right after that. The length of the whole
structure is also known.

So bank_contents[] and gva_list[] do overlap (and have different
meaning). We take special precautions when forming the structure
(e.g. fill_gva_list() takes 'offset').

-- 
Vitaly