| Message ID | 20190501083012.8608-1-oohall@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | pci/iov: Remove skiboot VF tracking | expand |
On Wed, May 1, 2019 at 6:30 PM Oliver O'Halloran <oohall@gmail.com> wrote: > > This feature was added a few years ago in response to a request to make > the MaxPayloadSize (MPS) field of a Virtual Function match the MPS of the > Physical Function that hosts it. > > The SR-IOV specification states the the MPS field of the VF is "ResvP". > This indicates the VF will use whatever MPS is configured on the PF and > that the field should be treated as a reserved field in the config space > of the VF. In other words, a SR-IOV spec compliant VF should always return > zero in the MPS field. Adding hacks in OPAL to make it non-zero is... > misguided at best. > > Additionally, there is a bug in the way pci_device structures are handled > by VFs that results in a crash on fast-reboot that occurs if VFs are > enabled and then disabled prior to rebooting. This patch fixes the bug by > removing the code entirely. This patch has no impact on SR-IOV support on > the host operating system. > > Cc: Sergey Miroshnichenko <s.miroshnichenko@yadro.com> > Cc: skiboot-stable@lists.ozlabs.org > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> The test team smoke tested it and it came back clean. Merged to master as of 22057f868f3b2b1fd02647a738f6da0858b5eb6c Vasant, you can pick this up for stable now if you like. > --- > core/Makefile.inc | 2 +- > core/pci-iov.c | 265 ---------------------------------------------- > core/pci.c | 2 - > include/pci-iov.h | 37 ------- > 4 files changed, 1 insertion(+), 305 deletions(-) > delete mode 100644 core/pci-iov.c > delete mode 100644 include/pci-iov.h > > diff --git a/core/Makefile.inc b/core/Makefile.inc > index 3b4387081a62..21c12fb8db31 100644 > --- a/core/Makefile.inc > +++ b/core/Makefile.inc > @@ -3,7 +3,7 @@ > SUBDIRS += core > CORE_OBJS = relocate.o console.o stack.o init.o chip.o mem_region.o > CORE_OBJS += malloc.o lock.o cpu.o utils.o fdt.o opal.o interrupts.o timebase.o > -CORE_OBJS += opal-msg.o pci.o pci-iov.o pci-virt.o pci-slot.o pcie-slot.o > +CORE_OBJS += opal-msg.o pci.o pci-virt.o pci-slot.o pcie-slot.o > CORE_OBJS += pci-opal.o fast-reboot.o device.o exceptions.o trace.o affinity.o > CORE_OBJS += vpd.o hostservices.o platform.o nvram.o nvram-format.o hmi.o > CORE_OBJS += console-log.o ipmi.o time-utils.o pel.o pool.o errorlog.o > diff --git a/core/pci-iov.c b/core/pci-iov.c > deleted file mode 100644 > index 8fecebc1292a..000000000000 > --- a/core/pci-iov.c > +++ /dev/null > @@ -1,265 +0,0 @@ > -/* Copyright 2013-2016 IBM Corp. > - * > - * Licensed under the Apache License, Version 2.0 (the "License"); > - * you may not use this file except in compliance with the License. > - * You may obtain a copy of the License at > - * > - * http://www.apache.org/licenses/LICENSE-2.0 > - * > - * Unless required by applicable law or agreed to in writing, software > - * distributed under the License is distributed on an "AS IS" BASIS, > - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > - * implied. > - * See the License for the specific language governing permissions and > - * limitations under the License. > - */ > - > -#include <skiboot.h> > -#include <pci.h> > -#include <pci-cfg.h> > -#include <pci-slot.h> > -#include <pci-iov.h> > - > -/* > - * Tackle the VF's MPS in PCIe capability. The field is read only. > - * This function caches what is written and returns the cached > - * MPS on read. > - */ > -static int64_t pci_iov_vf_devctl(void *dev, struct pci_cfg_reg_filter *pcrf, > - uint32_t offset, uint32_t len, > - uint32_t *data, bool write) > -{ > - struct pci_device *vf = (struct pci_device *)dev; > - uint32_t pos = pci_cap(vf, PCI_CFG_CAP_ID_EXP, false); > - uint8_t *pcache; > - > - if (offset != (pos + PCICAP_EXP_DEVCTL)) > - return OPAL_PARTIAL; > - > - pcache = &pcrf->data[0]; > - if (write) { > - *pcache = ((uint8_t)(*data >> (8 * (4 - len)))) & > - PCICAP_EXP_DEVCTL_MPS; > - } else { > - *data &= ~(PCICAP_EXP_DEVCTL_MPS << (8 * (4 - len))); > - *data |= (((uint32_t)(*pcache & PCICAP_EXP_DEVCTL_MPS)) > - << (8 * (4 - len))); > - } > - > - return OPAL_SUCCESS; > -} > - > -static void pci_iov_vf_quirk(struct phb *phb, struct pci_device *vf) > -{ > - struct pci_cfg_reg_filter *pcrf; > - uint32_t pos; > - > - if (!pci_has_cap(vf, PCI_CFG_CAP_ID_EXP, false)) > - return; > - > - /* > - * On Mellanox MT27500 Family [ConnectX-3], its VF's MPS field in > - * the corresponding config register is readonly. The MPS for PF/VF > - * are usually different. We are introducing a quirk to make them > - * look same to avoid confusion. > - */ > - if (vf->vdid != 0x100315b3) > - return; > - > - pos = pci_cap(vf, PCI_CFG_CAP_ID_EXP, false); > - pcrf = pci_add_cfg_reg_filter(vf, pos + PCICAP_EXP_DEVCTL, 4, > - PCI_REG_FLAG_MASK, pci_iov_vf_devctl); > - if (!pcrf) > - prlog(PR_WARNING, "%s: Missed DEVCTL filter on %04x:%02x:%02x.%01x\n", > - __func__, phb->opal_id, (vf->bdfn >> 8), > - ((vf->bdfn >> 3) & 0x1f), (vf->bdfn & 0x7)); > -} > - > -/* > - * Update the SRIOV parameters that change when the number of > - * VFs is configured. > - */ > -static bool pci_iov_update_parameters(struct pci_iov *iov) > -{ > - struct phb *phb = iov->phb; > - uint16_t bdfn = iov->pd->bdfn; > - uint32_t pos = iov->pos; > - uint16_t val; > - bool enabled; > - > - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_CTRL, &val); > - enabled = !!(val & PCIECAP_SRIOV_CTRL_VFE); > - if (iov->enabled == enabled) > - return false; > - > - if (enabled) { > - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_INITIAL_VF, > - &iov->init_VFs); > - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_NUM_VF, > - &iov->num_VFs); > - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_VF_OFFSET, > - &iov->offset); > - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_VF_STRIDE, > - &iov->stride); > - } else { > - iov->init_VFs = 0; > - iov->num_VFs = 0; > - iov->offset = 0; > - iov->stride = 0; > - } > - > - iov->enabled = enabled; > - return true; > -} > - > -static int64_t pci_iov_change(void *dev __unused, > - struct pci_cfg_reg_filter *pcrf, > - uint32_t offset __unused, > - uint32_t len __unused, > - uint32_t *data __unused, > - bool write __unused) > -{ > - struct pci_iov *iov = (struct pci_iov *)pcrf->data; > - struct phb *phb = iov->phb; > - struct pci_device *pd = iov->pd; > - struct pci_device *vf, *tmp; > - uint32_t i; > - bool changed; > - > - /* Update SRIOV variable parameters */ > - changed = pci_iov_update_parameters(iov); > - if (!changed) > - return OPAL_PARTIAL; > - > - /* Remove all VFs that have been attached to the parent */ > - if (!iov->enabled) { > - list_for_each_safe(&pd->children, vf, tmp, link) > - list_del(&vf->link); > - return OPAL_PARTIAL; > - } > - > - /* Initialize the VFs and attach them to parent */ > - for (changed = false, i = 0; i < iov->num_VFs; i++) { > - vf = &iov->VFs[i]; > - vf->bdfn = pd->bdfn + iov->offset + iov->stride * i; > - list_add_tail(&pd->children, &vf->link); > - > - /* > - * We don't populate the capabilities again if they have > - * been existing, to save time. Also, we need delay for > - * 100ms before the VF's config space becomes ready. > - */ > - if (!pci_has_cap(vf, PCI_CFG_CAP_ID_EXP, false)) { > - if (!changed) { > - changed = !changed; > - time_wait_ms(100); > - } > - > - pci_init_capabilities(phb, vf); > - pci_iov_vf_quirk(phb, vf); > - } > - > - /* Call PHB hook */ > - if (phb->ops->device_init) > - phb->ops->device_init(phb, pd, NULL); > - } > - > - return OPAL_PARTIAL; > -} > - > -/* > - * This function is called with disabled SRIOV capability. So the VF's > - * config address isn't finalized and its config space isn't accessible. > - */ > -static void pci_iov_init_VF(struct pci_device *pd, struct pci_device *vf) > -{ > - vf->is_bridge = false; > - vf->is_multifunction = false; > - vf->is_vf = true; > - vf->dev_type = PCIE_TYPE_ENDPOINT; > - vf->scan_map = -1; > - vf->vdid = pd->vdid; > - vf->sub_vdid = pd->sub_vdid; > - vf->class = pd->class; > - vf->dn = NULL; > - vf->slot = NULL; > - vf->parent = pd; > - vf->phb = pd->phb; > - list_head_init(&vf->pcrf); > - list_head_init(&vf->children); > -} > - > -static void pci_free_iov_cap(void *data) > -{ > - struct pci_iov *iov = data; > - free(iov->VFs); > - free(iov); > -} > - > -void pci_init_iov_cap(struct phb *phb, struct pci_device *pd) > -{ > - int64_t pos; > - struct pci_iov *iov; > - struct pci_cfg_reg_filter *pcrf; > - uint32_t i; > - > - /* Search for SRIOV capability */ > - if (!pci_has_cap(pd, PCI_CFG_CAP_ID_EXP, false)) > - return; > - > - pos = pci_find_ecap(phb, pd->bdfn, PCIECAP_ID_SRIOV, NULL); > - if (pos <= 0) > - return; > - > - /* Allocate IOV */ > - iov = zalloc(sizeof(*iov)); > - if (!iov) { > - prlog(PR_ERR, "%s: Cannot alloc IOV for %04x:%02x:%02x.%01x\n", > - __func__, phb->opal_id, (pd->bdfn >> 8), > - ((pd->bdfn >> 3) & 0x1f), (pd->bdfn & 0x7)); > - return; > - } > - > - /* Allocate VFs */ > - pci_cfg_read16(phb, pd->bdfn, pos + PCIECAP_SRIOV_TOTAL_VF, > - &iov->total_VFs); > - iov->VFs = zalloc(sizeof(*iov->VFs) * iov->total_VFs); > - if (!iov->VFs) { > - prlog(PR_ERR, "%s: Cannot alloc %d VFs for %04x:%02x:%02x.%01x\n", > - __func__, iov->total_VFs, phb->opal_id, > - (pd->bdfn >> 8), ((pd->bdfn >> 3) & 0x1f), > - (pd->bdfn & 0x7)); > - free(iov); > - return; > - } > - > - /* Initialize VFs */ > - for (i = 0; i < iov->total_VFs; i++) > - pci_iov_init_VF(pd, &iov->VFs[i]); > - > - /* Register filter for enabling or disabling SRIOV capability */ > - pcrf = pci_add_cfg_reg_filter(pd, pos + PCIECAP_SRIOV_CTRL, 2, > - PCI_REG_FLAG_WRITE, pci_iov_change); > - if (!pcrf) { > - prlog(PR_ERR, "%s: Cannot set filter on %04x:%02x:%02x.%01x\n", > - __func__, phb->opal_id, (pd->bdfn >> 8), > - ((pd->bdfn >> 3) & 0x1f), (pd->bdfn & 0x7)); > - free(iov->VFs); > - free(iov); > - return; > - } > - > - /* Associate filter and IOV capability */ > - pcrf->data = (void *)iov; > - > - /* > - * Retrieve the number of VFs and other information if applicable. > - * Register the SRIOV capability in the mean while. > - */ > - iov->phb = phb; > - iov->pd = pd; > - iov->pos = pos; > - iov->enabled = false; > - pci_iov_update_parameters(iov); > - pci_set_cap(pd, PCIECAP_ID_SRIOV, pos, iov, pci_free_iov_cap, true); > -} > diff --git a/core/pci.c b/core/pci.c > index ee563c2fc577..02f5caa8d901 100644 > --- a/core/pci.c > +++ b/core/pci.c > @@ -18,7 +18,6 @@ > #include <cpu.h> > #include <pci.h> > #include <pci-cfg.h> > -#include <pci-iov.h> > #include <pci-slot.h> > #include <pci-quirk.h> > #include <timebase.h> > @@ -193,7 +192,6 @@ void pci_init_capabilities(struct phb *phb, struct pci_device *pd) > { > pci_init_pcie_cap(phb, pd); > pci_init_aer_cap(phb, pd); > - pci_init_iov_cap(phb, pd); > pci_init_pm_cap(phb, pd); > } > > diff --git a/include/pci-iov.h b/include/pci-iov.h > deleted file mode 100644 > index 787b2cd1da69..000000000000 > --- a/include/pci-iov.h > +++ /dev/null > @@ -1,37 +0,0 @@ > -/* Copyright 2013-2016 IBM Corp. > - * > - * Licensed under the Apache License, Version 2.0 (the "License"); > - * you may not use this file except in compliance with the License. > - * You may obtain a copy of the License at > - * > - * http://www.apache.org/licenses/LICENSE-2.0 > - * > - * Unless required by applicable law or agreed to in writing, software > - * distributed under the License is distributed on an "AS IS" BASIS, > - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > - * implied. > - * See the License for the specific language governing permissions and > - * limitations under the License. > - */ > - > -#ifndef __PCI_IOV_H > -#define __PCI_IOV_H > - > -struct pci_iov { > - struct phb *phb; > - struct pci_device *pd; > - struct pci_device *VFs; > - uint32_t pos; > - bool enabled; > - struct pci_cfg_reg_filter pcrf; > - > - uint16_t init_VFs; > - uint16_t total_VFs; > - uint16_t num_VFs; > - uint16_t offset; > - uint16_t stride; > -}; > - > -extern void pci_init_iov_cap(struct phb *phb, struct pci_device *pd); > - > -#endif > -- > 2.20.1 >
On 5/7/19 1:36 PM, Oliver wrote: > On Wed, May 1, 2019 at 6:30 PM Oliver O'Halloran <oohall@gmail.com> wrote: >> >> This feature was added a few years ago in response to a request to make >> the MaxPayloadSize (MPS) field of a Virtual Function match the MPS of the >> Physical Function that hosts it. >> >> The SR-IOV specification states the the MPS field of the VF is "ResvP". >> This indicates the VF will use whatever MPS is configured on the PF and >> that the field should be treated as a reserved field in the config space >> of the VF. In other words, a SR-IOV spec compliant VF should always return >> zero in the MPS field. Adding hacks in OPAL to make it non-zero is... >> misguided at best. >> >> Additionally, there is a bug in the way pci_device structures are handled >> by VFs that results in a crash on fast-reboot that occurs if VFs are >> enabled and then disabled prior to rebooting. This patch fixes the bug by >> removing the code entirely. This patch has no impact on SR-IOV support on >> the host operating system. >> >> Cc: Sergey Miroshnichenko <s.miroshnichenko@yadro.com> >> Cc: skiboot-stable@lists.ozlabs.org >> Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > > The test team smoke tested it and it came back clean. Merged to master > as of 22057f868f3b2b1fd02647a738f6da0858b5eb6c > Works fine on our setup as well (platforms/vesnin). And it lets me gladly drop two patches from the "core/pci: Sync VFs and the changes of bdfns between the firmware and the OS" patchset. Serge > Vasant, you can pick this up for stable now if you like. > >> --- >> core/Makefile.inc | 2 +- >> core/pci-iov.c | 265 ---------------------------------------------- >> core/pci.c | 2 - >> include/pci-iov.h | 37 ------- >> 4 files changed, 1 insertion(+), 305 deletions(-) >> delete mode 100644 core/pci-iov.c >> delete mode 100644 include/pci-iov.h >> >> diff --git a/core/Makefile.inc b/core/Makefile.inc >> index 3b4387081a62..21c12fb8db31 100644 >> --- a/core/Makefile.inc >> +++ b/core/Makefile.inc >> @@ -3,7 +3,7 @@ >> SUBDIRS += core >> CORE_OBJS = relocate.o console.o stack.o init.o chip.o mem_region.o >> CORE_OBJS += malloc.o lock.o cpu.o utils.o fdt.o opal.o interrupts.o timebase.o >> -CORE_OBJS += opal-msg.o pci.o pci-iov.o pci-virt.o pci-slot.o pcie-slot.o >> +CORE_OBJS += opal-msg.o pci.o pci-virt.o pci-slot.o pcie-slot.o >> CORE_OBJS += pci-opal.o fast-reboot.o device.o exceptions.o trace.o affinity.o >> CORE_OBJS += vpd.o hostservices.o platform.o nvram.o nvram-format.o hmi.o >> CORE_OBJS += console-log.o ipmi.o time-utils.o pel.o pool.o errorlog.o >> diff --git a/core/pci-iov.c b/core/pci-iov.c >> deleted file mode 100644 >> index 8fecebc1292a..000000000000 >> --- a/core/pci-iov.c >> +++ /dev/null >> @@ -1,265 +0,0 @@ >> -/* Copyright 2013-2016 IBM Corp. >> - * >> - * Licensed under the Apache License, Version 2.0 (the "License"); >> - * you may not use this file except in compliance with the License. >> - * You may obtain a copy of the License at >> - * >> - * http://www.apache.org/licenses/LICENSE-2.0 >> - * >> - * Unless required by applicable law or agreed to in writing, software >> - * distributed under the License is distributed on an "AS IS" BASIS, >> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> - * implied. >> - * See the License for the specific language governing permissions and >> - * limitations under the License. >> - */ >> - >> -#include <skiboot.h> >> -#include <pci.h> >> -#include <pci-cfg.h> >> -#include <pci-slot.h> >> -#include <pci-iov.h> >> - >> -/* >> - * Tackle the VF's MPS in PCIe capability. The field is read only. >> - * This function caches what is written and returns the cached >> - * MPS on read. >> - */ >> -static int64_t pci_iov_vf_devctl(void *dev, struct pci_cfg_reg_filter *pcrf, >> - uint32_t offset, uint32_t len, >> - uint32_t *data, bool write) >> -{ >> - struct pci_device *vf = (struct pci_device *)dev; >> - uint32_t pos = pci_cap(vf, PCI_CFG_CAP_ID_EXP, false); >> - uint8_t *pcache; >> - >> - if (offset != (pos + PCICAP_EXP_DEVCTL)) >> - return OPAL_PARTIAL; >> - >> - pcache = &pcrf->data[0]; >> - if (write) { >> - *pcache = ((uint8_t)(*data >> (8 * (4 - len)))) & >> - PCICAP_EXP_DEVCTL_MPS; >> - } else { >> - *data &= ~(PCICAP_EXP_DEVCTL_MPS << (8 * (4 - len))); >> - *data |= (((uint32_t)(*pcache & PCICAP_EXP_DEVCTL_MPS)) >> - << (8 * (4 - len))); >> - } >> - >> - return OPAL_SUCCESS; >> -} >> - >> -static void pci_iov_vf_quirk(struct phb *phb, struct pci_device *vf) >> -{ >> - struct pci_cfg_reg_filter *pcrf; >> - uint32_t pos; >> - >> - if (!pci_has_cap(vf, PCI_CFG_CAP_ID_EXP, false)) >> - return; >> - >> - /* >> - * On Mellanox MT27500 Family [ConnectX-3], its VF's MPS field in >> - * the corresponding config register is readonly. The MPS for PF/VF >> - * are usually different. We are introducing a quirk to make them >> - * look same to avoid confusion. >> - */ >> - if (vf->vdid != 0x100315b3) >> - return; >> - >> - pos = pci_cap(vf, PCI_CFG_CAP_ID_EXP, false); >> - pcrf = pci_add_cfg_reg_filter(vf, pos + PCICAP_EXP_DEVCTL, 4, >> - PCI_REG_FLAG_MASK, pci_iov_vf_devctl); >> - if (!pcrf) >> - prlog(PR_WARNING, "%s: Missed DEVCTL filter on %04x:%02x:%02x.%01x\n", >> - __func__, phb->opal_id, (vf->bdfn >> 8), >> - ((vf->bdfn >> 3) & 0x1f), (vf->bdfn & 0x7)); >> -} >> - >> -/* >> - * Update the SRIOV parameters that change when the number of >> - * VFs is configured. >> - */ >> -static bool pci_iov_update_parameters(struct pci_iov *iov) >> -{ >> - struct phb *phb = iov->phb; >> - uint16_t bdfn = iov->pd->bdfn; >> - uint32_t pos = iov->pos; >> - uint16_t val; >> - bool enabled; >> - >> - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_CTRL, &val); >> - enabled = !!(val & PCIECAP_SRIOV_CTRL_VFE); >> - if (iov->enabled == enabled) >> - return false; >> - >> - if (enabled) { >> - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_INITIAL_VF, >> - &iov->init_VFs); >> - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_NUM_VF, >> - &iov->num_VFs); >> - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_VF_OFFSET, >> - &iov->offset); >> - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_VF_STRIDE, >> - &iov->stride); >> - } else { >> - iov->init_VFs = 0; >> - iov->num_VFs = 0; >> - iov->offset = 0; >> - iov->stride = 0; >> - } >> - >> - iov->enabled = enabled; >> - return true; >> -} >> - >> -static int64_t pci_iov_change(void *dev __unused, >> - struct pci_cfg_reg_filter *pcrf, >> - uint32_t offset __unused, >> - uint32_t len __unused, >> - uint32_t *data __unused, >> - bool write __unused) >> -{ >> - struct pci_iov *iov = (struct pci_iov *)pcrf->data; >> - struct phb *phb = iov->phb; >> - struct pci_device *pd = iov->pd; >> - struct pci_device *vf, *tmp; >> - uint32_t i; >> - bool changed; >> - >> - /* Update SRIOV variable parameters */ >> - changed = pci_iov_update_parameters(iov); >> - if (!changed) >> - return OPAL_PARTIAL; >> - >> - /* Remove all VFs that have been attached to the parent */ >> - if (!iov->enabled) { >> - list_for_each_safe(&pd->children, vf, tmp, link) >> - list_del(&vf->link); >> - return OPAL_PARTIAL; >> - } >> - >> - /* Initialize the VFs and attach them to parent */ >> - for (changed = false, i = 0; i < iov->num_VFs; i++) { >> - vf = &iov->VFs[i]; >> - vf->bdfn = pd->bdfn + iov->offset + iov->stride * i; >> - list_add_tail(&pd->children, &vf->link); >> - >> - /* >> - * We don't populate the capabilities again if they have >> - * been existing, to save time. Also, we need delay for >> - * 100ms before the VF's config space becomes ready. >> - */ >> - if (!pci_has_cap(vf, PCI_CFG_CAP_ID_EXP, false)) { >> - if (!changed) { >> - changed = !changed; >> - time_wait_ms(100); >> - } >> - >> - pci_init_capabilities(phb, vf); >> - pci_iov_vf_quirk(phb, vf); >> - } >> - >> - /* Call PHB hook */ >> - if (phb->ops->device_init) >> - phb->ops->device_init(phb, pd, NULL); >> - } >> - >> - return OPAL_PARTIAL; >> -} >> - >> -/* >> - * This function is called with disabled SRIOV capability. So the VF's >> - * config address isn't finalized and its config space isn't accessible. >> - */ >> -static void pci_iov_init_VF(struct pci_device *pd, struct pci_device *vf) >> -{ >> - vf->is_bridge = false; >> - vf->is_multifunction = false; >> - vf->is_vf = true; >> - vf->dev_type = PCIE_TYPE_ENDPOINT; >> - vf->scan_map = -1; >> - vf->vdid = pd->vdid; >> - vf->sub_vdid = pd->sub_vdid; >> - vf->class = pd->class; >> - vf->dn = NULL; >> - vf->slot = NULL; >> - vf->parent = pd; >> - vf->phb = pd->phb; >> - list_head_init(&vf->pcrf); >> - list_head_init(&vf->children); >> -} >> - >> -static void pci_free_iov_cap(void *data) >> -{ >> - struct pci_iov *iov = data; >> - free(iov->VFs); >> - free(iov); >> -} >> - >> -void pci_init_iov_cap(struct phb *phb, struct pci_device *pd) >> -{ >> - int64_t pos; >> - struct pci_iov *iov; >> - struct pci_cfg_reg_filter *pcrf; >> - uint32_t i; >> - >> - /* Search for SRIOV capability */ >> - if (!pci_has_cap(pd, PCI_CFG_CAP_ID_EXP, false)) >> - return; >> - >> - pos = pci_find_ecap(phb, pd->bdfn, PCIECAP_ID_SRIOV, NULL); >> - if (pos <= 0) >> - return; >> - >> - /* Allocate IOV */ >> - iov = zalloc(sizeof(*iov)); >> - if (!iov) { >> - prlog(PR_ERR, "%s: Cannot alloc IOV for %04x:%02x:%02x.%01x\n", >> - __func__, phb->opal_id, (pd->bdfn >> 8), >> - ((pd->bdfn >> 3) & 0x1f), (pd->bdfn & 0x7)); >> - return; >> - } >> - >> - /* Allocate VFs */ >> - pci_cfg_read16(phb, pd->bdfn, pos + PCIECAP_SRIOV_TOTAL_VF, >> - &iov->total_VFs); >> - iov->VFs = zalloc(sizeof(*iov->VFs) * iov->total_VFs); >> - if (!iov->VFs) { >> - prlog(PR_ERR, "%s: Cannot alloc %d VFs for %04x:%02x:%02x.%01x\n", >> - __func__, iov->total_VFs, phb->opal_id, >> - (pd->bdfn >> 8), ((pd->bdfn >> 3) & 0x1f), >> - (pd->bdfn & 0x7)); >> - free(iov); >> - return; >> - } >> - >> - /* Initialize VFs */ >> - for (i = 0; i < iov->total_VFs; i++) >> - pci_iov_init_VF(pd, &iov->VFs[i]); >> - >> - /* Register filter for enabling or disabling SRIOV capability */ >> - pcrf = pci_add_cfg_reg_filter(pd, pos + PCIECAP_SRIOV_CTRL, 2, >> - PCI_REG_FLAG_WRITE, pci_iov_change); >> - if (!pcrf) { >> - prlog(PR_ERR, "%s: Cannot set filter on %04x:%02x:%02x.%01x\n", >> - __func__, phb->opal_id, (pd->bdfn >> 8), >> - ((pd->bdfn >> 3) & 0x1f), (pd->bdfn & 0x7)); >> - free(iov->VFs); >> - free(iov); >> - return; >> - } >> - >> - /* Associate filter and IOV capability */ >> - pcrf->data = (void *)iov; >> - >> - /* >> - * Retrieve the number of VFs and other information if applicable. >> - * Register the SRIOV capability in the mean while. >> - */ >> - iov->phb = phb; >> - iov->pd = pd; >> - iov->pos = pos; >> - iov->enabled = false; >> - pci_iov_update_parameters(iov); >> - pci_set_cap(pd, PCIECAP_ID_SRIOV, pos, iov, pci_free_iov_cap, true); >> -} >> diff --git a/core/pci.c b/core/pci.c >> index ee563c2fc577..02f5caa8d901 100644 >> --- a/core/pci.c >> +++ b/core/pci.c >> @@ -18,7 +18,6 @@ >> #include <cpu.h> >> #include <pci.h> >> #include <pci-cfg.h> >> -#include <pci-iov.h> >> #include <pci-slot.h> >> #include <pci-quirk.h> >> #include <timebase.h> >> @@ -193,7 +192,6 @@ void pci_init_capabilities(struct phb *phb, struct pci_device *pd) >> { >> pci_init_pcie_cap(phb, pd); >> pci_init_aer_cap(phb, pd); >> - pci_init_iov_cap(phb, pd); >> pci_init_pm_cap(phb, pd); >> } >> >> diff --git a/include/pci-iov.h b/include/pci-iov.h >> deleted file mode 100644 >> index 787b2cd1da69..000000000000 >> --- a/include/pci-iov.h >> +++ /dev/null >> @@ -1,37 +0,0 @@ >> -/* Copyright 2013-2016 IBM Corp. >> - * >> - * Licensed under the Apache License, Version 2.0 (the "License"); >> - * you may not use this file except in compliance with the License. >> - * You may obtain a copy of the License at >> - * >> - * http://www.apache.org/licenses/LICENSE-2.0 >> - * >> - * Unless required by applicable law or agreed to in writing, software >> - * distributed under the License is distributed on an "AS IS" BASIS, >> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> - * implied. >> - * See the License for the specific language governing permissions and >> - * limitations under the License. >> - */ >> - >> -#ifndef __PCI_IOV_H >> -#define __PCI_IOV_H >> - >> -struct pci_iov { >> - struct phb *phb; >> - struct pci_device *pd; >> - struct pci_device *VFs; >> - uint32_t pos; >> - bool enabled; >> - struct pci_cfg_reg_filter pcrf; >> - >> - uint16_t init_VFs; >> - uint16_t total_VFs; >> - uint16_t num_VFs; >> - uint16_t offset; >> - uint16_t stride; >> -}; >> - >> -extern void pci_init_iov_cap(struct phb *phb, struct pci_device *pd); >> - >> -#endif >> -- >> 2.20.1 >>
diff --git a/core/Makefile.inc b/core/Makefile.inc index 3b4387081a62..21c12fb8db31 100644 --- a/core/Makefile.inc +++ b/core/Makefile.inc @@ -3,7 +3,7 @@ SUBDIRS += core CORE_OBJS = relocate.o console.o stack.o init.o chip.o mem_region.o CORE_OBJS += malloc.o lock.o cpu.o utils.o fdt.o opal.o interrupts.o timebase.o -CORE_OBJS += opal-msg.o pci.o pci-iov.o pci-virt.o pci-slot.o pcie-slot.o +CORE_OBJS += opal-msg.o pci.o pci-virt.o pci-slot.o pcie-slot.o CORE_OBJS += pci-opal.o fast-reboot.o device.o exceptions.o trace.o affinity.o CORE_OBJS += vpd.o hostservices.o platform.o nvram.o nvram-format.o hmi.o CORE_OBJS += console-log.o ipmi.o time-utils.o pel.o pool.o errorlog.o diff --git a/core/pci-iov.c b/core/pci-iov.c deleted file mode 100644 index 8fecebc1292a..000000000000 --- a/core/pci-iov.c +++ /dev/null @@ -1,265 +0,0 @@ -/* Copyright 2013-2016 IBM Corp. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or - * implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include <skiboot.h> -#include <pci.h> -#include <pci-cfg.h> -#include <pci-slot.h> -#include <pci-iov.h> - -/* - * Tackle the VF's MPS in PCIe capability. The field is read only. - * This function caches what is written and returns the cached - * MPS on read. - */ -static int64_t pci_iov_vf_devctl(void *dev, struct pci_cfg_reg_filter *pcrf, - uint32_t offset, uint32_t len, - uint32_t *data, bool write) -{ - struct pci_device *vf = (struct pci_device *)dev; - uint32_t pos = pci_cap(vf, PCI_CFG_CAP_ID_EXP, false); - uint8_t *pcache; - - if (offset != (pos + PCICAP_EXP_DEVCTL)) - return OPAL_PARTIAL; - - pcache = &pcrf->data[0]; - if (write) { - *pcache = ((uint8_t)(*data >> (8 * (4 - len)))) & - PCICAP_EXP_DEVCTL_MPS; - } else { - *data &= ~(PCICAP_EXP_DEVCTL_MPS << (8 * (4 - len))); - *data |= (((uint32_t)(*pcache & PCICAP_EXP_DEVCTL_MPS)) - << (8 * (4 - len))); - } - - return OPAL_SUCCESS; -} - -static void pci_iov_vf_quirk(struct phb *phb, struct pci_device *vf) -{ - struct pci_cfg_reg_filter *pcrf; - uint32_t pos; - - if (!pci_has_cap(vf, PCI_CFG_CAP_ID_EXP, false)) - return; - - /* - * On Mellanox MT27500 Family [ConnectX-3], its VF's MPS field in - * the corresponding config register is readonly. The MPS for PF/VF - * are usually different. We are introducing a quirk to make them - * look same to avoid confusion. - */ - if (vf->vdid != 0x100315b3) - return; - - pos = pci_cap(vf, PCI_CFG_CAP_ID_EXP, false); - pcrf = pci_add_cfg_reg_filter(vf, pos + PCICAP_EXP_DEVCTL, 4, - PCI_REG_FLAG_MASK, pci_iov_vf_devctl); - if (!pcrf) - prlog(PR_WARNING, "%s: Missed DEVCTL filter on %04x:%02x:%02x.%01x\n", - __func__, phb->opal_id, (vf->bdfn >> 8), - ((vf->bdfn >> 3) & 0x1f), (vf->bdfn & 0x7)); -} - -/* - * Update the SRIOV parameters that change when the number of - * VFs is configured. - */ -static bool pci_iov_update_parameters(struct pci_iov *iov) -{ - struct phb *phb = iov->phb; - uint16_t bdfn = iov->pd->bdfn; - uint32_t pos = iov->pos; - uint16_t val; - bool enabled; - - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_CTRL, &val); - enabled = !!(val & PCIECAP_SRIOV_CTRL_VFE); - if (iov->enabled == enabled) - return false; - - if (enabled) { - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_INITIAL_VF, - &iov->init_VFs); - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_NUM_VF, - &iov->num_VFs); - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_VF_OFFSET, - &iov->offset); - pci_cfg_read16(phb, bdfn, pos + PCIECAP_SRIOV_VF_STRIDE, - &iov->stride); - } else { - iov->init_VFs = 0; - iov->num_VFs = 0; - iov->offset = 0; - iov->stride = 0; - } - - iov->enabled = enabled; - return true; -} - -static int64_t pci_iov_change(void *dev __unused, - struct pci_cfg_reg_filter *pcrf, - uint32_t offset __unused, - uint32_t len __unused, - uint32_t *data __unused, - bool write __unused) -{ - struct pci_iov *iov = (struct pci_iov *)pcrf->data; - struct phb *phb = iov->phb; - struct pci_device *pd = iov->pd; - struct pci_device *vf, *tmp; - uint32_t i; - bool changed; - - /* Update SRIOV variable parameters */ - changed = pci_iov_update_parameters(iov); - if (!changed) - return OPAL_PARTIAL; - - /* Remove all VFs that have been attached to the parent */ - if (!iov->enabled) { - list_for_each_safe(&pd->children, vf, tmp, link) - list_del(&vf->link); - return OPAL_PARTIAL; - } - - /* Initialize the VFs and attach them to parent */ - for (changed = false, i = 0; i < iov->num_VFs; i++) { - vf = &iov->VFs[i]; - vf->bdfn = pd->bdfn + iov->offset + iov->stride * i; - list_add_tail(&pd->children, &vf->link); - - /* - * We don't populate the capabilities again if they have - * been existing, to save time. Also, we need delay for - * 100ms before the VF's config space becomes ready. - */ - if (!pci_has_cap(vf, PCI_CFG_CAP_ID_EXP, false)) { - if (!changed) { - changed = !changed; - time_wait_ms(100); - } - - pci_init_capabilities(phb, vf); - pci_iov_vf_quirk(phb, vf); - } - - /* Call PHB hook */ - if (phb->ops->device_init) - phb->ops->device_init(phb, pd, NULL); - } - - return OPAL_PARTIAL; -} - -/* - * This function is called with disabled SRIOV capability. So the VF's - * config address isn't finalized and its config space isn't accessible. - */ -static void pci_iov_init_VF(struct pci_device *pd, struct pci_device *vf) -{ - vf->is_bridge = false; - vf->is_multifunction = false; - vf->is_vf = true; - vf->dev_type = PCIE_TYPE_ENDPOINT; - vf->scan_map = -1; - vf->vdid = pd->vdid; - vf->sub_vdid = pd->sub_vdid; - vf->class = pd->class; - vf->dn = NULL; - vf->slot = NULL; - vf->parent = pd; - vf->phb = pd->phb; - list_head_init(&vf->pcrf); - list_head_init(&vf->children); -} - -static void pci_free_iov_cap(void *data) -{ - struct pci_iov *iov = data; - free(iov->VFs); - free(iov); -} - -void pci_init_iov_cap(struct phb *phb, struct pci_device *pd) -{ - int64_t pos; - struct pci_iov *iov; - struct pci_cfg_reg_filter *pcrf; - uint32_t i; - - /* Search for SRIOV capability */ - if (!pci_has_cap(pd, PCI_CFG_CAP_ID_EXP, false)) - return; - - pos = pci_find_ecap(phb, pd->bdfn, PCIECAP_ID_SRIOV, NULL); - if (pos <= 0) - return; - - /* Allocate IOV */ - iov = zalloc(sizeof(*iov)); - if (!iov) { - prlog(PR_ERR, "%s: Cannot alloc IOV for %04x:%02x:%02x.%01x\n", - __func__, phb->opal_id, (pd->bdfn >> 8), - ((pd->bdfn >> 3) & 0x1f), (pd->bdfn & 0x7)); - return; - } - - /* Allocate VFs */ - pci_cfg_read16(phb, pd->bdfn, pos + PCIECAP_SRIOV_TOTAL_VF, - &iov->total_VFs); - iov->VFs = zalloc(sizeof(*iov->VFs) * iov->total_VFs); - if (!iov->VFs) { - prlog(PR_ERR, "%s: Cannot alloc %d VFs for %04x:%02x:%02x.%01x\n", - __func__, iov->total_VFs, phb->opal_id, - (pd->bdfn >> 8), ((pd->bdfn >> 3) & 0x1f), - (pd->bdfn & 0x7)); - free(iov); - return; - } - - /* Initialize VFs */ - for (i = 0; i < iov->total_VFs; i++) - pci_iov_init_VF(pd, &iov->VFs[i]); - - /* Register filter for enabling or disabling SRIOV capability */ - pcrf = pci_add_cfg_reg_filter(pd, pos + PCIECAP_SRIOV_CTRL, 2, - PCI_REG_FLAG_WRITE, pci_iov_change); - if (!pcrf) { - prlog(PR_ERR, "%s: Cannot set filter on %04x:%02x:%02x.%01x\n", - __func__, phb->opal_id, (pd->bdfn >> 8), - ((pd->bdfn >> 3) & 0x1f), (pd->bdfn & 0x7)); - free(iov->VFs); - free(iov); - return; - } - - /* Associate filter and IOV capability */ - pcrf->data = (void *)iov; - - /* - * Retrieve the number of VFs and other information if applicable. - * Register the SRIOV capability in the mean while. - */ - iov->phb = phb; - iov->pd = pd; - iov->pos = pos; - iov->enabled = false; - pci_iov_update_parameters(iov); - pci_set_cap(pd, PCIECAP_ID_SRIOV, pos, iov, pci_free_iov_cap, true); -} diff --git a/core/pci.c b/core/pci.c index ee563c2fc577..02f5caa8d901 100644 --- a/core/pci.c +++ b/core/pci.c @@ -18,7 +18,6 @@ #include <cpu.h> #include <pci.h> #include <pci-cfg.h> -#include <pci-iov.h> #include <pci-slot.h> #include <pci-quirk.h> #include <timebase.h> @@ -193,7 +192,6 @@ void pci_init_capabilities(struct phb *phb, struct pci_device *pd) { pci_init_pcie_cap(phb, pd); pci_init_aer_cap(phb, pd); - pci_init_iov_cap(phb, pd); pci_init_pm_cap(phb, pd); } diff --git a/include/pci-iov.h b/include/pci-iov.h deleted file mode 100644 index 787b2cd1da69..000000000000 --- a/include/pci-iov.h +++ /dev/null @@ -1,37 +0,0 @@ -/* Copyright 2013-2016 IBM Corp. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or - * implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef __PCI_IOV_H -#define __PCI_IOV_H - -struct pci_iov { - struct phb *phb; - struct pci_device *pd; - struct pci_device *VFs; - uint32_t pos; - bool enabled; - struct pci_cfg_reg_filter pcrf; - - uint16_t init_VFs; - uint16_t total_VFs; - uint16_t num_VFs; - uint16_t offset; - uint16_t stride; -}; - -extern void pci_init_iov_cap(struct phb *phb, struct pci_device *pd); - -#endif
This feature was added a few years ago in response to a request to make the MaxPayloadSize (MPS) field of a Virtual Function match the MPS of the Physical Function that hosts it. The SR-IOV specification states the the MPS field of the VF is "ResvP". This indicates the VF will use whatever MPS is configured on the PF and that the field should be treated as a reserved field in the config space of the VF. In other words, a SR-IOV spec compliant VF should always return zero in the MPS field. Adding hacks in OPAL to make it non-zero is... misguided at best. Additionally, there is a bug in the way pci_device structures are handled by VFs that results in a crash on fast-reboot that occurs if VFs are enabled and then disabled prior to rebooting. This patch fixes the bug by removing the code entirely. This patch has no impact on SR-IOV support on the host operating system. Cc: Sergey Miroshnichenko <s.miroshnichenko@yadro.com> Cc: skiboot-stable@lists.ozlabs.org Signed-off-by: Oliver O'Halloran <oohall@gmail.com> --- core/Makefile.inc | 2 +- core/pci-iov.c | 265 ---------------------------------------------- core/pci.c | 2 - include/pci-iov.h | 37 ------- 4 files changed, 1 insertion(+), 305 deletions(-) delete mode 100644 core/pci-iov.c delete mode 100644 include/pci-iov.h