Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use wine-9 headers #17

Merged
merged 1 commit into from
Jan 24, 2024
Merged

Use wine-9 headers #17

merged 1 commit into from
Jan 24, 2024

Conversation

SveSop
Copy link
Contributor

@SveSop SveSop commented Jan 18, 2024

This updates headers for compilation with wine 9.0

Uncertain if actually hiding

__wine_unixlib_handle
__wine_unix_call_dispatcher
__wine_init_unix_call

manually should be done with something like __attribute((visibility("hidden"))) ?

@Saancreed
Copy link
Owner

Although I haven't dived into what exactly happened to these headers for a while now and what are the consequences of the changes, staying in sync with upstream Wine headers is probably desirable. Two questions though:

  1. Are resulting binaries still compatible with older versions of Wine/Proton? I'd be alright with dropping support for 7.0 but 8.0 should keep working until at least Proton 9 is released. If some older Wine no longer works, README should be updated as well.
  2. Is the visibility of these __wine_… symbols any different from upstream libraries that are also split into PE/unixlibs? If yes, then I believe ensuring nvml.h's DECLDIR macro expands to __declspec(dllexport) (by defining NVML_LIB_EXPORT) should take care of causing only NVML API entry points to be exported.

@SveSop
Copy link
Contributor Author

SveSop commented Jan 22, 2024

  1. There is no problem running the resulting binaries on wine-staging-8.20 and 8.19, so i would not think there would be issues with older wine-8 versions either, but as you said i guess wine-7 will not work due to the rework of this "helper" PE/ELF in wine-8+.
    I could make a test of wine-7 if you think it would be useful? (Although few would use wine-7 i guess)
  2. I checked the winedump of the new nvml.dll binary vs. older one before this change and found no differences, and i also checked with nm the dump of the nvml.so file and there was no difference. I have not checked other headers for wine-9, but it could be that these "internal" functions are hidden in some other manner. So from simple checks it actually does not seem to impact anything "interesting".

I guess the __wine_unix_call_funcs is what is taking care of the actual ELF calls "internally"? Other PE/ELF libs in wine like bcrypt only has the __wine_unix_call_wow64_funcs in addition, aswell as some __wine_dbg_ functions that i guess is some additional debugging symbols for particular libraries.

@Saancreed
Copy link
Owner

  1. There is no problem running the resulting binaries on wine-staging-8.20 and 8.19, so i would not think there would be issues with older wine-8 versions either, but as you said i guess wine-7 will not work due to the rework of this "helper" PE/ELF in wine-8+.
    I could make a test of wine-7 if you think it would be useful? (Although few would use wine-7 i guess)

Yeah, I think it's worth checking at least, but Wine/Proton 8.0 is imho more important than Wine/Proton 7.0. If the former works then it's no big deal. If the latter does not work, it's also fine but please also update the README to indicate that 8.0 is now the oldest supported version.

  1. I checked the winedump of the new nvml.dll binary vs. older one before this change and found no differences, and i also checked with nm the dump of the nvml.so file and there was no difference. I have not checked other headers for wine-9, but it could be that these "internal" functions are hidden in some other manner. So from simple checks it actually does not seem to impact anything "interesting".

Alright.

I guess the __wine_unix_call_funcs is what is taking care of the actual ELF calls "internally"? Other PE/ELF libs in wine like bcrypt only has the __wine_unix_call_wow64_funcs in addition, …

Yes, and existence of the latter is used to declare support for new experimental wow64 mode which we don't support so it's fine.

aswell as some __wine_dbg_ functions that i guess is some additional debugging symbols for particular libraries.

These symbols are externally defined anyway, they would show up in dumps of nvml.so if we did any more fancy logging. As an example, my winscard.so doesn't have them either.

So, to sum up, please make sure that it still works with Proton 8.0 and update the README if it no longer works with Wine 7.0 (I'd expect it to be broken for everything older than 7.22, and probably even before this change).

This updates headers for compilation with wine 9.0
@SveSop
Copy link
Contributor Author

SveSop commented Jan 23, 2024

Tested with wine-staging-7.12 that i had installed, and nvml does not seem to work there:
Loading nvml.dll failed with error code: 1114
I assume this is from dxvk-nvapi trying to load and initialize nvml lib.

Tested with GE-Proton-8.27 and running Batman AK i get this in the logs:

0128:trace:nvml:DllMain (00000000129F0000, 1, 0000000000000000)
0128:trace:nvml:nvmlInit_v2 ()
0128:trace:nvml:nvmlDeviceGetHandleByPciBusId_v2 (00000000:01:00.0, 000000000011EEA0)

And a successful shutdown upon exit:

0128:trace:nvml:DllMain (00000000129F0000, 0, 0000000000000001)
0128:trace:nvml:nvmlShutdown ()

Not very much uses the things NVML provides when gaming under steam/proton, but atleast it does seem as it is working proton-8 although this is the GE custom variant. I do not think it would fail using proton-next or whatnot that is newest valve provided version either tho.

If it works with wine-7.22 that was released kinda right before 8.0-rcX versions anyway, so it would be (imo) be pointless to report anything other than "Needs Wine-8 or newer".

Pushed change with update to README.md that hopefully covers this.

@Saancreed
Copy link
Owner

Yup, all good now, thank you.

@Saancreed Saancreed merged commit 009ee4c into Saancreed:master Jan 24, 2024
2 checks passed
@SveSop SveSop deleted the wine9 branch January 24, 2024 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants