Re: [PATCH v4 07/10] PCI/IDE: Add IDE establishment helpers

[Arto Merilainen]

On 25.9.2025 13.18, Xu Yilun wrote:
> > This is the change I am adding
> >
> > [...]
> >
> > +static int add_range_merge_overlap(struct range *range, int az, int nr_range,
> > +                                u64 start, u64 end)
> > +{
> > +     int i;
> > +
> > +     if (start >= end)
> > +             return nr_range;
> > +
> > +     /* get new start/end: */
> > +     for (i = 0; i < nr_range; i++) {
> > +
> > +             if (!range[i].end)
> > +                     continue;
> > +
> > +             /* Try to add to the end */
> > +             if (range[i].end + 1 == start) {
> > +                     range[i].end = end;
> > +                     return nr_range;
> > +             }
> > +
> > +             /* Try to add to the start */
> > +             if (range[i].start == end + 1) {
> > +                     range[i].start = start;
> > +                     return nr_range;
> > +             }
> > +     }
> > +
> > +     /* Need to add it: */
> > +     return add_range(range, az, nr_range, start, end);
> > +}

Should we use bus addresses while programming the ranges? The 32-bit 
ranges may be remapped (i.e. the range address doesn't match with the 
bus address).

> I noticed Arto has a good idea that there needs at most 2 blocks no
> matter how the mmio layout is for PF/VF/MFD..., one for 32 bit, one for
> 64 bit. And the direct connected upstream bridge to the DSM device has
> already aggregated the 2 ranges on enumeration. [1] That greatly reduces
> the complexity. No need for callers to iterate the devices/resources to
> collect the ranges again.

I agree that the implementation feels unnecessarily complex given that 
the aggregated ranges are already available.

For reference, I am using a routine like this for collecting the ranges 
before programming the address association registers:

static int pci_res_to_ide_addr(struct pci_dev *pdev,
                               struct ide_addr_range *ide_addr)
{
        struct pci_dev *bridge = pci_upstream_bridge(pdev);
        struct pci_bus_region region;
        struct resource *res;
        int naddr = 0;

        res = &bridge->resource[PCI_BRIDGE_MEM_WINDOW];
        if (res->flags & IORESOURCE_MEM) {
                pcibios_resource_to_bus(bridge->bus, &region, res);
                ide_addr[naddr].start = region.start;
                ide_addr[naddr].end = region.end;
                naddr++;
        }

        res = &bridge->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
        if (res->flags & IORESOURCE_PREFETCH) {
                pcibios_resource_to_bus(bridge->bus, &region, res);
                ide_addr[naddr].start = region.start;
                ide_addr[naddr].end = region.end;
                naddr++;
        }

        return naddr;
}

> For TDX, the firmware enforces to setup only one addr block for RP, no
> matter how many supported blocks the RP actually has. That means TDX
> could only support 64 bit IDE ranges. I'd like to require an input
> parameter like "max_nr_mem_rp" for this purpose.

I do not think having only a limited number of addr blocks is unique to 
TDX. Given that the SW needs only a couple of addr blocks when 
programmed using the information in the type-1 header of the upstream 
bridge, I would expect hardware implementations to not have more blocks 
than necessary.

- R2