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

From: Xu Yilun

Date: Fri Oct 16 2026 - 08:42:13 EST


On Fri, Oct 16, 2026 at 03:35:28PM +0300, Nikolay Borisov wrote:
>
>
> 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?

I agree, if (i == mmio->nr) is clearer.