Page MenuHomeFreeBSD

Clear the MEM/PORT_EN bit when updating PCI BAR
ClosedPublic

Authored by decui_microsoft.com on Sep 18 2016, 2:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 13 2024, 7:56 PM
Unknown Object (File)
Dec 19 2023, 9:56 AM
Unknown Object (File)
Nov 13 2023, 11:27 AM
Unknown Object (File)
Nov 11 2023, 1:25 AM
Unknown Object (File)
Nov 10 2023, 8:17 PM
Unknown Object (File)
Nov 8 2023, 8:08 PM
Unknown Object (File)
Nov 8 2023, 7:12 AM
Unknown Object (File)
Oct 25 2023, 5:48 AM
Subscribers
None

Details

Summary

It's unsafe to update the BAR when the EN bit is set.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

decui_microsoft.com retitled this revision from to Clear the MEM/PORT_EN bit when updating PCI BAR.
decui_microsoft.com updated this object.
decui_microsoft.com edited the test plan for this revision. (Show Details)

In theory the callers should be doing this (e.g. during something like resume where we write all the BARs). In addition, the ROM BAR shouldn't toggle MEMEN for all other memory BARs since it has its own bit. I'd rather we fix the callers that need this changed. There are some places in pci.c that aren't quite correct. Are you using this in out-of-tree changes or are you having issues with the existing uses (e.g. during boot-time probe)?

In D7914#165049, @jhb wrote:

In theory the callers should be doing this (e.g. during something like resume where we write all the BARs). In addition, the ROM BAR shouldn't toggle MEMEN for all other memory BARs since it has its own bit. I'd rather we fix the callers that need this changed. There are some places in pci.c that aren't quite correct. Are you using this in out-of-tree changes or are you having issues with the existing uses (e.g. during boot-time probe)?

It's for WIP Hyper-V PCI bridge for PCI passthrough/SR-IOV. The WIP bridge driver will program the BARs when its children try to allocate RES_MEMORY, however, if the MEMEN is set (e.g. Upon the first RES_MEMORY request w/ RF_ACTIVE, the MEMEN will be set. When second RES_MEMORY request is received, normally for MSI-X, the MEMEN has been set), the hypervisor will ignore the programmed value, the exact path is:
pci_alloc_resource -> pci_reserve_map -> pci_write_bar.

Do you mean we should move the MEMEN clear/set to pci_reserve_map?

In D7914#165049, @jhb wrote:

In theory the callers should be doing this (e.g. during something like resume where we write all the BARs). In addition, the ROM BAR shouldn't toggle MEMEN for all other memory BARs since it has its own bit. I'd rather we fix the callers that need this changed. There are some places in pci.c that aren't quite correct. Are you using this in out-of-tree changes or are you having issues with the existing uses (e.g. during boot-time probe)?

It's for WIP Hyper-V PCI bridge for PCI passthrough/SR-IOV. The WIP bridge driver will program the BARs when its children try to allocate RES_MEMORY, however, if the MEMEN is set (e.g. Upon the first RES_MEMORY request w/ RF_ACTIVE, the MEMEN will be set. When second RES_MEMORY request is received, normally for MSI-X, the MEMEN has been set), the hypervisor will ignore the programmed value, the exact path is:
pci_alloc_resource -> pci_reserve_map -> pci_write_bar.

Do you mean we should move the MEMEN clear/set to pci_reserve_map?

Yes. Truth be told, what we really should be doing is not setting MEMEN or IOEN unless/until all BARs of a given resource type are programmed/valid. I haven't gotten around to fixing that. For now, doing the disable/enable in pci_reserve_map() should be ok.

In D7914#165285, @jhb wrote:
In D7914#165049, @jhb wrote:

In theory the callers should be doing this (e.g. during something like resume where we write all the BARs). In addition, the ROM BAR shouldn't toggle MEMEN for all other memory BARs since it has its own bit. I'd rather we fix the callers that need this changed. There are some places in pci.c that aren't quite correct. Are you using this in out-of-tree changes or are you having issues with the existing uses (e.g. during boot-time probe)?

It's for WIP Hyper-V PCI bridge for PCI passthrough/SR-IOV. The WIP bridge driver will program the BARs when its children try to allocate RES_MEMORY, however, if the MEMEN is set (e.g. Upon the first RES_MEMORY request w/ RF_ACTIVE, the MEMEN will be set. When second RES_MEMORY request is received, normally for MSI-X, the MEMEN has been set), the hypervisor will ignore the programmed value, the exact path is:
pci_alloc_resource -> pci_reserve_map -> pci_write_bar.

Do you mean we should move the MEMEN clear/set to pci_reserve_map?

Yes. Truth be told, what we really should be doing is not setting MEMEN or IOEN unless/until all BARs of a given resource type are programmed/valid. I haven't gotten around to fixing that. For now, doing the disable/enable in pci_reserve_map() should be ok.

Yeah, agree w/ the complete fix.

We will move the MEMEN/IOEN to pci_reserve_map before the complete fix.

decui_microsoft.com edited edge metadata.
decui_microsoft.com removed rS FreeBSD src repository - subversion as the repository for this revision.

Please review v2.
I moved the MEMEN/PORTEN clear/restore from pci_write_bar() to pci_reserve_map().

jhb edited edge metadata.
This revision is now accepted and ready to land.Sep 26 2016, 5:36 PM
This revision was automatically updated to reflect the committed changes.