Re: [PATCH v3 2/2] pseries/eeh: Add Pseries pcibios_bus_add_device

[Juan Alvarez <jjalvare@linux.vnet.ibm.com>]

On 10/17/17 1:52 PM, Bjorn Helgaas wrote:
> Right.  But that patch isn't really on the path to inclusion (mainly
> because it's test framework and doesn't fix a bug or add support for
> new hardware or features).
I was not aware of this decision and this will cause changes to this patch.
>
> I'm suggesting that maybe there should be a generic way to prevent
> binding to VF devices that works for everybody and doesn't require any
> arch-specific code at all.

The patch that you have suggested in kernel 4.12 is also a generic way.

https://marc.info/?l=linux-kernel&m=149002335105804&w=2

Perhaps we can use the same constructs that this patch uses at our level. Nonetheless,
that will take a rework to this patch and possibly an export of a function to set drivers_autoprobe
globally. I know that was frowned upon on first email thread. Let me know if this is an
acceptable solution.
>>>>>> So like existing way of enabling SRIOV we still rely on the PF driver to
>>>>>> enable VFs - but in this case the attachment phase is done via a user
>>>>>> action via a management console in our case (novalink or hmc) triggered
>>>>>> event that will essentially act like a hotplug.
>>>>>>
>>>>>> So in the fine details of that user triggered action the system
>>>>>> firmware will bind the VFs, allowing resources to be allocated to
>>>>>> the VF.  - Which essentially does all the attaching as we know it
>>>>>> today but is managed by PHYP not by the kernel.
>>>>> What exactly does "firmware binding the VFs" mean?  I guess this must
>>>>> mean assigning a VF to a partition, injecting a hotplug add event to
>>>>> that partition, and making the VF visible in config space?
>>>> Firmware basically adds a device node to the device tree as defined
>>>> in the (PAPR) Power Architecture Platform Requirements document.
>>> From the point of view of the kernel that consumes this device tree
>>> and owns the VF, I guess this looks like a hot-add.  
>> Correct, this is intended. We want to minimize change and only focus
>> on configure SR-IOV path in the PF on PSeries platform.
>>> It would be nice
>>> if this could be exposed to that kernel by having the firmware inject
>>> a normal PCIe hotplug interrupt, but I'm guessing it's not that
>>> simple.
>> I don't understand entirely your suggestion. However, in the case of
>> interrupts PHYP owns all the resources and will be associated accordingly
>> with a partitionable endpoint (PE).
> Assume you have a Linux kernel, and PHYP assigns a VF to it.  What
> happens in that Linux kernel? 

When  kernel boots it goes through the lists of buses in the device tree. For each (physical,virtual)
bus the kernel reads the list of devices. After resources are read and assigned it will probe the devices.

If the kernel is up and running then this will start its flow through the dlpar add bus code.
Which then will read the device node and same operation as boot will entail of going
through the list of devices under the bus.

One should note that dynamically created VFs in configure SR-IOV path will have is_virtfn
set and others do not, therefore pcibios_bus_add_device will only be exercised in certain
cases.
>  How does it discover this new device?
This is treated as a normal hot plug operation for the Operating System that
the device gets assigned which can be  IBMi, AIX or Linux in this environment.
>
> I'm suggesting that it would be cool if that kernel received a hotplug
> interrupt from the Downstream Port leading to the new VF, and the
> pciehp driver could read the Slot Status register and see that
> "Presence Detect Changed" was set.  If that happened, the pciehp
> driver should automatically enumerate the new device and there
> wouldn't be much if any powerpc-specific code in the path.
To re-state Steve's earlier comment, PHYP will enumerate the virtual pci bus and is not
directly correlated to the real PCI bus that the PF is on. We have no control of the pci
bus enumeration for the device node added to the device tree.

Juan J. Alvarez