diff mbox series

[v3] misc: pci_endpoint_test: Handle BAR sizes larger than INT_MAX

Message ID 20250213133913.17391-2-cassel@kernel.org (mailing list archive)
State Superseded
Delegated to: Krzysztof WilczyƄski
Headers show
Series [v3] misc: pci_endpoint_test: Handle BAR sizes larger than INT_MAX | expand

Commit Message

Niklas Cassel Feb. 13, 2025, 1:39 p.m. UTC
Running 'pcitest -b 0' fails with "TEST FAILED" when the BAR0 size
is e.g. 8 GB.

The return value of the pci_resource_len() macro can be larger than that
of a signed integer type. Thus, when using 'pcitest' with an 8 GB BAR,
the bar_size of the integer type will overflow.

Change bar_size from integer to resource_size_t to prevent integer
overflow for large BAR sizes with 32-bit compilers.

In order to handle 64-bit resource_type_t on 32-bit platforms, we would
have needed to use a function like div_u64() or similar. Instead, change
the code to use addition instead of division. This avoids the need for
div_u64() or similar, while also simplifying the code.

Fixes: cda370ec6d1f ("misc: pci_endpoint_test: Avoid using hard-coded BAR sizes")
Co-developed-by: Hans Zhang <18255117159@163.com>
Signed-off-by: Hans Zhang <18255117159@163.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Changes since v2:
-Add Fixes: tag.

 drivers/misc/pci_endpoint_test.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

Comments

Frank Li Feb. 13, 2025, 3:58 p.m. UTC | #1
On Thu, Feb 13, 2025 at 02:39:14PM +0100, Niklas Cassel wrote:
> Running 'pcitest -b 0' fails with "TEST FAILED" when the BAR0 size
> is e.g. 8 GB.

pcitest is not exist now. Does it need update to pci_endpoint_test?

Frank

>
> The return value of the pci_resource_len() macro can be larger than that
> of a signed integer type. Thus, when using 'pcitest' with an 8 GB BAR,
> the bar_size of the integer type will overflow.
>
> Change bar_size from integer to resource_size_t to prevent integer
> overflow for large BAR sizes with 32-bit compilers.
>
> In order to handle 64-bit resource_type_t on 32-bit platforms, we would
> have needed to use a function like div_u64() or similar. Instead, change
> the code to use addition instead of division. This avoids the need for
> div_u64() or similar, while also simplifying the code.
>
> Fixes: cda370ec6d1f ("misc: pci_endpoint_test: Avoid using hard-coded BAR sizes")
> Co-developed-by: Hans Zhang <18255117159@163.com>
> Signed-off-by: Hans Zhang <18255117159@163.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Tested-by: Jon Hunter <jonathanh@nvidia.com>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> Changes since v2:
> -Add Fixes: tag.
>
>  drivers/misc/pci_endpoint_test.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index d5ac71a49386..8e48a15100f1 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -272,9 +272,9 @@ static const u32 bar_test_pattern[] = {
>  };
>
>  static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
> -					enum pci_barno barno, int offset,
> -					void *write_buf, void *read_buf,
> -					int size)
> +					enum pci_barno barno,
> +					resource_size_t offset, void *write_buf,
> +					void *read_buf, int size)
>  {
>  	memset(write_buf, bar_test_pattern[barno], size);
>  	memcpy_toio(test->bar[barno] + offset, write_buf, size);
> @@ -287,10 +287,11 @@ static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
>  static int pci_endpoint_test_bar(struct pci_endpoint_test *test,
>  				  enum pci_barno barno)
>  {
> -	int j, bar_size, buf_size, iters;
> +	resource_size_t bar_size, offset = 0;
>  	void *write_buf __free(kfree) = NULL;
>  	void *read_buf __free(kfree) = NULL;
>  	struct pci_dev *pdev = test->pdev;
> +	int buf_size;
>
>  	if (!test->bar[barno])
>  		return -ENOMEM;
> @@ -314,11 +315,12 @@ static int pci_endpoint_test_bar(struct pci_endpoint_test *test,
>  	if (!read_buf)
>  		return -ENOMEM;
>
> -	iters = bar_size / buf_size;
> -	for (j = 0; j < iters; j++)
> -		if (pci_endpoint_test_bar_memcmp(test, barno, buf_size * j,
> -						 write_buf, read_buf, buf_size))
> +	while (offset < bar_size) {
> +		if (pci_endpoint_test_bar_memcmp(test, barno, offset, write_buf,
> +						 read_buf, buf_size))
>  			return -EIO;
> +		offset += buf_size;
> +	}
>
>  	return 0;
>  }
> --
> 2.48.1
>
Hans Zhang Feb. 14, 2025, 6 a.m. UTC | #2
On 2025/2/13 23:58, Frank Li wrote:
> On Thu, Feb 13, 2025 at 02:39:14PM +0100, Niklas Cassel wrote:
>> Running 'pcitest -b 0' fails with "TEST FAILED" when the BAR0 size
>> is e.g. 8 GB.
> 
> pcitest is not exist now. Does it need update to pci_endpoint_test?
> 

Hi Frank,

Mani has modified the entire pci_endpoint_test test framework, BAR test 
is still required. So Niklas may need to modify the commit message.

392188bb0f6e ("selftests: pci_endpoint: Migrate to Kselftest framework")
e19bde2269ca ("selftests: Move PCI Endpoint tests from tools/pci to 
Kselftests")

Best regards
Hans
diff mbox series

Patch

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index d5ac71a49386..8e48a15100f1 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -272,9 +272,9 @@  static const u32 bar_test_pattern[] = {
 };
 
 static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
-					enum pci_barno barno, int offset,
-					void *write_buf, void *read_buf,
-					int size)
+					enum pci_barno barno,
+					resource_size_t offset, void *write_buf,
+					void *read_buf, int size)
 {
 	memset(write_buf, bar_test_pattern[barno], size);
 	memcpy_toio(test->bar[barno] + offset, write_buf, size);
@@ -287,10 +287,11 @@  static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
 static int pci_endpoint_test_bar(struct pci_endpoint_test *test,
 				  enum pci_barno barno)
 {
-	int j, bar_size, buf_size, iters;
+	resource_size_t bar_size, offset = 0;
 	void *write_buf __free(kfree) = NULL;
 	void *read_buf __free(kfree) = NULL;
 	struct pci_dev *pdev = test->pdev;
+	int buf_size;
 
 	if (!test->bar[barno])
 		return -ENOMEM;
@@ -314,11 +315,12 @@  static int pci_endpoint_test_bar(struct pci_endpoint_test *test,
 	if (!read_buf)
 		return -ENOMEM;
 
-	iters = bar_size / buf_size;
-	for (j = 0; j < iters; j++)
-		if (pci_endpoint_test_bar_memcmp(test, barno, buf_size * j,
-						 write_buf, read_buf, buf_size))
+	while (offset < bar_size) {
+		if (pci_endpoint_test_bar_memcmp(test, barno, offset, write_buf,
+						 read_buf, buf_size))
 			return -EIO;
+		offset += buf_size;
+	}
 
 	return 0;
 }