Re: [PATCH RESEND v2 7/7] PCI/hotplug: PowerPC PowerNV PCI hotplug driver

[Gavin Shan <gwshan@linux.vnet.ibm.com>]

On Tue, Feb 17, 2015 at 04:09:16PM -0600, Bjorn Helgaas wrote:
>On Tue, Feb 17, 2015 at 06:13:23PM +1100, Gavin Shan wrote:
>> The patch intends to add standalone driver to support PCI hotplug
>> for PowerPC PowerNV platform, which runs on top of skiboot firmware.
>> The firmware identified hotpluggable slots and marked their device
>> tree node with proper "ibm,slot-pluggable" and "ibm,reset-by-firmware".
>> The driver simply scans device-tree to create/register PCI hotplug slot
>> accordingly.
>> 
>> If the skiboot firmware doesn't support slot status retrieval, the PCI
>> slot device node shouldn't have property "ibm,reset-by-firmware". In
>> that case, none of valid PCI slots will be detected from device tree.
>> The skiboot firmware doesn't export the capability to access attention
>> LEDs yet and it's something for TBD.
>> 
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ...
>
>> +static int disable_slot(struct hotplug_slot *php_slot)
>> +{
>> +	struct powernv_php_slot *slot = php_slot->private;
>> +
>> +	if (slot->state != POWERNV_PHP_SLOT_STATE_POPULATED)
>> +		return 0;
>> +
>> +	pci_lock_rescan_remove();
>> +	pcibios_remove_pci_devices(slot->bus);
>> +	pci_unlock_rescan_remove();
>> +	vm_unmap_aliases();
>
>What is vm_unmap_aliases() for?  I see this is probably copied from
>rpaphp_core.c, where it was added by b4a26be9f6f8 ("powerpc/pseries: Flush
>lazy kernel mappings after unplug operations").
>
>But I don't know whether:
>
>  - this is something specific to powerpc,
>  - the lack of vm_unmap_aliases() in other hotplug paths is a bug,
>  - the fact that we only do this on powerpc is covering up a
>    powerpc bug somewhere
>

Yes, I copied this piece of code from rpaphp_core.c. I think Ben might
help to answer the questions as he added the patch. I had very quick
check on mm/vmalloc.c and it's reasonable to have vm_unmap_aliases()
here to flush TLB entries for ioremap() regions, which were unmapped
previously. if I'm correct. I don't think it's powerpc specific.

Thanks,
Gavin

>> +
>> +	/* Detach the child hotpluggable slots */
>> +	powernv_php_unregister(slot->dn);
>> +
>> +	/* Update slot state */
>> +	slot->state = POWERNV_PHP_SLOT_STATE_REGISTER;
>> +	return 0;
>> +}
>> 
>