[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <68c3220d47eea_75db100a9@dwillia2-mobl4.notmuch>
Date: Thu, 11 Sep 2025 12:25:01 -0700
From: <dan.j.williams@...el.com>
To: Aneesh Kumar K.V <aneesh.kumar@...nel.org>, Arto Merilainen
<amerilainen@...dia.com>, <dan.j.williams@...el.com>
CC: <linux-kernel@...r.kernel.org>, <bhelgaas@...gle.com>, <aik@....com>,
<lukas@...ner.de>, Samuel Ortiz <sameo@...osinc.com>, Yilun Xu
<yilun.xu@...ux.intel.com>, <linux-pci@...r.kernel.org>,
<linux-coco@...ts.linux.dev>
Subject: Re: [PATCH v4 07/10] PCI/IDE: Add IDE establishment helpers
Aneesh Kumar K.V wrote:
> Aneesh Kumar K.V <aneesh.kumar@...nel.org> writes:
[..]
> >> Aneesh, could you perhaps extend the IDE driver by adding the RP address
> >> association register programming in the next revision of the DA support
> >> series?
> >>
> >
> > Sure, I can add that change as part of next update.
> >
>
> This is the change I am adding
Just thinking out loud...
I assume we will soon get to the point where at least one of the vendors
is ready to have their implementation pulled into tsm.git.
For truly vendor-specific bits that can be a pull request that I just
blindly pull. For core update proposals like this I expect it would be
best to cherry-pick those into the base at the next staging tree update.
This would be in support of prepping tsm.git (at least the base
infrastructure) for inclusion in linux-next.
As always, open to ideas on how to coordinate this.
In the meantime the tsm.git plan is to continue to rebase the base
infrastrcture branch until the review comments subside and all new
changes can be handled as incremental updates.
> drivers/pci/ide.c | 128 ++++++++++++++++++++++-
> drivers/virt/coco/arm-cca-host/arm-cca.c | 13 +++
> include/linux/pci-ide.h | 7 ++
> 3 files changed, 147 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/ide.c b/drivers/pci/ide.c
> index 3f772979eacb..23d1712ba97a 100644
> --- a/drivers/pci/ide.c
> +++ b/drivers/pci/ide.c
> @@ -101,7 +101,7 @@ void pci_ide_init(struct pci_dev *pdev)
> pdev->ide_cap = ide_cap;
> pdev->nr_link_ide = nr_link_ide;
> pdev->nr_sel_ide = nr_streams;
> - pdev->nr_ide_mem = nr_ide_mem;
> + pdev->nr_ide_mem = min(nr_ide_mem, PCI_IDE_AASOC_REG_MAX);
> }
>
> struct stream_index {
> @@ -213,11 +213,13 @@ struct pci_ide *pci_ide_stream_alloc(struct pci_dev *pdev)
> .rid_start = pci_dev_id(rp),
> .rid_end = pci_dev_id(rp),
> .stream_index = no_free_ptr(ep_stream)->stream_index,
> + .nr_mem = 0,
Designated initializers already zero by default.
> },
> [PCI_IDE_RP] = {
> .rid_start = pci_dev_id(pdev),
> .rid_end = rid_end,
> .stream_index = no_free_ptr(rp_stream)->stream_index,
> + .nr_mem = 0,
> },
> },
> .host_bridge_stream = no_free_ptr(hb_stream)->stream_index,
> @@ -228,6 +230,109 @@ struct pci_ide *pci_ide_stream_alloc(struct pci_dev *pdev)
> }
> EXPORT_SYMBOL_GPL(pci_ide_stream_alloc);
>
> +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);
> +}
> +
> +int pci_ide_add_address_assoc_block(struct pci_dev *pdev,
> + struct pci_ide *ide,
> + u64 start, u64 end)
How about:
pci_ide_associate_address()?
...because the result is not always a new block.
> +{
> + struct pci_ide_partner *partner;
> +
> + if (!pci_is_pcie(pdev)) {
> + pci_warn_once(pdev, "not a PCIe device\n");
> + return -EINVAL;
> + }
> +
> + switch (pci_pcie_type(pdev)) {
> + case PCI_EXP_TYPE_ENDPOINT:
> +
> + if (pdev != ide->pdev)
> + return -EINVAL;
> + partner = &ide->partner[PCI_IDE_RP];
> + break;
> + default:
> + pci_warn_once(pdev, "invalid device type\n");
> + return -EINVAL;
> + }
> +
> + if (partner->nr_mem >= pdev->nr_ide_mem)
> + return -ENOMEM;
> +
> + partner->nr_mem = add_range_merge_overlap(partner->mem,
> + PCI_IDE_AASOC_REG_MAX, partner->nr_mem,
> + start, end);
> + return 0;
> +}
> +
> +
> +int pci_ide_merge_address_assoc_block(struct pci_dev *pdev,
> + struct pci_ide *ide, u64 start, u64 end)
Is this really "merge", or "expand_to_fit" similar to
insert_resource_expand_to_fit()?
pci_ide_associate_address_force()? ...or am I reading it wrong?
[..]
> diff --git a/drivers/virt/coco/arm-cca-host/arm-cca.c b/drivers/virt/coco/arm-cca-host/arm-cca.c
> index c9717698af56..28993f9277e4 100644
> --- a/drivers/virt/coco/arm-cca-host/arm-cca.c
> +++ b/drivers/virt/coco/arm-cca-host/arm-cca.c
> @@ -137,6 +137,7 @@ static int cca_tsm_connect(struct pci_dev *pdev)
> {
> struct pci_dev *rp = pcie_find_root_port(pdev);
> struct cca_host_pf0_dsc *dsc_pf0;
> + struct resource *res;
> struct pci_ide *ide;
> int rc, stream_id;
>
> @@ -163,9 +164,21 @@ static int cca_tsm_connect(struct pci_dev *pdev)
> if (rc)
> goto err_stream;
>
> + /*
> + * Try to use the available address assoc register blocks.
> + * If we fail with ENOMEM, create one block covering the entire
> + * address range. (Should work for arm64)
> + */
> + pci_dev_for_each_resource(pdev, res) {
> + rc = pci_ide_add_address_assoc_block(pdev, ide, res->start, res->end);
> + if (rc == -ENOMEM)
> + pci_ide_merge_address_assoc_block(pdev, ide, res->start, res->end);
How does this play with the "shared MSI-X MMIO" problem? Does this also
need to align with interface report expectations?
> + }
> +
> pci_ide_stream_setup(pdev, ide);
> pci_ide_stream_setup(rp, ide);
>
> +
> rc = tsm_ide_stream_register(ide);
> if (rc)
> goto err_tsm;
> diff --git a/include/linux/pci-ide.h b/include/linux/pci-ide.h
> index c3838d11af88..3d4f7f462a8d 100644
> --- a/include/linux/pci-ide.h
> +++ b/include/linux/pci-ide.h
> @@ -19,6 +19,7 @@ enum pci_ide_partner_select {
> PCI_IDE_HB = PCI_IDE_PARTNER_MAX,
> };
>
> +#define PCI_IDE_AASOC_REG_MAX 6
Where does 6 come from?
---
7.9.26.5.1 Selective IDE Stream Capability Register
The number of Selective IDE Address Association register blocks for a
given IDE Stream is hardware implementation specific, and is permitted
to be any number between 0 and 15.
---
Also, I would put this max in include/uapi/linux/pci_regs.h and match
the local naming.
Powered by blists - more mailing lists