linux-kernel - Re: [Patch v2] rpaphp: fix slot registration for multiple slots under a PHB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <578E2E14.2070606@linux.vnet.ibm.com>
Date:	Tue, 19 Jul 2016 08:41:40 -0500
From:	Nathan Fontenot <nfont@...ux.vnet.ibm.com>
To:	Tyrel Datwyler <tyreld@...ux.vnet.ibm.com>,
	linux-pci@...r.kernel.org
Cc:	bhelgaas@...gle.com, mpe@...erman.id.au, benh@...nel.crashing.org,
	mdroth@...ux.vnet.ibm.com, linux-kernel@...r.kernel.org,
	linuxppc-dev@...ts.ozlabs.org
Subject: Re: [Patch v2] rpaphp: fix slot registration for multiple slots under
 a PHB

On 07/11/2016 05:16 PM, Tyrel Datwyler wrote:
> PowerVM seems to only ever provide a single hotplug slot per PHB.
> The under lying slot hotplug registration code assumed multiple slots,
> but the actual implementation is broken for multiple slots. This went
> unnoticed for years due to the nature of PowerVM as mentioned
> previously. Under qemu/kvm the hotplug slot model aligns more with
> x86 where multiple slots are presented under a single PHB. As seen
> in the following each additional slot after the first fails to
> register due to each slot always being compared against the first
> child node of the PHB in the device tree.
> 
> [    6.492291] rpaphp: RPA HOT Plug PCI Controller Driver version: 0.1
> [    6.492569] rpaphp: Slot [Slot 0] registered
> [    6.492577] rpaphp: pci_hp_register failed with error -16
> [    6.493082] rpaphp: pci_hp_register failed with error -16
> [    6.493138] rpaphp: pci_hp_register failed with error -16
> [    6.493161] rpaphp: pci_hp_register failed with error -16
> 
> The registration logic is fixed so that each slot is compared
> against the existing child devices of the PHB in the device tree to
> determine present slots vs empty slots.
> 
> [   38.481750] rpaphp: RPA HOT Plug PCI Controller Driver version: 0.1
> [   38.482004] rpaphp: Slot [C0] registered
> [   38.482127] rpaphp: Slot [C1] registered
> [   38.482241] rpaphp: Slot [C2] registered
> [   38.482356] rpaphp: Slot [C3] registered
> [   38.482495] rpaphp: Slot [C4] registered
> 
> Signed-off-by: Tyrel Datwyler <tyreld@...ux.vnet.ibm.com>

Reviewed-by: Nathan Fontenot <nfont@...ux.vnet.ibm.com>

> ---
> 
> Changes in v2: corrected ibm,my-drc-index property name
> 
> ---
>  drivers/pci/hotplug/rpaphp_slot.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/rpaphp_slot.c b/drivers/pci/hotplug/rpaphp_slot.c
> index 6937c72..388c4d8 100644
> --- a/drivers/pci/hotplug/rpaphp_slot.c
> +++ b/drivers/pci/hotplug/rpaphp_slot.c
> @@ -117,8 +117,10 @@ EXPORT_SYMBOL_GPL(rpaphp_deregister_slot);
>  int rpaphp_register_slot(struct slot *slot)
>  {
>  	struct hotplug_slot *php_slot = slot->hotplug_slot;
> +	struct device_node *child;
> +	u32 my_index;
>  	int retval;
> -	int slotno;
> +	int slotno = -1;
> 
>  	dbg("%s registering slot:path[%s] index[%x], name[%s] pdomain[%x] type[%d]\n",
>  		__func__, slot->dn->full_name, slot->index, slot->name,
> @@ -130,10 +132,15 @@ int rpaphp_register_slot(struct slot *slot)
>  		return -EAGAIN;
>  	}
> 
> -	if (slot->dn->child)
> -		slotno = PCI_SLOT(PCI_DN(slot->dn->child)->devfn);
> -	else
> -		slotno = -1;
> +	for_each_child_of_node(slot->dn, child) {
> +		retval = of_property_read_u32(child, "ibm,my-drc-index", &my_index);
> +		if (my_index == slot->index) {
> +			slotno = PCI_SLOT(PCI_DN(child)->devfn);
> +			of_node_put(child);
> +			break;
> +		}
> +	}
> +
>  	retval = pci_hp_register(php_slot, slot->bus, slotno, slot->name);
>  	if (retval) {
>  		err("pci_hp_register failed with error %d\n", retval);
>