Clear the MEM/PORT_EN bit when updating PCI BAR

[decui_microsoft.com]

Summary

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

[jhb]

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)?

[sepherosa_gmail.com]

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?

[jhb]

In D7914#165153, @sepherosa_gmail.com 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.

[sepherosa_gmail.com]

In D7914#165285, @jhb wrote:

In D7914#165153, @sepherosa_gmail.com 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]

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