On 14/12/17 02:32, Bryant G. Ly wrote:
> When enabling SR-IOV in pseries platform,
> the VF bar properties for a PF are reported on
> the device node in the device tree.
>
> This patch adds the IOV Bar resources to Linux
> structures from the device tree for later use
> when configuring SR-IOV by PF driver.
>
> Signed-off-by: Bryant G. Ly <bryantly@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Juan J. Alvarez <jjalvare@xxxxxxxxxxxxxxxxxx>
> ---
> arch/powerpc/include/asm/pci.h | 2 +
> arch/powerpc/kernel/pci_of_scan.c | 2 +-
> arch/powerpc/platforms/pseries/setup.c | 183 +++++++++++++++++++++++++++++++++
> 3 files changed, 186 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
> index 8dc32eacc97c..d82802ff5088 100644
> --- a/arch/powerpc/include/asm/pci.h
> +++ b/arch/powerpc/include/asm/pci.h
> @@ -121,6 +121,8 @@ extern int remove_phb_dynamic(struct pci_controller *phb);
> extern struct pci_dev *of_create_pci_dev(struct device_node *node,
> struct pci_bus *bus, int devfn);
>
> +extern unsigned int pci_parse_of_flags(u32 addr0, int bridge);
> +
> extern void of_scan_pci_bridge(struct pci_dev *dev);
>
> extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
> diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
> index 0d790f8432d2..20ceec4a5f5e 100644
> --- a/arch/powerpc/kernel/pci_of_scan.c
> +++ b/arch/powerpc/kernel/pci_of_scan.c
> @@ -38,7 +38,7 @@ static u32 get_int_prop(struct device_node *np, const char *name, u32 def)
> * @addr0: value of 1st cell of a device tree PCI address.
> * @bridge: Set this flag if the address is from a bridge 'ranges' property
> */
> -static unsigned int pci_parse_of_flags(u32 addr0, int bridge)
> +unsigned int pci_parse_of_flags(u32 addr0, int bridge)
> {
> unsigned int flags = 0;
>
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index 5f1beb8367ac..ce28882cbde8 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -459,6 +459,181 @@ static void __init find_and_init_phbs(void)
> of_pci_check_probe_only();
> }
>
> +#ifdef CONFIG_PCI_IOV
> +enum rtas_iov_fw_value_map {
> + NUM_RES_PROPERTY = 0, ///< Number of Resources
> + LOW_INT = 1, ///< Lowest 32 bits of Address
> + START_OF_ENTRIES = 2, ///< Always start of entry
> + APERTURE_PROPERTY = 2, ///< Start of entry+ to Aperture Size
> + WDW_SIZE_PROPERTY = 4, ///< Start of entry+ to Window Size
> + NEXT_ENTRY = 7 ///< Go to next entry on array
> +};
> +
> +enum get_iov_fw_value_index {
> + BAR_ADDRS = 1, ///< Get Bar Address
> + APERTURE_SIZE = 2, ///< Get Aperture Size
> + WDW_SIZE = 3 ///< Get Window Size
> +};
> +
> +resource_size_t pseries_get_iov_fw_values(struct pci_dev *dev, int resno,
s/pseries_get_iov_fw_values/pseries_get_iov_fw_value/ as it returns a
single value.
> + enum get_iov_fw_value_index value)
> +{
> + struct vf_bar_wdw {
> + __be64 addr;
> + __be64 aperture_size;
> + __be64 wdw_size;
> + };
> +
> + struct vf_bar_wdw window_avail[PCI_SRIOV_NUM_BARS];
> + const int *indexes;
> + struct device_node *dn = pci_device_to_OF_node(dev);
> + int i, r, num_res;
> + resource_size_t return_value;
resource_size_t return_value = 0;
and remove initialization below. Also, way too long, @ret is a more common
name.
> +
> + indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);
> + if (!indexes)
> + return 0;
> +
> + memset(window_avail,
> + 0, sizeof(struct vf_bar_wdw) * PCI_SRIOV_NUM_BARS);
This is more common way of doing the same initialization:
struct vf_bar_wdw {
__be64 addr;
__be64 aperture_size;
__be64 wdw_size;
} window_avail[PCI_SRIOV_NUM_BARS] = { 0 };
> + return_value = 0;
> + /*
> + * First element in the array is the number of Bars
> + * returned. Search through the list to find the matching
> + * bar
> + */
> + num_res = of_read_number(&indexes[NUM_RES_PROPERTY], 1);
if (resno >= num_res)
return 0; /* or an errror */
i = START_OF_ENTRIES + NEXT_ENTRY * resno;
switch (value) {
case BAR_ADDRS:
ret = f_read_number(&indexes[i], 2);
break;
case APERTURE_SIZE:
ret = of_read_number(&indexes[i + APERTURE_PROPERTY], 2);
break;
case WDW_SIZE:
ret = of_read_number(&indexes[i + WDW_SIZE_PROPERTY], 2);
break;
}
return ret;
}
and remove the reminder of the function, and window_avail, and vf_bar_wdw?
> + for (i = START_OF_ENTRIES, r = 0; r < num_res && r < PCI_SRIOV_NUM_BARS;
> + i += NEXT_ENTRY, r++) {
> + window_avail[r].addr = of_read_number(&indexes[i], 2);
> + window_avail[r].aperture_size =
> + of_read_number(&indexes[i + APERTURE_PROPERTY], 2);
> + window_avail[r].wdw_size =
> + of_read_number(&indexes[i + WDW_SIZE_PROPERTY], 2);
> + }
> +
> + switch (value) {
> + case BAR_ADDRS:
> + return_value = window_avail[resno].addr;
> + break;
> + case APERTURE_SIZE:
> + return_value = window_avail[resno].aperture_size;
> + break;
> + case WDW_SIZE:
> + return_value = window_avail[resno].wdw_size;
> + break;
> + default:
> + break;
> + }
> + return return_value;
> +}
> +
> +void of_pci_parse_vf_bar_size(struct pci_dev *dev, const int *indexes)
It does not seem to make a lot of sense as a separate function imho...
> +{
> + struct resource *res;
> + resource_size_t base, size;
> + int i, r, num_res;
> +
> + num_res = of_read_number(&indexes[NUM_RES_PROPERTY], 1);
num_res = min_t(int, num_res, PCI_SRIOV_NUM_BARS) ? Matter of personal
taste though.
> + for (i = START_OF_ENTRIES, r = 0; r < num_res && r < PCI_SRIOV_NUM_BARS;
> + i += NEXT_ENTRY, r++) {
> + res = &dev->resource[r + PCI_IOV_RESOURCES];
> + base = of_read_number(&indexes[i], 2);
> + size = of_read_number(&indexes[i + APERTURE_PROPERTY], 2);
> + res->flags = pci_parse_of_flags(of_read_number
> + (&indexes[i + LOW_INT], 1), 0);
> + res->flags |= (IORESOURCE_MEM_64 | IORESOURCE_PCI_FIXED);
> + res->name = pci_name(dev);
> + res->start = base;
> + res->end = base + size - 1;
The function name suggests it only parses sizes but just above it assigns
all resource parameters - size, address, flags.
> + }
> +}
> +
> +void of_pci_parse_iov_addrs(struct pci_dev *dev, const int *indexes)
> +{
> + struct resource *res, *root, *conflict;
> + resource_size_t base, size;
> + int i, r, num_res;
> +
> + /*
> + * First element in the array is the number of Bars
> + * returned. Search through the list to find the matching
> + * bars assign them from firmware into resources structure.
> + */
> + num_res = of_read_number(&indexes[NUM_RES_PROPERTY], 1);
> + for (i = START_OF_ENTRIES, r = 0; r < num_res && r < PCI_SRIOV_NUM_BARS;
> + i += NEXT_ENTRY, r++) {
> + res = &dev->resource[r + PCI_IOV_RESOURCES];
> + base = of_read_number(&indexes[i], 2);
> + size = of_read_number(&indexes[i + WDW_SIZE_PROPERTY], 2);
> + res->name = pci_name(dev);
> + res->start = base;
> + res->end = base + size - 1;
> + root = pci_find_parent_resource(dev, res);
> +
> + if (!root)
> + root = &iomem_resource;
@dev here is a VF, right? I am not familiar with powervn much but from what
I see - the devices are sitting on a root bus of their own PHB and they all
either have a root returned from pci_find_parent_resource() or none of them
has a root and will fall back to &iomem_resource, or both cases are possible?
> + dev_dbg(&dev->dev,
> + "Pseries IOV BAR %d: trying firmware assignment %pR\n",
s/Pseries/pSeries/ ?
> + r + PCI_IOV_RESOURCES, res);
> + conflict = request_resource_conflict(root, res);
> + if (conflict) {
> + dev_info(&dev->dev,
> + "BAR %d: %pR conflicts with %s %pR\n",
> + r + PCI_IOV_RESOURCES, res,
> + conflict->name, conflict);
> + res->flags |= IORESOURCE_UNSET;
> + }
> + }
> +}
> +
> +static void pseries_pci_fixup_resources(struct pci_dev *pdev)
> +{
> + const int *indexes;
> + struct device_node *dn = pci_device_to_OF_node(pdev);
> +
> + /*Firmware must support open sriov otherwise dont configure*/
> + indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);
> + if (!indexes)
> + return;
> + /* Assign the addresses from device tree*/
> + of_pci_parse_vf_bar_size(pdev, indexes);
> +}
> +
> +static void pseries_pci_fixup_iov_resources(struct pci_dev *pdev)
> +{
> + const int *indexes;
> + struct device_node *dn = pci_device_to_OF_node(pdev);
> +
> + if (!pdev->is_physfn || pdev->is_added)
> + return;
> + /*Firmware must support open sriov otherwise dont configure*/
> + indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);
> + if (!indexes)
> + return;
> + /* Assign the addresses from device tree*/
> + of_pci_parse_iov_addrs(pdev, indexes);
> +}
> +
> +static resource_size_t pseries_pci_iov_resource_alignment(struct pci_dev *pdev,
> + int resno)
> +{
> + const __be32 *reg;
> + struct device_node *dn = pci_device_to_OF_node(pdev);
> +
> + /*Firmware must support open sriov otherwise report regular alignment*/
> + reg = of_get_property(dn, "ibm,is-open-sriov-pf", NULL);
> + if (!reg)
> + return pci_iov_resource_size(pdev, resno);
> +
> + if (!pdev->is_physfn)
> + return 0;
> + return pseries_get_iov_fw_values(pdev,
> + resno - PCI_IOV_RESOURCES,
> + APERTURE_SIZE);
> +}
> +#endif
> +
> static void __init pSeries_setup_arch(void)
> {
> set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT);
> @@ -490,6 +665,14 @@ static void __init pSeries_setup_arch(void)
> vpa_init(boot_cpuid);
> ppc_md.power_save = pseries_lpar_idle;
> ppc_md.enable_pmcs = pseries_lpar_enable_pmcs;
> +#ifdef CONFIG_PCI_IOV
> + ppc_md.pcibios_fixup_resources =
> + pseries_pci_fixup_resources;
> + ppc_md.pcibios_fixup_sriov =
> + pseries_pci_fixup_iov_resources;
> + ppc_md.pcibios_iov_resource_alignment =
> + pseries_pci_iov_resource_alignment;
> +#endif
I wish one day this horrible ppc_md monster dies and all these helpers will
go to PHB ops, hose ops, bus ops, whatever ops...
> } else {
> /* No special idle routine */
> ppc_md.enable_pmcs = power4_enable_pmcs;
>
--
Alexey
