Re: [Xen-devel] [PATCH] fix c/s 18938

["Jan Beulich" <jbeulich@xxxxxxxxxx>]

>>> Christoph Egger <Christoph.Egger@xxxxxxx> 27.03.09 18:21 >>>
>On Friday 27 March 2009 18:08:00 Jan Beulich wrote:
>> >Please make this a fixed sized array. There are users like Oracle who run
>> >a 32bit PAE Dom0 on a 64bit Xen ...
>>
>> And you expect a 32-bit kernel to be able to make sense of the MSRs
>> corresponding to 64-bit-only registers?
>>
>> But you remind me that I failed to handle the difference in size of that
>> array for 32-on-64 - I really need to check why the structure layout
>> checking logic didn't catch the difference in size. Oh, right, sizeof(void
>> *) needs special treatment (and I really don#t want to sue sizeof(long)
>> here due to the implied dependency on the OS ABI).
>
>Having the same size for both 32bit and 64bit makes the Dom0 code
>simpler to use it. Taking care about the different sizes would
>enforce either hackish code or two different implementations.
>That's not acceptable IMO.

After some more thinking about it I actually like the way the patch currently
is. This is because the formal structure layout now properly represents the
differences between the two bit widths, while it specifically does *not*
require different code paths in the guest: The way the structure gets used,
regardless of Dom0's bitness all registers will always be put in the structure
(since there's no is_pv_32on64_???() check in that code), and Dom0 will
also not have any issues reading them (unless it applies very pedantic
checks on the structure contents) since the structure is to be parsed using
its mcinfo_common header, which correctly states the size of the structure.

If anything, I'd be inclined to either make the structure variable length (but
that would imply all compilers possibly used to compile producing and
consuming code understand this construct) or add a comment to the effect
of the above explanation.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel