Re: QEMU patches for native windows support through clang-cl

Re: QEMU patches for native windows support through clang-cl

[Pierrick Bouvier]

Hi Erwin,

I'm adding also Peter and Alex, and the qemu developer mailing list.

Regards,
Pierrick

On 11/21/24 09:36, Erwin Jansen wrote:
> The Android Emulator team has successfully ported QEMU 8 to compile with 
> clang-cl, enabling Windows compilation and better integration with the 
> Microsoft ecosystem. This involved:
> 
>   * Replacing shell scripts with Python for enhanced Windows compatibility.
>   * Implementing fixes within existing scripts.
>   * Introducing a small set of header stubs and a series of compilation
>     fixes.
> 
> We would like to gauge the QEMU community's interest in these changes 
> and, if there is interest, discuss the best way to upstream them.
> 
> Kind regards,
> 
> Erwin.
> 

Re: QEMU patches for native windows support through clang-cl

[Paolo Bonzini]

On 11/21/24 19:17, Pierrick Bouvier wrote:
> On 11/21/24 09:36, Erwin Jansen wrote:
>> The Android Emulator team has successfully ported QEMU 8 to compile 
>> with clang-cl, enabling Windows compilation and better integration 
>> with the Microsoft ecosystem. This involved:
>>
>>   * Replacing shell scripts with Python for enhanced Windows 
>> compatibility.

It would be useful to know what scripts.  configure? :)

If it's just scripts/make-config-poison.sh or scripts/qemu-version.sh, 
no objection at all.

>>   * Implementing fixes within existing scripts.
>>   * Introducing a small set of header stubs and a series of compilation
>>     fixes.

If you can add this configuration to the GitLab CI, I don't see any 
blocker.  Otherwise there's risk of bitrot.

Paolo

Re: QEMU patches for native windows support through clang-cl

[Paolo Bonzini]

> NB As a general point, we actively block use of clang with Windows
> builds (more strictly in 9.2 now), because it lacks support for the
> 'gcc_struct' annotation that we rely on to guarantee correct ABI for
> structs exposed to guests in particular.

Ah, good point. This is
https://github.com/llvm/llvm-project/issues/24757 for the general
tracking issue, and https://github.com/llvm/llvm-project/pull/71148
for a recent PR that attempts to implement this.

Using -mno-ms-bitfields globally is unsafe because there are probably
Windows API structs that implement it.

One solution is to add `QEMU_BUILD_BUG_ON(sizeof(...) == ...)` to all
structs in QEMU that use bitfields. That will prove very quickly if
there are issues or not.

Paolo

> Many people try to simply remove that #ifdef, mistakenly assuming that
> because the code compiles without warnings, it must be correct. Did
> you have solution for this, as it would be a blocker for enabling
> clang on Windows currently ?
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>

Re: QEMU patches for native windows support through clang-cl

[Erwin Jansen]

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

Would a good next step be to work out some more details in a document that
outlines what process we are using, what we are planning to do and include
a set of suggestions as a starting point to see if we can upstream some of
the changes to the larger community?

We currently don't have a proper solution for packing. We are tracking the
issue here https://issuetracker.google.com/issues/380295845. All our code
is public and development is happening on the emu-dev repository:
https://android.googlesource.com/platform/external/qemu/+/emu-dev

Greetings,
Erwin.





On Thu, Nov 21, 2024 at 10:47 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> > NB As a general point, we actively block use of clang with Windows
> > builds (more strictly in 9.2 now), because it lacks support for the
> > 'gcc_struct' annotation that we rely on to guarantee correct ABI for
> > structs exposed to guests in particular.
>
> Ah, good point. This is
> https://github.com/llvm/llvm-project/issues/24757 for the general
> tracking issue, and https://github.com/llvm/llvm-project/pull/71148
> for a recent PR that attempts to implement this.
>
> Using -mno-ms-bitfields globally is unsafe because there are probably
> Windows API structs that implement it.
>
> One solution is to add `QEMU_BUILD_BUG_ON(sizeof(...) == ...)` to all
> structs in QEMU that use bitfields. That will prove very quickly if
> there are issues or not.
>
> Paolo
>
> > Many people try to simply remove that #ifdef, mistakenly assuming that
> > because the code compiles without warnings, it must be correct. Did
> > you have solution for this, as it would be a blocker for enabling
> > clang on Windows currently ?
> >
> > With regards,
> > Daniel
> > --
> > |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> > |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> > |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
> >
>
>

