Re: [PATCH v2 1/2] PCI: endpoint: pci-epf-test: Add support for capabilities

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Thu, Nov 21, 2024 at 04:23:20PM +0100, Niklas Cassel wrote:
> The test BAR is on the EP side is allocated using pci_epf_alloc_space(),
> which allocates the backing memory using dma_alloc_coherent(), which will
> return zeroed memory regardless of __GFP_ZERO was set or not.
> 
> This means that running a new version of pci-endpoint-test.c (host side)
> with an old version of pci-epf-test.c (EP side) will not see any
> capabilities being set (as intended), so this is backwards compatible.
> 
> Additionally, the EP side always allocates at least 128 bytes for the test
> BAR (excluding the MSI-X table), this means that adding another register at
> offset 0x30 is still within the 128 available bytes.
> 
> For now, we only add the CAP_UNALIGNED_ACCESS capability.
> 
> Set CAP_UNALIGNED_ACCESS if the EPC driver can handle any address (because
> it implements the .align_addr callback).
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Just a small nit below.

> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index ef6677f34116..7351289ecddd 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -44,6 +44,8 @@
>  
>  #define TIMER_RESOLUTION		1
>  
> +#define CAP_UNALIGNED_ACCESS		BIT(0)
> +
>  static struct workqueue_struct *kpcitest_workqueue;
>  
>  struct pci_epf_test {
> @@ -74,6 +76,7 @@ struct pci_epf_test_reg {
>  	u32	irq_type;
>  	u32	irq_number;
>  	u32	flags;
> +	u32	caps;

Can we rename the 'magic' register? It is not used since the beginning and I
don't know if we will ever have a usecase for it.

- Mani

>  } __packed;
>  
>  static struct pci_epf_header test_header = {
> @@ -739,6 +742,20 @@ static void pci_epf_test_clear_bar(struct pci_epf *epf)
>  	}
>  }
>  
> +static void pci_epf_test_set_capabilities(struct pci_epf *epf)
> +{
> +	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> +	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> +	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> +	struct pci_epc *epc = epf->epc;
> +	u32 caps = 0;
> +
> +	if (epc->ops->align_addr)
> +		caps |= CAP_UNALIGNED_ACCESS;
> +
> +	reg->caps = cpu_to_le32(caps);
> +}
> +
>  static int pci_epf_test_epc_init(struct pci_epf *epf)
>  {
>  	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> @@ -763,6 +780,8 @@ static int pci_epf_test_epc_init(struct pci_epf *epf)
>  		}
>  	}
>  
> +	pci_epf_test_set_capabilities(epf);
> +
>  	ret = pci_epf_test_set_bar(epf);
>  	if (ret)
>  		return ret;
> -- 
> 2.47.0
> 

-- 
மணிவண்ணன் சதாசிவம்