From: <dan.j.williams@intel.com>
To: Jonathan Cameron <jonathan.cameron@huawei.com>,
Dan Williams <dan.j.williams@intel.com>
Cc: <linux-pci@vger.kernel.org>, <linux-coco@lists.linux.dev>,
<bhelgaas@google.com>, <aneesh.kumar@kernel.org>,
<yilun.xu@linux.intel.com>, <aik@amd.com>,
Arto Merilainen <amerilainen@nvidia.com>
Subject: Re: [PATCH 2/6] PCI/IDE: Add Address Association Register setup for downstream MMIO
Date: Wed, 5 Nov 2025 15:04:05 -0800 [thread overview]
Message-ID: <690bd7e585c47_74f76100f@dwillia2-mobl4.notmuch> (raw)
In-Reply-To: <20251105095832.00000871@huawei.com>
Jonathan Cameron wrote:
> On Tue, 4 Nov 2025 20:00:51 -0800
> Dan Williams <dan.j.williams@intel.com> wrote:
>
> > From: Xu Yilun <yilun.xu@linux.intel.com>
> >
> > The address ranges for downstream Address Association Registers need to
> > cover memory addresses for all functions (PFs/VFs/downstream devices)
> > managed by a Device Security Manager (DSM). The proposed solution is get
> > the memory (32-bit only) range and prefetchable-memory (64-bit capable)
> > range from the immediate ancestor downstream port (either the direct-attach
> > RP or deepest switch port when switch attached).
> >
> > Similar to RID association, address associations will be set by default if
> > hardware sets 'Number of Address Association Register Blocks' in the
> > 'Selective IDE Stream Capability Register' to a non-zero value. TSM drivers
> > can opt-out of the settings by zero'ing out unwanted / unsupported address
> > ranges. E.g. TDX Connect only supports prefetachable (64-bit capable)
> > memory ranges for the Address Association setting.
> >
> > If the immediate downstream port provides both a memory range and
> > prefetchable-memory range, but the IDE partner port only provides 1 Address
> > Association Register block then the TSM driver can pick which range to
> > associate, or let the PCI core prioritize memory.
> >
> > Note, the Address Association Register setup for upstream requests is still
> > uncertain so is not included.
> >
> > Co-developed-by: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
> > Co-developed-by: Arto Merilainen <amerilainen@nvidia.com>
> > Signed-off-by: Arto Merilainen <amerilainen@nvidia.com>
> > Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> > Co-developed-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> > include/linux/pci-ide.h | 27 ++++++++++
> > include/linux/pci.h | 5 ++
> > drivers/pci/ide.c | 115 ++++++++++++++++++++++++++++++++++++----
> > 3 files changed, 138 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/pci-ide.h b/include/linux/pci-ide.h
> > index d0f10f3c89fc..55283c8490e4 100644
> > --- a/include/linux/pci-ide.h
> > +++ b/include/linux/pci-ide.h
> > @@ -28,6 +28,9 @@ enum pci_ide_partner_select {
> > * @rid_start: Partner Port Requester ID range start
> > * @rid_end: Partner Port Requester ID range end
> > * @stream_index: Selective IDE Stream Register Block selection
> > + * @mem_assoc: PCI bus memory address association for targeting peer partner
>
> The text above about TDX only support prefetchable to me suggestions this
> is optional so should be marked so like pref_assoc?
Maybe... I think I was more considering the fact that PCI compliant
devices always have a 32-bit MMIO range. Given both are optional it
might be better to detail that in the Description section:
diff --git a/include/linux/pci-ide.h b/include/linux/pci-ide.h
index 40f0be185120..37a1ad9501b0 100644
--- a/include/linux/pci-ide.h
+++ b/include/linux/pci-ide.h
@@ -29,13 +29,18 @@ enum pci_ide_partner_select {
* @rid_end: Partner Port Requester ID range end
* @stream_index: Selective IDE Stream Register Block selection
* @mem_assoc: PCI bus memory address association for targeting peer partner
- * @pref_assoc: (optional) PCI bus prefetchable memory address association for
+ * @pref_assoc: PCI bus prefetchable memory address association for
* targeting peer partner
* @default_stream: Endpoint uses this stream for all upstream TLPs regardless of
* address and RID association registers
* @setup: flag to track whether to run pci_ide_stream_teardown() for this
* partner slot
* @enable: flag whether to run pci_ide_stream_disable() for this partner slot
+ *
+ * By default, pci_ide_stream_alloc() initializes @mem_assoc and @pref_assoc
+ * with the immediate ancestor downstream port memory ranges (i.e. Type 1
+ * Configuration Space Header values). Caller may zero size ({0, -1}) the range
+ * to drop it from consideration at pci_ide_stream_setup() time.
*/
struct pci_ide_partner {
u16 rid_start;
>
> > + * @pref_assoc: (optional) PCI bus prefetchable memory address association for
> > + * targeting peer partner
> > * @default_stream: Endpoint uses this stream for all upstream TLPs regardless of
> > * address and RID association registers
> > * @setup: flag to track whether to run pci_ide_stream_teardown() for this
> > @@ -38,11 +41,35 @@ struct pci_ide_partner {
> > u16 rid_start;
> > u16 rid_end;
> > u8 stream_index;
> > + struct pci_bus_region mem_assoc;
> > + struct pci_bus_region pref_assoc;
> > unsigned int default_stream:1;
> > unsigned int setup:1;
> > unsigned int enable:1;
> > };
>
>
> > diff --git a/drivers/pci/ide.c b/drivers/pci/ide.c
> > index da5b1acccbb4..d7fc741f3a26 100644
> > --- a/drivers/pci/ide.c
> > +++ b/drivers/pci/ide.c
>
>
> > @@ -385,6 +408,61 @@ static void set_ide_sel_ctl(struct pci_dev *pdev, struct pci_ide *ide,
> > pci_write_config_dword(pdev, pos + PCI_IDE_SEL_CTL, val);
> > }
> >
> > +#define SEL_ADDR1_LOWER GENMASK(31, 20)
> > +#define SEL_ADDR_UPPER GENMASK_ULL(63, 32)
> > +#define PREP_PCI_IDE_SEL_ADDR1(base, limit) \
> > + (FIELD_PREP(PCI_IDE_SEL_ADDR_1_VALID, 1) | \
> > + FIELD_PREP(PCI_IDE_SEL_ADDR_1_BASE_LOW, \
> > + FIELD_GET(SEL_ADDR1_LOWER, (base))) | \
> > + FIELD_PREP(PCI_IDE_SEL_ADDR_1_LIMIT_LOW, \
> > + FIELD_GET(SEL_ADDR1_LOWER, (limit))))
>
> Whilst complex, if it is only going to get one use, I'd just put the
> complexity inline. If it's getting lots of use in later patches then
> fair enough having this macro.
Not sure if that buys much just to move this down a few lines into
mem_assoc_to_regs().
> > +static void mem_assoc_to_regs(struct pci_bus_region *region,
> > + struct pci_ide_regs *regs, int idx)
> > +{
> > + regs->addr[idx].assoc1 =
> > + PREP_PCI_IDE_SEL_ADDR1(region->start, region->end);
> > + regs->addr[idx].assoc2 = FIELD_GET(SEL_ADDR_UPPER, region->end);
> > + regs->addr[idx].assoc3 = FIELD_GET(SEL_ADDR_UPPER, region->start);
> > +}
>
> > /**
> > * pci_ide_stream_setup() - program settings to Selective IDE Stream registers
> > * @pdev: PCIe device object for either a Root Port or Endpoint Partner Port
> > @@ -398,22 +476,34 @@ static void set_ide_sel_ctl(struct pci_dev *pdev, struct pci_ide *ide,
> > void pci_ide_stream_setup(struct pci_dev *pdev, struct pci_ide *ide)
> > {
> > struct pci_ide_partner *settings = pci_ide_to_settings(pdev, ide);
> > + struct pci_ide_regs regs;
> > int pos;
> > - u32 val;
> >
> > if (!settings)
> > return;
> >
> > + pci_ide_stream_to_regs(pdev, ide, ®s);
>
> If I were being super fussy, I'd suggest doing the factor out to a structure
> + helper as a precursor patch then just add the new stuff here.
> meh. I'm not that bothered but it would slightly simply review.
>
> I'm not entirely convinced by the helper as a readability improvement
> but don't hate it.
Note that the helper has an ulterior motive. TDX Connect wants to have a
copy of the desired register settings to pass to a TSM ABI for root port
setup. That will become clearer / documented later when this helper gets
an export.
next prev parent reply other threads:[~2025-11-05 23:04 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-05 4:00 [PATCH 0/6] PCI/TSM: Finalize "Link" TSM infrastructure Dan Williams
2025-11-05 4:00 ` [PATCH 1/6] resource: Introduce resource_assigned() for discerning active resources Dan Williams
2025-11-05 9:17 ` Jonathan Cameron
2025-11-05 21:57 ` dan.j.williams
2025-11-05 4:00 ` [PATCH 2/6] PCI/IDE: Add Address Association Register setup for downstream MMIO Dan Williams
2025-11-05 9:58 ` Jonathan Cameron
2025-11-05 23:04 ` dan.j.williams [this message]
2025-11-10 11:49 ` Jonathan Cameron
2025-11-05 4:00 ` [PATCH 3/6] PCI/IDE: Initialize an ID for all IDE streams Dan Williams
2025-11-05 15:27 ` Jonathan Cameron
2025-11-05 23:51 ` dan.j.williams
2025-11-10 11:52 ` Jonathan Cameron
2025-11-05 4:00 ` [PATCH 4/6] PCI/TSM: Add pci_tsm_bind() helper for instantiating TDIs Dan Williams
2025-11-05 4:59 ` Aneesh Kumar K.V
2025-11-05 21:49 ` dan.j.williams
2025-11-05 15:31 ` Jonathan Cameron
2025-11-06 0:11 ` dan.j.williams
2025-11-05 4:00 ` [PATCH 5/6] PCI/TSM: Add pci_tsm_guest_req() for managing TDIs Dan Williams
2025-11-05 15:38 ` Jonathan Cameron
2025-11-06 0:13 ` dan.j.williams
2025-11-05 4:00 ` [PATCH 6/6] PCI/TSM: Add 'dsm' and 'bound' attributes for dependent functions Dan Williams
2025-11-05 17:53 ` Jonathan Cameron
2025-11-13 12:10 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=690bd7e585c47_74f76100f@dwillia2-mobl4.notmuch \
--to=dan.j.williams@intel.com \
--cc=aik@amd.com \
--cc=amerilainen@nvidia.com \
--cc=aneesh.kumar@kernel.org \
--cc=bhelgaas@google.com \
--cc=jonathan.cameron@huawei.com \
--cc=linux-coco@lists.linux.dev \
--cc=linux-pci@vger.kernel.org \
--cc=yilun.xu@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).