Re: [Xen-devel] [PATCH v3 3/5] x86/hyperv: provide percpu hypercall input page

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.01.2020 17:33, Wei Liu wrote:
> On Mon, Jan 06, 2020 at 11:27:18AM +0100, Jan Beulich wrote:
>> On 05.01.2020 17:47, Wei Liu wrote:
>>> Hyper-V's input / output argument must be 8 bytes aligned an not cross
>>> page boundary. The easiest way to satisfy those requirements is to use
>>> percpu page.
>>
>> I'm not sure "easiest" is really true here. Others could consider adding
>> __aligned() attributes as easy or even easier (by being even more
>> transparent to use sites). Could we settle on "One way ..."?
> 
> Do you mean something like
> 
>    struct foo __aligned(8);

If this is in a header and ...

>    hv_do_hypercall(OP, virt_to_maddr(&foo), ...);

... this in actual code, then yes.

> ?
> 
> I don't think this is transparent to user sites. Plus, foo is on stack
> which is 1) difficult to get its maddr,

It being on the stack may indeed complicate getting its machine address
(if not now, then down the road) - valid point.

> 2) may cross page boundary.

The __aligned() of course needs to be large enough to avoid this
happening.

>> Also, while looking at this I notice that - despite my earlier
>> comment when giving the respective, sort-of-conditional ack -
>> there are (still) many apparently pointless __packed attributes
>> in hyperv-tlfs.h. Care to comment on this?
> 
> Again, that's a straight import from Linux. I tried not to deviate too
> much. A commit in Linux (ec084491727b0) claims "compiler can add
> alignment padding to structures or reorder struct members for
> randomization and optimization".

Would a compiler doing so (without explicitly being told to) even
be in line with the C spec? I'd buy such a claim only if I see an
example proving it.

> I just checked all the packed structures. They seem to have all the
> required manual paddings already. I can only assume they tried to erred
> on the safe side.

And you surely recall we had to remove quite a few instances of
__packed for gcc 9 compatibility?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel