Re: [RFC PATCH 1/4] PCI/TSM: Create MMIO descriptors via TDISP Report

[Nikolay Borisov <nik.borisov@suse.com>]

On 16.10.26 г. 14:57 ч., Xu Yilun wrote:
>>> +int pci_tsm_mmio_setup(struct pci_dev *pdev, struct pci_tsm_mmio *mmio)
>>> +{
>>> +	int i;
>>> +
>>> +	device_lock_assert(&pdev->dev);
>>> +	if (pdev->dev.driver)
>>> +		return -EBUSY;
>>> +
>>> +	for (i = 0; i < mmio->nr; i++) {
>>> +		struct resource *res = pci_tsm_mmio_resource(mmio, i);
>>> +		int j;
>>> +
>>> +		if (resource_size(res) == 0 || !(res->flags & IORESOURCE_MEM))
>>> +			break;
>>> +
>>> +		/* Only require the caller to set the range, init remainder */
>>> +		*res = DEFINE_RES_NAMED_DESC(res->start, resource_size(res),
>>> +					     pci_name(pdev), IORESOURCE_MEM,
>>> +					     IORES_DESC_ENCRYPTED);
>>> +
>>> +		for (j = 0; j < PCI_NUM_RESOURCES; j++)
>>> +			if (resource_contains(pci_resource_n(pdev, j), res))
>>> +				break;
>>> +
>>> +		/* Request is outside of device MMIO */
>>> +		if (j >= PCI_NUM_RESOURCES)
>>> +			break;
>>> +
>>> +		if (insert_resource(&encrypted_iomem_resource, res) != 0)
>>> +			break;
>>> +	}
>>> +
>>> +	if (i >= mmio->nr)
>>> +		return 0;
>>
>> Shouldn't this check be if (i == mmio->nr -1 ) i.e all descriptors have been
>> initialized, given the for loop condition i can ever be at most mmio->nr - 1
>> i.e the actual index of a descriptor?
> 
> The original if (i >= mmio->nr) check is correct. Valid i is at most mmio->nr -1,
> but you need an invalid i to finish the loop.


Shouldn't the check strictly check for i  == mmio->nr the loop 
terminating condition, which means "we've initialized ALL mmio 
resources" otherwise perform the cleanup, that '>' can never be true.

  While not wrong per-se having a strict check would be far clearer, no?