Re: [PATCH] pvcalls: Document explicitly the padding for all arches

From: Julien Grall <julien@xen.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: Juergen Gross <jgross@suse.com>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Julien Grall <jgrall@amazon.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH] pvcalls: Document explicitly the padding for all arches
Date: Wed, 29 Apr 2020 15:14:55 +0100	[thread overview]
Message-ID: <4cd108f9-3ad0-2262-fa7c-d2247660c635@xen.org> (raw)
In-Reply-To: <423c0369-9c90-dbfe-2f90-d49a2ce5b283@suse.com>

Hi Jan,

On 29/04/2020 15:05, Jan Beulich wrote:
> On 29.04.2020 16:01, Julien Grall wrote:
>> Hi,
>>
>> On 22/04/2020 10:20, Jan Beulich wrote:
>>>> Even if it was possible to use the sub-structs defined in the header
>>>> that way, keep in mind that we also wrote:
>>>>
>>>>           /* dummy member to force sizeof(struct xen_pvcalls_request)
>>>>            * to match across archs */
>>>>           struct xen_pvcalls_dummy {
>>>>               uint8_t dummy[56];
>>>>           } dummy;
>>>
>>> This has nothing to do with how a consumer may use the structs.
>>>
>>>> And the spec also clarifies that the size of each specific request is
>>>> always 56 bytes.
>>>
>>> Sure, and I didn't mean to imply that a consumer would be allowed
>>> to break this requirement. Still something like this
>>>
>>> int pvcall_new_socket(struct xen_pvcalls_socket *s) {
>>>       struct xen_pvcalls_request req = {
>>>           .req_id = REQ_ID,
>>>           .cmd = PVCALLS_SOCKET,
>>>           .u.socket = *s,
>>>       };
>>>
>>>       return pvcall(&req);
>>> }
>>>
>>> may break.
>>
>> I think I understand your concern now. So yes I agree this would break 32-bit consumer.
>>
>> As the padding is at the end of the structure, I think a 32-bit frontend and 64-bit backend (or vice-versa) should currently work without any trouble. The problem would come later if we decide to extend a command.
> 
> Can commands be extended at all, i.e. don't extensions require new
> commands? The issue I've described has nothing to do with future
> extending of any of the affected structures.

I think my point wasn't conveyed correctly. The implicit padding is at 
the end of the structure for all the consumers but 32-bit x86. So 
without any modification, I think 32-bit frontend can still communicate 
with 64-bit backend (or vice-versa).

Therefore I suggest to rework the documentation and add the implicit 
padding just for all the architectures but 32-bit x86.

Cheers,

-- 
Julien Grall