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

[Roman Kagan <rkagan@virtuozzo.com>]

On Mon, Dec 03, 2018 at 12:35:35AM +0100, Vitaly Kuznetsov wrote:
> 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').

This basically means that the argument of this hypercall can't be
represented by a C struct.  gva_list just can't be used.  So I'd rather
remove it from the struct (but leave a comment to that end perhaps), and
construct the message in place (as is done now anyway).

Roman.