* [PATCH v5 0/3] vfio/pci: Restore MMIO access for s390 detached VFs @ 2020-09-10 14:59 Matthew Rosato 2020-09-10 14:59 ` [PATCH v5 1/3] PCI/IOV: Mark VFs as not implementing PCI_COMMAND_MEMORY Matthew Rosato ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Matthew Rosato @ 2020-09-10 14:59 UTC (permalink / raw) To: alex.williamson, bhelgaas Cc: schnelle, pmorel, mpe, oohall, cohuck, kevin.tian, hca, gor, borntraeger, linux-s390, linux-kernel, kvm, linux-pci Changes from v4: - Switch from dev_flags to a bitfield - Scrubbed improper use of MSE acronym - Restored the fixes tag to patch 3 (but the other 2 patches are now pre-reqs -- cc stable 5.8?) Since commit abafbc551fdd ("vfio-pci: Invalidate mmaps and block MMIO access on disabled memory") VFIO now rejects guest MMIO access when the PCI_COMMAND_MEMORY bit is OFF. This is however not the case for VFs (fixed in commit ebfa440ce38b ("vfio/pci: Fix SR-IOV VF handling with MMIO blocking")). Furthermore, on s390 where we always run with at least a bare-metal hypervisor (LPAR) PCI_COMMAND_MEMORY, unlike Device/ Vendor IDs and BARs, is not emulated when VFs are passed-through to the OS independently. Based upon Bjorn's most recent comment [1], I investigated the notion of setting is_virtfn=1 for VFs passed-through to Linux and not linked to a parent PF (referred to as a 'detached VF' in my prior post). However, we rapidly run into issues on how to treat an is_virtfn device with no linked PF. Further complicating the issue is when you consider the guest kernel has a passed-through VF but has CONFIG_PCI_IOV=n as in many locations is_virtfn checking is ifdef'd out altogether and the device is assumed to be an independent PCI function. The decision made by VFIO whether to require or emulate a PCI feature (in this case PCI_COMMAND_MEMORY) is based upon the knowledge it has about the device, including implicit expectations of what/is not emulated below VFIO. (ex: is it safe to read vendor/id from config space?) -- Our firmware layer attempts similar behavior by emulating things such as vendor/id/BAR access - without these an unlinked VF would not be usable. But what is or is not emulated by the layer below may be different based upon which entity is providing the emulation (vfio, LPAR, some other hypervisor) So, the proposal here aims to fix the immediate issue of s390 pass-through VFs becoming suddenly unusable by vfio by using a new bit to identify a VF feature that we know is hardwired to 0 for any VF (PCI_COMMAND_MEMORY) and de-coupling the need for emulating PCI_COMMAND_MEMORY from the is_virtfn flag. The exact scope of is_virtfn and physfn for bare-metal vs guest scenarios and identifying what features are / are not emulated by the lower-level hypervisors is a much bigger discussion independent of this limited proposal. [1]: https://marc.info/?l=linux-pci&m=159856041930022&w=2 Matthew Rosato (3): PCI/IOV: Mark VFs as not implementing PCI_COMMAND_MEMORY s390/pci: Mark all VFs as not implementing PCI_COMMAND_MEMORY vfio/pci: Decouple PCI_COMMAND_MEMORY bit checks from is_virtfn arch/s390/pci/pci_bus.c | 5 +++-- drivers/pci/iov.c | 1 + drivers/vfio/pci/vfio_pci_config.c | 24 ++++++++++++++---------- include/linux/pci.h | 1 + 4 files changed, 19 insertions(+), 12 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 1/3] PCI/IOV: Mark VFs as not implementing PCI_COMMAND_MEMORY 2020-09-10 14:59 [PATCH v5 0/3] vfio/pci: Restore MMIO access for s390 detached VFs Matthew Rosato @ 2020-09-10 14:59 ` Matthew Rosato 2020-09-16 21:55 ` Bjorn Helgaas 2020-09-10 14:59 ` [PATCH v5 2/3] s390/pci: Mark all " Matthew Rosato 2020-09-10 14:59 ` [PATCH v5 3/3] vfio/pci: Decouple PCI_COMMAND_MEMORY bit checks from is_virtfn Matthew Rosato 2 siblings, 1 reply; 9+ messages in thread From: Matthew Rosato @ 2020-09-10 14:59 UTC (permalink / raw) To: alex.williamson, bhelgaas Cc: schnelle, pmorel, mpe, oohall, cohuck, kevin.tian, hca, gor, borntraeger, linux-s390, linux-kernel, kvm, linux-pci For VFs, the Memory Space Enable bit in the Command Register is hard-wired to 0. Add a new bit to signify devices where the Command Register Memory Space Enable bit does not control the device's response to MMIO accesses. Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> --- drivers/pci/iov.c | 1 + include/linux/pci.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index b37e08c..4afd4ee 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -180,6 +180,7 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id) virtfn->device = iov->vf_device; virtfn->is_virtfn = 1; virtfn->physfn = pci_dev_get(dev); + virtfn->no_command_memory = 1; if (id == 0) pci_read_vf_config_common(virtfn); diff --git a/include/linux/pci.h b/include/linux/pci.h index 8355306..3ff72312 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -445,6 +445,7 @@ struct pci_dev { unsigned int is_probed:1; /* Device probing in progress */ unsigned int link_active_reporting:1;/* Device capable of reporting link active */ unsigned int no_vf_scan:1; /* Don't scan for VFs after IOV enablement */ + unsigned int no_command_memory:1; /* No PCI_COMMAND_MEMORY */ pci_dev_flags_t dev_flags; atomic_t enable_cnt; /* pci_enable_device has been called */ -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 1/3] PCI/IOV: Mark VFs as not implementing PCI_COMMAND_MEMORY 2020-09-10 14:59 ` [PATCH v5 1/3] PCI/IOV: Mark VFs as not implementing PCI_COMMAND_MEMORY Matthew Rosato @ 2020-09-16 21:55 ` Bjorn Helgaas 0 siblings, 0 replies; 9+ messages in thread From: Bjorn Helgaas @ 2020-09-16 21:55 UTC (permalink / raw) To: Matthew Rosato Cc: alex.williamson, bhelgaas, schnelle, pmorel, mpe, oohall, cohuck, kevin.tian, hca, gor, borntraeger, linux-s390, linux-kernel, kvm, linux-pci On Thu, Sep 10, 2020 at 10:59:55AM -0400, Matthew Rosato wrote: > For VFs, the Memory Space Enable bit in the Command Register is > hard-wired to 0. > > Add a new bit to signify devices where the Command Register Memory > Space Enable bit does not control the device's response to MMIO > accesses. > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/pci/iov.c | 1 + > include/linux/pci.h | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c > index b37e08c..4afd4ee 100644 > --- a/drivers/pci/iov.c > +++ b/drivers/pci/iov.c > @@ -180,6 +180,7 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id) > virtfn->device = iov->vf_device; > virtfn->is_virtfn = 1; > virtfn->physfn = pci_dev_get(dev); > + virtfn->no_command_memory = 1; > > if (id == 0) > pci_read_vf_config_common(virtfn); > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 8355306..3ff72312 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -445,6 +445,7 @@ struct pci_dev { > unsigned int is_probed:1; /* Device probing in progress */ > unsigned int link_active_reporting:1;/* Device capable of reporting link active */ > unsigned int no_vf_scan:1; /* Don't scan for VFs after IOV enablement */ > + unsigned int no_command_memory:1; /* No PCI_COMMAND_MEMORY */ > pci_dev_flags_t dev_flags; > atomic_t enable_cnt; /* pci_enable_device has been called */ > > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 2/3] s390/pci: Mark all VFs as not implementing PCI_COMMAND_MEMORY 2020-09-10 14:59 [PATCH v5 0/3] vfio/pci: Restore MMIO access for s390 detached VFs Matthew Rosato 2020-09-10 14:59 ` [PATCH v5 1/3] PCI/IOV: Mark VFs as not implementing PCI_COMMAND_MEMORY Matthew Rosato @ 2020-09-10 14:59 ` Matthew Rosato 2020-09-10 14:59 ` [PATCH v5 3/3] vfio/pci: Decouple PCI_COMMAND_MEMORY bit checks from is_virtfn Matthew Rosato 2 siblings, 0 replies; 9+ messages in thread From: Matthew Rosato @ 2020-09-10 14:59 UTC (permalink / raw) To: alex.williamson, bhelgaas Cc: schnelle, pmorel, mpe, oohall, cohuck, kevin.tian, hca, gor, borntraeger, linux-s390, linux-kernel, kvm, linux-pci For s390 we can have VFs that are passed-through without the associated PF. Firmware provides an emulation layer to allow these devices to operate independently, but is missing emulation of the Memory Space Enable bit. For these as well as linked VFs, set no_command_memory which specifies these devices do not implement PCI_COMMAND_MEMORY. Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> --- arch/s390/pci/pci_bus.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/s390/pci/pci_bus.c b/arch/s390/pci/pci_bus.c index 5967f30..c93486a 100644 --- a/arch/s390/pci/pci_bus.c +++ b/arch/s390/pci/pci_bus.c @@ -197,9 +197,10 @@ void pcibios_bus_add_device(struct pci_dev *pdev) * With pdev->no_vf_scan the common PCI probing code does not * perform PF/VF linking. */ - if (zdev->vfn) + if (zdev->vfn) { zpci_bus_setup_virtfn(zdev->zbus, pdev, zdev->vfn); - + pdev->no_command_memory = 1; + } } static int zpci_bus_add_device(struct zpci_bus *zbus, struct zpci_dev *zdev) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v5 3/3] vfio/pci: Decouple PCI_COMMAND_MEMORY bit checks from is_virtfn 2020-09-10 14:59 [PATCH v5 0/3] vfio/pci: Restore MMIO access for s390 detached VFs Matthew Rosato 2020-09-10 14:59 ` [PATCH v5 1/3] PCI/IOV: Mark VFs as not implementing PCI_COMMAND_MEMORY Matthew Rosato 2020-09-10 14:59 ` [PATCH v5 2/3] s390/pci: Mark all " Matthew Rosato @ 2020-09-10 14:59 ` Matthew Rosato 2020-09-21 12:43 ` Matthew Rosato 2 siblings, 1 reply; 9+ messages in thread From: Matthew Rosato @ 2020-09-10 14:59 UTC (permalink / raw) To: alex.williamson, bhelgaas Cc: schnelle, pmorel, mpe, oohall, cohuck, kevin.tian, hca, gor, borntraeger, linux-s390, linux-kernel, kvm, linux-pci While it is true that devices with is_virtfn=1 will have a Memory Space Enable bit that is hard-wired to 0, this is not the only case where we see this behavior -- For example some bare-metal hypervisors lack Memory Space Enable bit emulation for devices not setting is_virtfn (s390). Fix this by instead checking for the newly-added no_command_memory bit which directly denotes the need for PCI_COMMAND_MEMORY emulation in vfio. Fixes: abafbc551fdd ("vfio-pci: Invalidate mmaps and block MMIO access on disabled memory") Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> --- drivers/vfio/pci/vfio_pci_config.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c index d98843f..5076d01 100644 --- a/drivers/vfio/pci/vfio_pci_config.c +++ b/drivers/vfio/pci/vfio_pci_config.c @@ -406,7 +406,7 @@ bool __vfio_pci_memory_enabled(struct vfio_pci_device *vdev) * PF SR-IOV capability, there's therefore no need to trigger * faults based on the virtual value. */ - return pdev->is_virtfn || (cmd & PCI_COMMAND_MEMORY); + return pdev->no_command_memory || (cmd & PCI_COMMAND_MEMORY); } /* @@ -520,8 +520,8 @@ static int vfio_basic_config_read(struct vfio_pci_device *vdev, int pos, count = vfio_default_config_read(vdev, pos, count, perm, offset, val); - /* Mask in virtual memory enable for SR-IOV devices */ - if (offset == PCI_COMMAND && vdev->pdev->is_virtfn) { + /* Mask in virtual memory enable */ + if (offset == PCI_COMMAND && vdev->pdev->no_command_memory) { u16 cmd = le16_to_cpu(*(__le16 *)&vdev->vconfig[PCI_COMMAND]); u32 tmp_val = le32_to_cpu(*val); @@ -589,9 +589,11 @@ static int vfio_basic_config_write(struct vfio_pci_device *vdev, int pos, * shows it disabled (phys_mem/io, then the device has * undergone some kind of backdoor reset and needs to be * restored before we allow it to enable the bars. - * SR-IOV devices will trigger this, but we catch them later + * SR-IOV devices will trigger this - for mem enable let's + * catch this now and for io enable it will be caught later */ - if ((new_mem && virt_mem && !phys_mem) || + if ((new_mem && virt_mem && !phys_mem && + !pdev->no_command_memory) || (new_io && virt_io && !phys_io) || vfio_need_bar_restore(vdev)) vfio_bar_restore(vdev); @@ -1734,12 +1736,14 @@ int vfio_config_init(struct vfio_pci_device *vdev) vconfig[PCI_INTERRUPT_PIN]); vconfig[PCI_INTERRUPT_PIN] = 0; /* Gratuitous for good VFs */ - + } + if (pdev->no_command_memory) { /* - * VFs do no implement the memory enable bit of the COMMAND - * register therefore we'll not have it set in our initial - * copy of config space after pci_enable_device(). For - * consistency with PFs, set the virtual enable bit here. + * VFs and devices that set pdev->no_command_memory do not + * implement the memory enable bit of the COMMAND register + * therefore we'll not have it set in our initial copy of + * config space after pci_enable_device(). For consistency + * with PFs, set the virtual enable bit here. */ *(__le16 *)&vconfig[PCI_COMMAND] |= cpu_to_le16(PCI_COMMAND_MEMORY); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 3/3] vfio/pci: Decouple PCI_COMMAND_MEMORY bit checks from is_virtfn 2020-09-10 14:59 ` [PATCH v5 3/3] vfio/pci: Decouple PCI_COMMAND_MEMORY bit checks from is_virtfn Matthew Rosato @ 2020-09-21 12:43 ` Matthew Rosato 2020-09-22 16:40 ` Alex Williamson 0 siblings, 1 reply; 9+ messages in thread From: Matthew Rosato @ 2020-09-21 12:43 UTC (permalink / raw) To: alex.williamson, bhelgaas Cc: schnelle, pmorel, mpe, oohall, cohuck, kevin.tian, hca, gor, borntraeger, linux-s390, linux-kernel, kvm, linux-pci On 9/10/20 10:59 AM, Matthew Rosato wrote: > While it is true that devices with is_virtfn=1 will have a Memory Space > Enable bit that is hard-wired to 0, this is not the only case where we > see this behavior -- For example some bare-metal hypervisors lack > Memory Space Enable bit emulation for devices not setting is_virtfn > (s390). Fix this by instead checking for the newly-added > no_command_memory bit which directly denotes the need for > PCI_COMMAND_MEMORY emulation in vfio. > > Fixes: abafbc551fdd ("vfio-pci: Invalidate mmaps and block MMIO access on disabled memory") > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com> > Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> Polite ping on this patch as the other 2 have now received maintainer ACKs or reviews. I'm concerned about this popping up in distros as abafbc551fdd was a CVE fix. Related, see question from the cover: - Restored the fixes tag to patch 3 (but the other 2 patches are now pre-reqs -- cc stable 5.8?) Thanks, Matt > --- > drivers/vfio/pci/vfio_pci_config.c | 24 ++++++++++++++---------- > 1 file changed, 14 insertions(+), 10 deletions(-) > > diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c > index d98843f..5076d01 100644 > --- a/drivers/vfio/pci/vfio_pci_config.c > +++ b/drivers/vfio/pci/vfio_pci_config.c > @@ -406,7 +406,7 @@ bool __vfio_pci_memory_enabled(struct vfio_pci_device *vdev) > * PF SR-IOV capability, there's therefore no need to trigger > * faults based on the virtual value. > */ > - return pdev->is_virtfn || (cmd & PCI_COMMAND_MEMORY); > + return pdev->no_command_memory || (cmd & PCI_COMMAND_MEMORY); > } > > /* > @@ -520,8 +520,8 @@ static int vfio_basic_config_read(struct vfio_pci_device *vdev, int pos, > > count = vfio_default_config_read(vdev, pos, count, perm, offset, val); > > - /* Mask in virtual memory enable for SR-IOV devices */ > - if (offset == PCI_COMMAND && vdev->pdev->is_virtfn) { > + /* Mask in virtual memory enable */ > + if (offset == PCI_COMMAND && vdev->pdev->no_command_memory) { > u16 cmd = le16_to_cpu(*(__le16 *)&vdev->vconfig[PCI_COMMAND]); > u32 tmp_val = le32_to_cpu(*val); > > @@ -589,9 +589,11 @@ static int vfio_basic_config_write(struct vfio_pci_device *vdev, int pos, > * shows it disabled (phys_mem/io, then the device has > * undergone some kind of backdoor reset and needs to be > * restored before we allow it to enable the bars. > - * SR-IOV devices will trigger this, but we catch them later > + * SR-IOV devices will trigger this - for mem enable let's > + * catch this now and for io enable it will be caught later > */ > - if ((new_mem && virt_mem && !phys_mem) || > + if ((new_mem && virt_mem && !phys_mem && > + !pdev->no_command_memory) || > (new_io && virt_io && !phys_io) || > vfio_need_bar_restore(vdev)) > vfio_bar_restore(vdev); > @@ -1734,12 +1736,14 @@ int vfio_config_init(struct vfio_pci_device *vdev) > vconfig[PCI_INTERRUPT_PIN]); > > vconfig[PCI_INTERRUPT_PIN] = 0; /* Gratuitous for good VFs */ > - > + } > + if (pdev->no_command_memory) { > /* > - * VFs do no implement the memory enable bit of the COMMAND > - * register therefore we'll not have it set in our initial > - * copy of config space after pci_enable_device(). For > - * consistency with PFs, set the virtual enable bit here. > + * VFs and devices that set pdev->no_command_memory do not > + * implement the memory enable bit of the COMMAND register > + * therefore we'll not have it set in our initial copy of > + * config space after pci_enable_device(). For consistency > + * with PFs, set the virtual enable bit here. > */ > *(__le16 *)&vconfig[PCI_COMMAND] |= > cpu_to_le16(PCI_COMMAND_MEMORY); > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 3/3] vfio/pci: Decouple PCI_COMMAND_MEMORY bit checks from is_virtfn 2020-09-21 12:43 ` Matthew Rosato @ 2020-09-22 16:40 ` Alex Williamson 2020-09-22 16:57 ` Matthew Rosato 2020-09-23 8:51 ` Pierre Morel 0 siblings, 2 replies; 9+ messages in thread From: Alex Williamson @ 2020-09-22 16:40 UTC (permalink / raw) To: Matthew Rosato Cc: bhelgaas, schnelle, pmorel, mpe, oohall, cohuck, kevin.tian, hca, gor, borntraeger, linux-s390, linux-kernel, kvm, linux-pci On Mon, 21 Sep 2020 08:43:29 -0400 Matthew Rosato <mjrosato@linux.ibm.com> wrote: > On 9/10/20 10:59 AM, Matthew Rosato wrote: > > While it is true that devices with is_virtfn=1 will have a Memory Space > > Enable bit that is hard-wired to 0, this is not the only case where we > > see this behavior -- For example some bare-metal hypervisors lack > > Memory Space Enable bit emulation for devices not setting is_virtfn > > (s390). Fix this by instead checking for the newly-added > > no_command_memory bit which directly denotes the need for > > PCI_COMMAND_MEMORY emulation in vfio. > > > > Fixes: abafbc551fdd ("vfio-pci: Invalidate mmaps and block MMIO access on disabled memory") > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > > Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com> > > Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> > > Polite ping on this patch as the other 2 have now received maintainer > ACKs or reviews. I'm concerned about this popping up in distros as > abafbc551fdd was a CVE fix. Related, see question from the cover: > > - Restored the fixes tag to patch 3 (but the other 2 patches are > now pre-reqs -- cc stable 5.8?) I've got these queued in my local branch which I'll push to next for v5.10. I'm thinking that perhaps the right thing would be to add the fixes tag to all three patches, otherwise I could see that the PCI/VF change might get picked as a dependency, but not the s390 specific one. Does this sound correct to everyone? Thanks, Alex > > --- > > drivers/vfio/pci/vfio_pci_config.c | 24 ++++++++++++++---------- > > 1 file changed, 14 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c > > index d98843f..5076d01 100644 > > --- a/drivers/vfio/pci/vfio_pci_config.c > > +++ b/drivers/vfio/pci/vfio_pci_config.c > > @@ -406,7 +406,7 @@ bool __vfio_pci_memory_enabled(struct vfio_pci_device *vdev) > > * PF SR-IOV capability, there's therefore no need to trigger > > * faults based on the virtual value. > > */ > > - return pdev->is_virtfn || (cmd & PCI_COMMAND_MEMORY); > > + return pdev->no_command_memory || (cmd & PCI_COMMAND_MEMORY); > > } > > > > /* > > @@ -520,8 +520,8 @@ static int vfio_basic_config_read(struct vfio_pci_device *vdev, int pos, > > > > count = vfio_default_config_read(vdev, pos, count, perm, offset, val); > > > > - /* Mask in virtual memory enable for SR-IOV devices */ > > - if (offset == PCI_COMMAND && vdev->pdev->is_virtfn) { > > + /* Mask in virtual memory enable */ > > + if (offset == PCI_COMMAND && vdev->pdev->no_command_memory) { > > u16 cmd = le16_to_cpu(*(__le16 *)&vdev->vconfig[PCI_COMMAND]); > > u32 tmp_val = le32_to_cpu(*val); > > > > @@ -589,9 +589,11 @@ static int vfio_basic_config_write(struct vfio_pci_device *vdev, int pos, > > * shows it disabled (phys_mem/io, then the device has > > * undergone some kind of backdoor reset and needs to be > > * restored before we allow it to enable the bars. > > - * SR-IOV devices will trigger this, but we catch them later > > + * SR-IOV devices will trigger this - for mem enable let's > > + * catch this now and for io enable it will be caught later > > */ > > - if ((new_mem && virt_mem && !phys_mem) || > > + if ((new_mem && virt_mem && !phys_mem && > > + !pdev->no_command_memory) || > > (new_io && virt_io && !phys_io) || > > vfio_need_bar_restore(vdev)) > > vfio_bar_restore(vdev); > > @@ -1734,12 +1736,14 @@ int vfio_config_init(struct vfio_pci_device *vdev) > > vconfig[PCI_INTERRUPT_PIN]); > > > > vconfig[PCI_INTERRUPT_PIN] = 0; /* Gratuitous for good VFs */ > > - > > + } > > + if (pdev->no_command_memory) { > > /* > > - * VFs do no implement the memory enable bit of the COMMAND > > - * register therefore we'll not have it set in our initial > > - * copy of config space after pci_enable_device(). For > > - * consistency with PFs, set the virtual enable bit here. > > + * VFs and devices that set pdev->no_command_memory do not > > + * implement the memory enable bit of the COMMAND register > > + * therefore we'll not have it set in our initial copy of > > + * config space after pci_enable_device(). For consistency > > + * with PFs, set the virtual enable bit here. > > */ > > *(__le16 *)&vconfig[PCI_COMMAND] |= > > cpu_to_le16(PCI_COMMAND_MEMORY); > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 3/3] vfio/pci: Decouple PCI_COMMAND_MEMORY bit checks from is_virtfn 2020-09-22 16:40 ` Alex Williamson @ 2020-09-22 16:57 ` Matthew Rosato 2020-09-23 8:51 ` Pierre Morel 1 sibling, 0 replies; 9+ messages in thread From: Matthew Rosato @ 2020-09-22 16:57 UTC (permalink / raw) To: Alex Williamson Cc: bhelgaas, schnelle, pmorel, mpe, oohall, cohuck, kevin.tian, hca, gor, borntraeger, linux-s390, linux-kernel, kvm, linux-pci On 9/22/20 12:40 PM, Alex Williamson wrote: > On Mon, 21 Sep 2020 08:43:29 -0400 > Matthew Rosato <mjrosato@linux.ibm.com> wrote: > >> On 9/10/20 10:59 AM, Matthew Rosato wrote: >>> While it is true that devices with is_virtfn=1 will have a Memory Space >>> Enable bit that is hard-wired to 0, this is not the only case where we >>> see this behavior -- For example some bare-metal hypervisors lack >>> Memory Space Enable bit emulation for devices not setting is_virtfn >>> (s390). Fix this by instead checking for the newly-added >>> no_command_memory bit which directly denotes the need for >>> PCI_COMMAND_MEMORY emulation in vfio. >>> >>> Fixes: abafbc551fdd ("vfio-pci: Invalidate mmaps and block MMIO access on disabled memory") >>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >>> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com> >>> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> >> >> Polite ping on this patch as the other 2 have now received maintainer >> ACKs or reviews. I'm concerned about this popping up in distros as >> abafbc551fdd was a CVE fix. Related, see question from the cover: >> >> - Restored the fixes tag to patch 3 (but the other 2 patches are >> now pre-reqs -- cc stable 5.8?) > > I've got these queued in my local branch which I'll push to next for > v5.10. I'm thinking that perhaps the right thing would be to add the > fixes tag to all three patches, otherwise I could see that the PCI/VF > change might get picked as a dependency, but not the s390 specific one. > Does this sound correct to everyone? Thanks, Sounds good to me. Thanks! ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 3/3] vfio/pci: Decouple PCI_COMMAND_MEMORY bit checks from is_virtfn 2020-09-22 16:40 ` Alex Williamson 2020-09-22 16:57 ` Matthew Rosato @ 2020-09-23 8:51 ` Pierre Morel 1 sibling, 0 replies; 9+ messages in thread From: Pierre Morel @ 2020-09-23 8:51 UTC (permalink / raw) To: Alex Williamson, Matthew Rosato Cc: bhelgaas, schnelle, mpe, oohall, cohuck, kevin.tian, hca, gor, borntraeger, linux-s390, linux-kernel, kvm, linux-pci On 2020-09-22 18:40, Alex Williamson wrote: > On Mon, 21 Sep 2020 08:43:29 -0400 > Matthew Rosato <mjrosato@linux.ibm.com> wrote: > >> On 9/10/20 10:59 AM, Matthew Rosato wrote: >>> While it is true that devices with is_virtfn=1 will have a Memory Space >>> Enable bit that is hard-wired to 0, this is not the only case where we >>> see this behavior -- For example some bare-metal hypervisors lack >>> Memory Space Enable bit emulation for devices not setting is_virtfn >>> (s390). Fix this by instead checking for the newly-added >>> no_command_memory bit which directly denotes the need for >>> PCI_COMMAND_MEMORY emulation in vfio. >>> >>> Fixes: abafbc551fdd ("vfio-pci: Invalidate mmaps and block MMIO access on disabled memory") >>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >>> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com> >>> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com> >> >> Polite ping on this patch as the other 2 have now received maintainer >> ACKs or reviews. I'm concerned about this popping up in distros as >> abafbc551fdd was a CVE fix. Related, see question from the cover: >> >> - Restored the fixes tag to patch 3 (but the other 2 patches are >> now pre-reqs -- cc stable 5.8?) > > I've got these queued in my local branch which I'll push to next for > v5.10. I'm thinking that perhaps the right thing would be to add the > fixes tag to all three patches, otherwise I could see that the PCI/VF > change might get picked as a dependency, but not the s390 specific one. > Does this sound correct to everyone? Thanks, > > Alex > sound correct for me. Thanks. Pierre -- Pierre Morel IBM Lab Boeblingen ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-09-23 8:51 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-10 14:59 [PATCH v5 0/3] vfio/pci: Restore MMIO access for s390 detached VFs Matthew Rosato 2020-09-10 14:59 ` [PATCH v5 1/3] PCI/IOV: Mark VFs as not implementing PCI_COMMAND_MEMORY Matthew Rosato 2020-09-16 21:55 ` Bjorn Helgaas 2020-09-10 14:59 ` [PATCH v5 2/3] s390/pci: Mark all " Matthew Rosato 2020-09-10 14:59 ` [PATCH v5 3/3] vfio/pci: Decouple PCI_COMMAND_MEMORY bit checks from is_virtfn Matthew Rosato 2020-09-21 12:43 ` Matthew Rosato 2020-09-22 16:40 ` Alex Williamson 2020-09-22 16:57 ` Matthew Rosato 2020-09-23 8:51 ` Pierre Morel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).