Re: [PATCH] PCI: PTM preliminary implementation

["Yong, Jonathan" <jonathan.yong@intel.com>]

On 04/30/2016 00:20, Bjorn Helgaas wrote:

>> +	return sprintf(buf, "%u\n", word & PCI_PTM_CTRL_ENABLE ? 1 : 0);
>> +}
>> +
>> +static ssize_t ptm_status_store(struct device *dev,
>> +	struct device_attribute *attr, const char *buf, size_t count)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	unsigned long val;
>> +	ssize_t ret;
>> +
>> +	ret = kstrtoul(buf, 0, &val);
>> +	if (ret)
>> +		return ret;
>> +	if (val)
>> +		return pci_enable_ptm(pdev);
>> +	pci_disable_ptm(pdev);
>> +	return 0;
>> +}
>> +
>> +static DEVICE_ATTR_RW(ptm_status);
>> +
>> +void pcie_ptm_remove_sysfs_dev_files(struct pci_dev *dev)
>> +{
>> +	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
>> +		return;
>> +	device_remove_file(&dev->dev, &dev_attr_ptm_status);
>> +}
>
> This is called in the device remove path.  Obviously we want to
> remove the sysfs file.  But we currently don't do anything to the PTM
> capability itself.
>
> What happens if the device has PTM enabled, we hotplug remove it
> (using the sysfs interface so the device stays physically present and
> powered up), manually disable PTM on the upstream switch, then hotplug
> add the device back?  If the switch has PTM disabled but the device
> has PTM enabled, that sounds like an illegal configuration.
>
> Is this scenario possible?  Maybe the hotplug remove would power off
> the device?  I guess the current pci_ptm_init() path actually would
> disable PTM on the new device because it's disabled on the upstream
> switch.
>
> But since PTM is autonomous once enabled, I would assume the new
> device (with PTM enabled) could send PTM messages upstream, and it
> sounds (per sec 6.22.3) like those would be treated as Unsupported
> Requests, which means we'd report errors.
>
> I think we might want to disable PTM before we remove a device.
>

I'm not familiar with PCI hotplug, release_slot in acpiphp_core.c?