Re: [Xen-devel] [PATCH] x86: drop unneeded __packed attributes

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.03.17 at 18:15, <roger.pau@xxxxxxxxxx> wrote:
> On Mon, Mar 06, 2017 at 09:57:23AM -0700, Jan Beulich wrote:
>> >>> On 06.03.17 at 17:42, <roger.pau@xxxxxxxxxx> wrote:
>> > There where a couple of unneeded packed attributes in several x86-specific
>> > structures, that are obviously aligned. The only non-trivial one is
>> > vmcb_struct, which has been checked to have the same layout with and 
> without
>> > the packed attribute using pahole. In that case add a build-time size 
>> > check 
> to
>> > be on the safe side.
>> > 
>> > No functional change is expected as a result of this commit.
>> > 
>> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> 
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>> albeit ...
>> 
>> > --- a/xen/arch/x86/hvm/svm/vmcb.c
>> > +++ b/xen/arch/x86/hvm/svm/vmcb.c
>> > @@ -71,6 +71,9 @@ static int construct_vmcb(struct vcpu *v)
>> >      struct arch_svm_struct *arch_svm = &v->arch.hvm_svm;
>> >      struct vmcb_struct *vmcb = arch_svm->vmcb;
>> >  
>> > +    /* Build-time check of the size of VMCB AMD structure. */
>> > +    BUILD_BUG_ON(sizeof(*vmcb) != 4096);
>> 
>> ... I wonder whether this wouldn't better be PAGE_SIZE.
> 
> I wondered the same, but then if we ever bump PAGE_SIZE to something else
> (which is not likely to happen), this still needs to be 4096.

Well, that depends on what meaning we assign to PAGE_SIZE: To
me this is a constant dictated by the architecture. If we meant to
invent a software page covering multiple hardware pages, I think
we should introduce a distinct constant. But this is close to
impossible to do, as discussed on a hackathon a while ago - the
page size is baked into the public interface.

> Feel free to change this to PAGE_SIZE if you prefer.

Indeed I may.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel