Re: [PATCH v5] PCI: vmd: Clean up domain before enumeration

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Nov 19, 2021 at 01:48:59PM -0700, Patel, Nirmal wrote:
> On 11/16/2021 3:11 PM, Nirmal Patel wrote:
> > During VT-d pass-through, the VMD driver occasionally fails to
> > enumerate underlying NVMe devices when repetitive reboots are
> > performed in the guest OS. The issue can be resolved by resetting
> > VMD root ports for proper enumeration and triggering secondary bus
> > reset which will also propagate reset through downstream bridges.
> >
> > Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
> > Reviewed-by: Jon Derrick <jonathan.derrick@linux.dev>
> > ---
> > ---
> > v4->v5: Fixing small nitpick fix.
> > v3->v4: Using pci_reset_bus function for secondary bus reset instead of
> >         manually triggering secondary bus reset, addressing review
> >         comments of v3.
> > v2->v3: Combining two functions into one, Remove redundant definations
> >         and Formatting fixes
> >
> >  drivers/pci/controller/vmd.c | 37 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >
> > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> > index a5987e52700e..a905fce6232f 100644
> > --- a/drivers/pci/controller/vmd.c
> > +++ b/drivers/pci/controller/vmd.c
> > @@ -498,6 +498,40 @@ static inline void vmd_acpi_begin(void) { }
> >  static inline void vmd_acpi_end(void) { }
> >  #endif /* CONFIG_ACPI */
> >  
> > +static void vmd_domain_reset(struct vmd_dev *vmd)
> > +{
> > +	u16 bus, max_buses = resource_size(&vmd->resources[0]);
> > +	u8 dev, functions, fn, hdr_type;
> > +	char __iomem *base;
> > +
> > +	for (bus = 0; bus < max_buses; bus++) {
> > +		for (dev = 0; dev < 32; dev++) {
> > +			base = vmd->cfgbar + PCIE_ECAM_OFFSET(bus,
> > +						PCI_DEVFN(dev, 0), 0);
> > +
> > +			hdr_type = readb(base + PCI_HEADER_TYPE) &
> > +					 PCI_HEADER_TYPE_MASK;
> > +
> > +			functions = (hdr_type & 0x80) ? 8 : 1;
> > +			for (fn = 0; fn < functions; fn++) {
> > +				base = vmd->cfgbar + PCIE_ECAM_OFFSET(bus,
> > +						PCI_DEVFN(dev, fn), 0);
> > +
> > +				hdr_type = readb(base + PCI_HEADER_TYPE) &
> > +						PCI_HEADER_TYPE_MASK;
> > +
> > +				if (hdr_type != PCI_HEADER_TYPE_BRIDGE ||
> > +				    (readw(base + PCI_CLASS_DEVICE) !=
> > +				     PCI_CLASS_BRIDGE_PCI))
> > +					continue;
> > +
> > +				memset_io(base + PCI_IO_BASE, 0,
> > +					  PCI_ROM_ADDRESS1 - PCI_IO_BASE);
> > +			}
> > +		}
> > +	}
> > +}
> > +
> >  static void vmd_attach_resources(struct vmd_dev *vmd)
> >  {
> >  	vmd->dev->resource[VMD_MEMBAR1].child = &vmd->resources[1];
> > @@ -801,6 +835,9 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
> >  	vmd_acpi_begin();
> >  
> >  	pci_scan_child_bus(vmd->bus);
> > +	vmd_domain_reset(vmd);
> > +	list_for_each_entry(child, &vmd->bus->children, node)
> > +		pci_reset_bus(child->self);
> >  	pci_assign_unassigned_bus_resources(vmd->bus);
> >  
> >  	/*
> 
> Hi
> 
> Gentle ping. Please let me know if you are okay with these changes
> (with Jon's Reviewed-by). Thanks.

This got assigned to me by mistake, so I just reassigned it to Lorenzo
in patchwork.  There's no hurry, we're only at v5.16-rc1.  It doesn't
appear to be a candidate for v5.16, and there's always "cc: stable" if
appropriate.

Bjorn