Saancreed / wine-nvml

NVIDIA Management Library wrapper for Wine
GNU Lesser General Public License v2.1
25 stars 5 forks source link

nvml - Upstream or proton/GE/TkG? #5

Open SveSop opened 3 years ago

SveSop commented 3 years ago

First off, it is a very easy implementation to compile this together with staging (or with a quick rebase regular wine). I implemented this in my custom wine-tree like this: https://github.com/SveSop/wine/commit/c8e036099c3e4493a8a565660cd813d951988446 https://github.com/SveSop/wine/commit/f74559c8fc66da52bb5610a6207d5304d1d5b951 And that compiles together with wine and seems to work as it should.

However, i have a nagging feeling about the nvml header from nVidia. Would this be a problem given the copyright notice there?

  • This source code is subject to NVIDIA ownership rights under U.S. and
  • international Copyright laws. Users and possessors of this source code
  • are hereby granted a nonexclusive, royalty-free license to use this code
  • in individual and commercial software.

On first glance, i would say it is a slam-dunk home-free kind of thing, but i do not really know that much about "copyright law" (other than it READS like its a free for all).

This brings me to the second thought i have - Will the WineHQ team accept this header as-is? Or (as i might suspect) will they NOT do it cos it's "not their property"? Thoughts on this?

Third note: I know (if going for upstream wine) the WineHQ team is a stickler for tests. I would think there HAS to be some sort of tests written for this to be implemented upstream.

Final note: Posting this on TkG's repo https://github.com/Frogging-Family/community-patches/tree/master/wine-tkg-git is a easy way to maybe get this to be used? If used - it gets noticed - and thus more wind in the sails.

Saancreed commented 3 years ago

It's more or less what I intended to do (and then send it for inclusion to Staging) but I found myself quite busy lately. I think what we already have here is good enough baseline to be useful so perhaps it's high time to send this and at the very least get some initial feedback.

Fwiw I also have a branch that removes the need for Nvidia's header; it's not exactly up to date but seems to work fine and has one cool benefit: it allows me to stub out all remaining functions so they return NVML_ERROR_FUNCTION_NOT_FOUND instead of not being there or crashing the whole application when called (but that's based on my assumption that used calling conventions allow me to ignore parameters passed to each function, otherwise I'd be in great need for some automation tools for this stuff).

But on the other hand, maybe there's no need for it: perhaps the license is simply compatible with LGPLv2 (IANAL so I can't tell) and if not, I recall @liam-middlebrook saying on Discord a few months ago that he'd be glad to forward my questions and/or requests wrt licensing to their NVML team.

I'm not sure yet what to do with tests as their results would rely on host maching having Nvidia's driver installed. Perhaps Wine people have an idea what can be done here.

Finally, if you don't mind doing this stuff yourself, you have my full blessing to send it to WineHQ as a bug that NVML is not supported with patchset attached for inclusion to Staging. If you do, I'll join the discussion and maybe we will be able to work out some solution that will satisfy everyone involved (though it would be nice if we had some known Windows applications that make direct use of NVML as examples we could fix instead of just DXVK-NVAPI).

SveSop commented 3 years ago

I looked a bit at your other branch, but i think if that was to be used it would perhaps need something in configure to check for the header, and set the equivalent of -DUSE_NVIDIA_HEADER for winegcc. Not sure how to do that tho. I think it looks a lot tidier to actually have the @ stub in the .spec file, rather than re-declare them in the source. How do you handle functions that currently do not exist?

I dont know how to start with the tests, but i agree it would be rather moot to run a LOT of function tests if the machine does not have a physical adapter. I see there is written multiple tests for the nvcuda lib in staging, but that would also depend on libcuda.so being present at test-time? (Something not available without an nvidia adapter either). Have to be discussed further i guess.

Maybe i could post a RFC on the WineHQ mailing list asking about this package including questions about header/copyright? Are you signed up on that? https://www.winehq.org/mailman/listinfo/wine-devel

PS. Quite a bit of traffic on that mailinglist tho, so i would not recommending dropping incoming directly in your inbox...

Saancreed commented 3 years ago

I looked a bit at your other branch, but i think if that was to be used it would perhaps need something in configure to check for the header, and set the equivalent of -DUSE_NVIDIA_HEADER for winegcc. Not sure how to do that tho.

I don't think this should be checking if the header exists. I believe the way to go would be to either remove the header and never use it even if present or include it and expect it to always be there.

