Re: [PATCH v4 for-4.14 2/2] pvcalls: Document correctly and explicitely the padding for all arches

From: Stefano Stabellini <sstabellini@kernel.org>
To: Julien Grall <julien@xen.org>
Cc: Juergen Gross <jgross@suse.com>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	paul@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>,
	Jan Beulich <jbeulich@suse.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v4 for-4.14 2/2] pvcalls: Document correctly and explicitely the padding for all arches
Date: Thu, 9 Jul 2020 14:43:43 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.21.2007091443290.4124@sstabellini-ThinkPad-T480s> (raw)
In-Reply-To: <2e0a451e-58be-eff1-0030-be489e5098f3@xen.org>

On Thu, 9 Jul 2020, Julien Grall wrote:
> Hi,
> 
> Gentle ping.
> 
> Is the new commit message fine?
> 
> Cheers,
> 
> On 04/07/2020 16:29, Julien Grall wrote:
> > Hi,
> > 
> > On 27/06/2020 10:55, Julien Grall wrote:
> > > From: Julien Grall <jgrall@amazon.com>
> > > 
> > > The specification of pvcalls suggests there is padding for 32-bit x86
> > > at the end of most the structure. However, they are not described in
> > > in the public header.
> > > 
> > > Because of that all the structures would be 32-bit aligned and not
> > > 64-bit aligned for 32-bit x86.
> > > 
> > > For all the other architectures supported (Arm and 64-bit x86), the
> > > structure are aligned to 64-bit because they contain uint64_t field.
> > > Therefore all the structures contain implicit padding.
> > > 
> > > Given the specification is authoriitative, the padding will the same for
> > > the all architectures. The potential breakage of compatibility is ought
> > > to be fine as pvcalls is still a tech preview.
> > > 
> > > As an aside, the padding sadly cannot be mandated to be 0 as they are
> > > already present. So it is not going to be possible to use the padding
> > > for extending a command in the future.
> > > 
> > > Signed-off-by: Julien Grall <jgrall@amazon.com>
> > 
> > It looks like most of the comments are on the commit message. So rather than
> > sending the series again, below a new version of the commit message:
> > 
> > "
> > The specification of pvcalls suggests there is padding for 32-bit x86
> > at the end of most the structure. However, they are not described in
> > in the public header.
> > 
> > Because of that all the structures would have a different size between
> > 32-bit x86 and 64-bit x86.
> > 
> > For all the other architectures supported (Arm and 64-bit x86), the
> > structure have the sames sizes because they contain implicit padding thanks
> > to the 64-bit alinment of the field uint64_t field.
> > 
> > Given the specification is authoritative, the padding will now be the same
> > for all architectures. The potential breakage of compatibility is ought to
> > be fine as pvcalls is still a tech preview.
> > "

Looks good to me

Acked-by: Stefano Stabellini <sstabellini@kernel.org>