Re: [PATCH 2/3] PCI: vmd: Expose VMD details from BIOS

["Derrick, Jonathan" <jonathan.derrick@intel.com>]

Hi Bjorn,

On Fri, 2019-11-01 at 16:53 -0500, Bjorn Helgaas wrote:
> [+cc Andrew]
> 
> On Wed, Oct 16, 2019 at 11:04:47AM -0600, Jon Derrick wrote:
> > When some VMDs are enabled and others are not, it's difficult to
> > determine which IIO stack corresponds to the enabled VMD.
> > 
> > To assist userspace with management tasks, VMD BIOS will write the VMD
> > instance number and socket number into the first enabled root port's IO
> > Base/Limit registers prior to OS handoff. VMD driver can capture this
> > information and expose it to userspace.
> 
> Hmmm, I'm not sure I understand this, but it sounds possibly fragile.
> Are these Root Ports visible to the generic PCI core device
> enumeration?  If so, it will find them and read these I/O window
> registers.  Maybe today the PCI core doesn't change them, but I'm not
> sure we should rely on them always being preserved until the vmd
> driver can claim the device.
> 

The Root Ports are on the VMD PCI domain, and this IO BASE/LIMIT
parsing occurs before this PCI domain is exposed to the generic PCI
scancode with pci_scan_child_bus(). Until that point the VMD PCI domain
is invisible to the kernel outside of /dev/mem or resource0.

However, yes, it is somewhat fragile in that a third-party driver could
attach to the VMD endpoint prior to the VMD driver and modify the
values. A /dev/mem or resource0 user could also do this on an
unattached VMD endpoint.

I'm wondering if this would also be better suited for a special reset
in quirks.c, but it would need to expose a bit of VMD config accessing
in quirks.c to do that.

> But I guess you're using a special config accessor (vmd_cfg_read()),
> so these are probably invisible to the generic enumeration?
> 

Yes the VMD domain is invisible to generic PCI until the domain is
enumerated late in vmd_enable_domain().

> > + * for_each_vmd_root_port - iterate over all enabled VMD Root Ports
> > + * @vmd: &struct vmd_dev VMD device descriptor
> > + * @rp: int iterator cursor
> > + * @temp: u32 temporary value for config read
> > + *
> > + * VMD Root Ports are located in the VMD PCIe Domain at 00:[0-3].0, and config
> > + * space can be determinately accessed through the VMD Config BAR. Because VMD
> 
> I'm not sure how to parse "determinately accessed".  Maybe this config
> space can *only* be accessed via the VMD Config BAR?

Perhaps it should instead say determinately addressed, as each Root
Port config space is addressable at some offset of N * 0x8000 from the
base of the VMD endpoint config bar. I can see the comment may not be
helpful as that detail is abstracted using the vmd_cfg_read() helper.


> 
> > + * Root Ports can be individually disabled, it's important to iterate for the
> > + * first enabled Root Port as determined by reading the Vendor/Device register.
> > + */
> > +#define for_each_vmd_root_port(vmd, rp, temp)				\
> > +	for (rp = 0; rp < 4; rp++)					\
> > +		if (vmd_cfg_read(vmd, 0, PCI_DEVFN(root_port, 0),	\
> > +				 PCI_VENDOR_ID, 4, &temp) ||		\
> > +		    temp == 0xffffffff) {} else