Re: [PATCH 2/6] PCI/IDE: Add Address Association Register setup for downstream MMIO

[Jonathan Cameron <jonathan.cameron@huawei.com>]

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?

> + * @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.

> +
> +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, &regs);

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.


> +
>  	pos = sel_ide_offset(pdev, settings);
>  
> -	val = FIELD_PREP(PCI_IDE_SEL_RID_1_LIMIT, settings->rid_end);
> -	pci_write_config_dword(pdev, pos + PCI_IDE_SEL_RID_1, val);
> +	pci_write_config_dword(pdev, pos + PCI_IDE_SEL_RID_1, regs.rid1);
> +	pci_write_config_dword(pdev, pos + PCI_IDE_SEL_RID_2, regs.rid2);
>  
> -	val = FIELD_PREP(PCI_IDE_SEL_RID_2_VALID, 1) |
> -	      FIELD_PREP(PCI_IDE_SEL_RID_2_BASE, settings->rid_start) |
> -	      FIELD_PREP(PCI_IDE_SEL_RID_2_SEG, pci_ide_domain(pdev));
> +	for (int i = 0; i < regs.nr_addr; i++) {
> +		pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_1(i),
> +				       regs.addr[i].assoc1);
> +		pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_2(i),
> +				       regs.addr[i].assoc2);
> +		pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_3(i),
> +				       regs.addr[i].assoc3);
> +	}
>  
> -	pci_write_config_dword(pdev, pos + PCI_IDE_SEL_RID_2, val);
> +	/* clear extra unused address association blocks */
> +	for (int i = regs.nr_addr; i < pdev->nr_ide_mem; i++) {
> +		pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_1(i), 0);
> +		pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_2(i), 0);
> +		pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_3(i), 0);
> +	}