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

Alpha 11 breaks compatibility with my Windows 10 guest #76

Closed
Djhg2000 opened this issue Jun 3, 2018 · 7 comments
Closed

Alpha 11 breaks compatibility with my Windows 10 guest #76

Djhg2000 opened this issue Jun 3, 2018 · 7 comments

Comments

@Djhg2000
Copy link

Djhg2000 commented Jun 3, 2018

Host CPU: AMD Ryzen 2400G
Host GPU: AMD Ryzen 2400G
Guest GPU: Sapphire Radeon RX 580
Host Kernel version: 4.17-rc7
Host QEMU version: version 2.12.0 (Debian 1:2.12+dfsg-3)

LG host behaves like the SSE issue is described, but SSE seems to be enabled (PCSX2 works with SSE2 enhancements enabled). Is there any more reliable way to test if SSE is the issue rather than just "LG silently crashes"? Guest is running on core2duo vcpus for compatibility reasons, I might be able to set up a test machine with host-passthrough vcpus later today.

I managed to capture a screenshot just before the crash here.

Alpha 10 works flawlessly and performs well at 1080p, if this turns out to be SSE related is there any chance we can get back the generic memcpy() implementation behind a compile flag? I don't have a Windows compatible build environment and I doubt I'll have time to set it up until next weekend.

No looking-glass-host.dmp file is created which I presume means a hard crash.

@Lessaj
Copy link

Lessaj commented Jun 3, 2018

core2duo is not compatible, it does not have the extensions needed. You will need to manually add them if you want to continue to use that model but that may break your compatibility with other applications. In my case, I was using core2duo because otherwise Asus GPU Tweak would cause a BSOD, I was using that to control the external fan headers on my card but I could not come up with an extension list that allowed both the LG host and Asus GPU Tweak to work without a BSOD, I ended up using host-passthrough and bought a y-splitter to control both fans from one header on the mobo.

You can add features under the section like this. I kind of threw away the notes I made when I was trying this all out but I think these are the policies you need to add, you can see all of them in /usr/share/libvirt/cpu_map.xml.

  <feature policy='require' name='avx'/>
  <feature policy='require' name='x2apic'/>
  <feature policy='require' name='xgetbv1'/>
  <feature policy='require' name='xsave'/>
  <feature policy='require' name='xsavec'/>
  <feature policy='require' name='xsaveopt'/>

The error BTW is "illegal instruction", I ran the host code in debug mode in Visual Studio to find the instruction set(s) that allowed it to continue running without that error.

@Djhg2000
Copy link
Author

Djhg2000 commented Jun 3, 2018

@Lessaj Thank you so much, after a few rounds through the Windows startup repair (which always reported it failed to find a problem) it booted just fine with the added CPU features.

My cpu config block looks like this now:

  <cpu mode='custom' match='exact' check='partial'>
    <model fallback='allow'>core2duo</model>
    <topology sockets='1' cores='3' threads='2'/>
    <feature policy='require' name='avx'/>
    <feature policy='require' name='x2apic'/>
    <feature policy='require' name='xgetbv1'/>
    <feature policy='require' name='xsave'/>
    <feature policy='require' name='xsavec'/>
    <feature policy='require' name='xsaveopt'/>
  </cpu>

Unfortunately from the look of things this might also break compatibility with Sandy Bridge. I don't think we have too many users besides me who might be trying to use LG on SB yet but it's something worth considering. That said my SB system isn't running any gaming VMs anymore since the GPU I had in there was killed by a particularly evil DP adapter (always verify the shield is actually connected at both ends with a multimeter), so I might finally have to track down the bug that makes it hard to install Windows 10 without emulating a core2duo.

@namidairo
Copy link

Alpha 10 works flawlessly and performs well at 1080p, if this turns out to be SSE related is there any chance we can get back the generic memcpy() implementation behind a compile flag?

Yes I believe you can ifdef out, but you can just as easily do it at runtime by checking cpuid then AND'ing the bit for SSE2 support, so that people don't have to pick different binaries to download.

@Djhg2000
Copy link
Author

Djhg2000 commented Jun 4, 2018

@namidairo It's been a while since I did a lot of C coding, but wouldn't a variable compare at every memory access negate some of the performance gains we're seeing from having switched to handwritten assembly? Or did you mean we would pick a different main loop depending on the SSE2 flag?

Looking through the memcpySSE.asm code I don't see why we need any extra instructions besides AVX on core2duo, but I've never written assembly so I might be missing something. If I had a Windows build environment I'd investigate it myself.

@alama
Copy link
Contributor

alama commented Jun 4, 2018

Why not just make a pointer void *(*mem_cpy)(void*,void*,size_t*), and by default, it points a wrapper for C memcpy() and during startup, points to the SSE2 version if the CPU have support for it?

@gnif
Copy link
Owner

gnif commented Jun 5, 2018

After months of testing, evaluation and reverse engineering we opted to switch to assembler for memcpy due to the poor performance of the default Microsoft provided memcpy. This problem of incompatibility has already been pointed out and a future update will include CPU detection before assuming it is safe to use this method.

@gnif
Copy link
Owner

gnif commented Jul 19, 2018

This issuse has been resolved by switching to SSE2 instructions over AVX instructions. The use of AVX instructions were in error.

@gnif gnif closed this as completed Jul 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants