Hi!
I am using this card for about a year and I would like first to say thanks for open source driver that you made for it, for the big navi and for the threadripper which brought back fun to the computing.
I bought that card primary to use as a host GPU in VFIO enabled multi-seat system I am building, and recently I was able (with a minor issue I managed to solve, more about it later) to pass that GPU to both linux and windows guest mostly flawlessly.
I do have experience in kernel development, and debugging so I am willing to test patches, etc. Any help is welcome!
So these are the issues:
1.(the biggest issue): The amdgpu driver often crashes when plugging an input.
I tested this now on purpose with 'amdgpu.dc=1' by slowly plugging and unplugging an input connector while I wait for the output to stabilize between each cycle, and still the issue reproduced after a dozen (or so) tries. (It only happens when I plug the connector, and never happens when I unplug it)
Then I unloaded the amdgpu driver and loaded it again with dc=0. This does sort of work but takes a lot of time. The dmesg output is attached (amdgpu_dc1_plug_bug.txt)
I did try to increase the number of tries in dm_helpers_read_local_edid, to something silly like 1000, but no luck.
I also tried to remove the code below the 'Abort detection for non-DP connectors if we have no EDID' Also no luck.
This bug pretty much makes it impossible to use the card daily as is since I do connect/disconnect monitors often, especially due to VFIO usage.
2. I found out that running without the new DC framework (amdgpu.dc=0) solves issue 1 completely (but costs HDMI sound - HDMI sound only works with amdgpu.dc=1)
I am using this card like that for about at least half an year and haven't had a single connector plug/unplug related crash.
Issue 2 however is that in this mode (I haven't tried to reproduce this with amdgpu.dc=1 yet), sometimes when I unbind the amdgpu driver the amdgpu complains about a leaked connector and crashes a bit later on. I haven't yet tracked the combination of things needed to trigger this, but it did happen to me about 3 times already.
I did put a WARN_ON(1) to __drm_connector_put_safe, to see who is the caller that triggers the delayed work that frees the connector when it is too late.
I attached a backtrace with the above WARN_ON and the crash (connector_leak_bug.txt) I also attached the script 'amdgpu_unbind' for the reference that I use to unbind the amdgpu driver.
3. When doing VFIO passthrough of this card, I found out that it doesn't suffer that much from the reset bug. As long as I shut down the guest in clean manner, I can start it again). The vendor_reset module however makes the reset work even when I shut down the guest right in the middle of a 3D app running and I tested it many times.
_However_ this only works if I never load the amdgpu linux driver. Otherwise a windows guest still boots but all 3D apps in it crash very early.
I tried both the stock drivers that windows auto installs and latest AMD workstation drivers from AMD site.
Linux guests do work.
I found out that amdgpu driver resizes the device bars (I have TRX40 platform, so I don't know if this platform supports the AMD Smart Memory or not, but according to lspci the device does support resizable BARs).
If I patch the amdgpu's bar resize out, then, the windows guest _does_ work regardless if I loaded amdgpu prior or not. Linux guests also still work. I haven't measured the performance impact of this.
For debugging this, I did try to hide the PCI_EXT_CAP_ID_REBAR capability from the VM, but it made no difference.
I suspect that once the GPU is resetted, the bars revert to their original sizes, but VFIO uses the sizes that are cached by the kernel, so that the guest thinks that the bars are of one size while they are of an another. I don't have an idea though why this does work with a Linux guest.
I had attached the pci config with amdgpu running, once with my patch that stops it from resizing the bars, and once without that patch for reference. (amdgpu_pciconfig_noresize.txt, amdgpu_pciconfig_resize.txt)
4. I found out that amdgpu runtime PM sometimes breaks the card if last output is disconnected from it. I didn't debug it much as I just disabled it with amdgpu.runpm=0) I will do more debug on this later.
Please let me know if you have any questions, Don't hesitate to ask me for more information.
My setup: 3 outputs, all HDMI, converted with DP->HDMI adapters, of which 2 are 1080P monitors, and 1 is a 1080P TV. The issues I describe above are reproducible on all the outputs.
I am running 5.10.0 kernel with few patches and kvm-queue branch merged for my day to day work on KVM.
You can find the exact kernel I use and its .config on https://gitlab.com/maximlevitsky/linux/-/commits/kernel-starship-5.10
Best regards, Maxim Levitsky
Hi Maxim,
I can't help with the display related stuff. Probably best approach to get this fixes would be to open up a bug tracker for this on FDO.
But I'm the one who implemented the resizeable BAR support and your analysis of the problem sounds about correct to me.
The reason why this works on Linux is most likely because we restore the BAR size on resume (and maybe during initial boot as well).
See this patch for reference:
commit d3252ace0bc652a1a244455556b6a549f969bf99 Author: Christian König ckoenig.leichtzumerken@gmail.com Date: Fri Jun 29 19:54:55 2018 -0500
PCI: Restore resized BAR state on resume
Resize BARs after resume to the expected size again.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199959 Fixes: d6895ad39f3b ("drm/amdgpu: resize VRAM BAR for CPU access v6") Fixes: 276b738deb5b ("PCI: Add resizable BAR infrastructure") Signed-off-by: Christian König christian.koenig@amd.com Signed-off-by: Bjorn Helgaas bhelgaas@google.com CC: stable@vger.kernel.org # v4.15+
It should be trivial to add this to the reset module as well. Most likely even completely vendor independent since I'm not sure what a bus reset will do to this configuration and restoring it all the time should be the most defensive approach.
Let me know if you got any more questions on this.
Regards, Christian.
Am 02.01.21 um 23:42 schrieb Maxim Levitsky:
Hi!
I am using this card for about a year and I would like first to say thanks for open source driver that you made for it, for the big navi and for the threadripper which brought back fun to the computing.
I bought that card primary to use as a host GPU in VFIO enabled multi-seat system I am building, and recently I was able (with a minor issue I managed to solve, more about it later) to pass that GPU to both linux and windows guest mostly flawlessly.
I do have experience in kernel development, and debugging so I am willing to test patches, etc. Any help is welcome!
So these are the issues:
1.(the biggest issue): The amdgpu driver often crashes when plugging an input.
I tested this now on purpose with 'amdgpu.dc=1' by slowly plugging and unplugging an input connector while I wait for the output to stabilize between each cycle, and still the issue reproduced after a dozen (or so) tries. (It only happens when I plug the connector, and never happens when I unplug it)
Then I unloaded the amdgpu driver and loaded it again with dc=0. This does sort of work but takes a lot of time. The dmesg output is attached (amdgpu_dc1_plug_bug.txt)
I did try to increase the number of tries in dm_helpers_read_local_edid, to something silly like 1000, but no luck.
I also tried to remove the code below the 'Abort detection for non-DP connectors if we have no EDID' Also no luck.
This bug pretty much makes it impossible to use the card daily as is since I do connect/disconnect monitors often, especially due to VFIO usage.
- I found out that running without the new DC framework (amdgpu.dc=0) solves
issue 1 completely (but costs HDMI sound - HDMI sound only works with amdgpu.dc=1)
I am using this card like that for about at least half an year and haven't had a single connector plug/unplug related crash.
Issue 2 however is that in this mode (I haven't tried to reproduce this with amdgpu.dc=1 yet), sometimes when I unbind the amdgpu driver the amdgpu complains about a leaked connector and crashes a bit later on. I haven't yet tracked the combination of things needed to trigger this, but it did happen to me about 3 times already.
I did put a WARN_ON(1) to __drm_connector_put_safe, to see who is the caller that triggers the delayed work that frees the connector when it is too late.
I attached a backtrace with the above WARN_ON and the crash (connector_leak_bug.txt) I also attached the script 'amdgpu_unbind' for the reference that I use to unbind the amdgpu driver.
- When doing VFIO passthrough of this card, I found out that it doesn't
suffer that much from the reset bug. As long as I shut down the guest in clean manner, I can start it again). The vendor_reset module however makes the reset work even when I shut down the guest right in the middle of a 3D app running and I tested it many times.
_However_ this only works if I never load the amdgpu linux driver. Otherwise a windows guest still boots but all 3D apps in it crash very early.
I tried both the stock drivers that windows auto installs and latest AMD workstation drivers from AMD site.
Linux guests do work.
I found out that amdgpu driver resizes the device bars (I have TRX40 platform, so I don't know if this platform supports the AMD Smart Memory or not, but according to lspci the device does support resizable BARs).
If I patch the amdgpu's bar resize out, then, the windows guest _does_ work regardless if I loaded amdgpu prior or not. Linux guests also still work. I haven't measured the performance impact of this.
For debugging this, I did try to hide the PCI_EXT_CAP_ID_REBAR capability from the VM, but it made no difference.
I suspect that once the GPU is resetted, the bars revert to their original sizes, but VFIO uses the sizes that are cached by the kernel, so that the guest thinks that the bars are of one size while they are of an another. I don't have an idea though why this does work with a Linux guest.
I had attached the pci config with amdgpu running, once with my patch that stops it from resizing the bars, and once without that patch for reference. (amdgpu_pciconfig_noresize.txt, amdgpu_pciconfig_resize.txt)
- I found out that amdgpu runtime PM sometimes breaks the card if last
output is disconnected from it. I didn't debug it much as I just disabled it with amdgpu.runpm=0) I will do more debug on this later.
Please let me know if you have any questions, Don't hesitate to ask me for more information.
My setup: 3 outputs, all HDMI, converted with DP->HDMI adapters, of which 2 are 1080P monitors, and 1 is a 1080P TV. The issues I describe above are reproducible on all the outputs.
I am running 5.10.0 kernel with few patches and kvm-queue branch merged for my day to day work on KVM.
You can find the exact kernel I use and its .config on https://gitlab.com/maximlevitsky/linux/-/commits/kernel-starship-5.10
Best regards, Maxim Levitsky
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Mon, 4 Jan 2021 12:34:34 +0100 Christian König christian.koenig@amd.com wrote:
Hi Maxim,
I can't help with the display related stuff. Probably best approach to get this fixes would be to open up a bug tracker for this on FDO.
But I'm the one who implemented the resizeable BAR support and your analysis of the problem sounds about correct to me.
The reason why this works on Linux is most likely because we restore the BAR size on resume (and maybe during initial boot as well).
See this patch for reference:
commit d3252ace0bc652a1a244455556b6a549f969bf99 Author: Christian König ckoenig.leichtzumerken@gmail.com Date: Fri Jun 29 19:54:55 2018 -0500
PCI: Restore resized BAR state on resume
Resize BARs after resume to the expected size again.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199959 Fixes: d6895ad39f3b ("drm/amdgpu: resize VRAM BAR for CPU access v6") Fixes: 276b738deb5b ("PCI: Add resizable BAR infrastructure") Signed-off-by: Christian König christian.koenig@amd.com Signed-off-by: Bjorn Helgaas bhelgaas@google.com CC: stable@vger.kernel.org # v4.15+
It should be trivial to add this to the reset module as well. Most likely even completely vendor independent since I'm not sure what a bus reset will do to this configuration and restoring it all the time should be the most defensive approach.
Hmm, this should already be used by the bus/slot reset path:
pci_bus_restore_locked()/pci_slot_restore_locked() pci_dev_restore() pci_restore_state() pci_restore_rebar_state()
VFIO support for resizeable BARs has been on my todo list, but I don't have access to any systems that have both a capable device and >4G decoding enabled in the BIOS. If we have a consistent view of the BAR size after the BARs are expanded, I'm not sure why it doesn't just work. FWIW, QEMU currently hides the REBAR capability to the guest because the kernel driver doesn't support emulation through config space (ie. it's read-only, which the spec doesn't support).
AIUI, resource allocation can fail when enabling REBAR support, which is a problem if the failure occurs on the host but not the guest since we have no means via the hardware protocol to expose such a condition. Therefore the model I was considering for vfio-pci would be to simply pre-enable REBAR at the max size. It might be sufficiently safe to test BAR expansion on initialization and then allow user control, but I'm concerned that resource availability could change while already in use by the user. Thanks,
Alex
Am 04.01.21 um 17:45 schrieb Alex Williamson:
On Mon, 4 Jan 2021 12:34:34 +0100 Christian König christian.koenig@amd.com wrote:
Hi Maxim,
I can't help with the display related stuff. Probably best approach to get this fixes would be to open up a bug tracker for this on FDO.
But I'm the one who implemented the resizeable BAR support and your analysis of the problem sounds about correct to me.
The reason why this works on Linux is most likely because we restore the BAR size on resume (and maybe during initial boot as well).
See this patch for reference:
commit d3252ace0bc652a1a244455556b6a549f969bf99 Author: Christian König ckoenig.leichtzumerken@gmail.com Date: Fri Jun 29 19:54:55 2018 -0500
PCI: Restore resized BAR state on resume
Resize BARs after resume to the expected size again.
BugLink: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.k... Fixes: d6895ad39f3b ("drm/amdgpu: resize VRAM BAR for CPU access v6") Fixes: 276b738deb5b ("PCI: Add resizable BAR infrastructure") Signed-off-by: Christian König christian.koenig@amd.com Signed-off-by: Bjorn Helgaas bhelgaas@google.com CC: stable@vger.kernel.org # v4.15+
It should be trivial to add this to the reset module as well. Most likely even completely vendor independent since I'm not sure what a bus reset will do to this configuration and restoring it all the time should be the most defensive approach.
Hmm, this should already be used by the bus/slot reset path:
pci_bus_restore_locked()/pci_slot_restore_locked() pci_dev_restore() pci_restore_state() pci_restore_rebar_state()
VFIO support for resizeable BARs has been on my todo list, but I don't have access to any systems that have both a capable device and >4G decoding enabled in the BIOS. If we have a consistent view of the BAR size after the BARs are expanded, I'm not sure why it doesn't just work. FWIW, QEMU currently hides the REBAR capability to the guest because the kernel driver doesn't support emulation through config space (ie. it's read-only, which the spec doesn't support).
In this case the guest shouldn't be able to change the config at all and I have no idea what's going wrong here.
AIUI, resource allocation can fail when enabling REBAR support, which is a problem if the failure occurs on the host but not the guest since we have no means via the hardware protocol to expose such a condition. Therefore the model I was considering for vfio-pci would be to simply pre-enable REBAR at the max size.
That's a rather bad idea. See our GPUs for example return way more than they actually need.
E.g. a Polaris usually returns 4GiB even when only 2GiB are installed, because 4GiB is just the maximum amount of RAM you can put together with the ASIC on a board.
Some devices even return a mask of all 1 even when they need only 2MiB, resulting in nearly 1TiB of wasted address space with this approach.
Regards, Christian.
It might be sufficiently safe to test BAR expansion on initialization and then allow user control, but I'm concerned that resource availability could change while already in use by the user. Thanks,
Alex
On Mon, 4 Jan 2021 18:39:33 +0100 Christian König christian.koenig@amd.com wrote:
Am 04.01.21 um 17:45 schrieb Alex Williamson:
On Mon, 4 Jan 2021 12:34:34 +0100 Christian König christian.koenig@amd.com wrote:
Hi Maxim,
I can't help with the display related stuff. Probably best approach to get this fixes would be to open up a bug tracker for this on FDO.
But I'm the one who implemented the resizeable BAR support and your analysis of the problem sounds about correct to me.
The reason why this works on Linux is most likely because we restore the BAR size on resume (and maybe during initial boot as well).
See this patch for reference:
commit d3252ace0bc652a1a244455556b6a549f969bf99 Author: Christian König ckoenig.leichtzumerken@gmail.com Date: Fri Jun 29 19:54:55 2018 -0500
PCI: Restore resized BAR state on resume
Resize BARs after resume to the expected size again.
BugLink: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.k... Fixes: d6895ad39f3b ("drm/amdgpu: resize VRAM BAR for CPU access v6") Fixes: 276b738deb5b ("PCI: Add resizable BAR infrastructure") Signed-off-by: Christian König christian.koenig@amd.com Signed-off-by: Bjorn Helgaas bhelgaas@google.com CC: stable@vger.kernel.org # v4.15+
It should be trivial to add this to the reset module as well. Most likely even completely vendor independent since I'm not sure what a bus reset will do to this configuration and restoring it all the time should be the most defensive approach.
Hmm, this should already be used by the bus/slot reset path:
pci_bus_restore_locked()/pci_slot_restore_locked() pci_dev_restore() pci_restore_state() pci_restore_rebar_state()
VFIO support for resizeable BARs has been on my todo list, but I don't have access to any systems that have both a capable device and >4G decoding enabled in the BIOS. If we have a consistent view of the BAR size after the BARs are expanded, I'm not sure why it doesn't just work. FWIW, QEMU currently hides the REBAR capability to the guest because the kernel driver doesn't support emulation through config space (ie. it's read-only, which the spec doesn't support).
In this case the guest shouldn't be able to change the config at all and I have no idea what's going wrong here.
AIUI, resource allocation can fail when enabling REBAR support, which is a problem if the failure occurs on the host but not the guest since we have no means via the hardware protocol to expose such a condition. Therefore the model I was considering for vfio-pci would be to simply pre-enable REBAR at the max size.
That's a rather bad idea. See our GPUs for example return way more than they actually need.
E.g. a Polaris usually returns 4GiB even when only 2GiB are installed, because 4GiB is just the maximum amount of RAM you can put together with the ASIC on a board.
Would the driver fail or misbehave if the BAR is sized larger than the amount of memory on the card or is memory size determined independently of BAR size?
Some devices even return a mask of all 1 even when they need only 2MiB, resulting in nearly 1TiB of wasted address space with this approach.
Ugh. I'm afraid to ask why a device with a 2MiB BAR would implement a REBAR capability, but I guess we really can't make any assumptions about the breadth of SKUs that ASIC might support (or sanity of the designers).
We could probe to determine the maximum size the host can support and potentially emulate the capability to remove sizes that we can't allocate, but without any ability for the device to reject a size advertised as supported via the capability protocol it makes me nervous how we can guarantee the resources are available when the user re-configures the device. That might mean we'd need to reserve the resources, up to what the host can support, regardless of what the device can actually use. I'm not sure how else to know how much to reserve without device specific code in vfio-pci. Thanks,
Alex
Am 04.01.21 um 19:43 schrieb Alex Williamson:
On Mon, 4 Jan 2021 18:39:33 +0100 Christian König christian.koenig@amd.com wrote:
Am 04.01.21 um 17:45 schrieb Alex Williamson:
On Mon, 4 Jan 2021 12:34:34 +0100 Christian König christian.koenig@amd.com wrote:
[SNIP]
That's a rather bad idea. See our GPUs for example return way more than they actually need.
E.g. a Polaris usually returns 4GiB even when only 2GiB are installed, because 4GiB is just the maximum amount of RAM you can put together with the ASIC on a board.
Would the driver fail or misbehave if the BAR is sized larger than the amount of memory on the card or is memory size determined independently of BAR size?
Uff, good question. I have no idea.
At least the Linux driver should behave well, but no idea about the Windows driver stack.
Some devices even return a mask of all 1 even when they need only 2MiB, resulting in nearly 1TiB of wasted address space with this approach.
Ugh. I'm afraid to ask why a device with a 2MiB BAR would implement a REBAR capability, but I guess we really can't make any assumptions about the breadth of SKUs that ASIC might support (or sanity of the designers).
It's a standard feature for FPGAs these days since how much BAR you need depends on what you load on it, and that in turn usually only happens after the OS is already started and you fire up your development environment.
We could probe to determine the maximum size the host can support and potentially emulate the capability to remove sizes that we can't allocate, but without any ability for the device to reject a size advertised as supported via the capability protocol it makes me nervous how we can guarantee the resources are available when the user re-configures the device. That might mean we'd need to reserve the resources, up to what the host can support, regardless of what the device can actually use. I'm not sure how else to know how much to reserve without device specific code in vfio-pci. Thanks,
Well in the FPGA case I outlined above you don't really know how much BAR you need until the setup is completed.
E.g. you could need one BAR with just 2MiB and another with 128GB, or two with 64GB or.... That's the reason why somebody came up with the REBAR standard in the first place.
I think I can summarize that static resizing might work for some devices like our GPUs, but it doesn't solve the problem in general.
Regards, Christian.
Alex
On Mon, 4 Jan 2021 21:13:53 +0100 Christian König christian.koenig@amd.com wrote:
Am 04.01.21 um 19:43 schrieb Alex Williamson:
On Mon, 4 Jan 2021 18:39:33 +0100 Christian König christian.koenig@amd.com wrote:
Am 04.01.21 um 17:45 schrieb Alex Williamson:
On Mon, 4 Jan 2021 12:34:34 +0100 Christian König christian.koenig@amd.com wrote:
[SNIP]
That's a rather bad idea. See our GPUs for example return way more than they actually need.
E.g. a Polaris usually returns 4GiB even when only 2GiB are installed, because 4GiB is just the maximum amount of RAM you can put together with the ASIC on a board.
Would the driver fail or misbehave if the BAR is sized larger than the amount of memory on the card or is memory size determined independently of BAR size?
Uff, good question. I have no idea.
At least the Linux driver should behave well, but no idea about the Windows driver stack.
Some devices even return a mask of all 1 even when they need only 2MiB, resulting in nearly 1TiB of wasted address space with this approach.
Ugh. I'm afraid to ask why a device with a 2MiB BAR would implement a REBAR capability, but I guess we really can't make any assumptions about the breadth of SKUs that ASIC might support (or sanity of the designers).
It's a standard feature for FPGAs these days since how much BAR you need depends on what you load on it, and that in turn usually only happens after the OS is already started and you fire up your development environment.
We could probe to determine the maximum size the host can support and potentially emulate the capability to remove sizes that we can't allocate, but without any ability for the device to reject a size advertised as supported via the capability protocol it makes me nervous how we can guarantee the resources are available when the user re-configures the device. That might mean we'd need to reserve the resources, up to what the host can support, regardless of what the device can actually use. I'm not sure how else to know how much to reserve without device specific code in vfio-pci. Thanks,
Well in the FPGA case I outlined above you don't really know how much BAR you need until the setup is completed.
E.g. you could need one BAR with just 2MiB and another with 128GB, or two with 64GB or.... That's the reason why somebody came up with the REBAR standard in the first place.
Yes, I suppose without a full bus-reset and soft-hotplug event, resizable BARs are the best way to reconfigure a device based on FPGA programming. Anyway, thanks for the insights here.
I think I can summarize that static resizing might work for some devices like our GPUs, but it doesn't solve the problem in general.
Yup, I don't have a good approach for the general case for a VM yet. We could add a sysfs or side channel mechanism to preconfigure a BAR size, but once we're dealing with a VM interacting with the REBAR capability itself, it's far too easy for the guest to create a configuration that the host might not have bus resources to support, especially if there are multiple resizable BARs under a bridge. Thanks,
Alex
On Mon, 2021-01-04 at 09:45 -0700, Alex Williamson wrote:
On Mon, 4 Jan 2021 12:34:34 +0100 Christian König christian.koenig@amd.com wrote:
Hi Maxim,
I can't help with the display related stuff. Probably best approach to get this fixes would be to open up a bug tracker for this on FDO.
But I'm the one who implemented the resizeable BAR support and your analysis of the problem sounds about correct to me.
The reason why this works on Linux is most likely because we restore the BAR size on resume (and maybe during initial boot as well).
See this patch for reference:
commit d3252ace0bc652a1a244455556b6a549f969bf99 Author: Christian König ckoenig.leichtzumerken@gmail.com Date: Fri Jun 29 19:54:55 2018 -0500
PCI: Restore resized BAR state on resume Resize BARs after resume to the expected size again. BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199959 Fixes: d6895ad39f3b ("drm/amdgpu: resize VRAM BAR for CPU access v6") Fixes: 276b738deb5b ("PCI: Add resizable BAR infrastructure") Signed-off-by: Christian König <christian.koenig@amd.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> CC: stable@vger.kernel.org # v4.15+
Hi! Thanks for the feedback!
So I went over qemu code and indeed the qemu (as opposed to the kernel where I tried to hide the PCI_EXT_CAP_ID_REBAR) indeed does hide this pci capability from the guest.
However exactly as Alex mentioned the kernel does indeed restore the rebar state, and even with that code patched out I found out that rebar state persists across the reset that the vendor_reset module does (BACO I think).
Therefore the Linux guest sees the full 4G bar and happily uses it, while the windows guest's driver apparently has a bug when the bar is that large.
I patched the amdgpu to resize the bar to various other sizes, and the windows driver apparently works up to a 2GB bar.
So pretty much other than a bug in the windows driver, and fact that VFIO doesn't support resizable bars there is nothing wrong here.
Since my system does support above 4G decoding and I do have a nice vfio friendly device that does support a resizable bar, I do volunteer to add support for this to VFIO as time and resources permit.
Also it would be nice if it was either possible to make amdgpu (or the whole system) optionally avoid resizing bars when a kernel command line / module param is given, or even better let the amdgpu resize the bar to its original size when it is unloaded which IMHO is the best solution for this problem.
I think I can prepare a patch to make amdgpu restore the bar size on unload if you think that this is the right solution.
It should be trivial to add this to the reset module as well. Most likely even completely vendor independent since I'm not sure what a bus reset will do to this configuration and restoring it all the time should be the most defensive approach.
Hmm, this should already be used by the bus/slot reset path:
pci_bus_restore_locked()/pci_slot_restore_locked() pci_dev_restore() pci_restore_state() pci_restore_rebar_state()
VFIO support for resizeable BARs has been on my todo list, but I don't have access to any systems that have both a capable device and >4G decoding enabled in the BIOS. If we have a consistent view of the BAR size after the BARs are expanded, I'm not sure why it doesn't just work. FWIW, QEMU currently hides the REBAR capability to the guest because the kernel driver doesn't support emulation through config space (ie. it's read-only, which the spec doesn't support).
AIUI, resource allocation can fail when enabling REBAR support, which is a problem if the failure occurs on the host but not the guest since we have no means via the hardware protocol to expose such a condition. Therefore the model I was considering for vfio-pci would be to simply pre-enable REBAR at the max size. It might be sufficiently safe to test BAR expansion on initialization and then allow user control, but I'm concerned that resource availability could change while already in use by the user. Thanks,
As mentioned in other replies in this thread and what my first thought about this, this will indeed will break on devices which don't accurately report the maximum bar size that they actually need. Even the spec itself says that it is vendor specific to determine the optimal bar size.
We can also allow guest to resize the bar and if that fails, expose the error via a virtual AER message on the root port where the device is attached?
I personally don't know if this is possible/worth it.
Best regards, Maxim Levitsky
Alex
Am 06.01.21 um 21:21 schrieb Maxim Levitsky:
On Mon, 2021-01-04 at 09:45 -0700, Alex Williamson wrote:
On Mon, 4 Jan 2021 12:34:34 +0100 Christian König christian.koenig@amd.com wrote:
Hi Maxim,
I can't help with the display related stuff. Probably best approach to get this fixes would be to open up a bug tracker for this on FDO.
But I'm the one who implemented the resizeable BAR support and your analysis of the problem sounds about correct to me.
The reason why this works on Linux is most likely because we restore the BAR size on resume (and maybe during initial boot as well).
See this patch for reference:
commit d3252ace0bc652a1a244455556b6a549f969bf99 Author: Christian König ckoenig.leichtzumerken@gmail.com Date: Fri Jun 29 19:54:55 2018 -0500
PCI: Restore resized BAR state on resume Resize BARs after resume to the expected size again. BugLink: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D199959&data=04%7C01%7Cchristian.koenig%40amd.com%7C04878f8babc64386353908d8b280a23b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637455612845286179%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=iRk9S4IgfQHZgVf1m1n%2F9LpOQzO41pLoc7EWmzH%2Fym4%3D&reserved=0 Fixes: d6895ad39f3b ("drm/amdgpu: resize VRAM BAR for CPU access v6") Fixes: 276b738deb5b ("PCI: Add resizable BAR infrastructure") Signed-off-by: Christian König <christian.koenig@amd.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> CC: stable@vger.kernel.org # v4.15+Hi! Thanks for the feedback!
So I went over qemu code and indeed the qemu (as opposed to the kernel where I tried to hide the PCI_EXT_CAP_ID_REBAR) indeed does hide this pci capability from the guest.
However exactly as Alex mentioned the kernel does indeed restore the rebar state, and even with that code patched out I found out that rebar state persists across the reset that the vendor_reset module does (BACO I think).
Therefore the Linux guest sees the full 4G bar and happily uses it, while the windows guest's driver apparently has a bug when the bar is that large.
I patched the amdgpu to resize the bar to various other sizes, and the windows driver apparently works up to a 2GB bar.
So pretty much other than a bug in the windows driver, and fact that VFIO doesn't support resizable bars there is nothing wrong here.
Since my system does support above 4G decoding and I do have a nice vfio friendly device that does support a resizable bar, I do volunteer to add support for this to VFIO as time and resources permit.
Also it would be nice if it was either possible to make amdgpu (or the whole system) optionally avoid resizing bars when a kernel command line / module param is given, or even better let the amdgpu resize the bar to its original size when it is unloaded which IMHO is the best solution for this problem.
I think I can prepare a patch to make amdgpu restore the bar size on unload if you think that this is the right solution.
Coming back to this topic now, sorry been a bit busy over the last few days.
Basically I don't think that amdgpu should do anything when it quits.
What you should rather do is to resize the BAR to the default value of the BIOS when you trigger the device reset.
It should be trivial to add this to the reset module as well. Most likely even completely vendor independent since I'm not sure what a bus reset will do to this configuration and restoring it all the time should be the most defensive approach.
Hmm, this should already be used by the bus/slot reset path:
pci_bus_restore_locked()/pci_slot_restore_locked() pci_dev_restore() pci_restore_state() pci_restore_rebar_state()
VFIO support for resizeable BARs has been on my todo list, but I don't have access to any systems that have both a capable device and >4G decoding enabled in the BIOS. If we have a consistent view of the BAR size after the BARs are expanded, I'm not sure why it doesn't just work. FWIW, QEMU currently hides the REBAR capability to the guest because the kernel driver doesn't support emulation through config space (ie. it's read-only, which the spec doesn't support).
AIUI, resource allocation can fail when enabling REBAR support, which is a problem if the failure occurs on the host but not the guest since we have no means via the hardware protocol to expose such a condition. Therefore the model I was considering for vfio-pci would be to simply pre-enable REBAR at the max size. It might be sufficiently safe to test BAR expansion on initialization and then allow user control, but I'm concerned that resource availability could change while already in use by the user. Thanks,
As mentioned in other replies in this thread and what my first thought about this, this will indeed will break on devices which don't accurately report the maximum bar size that they actually need. Even the spec itself says that it is vendor specific to determine the optimal bar size.
We can also allow guest to resize the bar and if that fails, expose the error via a virtual AER message on the root port where the device is attached?
Sounds like it might work in theory, but I'm not an expert for KVM.
Regards, Christian.
I personally don't know if this is possible/worth it.
Best regards, Maxim Levitsky
Alex
On Mon, 2021-01-04 at 12:34 +0100, Christian König wrote:
Hi Maxim,
I can't help with the display related stuff. Probably best approach to get this fixes would be to open up a bug tracker for this on FDO.
Done, bugs are opened https://gitlab.freedesktop.org/drm/amd/-/issues/1429 https://gitlab.freedesktop.org/drm/amd/-/issues/1430
About the EDID issue, there do seem to be few open bugs about it, but what differs in my case I think is that EDID failure happens only once in a while, rather that always, and it seems to bring the whole device down.
Best regards, Maxim Levitsky
dri-devel@lists.freedesktop.org
-
Alex Williamson -
Christian König -
Maxim Levitsky