[-- Attachment #2: Type: text/html, Size: 3405 bytes --]

Re: QEMU patches for native windows support through clang-cl

[Paolo Bonzini]

On Thu, Nov 21, 2024 at 10:43 PM Erwin Jansen <jansene@google.com> wrote:
>
> Would a good next step be to work out some more details in a document that outlines what process we are using, what we are planning to do and include a set of suggestions as a starting point to see if we can upstream some of the changes to the larger community?

Yes, you can write it at https://wiki.qemu.org/Features/ClangCL. I'll
send you credentials off list.

Paolo

> We currently don't have a proper solution for packing. We are tracking the issue here https://issuetracker.google.com/issues/380295845. All our code is public and development is happening on the emu-dev repository: https://android.googlesource.com/platform/external/qemu/+/emu-dev
>
> Greetings,
> Erwin.
>
>
>
>
>
> On Thu, Nov 21, 2024 at 10:47 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> > NB As a general point, we actively block use of clang with Windows
>> > builds (more strictly in 9.2 now), because it lacks support for the
>> > 'gcc_struct' annotation that we rely on to guarantee correct ABI for
>> > structs exposed to guests in particular.
>>
>> Ah, good point. This is
>> https://github.com/llvm/llvm-project/issues/24757 for the general
>> tracking issue, and https://github.com/llvm/llvm-project/pull/71148
>> for a recent PR that attempts to implement this.
>>
>> Using -mno-ms-bitfields globally is unsafe because there are probably
>> Windows API structs that implement it.
>>
>> One solution is to add `QEMU_BUILD_BUG_ON(sizeof(...) == ...)` to all
>> structs in QEMU that use bitfields. That will prove very quickly if
>> there are issues or not.
>>
>> Paolo
>>
>> > Many people try to simply remove that #ifdef, mistakenly assuming that
>> > because the code compiles without warnings, it must be correct. Did
>> > you have solution for this, as it would be a blocker for enabling
>> > clang on Windows currently ?
>> >
>> > With regards,
>> > Daniel
>> > --
>> > |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>> > |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>> > |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>> >
>>

Re: QEMU patches for native windows support through clang-cl

[Pierrick Bouvier]

Hi,

On 11/21/24 13:42, Erwin Jansen wrote:
> Would a good next step be to work out some more details in a document 
> that outlines what process we are using, what we are planning to do and 
> include a set of suggestions as a starting point to see if we can 
> upstream some of the changes to the larger community?
> 
> We currently don't have a proper solution for packing. We are tracking 
> the issue here https://issuetracker.google.com/issues/380295845 
> <https://issuetracker.google.com/issues/380295845>. All our code is 
> public and development is happening on the emu-dev repository: https:// 
> android.googlesource.com/platform/external/qemu/+/emu-dev <https:// 
> android.googlesource.com/platform/external/qemu/+/emu-dev>
> 

Regarding the gcc_struct/clang issue, I've been checking at the full 
impact to use -mno-ms-bitfields as a replacement [1]. And from the 
difference observed, it seems to be a better default choice than using 
selectively gcc_struct for packed structs.

Feedback is welcome [1], and enabling this would allow to support 
clang/clang-cl, and windows-arm64 (where gcc is not supported yet) as a 
side effect.

Regards,
Pierrick

[1] 
https://lore.kernel.org/qemu-devel/66c346de-7e20-4831-b3eb-1cda83240af9@linaro.org/T/#u

> Greetings,
> Erwin.
> 
> 
> 
> 
> 
> On Thu, Nov 21, 2024 at 10:47 AM Paolo Bonzini <pbonzini@redhat.com 
> <mailto:pbonzini@redhat.com>> wrote:
> 
>      > NB As a general point, we actively block use of clang with Windows
>      > builds (more strictly in 9.2 now), because it lacks support for the
>      > 'gcc_struct' annotation that we rely on to guarantee correct ABI for
>      > structs exposed to guests in particular.
> 
>     Ah, good point. This is
>     https://github.com/llvm/llvm-project/issues/24757 <https://
>     github.com/llvm/llvm-project/issues/24757> for the general
>     tracking issue, and https://github.com/llvm/llvm-project/pull/71148
>     <https://github.com/llvm/llvm-project/pull/71148>
>     for a recent PR that attempts to implement this.
> 
>     Using -mno-ms-bitfields globally is unsafe because there are probably
>     Windows API structs that implement it.
> 
>     One solution is to add `QEMU_BUILD_BUG_ON(sizeof(...) == ...)` to all
>     structs in QEMU that use bitfields. That will prove very quickly if
>     there are issues or not.
> 
>     Paolo
> 
>      > Many people try to simply remove that #ifdef, mistakenly assuming
>     that
>      > because the code compiles without warnings, it must be correct. Did
>      > you have solution for this, as it would be a blocker for enabling
>      > clang on Windows currently ?
>      >
>      > With regards,
>      > Daniel
>      > --
>      > |: https://berrange.com <https://berrange.com>      -o- https://
>     www.flickr.com/photos/dberrange <https://www.flickr.com/photos/
>     dberrange> :|
>      > |: https://libvirt.org <https://libvirt.org>         -o- https://
>     fstop138.berrange.com <https://fstop138.berrange.com> :|
>      > |: https://entangle-photo.org <https://entangle-photo.org>    -o-
>     https://www.instagram.com/dberrange <https://www.instagram.com/
>     dberrange> :|
>      >
>