I think it looks a lot tidier to actually have the @ stub in the .spec file, rather than re-declare them in the source. How do you handle functions that currently do not exist?

Well, if they are commented out, they effectively don't exist, GetProcAddress would return NULL and programs linked against nvml.dll with those symbols would fail to load. Leaving them as @ stub should make them visible but crash the application when called; I think I saw some Wine DLLs having functions only in the spec file which would result in having them generated at compile time, but when building my library with Meson, winegcc complained that they don't exist. Most likely I'm missing some Wine secret sauce here. Having them as @ cdecl and manually "implemented" as stubs in nvml.c allows me to correctly handle all unimplemented functions without crashing calling application, but this doesn't exactly work nicely with NVML header because my macro is too dumb to correctly replicate full function signature.

I dont know how to start with the tests, but i agree it would be rather moot to run a LOT of function tests if the machine does not have a physical adapter. I see there is written multiple tests for the nvcuda lib in staging, but that would also depend on libcuda.so being present at test-time? (Something not available without an nvidia adapter either). Have to be discussed further i guess.

Yup.

Maybe i could post a RFC on the WineHQ mailing list asking about this package including questions about header/copyright? Are you signed up on that? https://www.winehq.org/mailman/listinfo/wine-devel

I am now :slightly_smiling_face:

jp7677 commented 3 years ago

As an intermediate step, would be cool if you could provide a release including a binary that can be easily dropped into Proton (I mean 6.3. based Protons).

SveSop commented 3 years ago

Just for reference to a combined discussion: https://github.com/jp7677/dxvk-nvapi/issues/27#issuecomment-890334455

Just find the argument of implementing nvml upstream loosing a bit of footing from Zeb's comment there...

Saancreed commented 3 years ago

@jp7677 Done, see https://github.com/Saancreed/wine-nvml/releases/tag/v0.1

I'd be grateful if you guys checked on your machines if attached binary release correctly works with Proton 6.3 and/or Experimental with runtime enabled.

And what about nvml then? So far the only real usage is with nvapi, as there is no real point in running nvidia-smi.exe like tools under wine, when we can (and should for safety) use that directly in Linux.

nvidia-smi.exe won't be running anytime soon because afaict it relies on nvmlInternalGetExportTable working correctly.

Just find the argument of implementing nvml upstream loosing a bit of footing from Zeb's comment there...

Yeah, that's unfortunate, but it also doesn't mean that nvcuda will be thrown out of staging soon, or that nvml will automatically be rejected for similar reasons. There's still a discussion to be had.

SveSop commented 3 years ago

@jp7677 Done, see https://github.com/Saancreed/wine-nvml/releases/tag/v0.1

I'd be grateful if you guys checked on your machines if attached binary release correctly works with Proton 6.3 and/or Experimental with runtime enabled.

Tested with Proton-6.13-GE-1 on Batman Arkham Knight: From dxvk-nvapi log:

DXVK-NVAPI v0.3-108-g1ffd573 (BatmanAK.exe)
NVML loaded and initialized successfully

From steam log:

0114:trace:nvml:DllMain (0x7f346c400000, 1, (nil))
0114:trace:nvml:nvmlInit_v2 ()
0114:trace:nvml:nvmlDeviceGetHandleByPciBusId_v2 (00000000:01:00.0, 0x11eb90)

So it does seem as it is working. Proton-6.3 does not currently use DXVK-NVAPI, so you have to use proton-experimental. Or, as i did - Proton-6.13-GE-1.

Just find the argument of implementing nvml upstream loosing a bit of footing from Zeb's comment there...

Yeah, that's unfortunate, but it also doesn't mean that nvcuda will be thrown out of staging soon, or that nvml will automatically be rejected for similar reasons. There's still a discussion to be had.

I have posted a comment on that bugpost to Zeb with the list of PhysX games, so ill see what she sais. If she does not think the list of games using PhysX is warranted enough for wine to keep using nvapi/nvcuda, then i am afraid i kinda see little use for nvml when it comes to upstream. Ill see if i can manage to articulate a RFC on the devel mailing list and see what the thoughts are.

SveSop commented 3 years ago

https://bugs.winehq.org/show_bug.cgi?id=51541#c6

I guess I should be fair, it's not like there's a generic alternative to PhysX. But we don't really like to add vendor-specific libraries, for obvious reasons.

I know they have that stance when it comes to vendor specific things. What does that leave for nvml?

