Re: [Xen-devel] [PATCH RFC 1/3] xen/save: pass a size paramter to the HVM compat functions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.10.15 at 11:49, <roger.pau@xxxxxxxxxx> wrote:
> El 15/10/15 a les 10.12, Jan Beulich ha escrit:
>>>>> On 14.10.15 at 18:24, <roger.pau@xxxxxxxxxx> wrote:
>>> In order to cope with types having multiple compat versions pass a parameter
>>> to the fixup function so we can identify which compat version Xen is dealing
>>> with.
>> 
>> Having peeked at patch 2, this won't help once another bit gets added
>> to the tail of that structure. Also it doesn't seem logical that the
>> previous compat handling got around without being passed the size
>> explicitly. I.e. while perhaps more involved, I think the compat
>> handling needs to be extended to allow for multiple versions.
> 
> Yes, patch #2 needs to be fixed by making fpu_initialised an uint32_t.
> Adding versions would be the best option IMHO, but I don't think I have
> enough time to do that myself right now.

Understood. An I don't think we really need versioning here.

>> Or, since we have this under control going forward, don't even declare
>> all the various compat structures in the public header (and only ever
>> add to the tail). Then staying with the passing of size probably makes
>> sense, but the fixup function then should use offsetof() instead of
>> sizeof() (and validate unused tail bits are zero, so they can be used for
>> something later on).
> 
> Since we use hvm_load_entry_zeroextend I think we can assert that all
> new tail bits are going to be 0, but I'm not sure I follow you regarding
> the usage of offsetof instead of sizeof. What are we going to compare
> with offsetof?

Instead of introducing compat1 and compat2 structures, just add
the new field to the existing structure, and use offsetof() to compare
the passed in size with the that of the structure in its original state.

> Also, we already have a compat version of hvm_hw_cpu that didn't add the
> new field to the end.

Right, but we can avoid making the same mistake again.

Jan


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