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

[Nikolay Borisov]

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?