I'll probably try to push nvcuda upstream anyway.

One might interpret this as a kind of "reluctant reply". Is now a bad time to suggest "yet another vendor specific library" like nvml? I mean - "Ah well.. ill probably try anyway" and we ask "How about nvml?" Might that end up with a response in the lines of: "Jeez.. no, ENOUGH IS ENOUGH!"

What are your thoughts? Current argument for nvml at this point would ONLY be for use by nvapi (dxvk-nvapi currently, although could be used for staging nvapi). And on the previous post from Zeb about using the .so.1 libname she wants to check for WINE_CHECK_SONAME to determine this. I fear that without this check, it might not fly for submitting an upstream patch currently. (Although, granted the staging team could write this aswell if it gets included in staging).

Arf.. i really hate this. I am not good at "doing it right" in the terms of things like this, and i have a hard time making a solid argument atm.

Saancreed commented 3 years ago

I know they have that stance when it comes to vendor specific things. What does that leave for nvml?

Sadness, I suppose? But even if upstream is not interested in this, maybe Valve will be.

What are your thoughts? Current argument for nvml at this point would ONLY be for use by nvapi (dxvk-nvapi currently, although could be used for staging nvapi). And on the previous post from Zeb about using the .so.1 libname she wants to check for WINE_CHECK_SONAME to determine this. I fear that without this check, it might not fly for submitting an upstream patch currently. (Although, granted the staging team could write this aswell if it gets included in staging).

If I understand this correctly, this would require libnvidia-ml.so to be available at build time, which would make building Wine quite a bit more annoying for distro package maintainers. I'd prefer if we kept just .so.1, perhaps with a fallback to .so in case the soname ever changes, but that would need a fix to continue working in Proton :/

On the other hand, I expect Nvidia to be very careful about keeping this stuff compatible, so probably we don't have to worry about this.

Arf.. i really hate this. I am not good at "doing it right" in the terms of things like this, and i have a hard time making a solid argument atm.

Yeah, well, I was afraid that would be the case. There's not much to be gained if there aren't any Windows apps that use NVML.

SveSop commented 3 years ago

If I understand this correctly, this would require libnvidia-ml.so to be available at build time, which would make building Wine quite a bit more annoying for distro package maintainers. I'd prefer if we kept just .so.1, perhaps with a fallback to .so in case the soname ever changes, but that would need a fix to continue working in Proton :/

On the other hand, I expect Nvidia to be very careful about keeping this stuff compatible, so probably we don't have to worry about this.

Well.. if DXVK-NVAPI was built as a winelib, libnvml.so.1 could have just been loaded directly with the nvml functions included in the DXVK-NVAPI source, and there would be no worry about it i guess, since they (Valve) already use it for Proton. No fuzz, no worries...

Lets assume games that uses nvcuda for PhysX setup needs nvapi and nvcuda. Currently nvcuda is built as winelib cos it loads libcuda.so.1. How to get around that, i do not know. I know Wine is all about converting everything to PE libs these days, but i think it might be problematic to get around nVidia closed source libraries (libcuda) (maybe one of the reasons winedev team is not too keen on implementing vendor specific libs in the first place).

Is it too much of a "step-back" to build DXVK-NVAPI as a winelib, so it can load libnvml.so.1 directly? Not saying that is what should be done, but it would solve a lot of problems...

Yeah, well, I was afraid that would be the case. There's not much to be gained if there aren't any Windows apps that use NVML.

There are probably some monitoring software, and i think i found some rendering software (Arnold) that used nvml... however, that is also available directly for Linux, so there is no real reason to HAVE to run this under wine. Same with some bitcoin mining software. I see no reason to run mining software under wine (with possible additional performance penalty) when you can get Linux version of the software. Some are not available on Linux, and already have a bug report: https://honeyminer.com/ Seems to only be available on Windows, and got a bug here: https://bugs.winehq.org/show_bug.cgi?id=45792 3 year old bug, with no developer replies.

Another example: https://www.topazlabs.com/video-enhance-ai relevant bug: https://bugs.winehq.org/show_bug.cgi?id=49026

That was the 2 bugs i could dig up.

The Topazlabs one have a trial/demo thing available. Maybe i should try and see if it works when using nvml.

jp7677 commented 3 years ago

@Saancreed thanks a lot for the release, having a binary that people can just drop into Proton makes it a lot more accessible imho.

