Re: xen bits broke x32 build

[David Woodhouse]

On 1 April 2023 12:57:33 BST, Michael Tokarev <mjt@tls.msk.ru> wrote:
>01.04.2023 14:45, Joao Martins пишет:
>> On 01/04/2023 09:40, Michael Tokarev wrote:
>>> After bringing in xen guest support, qemu fails to build on x32:
>>> Adding the folks who added the feature too
>> 
>>> target/i386/kvm/xen-emu.c:876:5: note: in expansion of macro 
>>> ‘qemu_build_assert’
>>>    876 |     qemu_build_assert(sizeof(struct vcpu_info) == 64);
>>>        |     ^~~~~~~~~~~~~~~~~
>>> 
>>> This one should be easy to fix, but I wonder if there are other issues
>>> with x32 exists..
>>> 
>> Not sure.
>> 
>> struct vcpu_info is supposed to be 64bytes on both 32-bit and 64-bit builds.
>> 
>> If anything maybe arch_vcpu_info struct is different on 32-bit and 64-bit...
>> David, Paul, any ideas?
>
>Yes, it is arch_vcpu_info. I assumed it is a trivial thing, but let me explain
>if it's not the case.
>
>include/hw/xen/interface/arch-x86/xen.h :
>
>#if defined(__i386__)
>#include "xen-x86_32.h"
>#elif defined(__x86_64__)
>#include "xen-x86_64.h"
>#endif
>
>(I wonder if it ever possible to have none of the two defined).
>
>Now, xen-x86_32.h has:
>
>struct arch_vcpu_info {
>    unsigned long cr2;
>    unsigned long pad[5]; /* sizeof(vcpu_info_t) == 64 */
>};
>
>Assuming sizeof(long)==32bits. But actually it is 64bits on x32.
>
>While xen-x86_64.h has:
>
>struct arch_vcpu_info {
>    unsigned long cr2;
>    unsigned long pad; /* sizeof(vcpu_info_t) == 64 */
>};
>
>
>It looks like for x32, the test in arch-x86/xen.h should be:
>
>#if defined(__x86_64__)
>#include "xen-x86_64.h"
>#else
>#include "xen-x86_32.h"
>#endif
>
>since x32 is almost like x86_64. The only difference from x86_64
>is sizeof(pointer), which is 32bits.

Hm, doesn't x32 also align uint64_t to 64 bits (unlike i386 which only aligns 
it to 32 bits)?

>Well. Maybe xen support should be disabled entirely on x32.
>Or maybe x32 should be declared as unsupported entirely.
>I dunno.

We rely heavily on the struct layouts being precisely the same, since these are 
ABI for the Xen guests. Wherever there was some 32/64 compatibility issue — and 
often where there wasn't — that's why I littered it with those build 
assertions. 

But while there are enough in there to sanity check the i386 vs. x86_64 ABI 
differences, I wouldn't swear that I've put in enough checks for x32. So "it 
builds without hitting an assertion" does not necessarily mean it'll be 
*correct*. Let's disable it on x32.