@SveSop lets wait a bit how things turn out (it’s weekend for Liam and Zeb) and how the discussion for the .1 naming change continues. Similar like nvml, I think it would be cool to have a wine-nvcuda binary ready that can be easily dropped into Proton. Having this upstreamed would be the coolest way to go (like nvml), but having stand-alone repos (which would be the alternative for nvcuda) like this project or your nvapi-stand-alone isn’t that bad either as long as it is simple to get things running in Proton. (Well, I guess you figured I care mostly for Proton :))

SveSop commented 3 years ago

I mostly care about Proton and custom wine anyway, since i have a somewhat massively patched wine i run for myself :+1:

So for ME personally, i would be very happy having a single package "nVidia libs" (or whatnot) with nvapi, nvcuda, nvcuvid, nvencodeapi and nvml - with a script similar to that of dxvk where you just type WINEPREFIX=/my/wine/prefix ./setup.sh install and Bob's your uncle. It is not too hard to write a similar script for putting the binaries in the Proton archive either, but it would probably mean doing a refresh of that every new Proton release.

That said, i'll keep an eye on the discussion from Zeb/Liam and see what turns up.

SveSop commented 3 years ago

https://github.com/SveSop/nvidia-libs.git

Play around with it a bit. Builds the bare-bones libraries for nvapi/nvcuda/nvencodeapi/nvcuvid/nvml and puts them in the "new" library structure used by current wine.

Ended up just building nvml from subdir instead of making it a submodule (since it would have cloned the entirety of wine). Minor change to dxvk-nvapi meson settings to get the directory structure. Can probably manage to implement what Saancreed did with the "old-structure" setting thing tho, if it seems better?

Hmm.. maybe i would not have to pull all submodules from nvml repo.. some .gitignore stuff or something? Rather not want to clone wine just for a few headers, although it would probably be the right thing to do. Lets discuss that maybe.

Point is: Its a collection of the libs we are interested in :+1:

Saancreed commented 3 years ago

https://github.com/SveSop/nvidia-libs.git

Looks cool and seems like a good way to keep these modules alive if Staging drops related patchsets. I'll take a closer look sometime soon.

Ended up just building nvml from subdir instead of making it a submodule (since it would have cloned the entirety of wine).

Yeah, it's fine. I was hoping that headers we need there would be shipped with binary Wine packages, but apparently that's not the case. So I applied a bit of my laziness and just included the entire Wine repo as a submodule (same with nvidia-settings really but at least that one is much smaller).

Can probably manage to implement what Saancreed did with the "old-structure" setting thing tho, if it seems better?

Up to you, I included that setting only because the current Proton is still based on Wine 6.3 which uses old directory layout. Pretty much the same can be achieved just by moving the files manually.

Hmm.. maybe i would not have to pull all submodules from nvml repo.. some .gitignore stuff or something? Rather not want to clone wine just for a few headers, although it would probably be the right thing to do. Lets discuss that maybe.

You can always update submodules without doing so recursively, doing sparse checkout will be quite a bit more problematic.

SveSop commented 3 years ago

Added an installscript for installing symlinks in a WINEPREFIX

Going to be some head-twisting to make a script for Proton-drop-in-replacement binaries... Will think about it some. But those with interest can do it easily anyway... and IF someone (ex. GloriousEggroll) were to pick up on it, it would be scripted anyway...

EDIT: Fun part being that wine uses 1 setup of folders - /lib/wine/x86_64-windows and so on, aswell as /lib/wine/i386-windows (everything under /lib/wine) Proton-Experimental uses /lib/wine and /lib64/wine + /lib/wine/fakedlls and lib64/wine/fakedlls Proton-GE uses /lib/wine/i386-windows and so on + /lib64/wine/x86_64-windows and its like.

So, basically there is 3 different folder structures... for SOME reason (even tho Proton-GE is built on wine-staging-6.14 currently)

SveSop commented 3 years ago

Added another script that will take the binaries from the package and drop them in to Proton Experimental or Proton - GE.

I tested Batman Arkham Knight with this, creating a new prefix with Proton - Experimental, and after adding nvidia libs + installing PhysX-9.19 i got PhysX to work.

https://github.com/SveSop/nvidia-libs/blob/master/proton_setup.sh

Probably a million ways to solve the scripting tho.. It is a bit tricky due to Proton Experimental and Proton GE using different filestructures.

Ill see if i can make a "release" binary blob later tonight.