[v8] misc: pci_endpoint_test: Fix overflow of bar_size

[v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Hans Zhang]

With 8GB BAR2, running pcitest -b 2 fails with "TEST FAILED".

The return value of the `pci_resource_len` interface is not an integer.
Using `pcitest` with an 8GB BAR2, the bar_size of integer type will
overflow.

Change the data type of bar_size from integer to resource_size_t, to fix
the above issue.

Signed-off-by: Hans Zhang <18255117159@163.com>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
Changes since v8:

- Add reviewer.

Changes since v4-v7:
https://lore.kernel.org/linux-pci/20250102120222.1403906-1-18255117159@163.com/
https://lore.kernel.org/linux-pci/20250101151509.570341-1-18255117159@163.com/

- Fix 32-bit OS warnings and errors.
- Fix undefined reference to `__udivmoddi4`

Changes since v3:
https://lore.kernel.org/linux-pci/20241221141009.27317-1-18255117159@163.com/

- The patch subject were modified.

Changes since v2:
https://lore.kernel.org/linux-pci/20241220075253.16791-1-18255117159@163.com/

- Fix "changes" part goes below the --- line
- The patch commit message were modified.

Changes since v1:
https://lore.kernel.org/linux-pci/20241217121220.19676-1-18255117159@163.com/

- The patch subject and commit message were modified.
---
 drivers/misc/pci_endpoint_test.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 3aaaf47fa4ee..50d4616119af 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -280,10 +280,11 @@ static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
 static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
 				  enum pci_barno barno)
 {
-	int j, bar_size, buf_size, iters, remain;
 	void *write_buf __free(kfree) = NULL;
 	void *read_buf __free(kfree) = NULL;
 	struct pci_dev *pdev = test->pdev;
+	int j, buf_size, iters, remain;
+	resource_size_t bar_size;
 
 	if (!test->bar[barno])
 		return false;
@@ -307,13 +308,18 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
 	if (!read_buf)
 		return false;
 
-	iters = bar_size / buf_size;
+	if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT)) {
+		remain = do_div(bar_size, buf_size);
+		iters = bar_size;
+	} else {
+		iters = bar_size / buf_size;
+		remain = 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))
 			return false;
 
-	remain = bar_size % buf_size;
 	if (remain)
 		if (pci_endpoint_test_bar_memcmp(test, barno, buf_size * iters,
 						 write_buf, read_buf, remain))

base-commit: ccb98ccef0e543c2bd4ef1a72270461957f3d8d0
-- 
2.25.1

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Niklas Cassel]

On Sat, Jan 04, 2025 at 11:16:52PM +0800, Hans Zhang wrote:
> With 8GB BAR2, running pcitest -b 2 fails with "TEST FAILED".
> 
> The return value of the `pci_resource_len` interface is not an integer.
> Using `pcitest` with an 8GB BAR2, the bar_size of integer type will
> overflow.
> 
> Change the data type of bar_size from integer to resource_size_t, to fix
> the above issue.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> Reviewed-by: Niklas Cassel <cassel@kernel.org>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

When significantly changing the patch from one version to another,
(as in this case), you are supposed to drop the Reviewed-by tags.


Doing a:
$ git grep -A 10 "IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT"
does not show very many hits, which suggests that this is not the proper
way to solve this.

I don't know the proper solution to this. How is resource_size_t handled
in other PCI driver when being built on with 32-bit PHYS_ADDR_T ?

Can't you just cast the resource_size_t to u64 before doing the division?


> ---
> Changes since v8:
> 
> - Add reviewer.
> 
> Changes since v4-v7:
> https://lore.kernel.org/linux-pci/20250102120222.1403906-1-18255117159@163.com/
> https://lore.kernel.org/linux-pci/20250101151509.570341-1-18255117159@163.com/
> 
> - Fix 32-bit OS warnings and errors.
> - Fix undefined reference to `__udivmoddi4`
> 
> Changes since v3:
> https://lore.kernel.org/linux-pci/20241221141009.27317-1-18255117159@163.com/
> 
> - The patch subject were modified.
> 
> Changes since v2:
> https://lore.kernel.org/linux-pci/20241220075253.16791-1-18255117159@163.com/
> 
> - Fix "changes" part goes below the --- line
> - The patch commit message were modified.
> 
> Changes since v1:
> https://lore.kernel.org/linux-pci/20241217121220.19676-1-18255117159@163.com/
> 
> - The patch subject and commit message were modified.
> ---
>  drivers/misc/pci_endpoint_test.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 3aaaf47fa4ee..50d4616119af 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -280,10 +280,11 @@ static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
>  static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>  				  enum pci_barno barno)
>  {
> -	int j, bar_size, buf_size, iters, remain;
>  	void *write_buf __free(kfree) = NULL;
>  	void *read_buf __free(kfree) = NULL;
>  	struct pci_dev *pdev = test->pdev;
> +	int j, buf_size, iters, remain;
> +	resource_size_t bar_size;
>  
>  	if (!test->bar[barno])
>  		return false;
> @@ -307,13 +308,18 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>  	if (!read_buf)
>  		return false;
>  
> -	iters = bar_size / buf_size;
> +	if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT)) {
> +		remain = do_div(bar_size, buf_size);
> +		iters = bar_size;
> +	} else {
> +		iters = bar_size / buf_size;
> +		remain = 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))
>  			return false;
>  
> -	remain = bar_size % buf_size;
>  	if (remain)
>  		if (pci_endpoint_test_bar_memcmp(test, barno, buf_size * iters,
>  						 write_buf, read_buf, remain))
> 
> base-commit: ccb98ccef0e543c2bd4ef1a72270461957f3d8d0
> -- 
> 2.25.1
> 

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Hans Zhang]

On 2025/1/6 19:49, Niklas Cassel wrote:
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> Reviewed-by: Niklas Cassel <cassel@kernel.org>
>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> When significantly changing the patch from one version to another,
> (as in this case), you are supposed to drop the Reviewed-by tags.

Okay, i will remove the reviewer.

> 
> Doing a:
> $ git grep -A 10 "IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT"
> does not show very many hits, which suggests that this is not the proper
> way to solve this.
> 
> I don't know the proper solution to this. How is resource_size_t handled
> in other PCI driver when being built on with 32-bit PHYS_ADDR_T ?
> 
> Can't you just cast the resource_size_t to u64 before doing the division?
> 

Thank you Niklas. I'll try it.

Regards
Hans

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Hans Zhang]

On 2025/1/6 19:49, Niklas Cassel wrote:
> Doing a:
> $ git grep -A 10 "IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT"
> does not show very many hits, which suggests that this is not the proper
> way to solve this.
> 
> I don't know the proper solution to this. How is resource_size_t handled
> in other PCI driver when being built on with 32-bit PHYS_ADDR_T ?
> 
> Can't you just cast the resource_size_t to u64 before doing the division?

Hi Niklas,

Modify as follows, if you have no opinion, I will fix the next version.

>> ---
>>   drivers/misc/pci_endpoint_test.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
>> index 3aaaf47fa4ee..50d4616119af 100644
>> --- a/drivers/misc/pci_endpoint_test.c
>> +++ b/drivers/misc/pci_endpoint_test.c
>> @@ -280,10 +280,11 @@ static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
>>   static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>>   				  enum pci_barno barno)
>>   {
>> -	int j, bar_size, buf_size, iters, remain;
>>   	void *write_buf __free(kfree) = NULL;
>>   	void *read_buf __free(kfree) = NULL;
>>   	struct pci_dev *pdev = test->pdev;
>> +	int j, buf_size, iters, remain;
>> +	resource_size_t bar_size;

Fix resource_size_t to u64 bar_size.
u64 bar_size;

>>   
>>   	if (!test->bar[barno])
>>   		return false;
>> @@ -307,13 +308,18 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>>   	if (!read_buf)
>>   		return false;
>>   
>> -	iters = bar_size / buf_size;
>> +	if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT)) {
>> +		remain = do_div(bar_size, buf_size);
>> +		iters = bar_size;
>> +	} else {
>> +		iters = bar_size / buf_size;
>> +		remain = bar_size % buf_size;
>> +	}

Removed IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT), Execute the following code.

remain = do_div(bar_size, buf_size);
iters = bar_size;

>>   	for (j = 0; j < iters; j++)
>>   		if (pci_endpoint_test_bar_memcmp(test, barno, buf_size * j,
>>   						 write_buf, read_buf, buf_size))
>>   			return false;
>>   
>> -	remain = bar_size % buf_size;
>>   	if (remain)
>>   		if (pci_endpoint_test_bar_memcmp(test, barno, buf_size * iters,
>>   						 write_buf, read_buf, remain))
>>
>> base-commit: ccb98ccef0e543c2bd4ef1a72270461957f3d8d0
>> -- 
>> 2.25.1
>>

Best regards
Hans

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Niklas Cassel]

Hello Hans,

On Mon, Jan 06, 2025 at 11:32:33PM +0800, Hans Zhang wrote:
> 
> 
> On 2025/1/6 19:49, Niklas Cassel wrote:
> > Doing a:
> > $ git grep -A 10 "IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT"
> > does not show very many hits, which suggests that this is not the proper
> > way to solve this.
> > 
> > I don't know the proper solution to this. How is resource_size_t handled
> > in other PCI driver when being built on with 32-bit PHYS_ADDR_T ?
> > 
> > Can't you just cast the resource_size_t to u64 before doing the division?
> 
> Hi Niklas,
> 
> Modify as follows, if you have no opinion, I will fix the next version.
> 
> > > ---
> > >   drivers/misc/pci_endpoint_test.c | 12 +++++++++---
> > >   1 file changed, 9 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> > > index 3aaaf47fa4ee..50d4616119af 100644
> > > --- a/drivers/misc/pci_endpoint_test.c
> > > +++ b/drivers/misc/pci_endpoint_test.c
> > > @@ -280,10 +280,11 @@ static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
> > >   static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
> > >   				  enum pci_barno barno)
> > >   {
> > > -	int j, bar_size, buf_size, iters, remain;
> > >   	void *write_buf __free(kfree) = NULL;
> > >   	void *read_buf __free(kfree) = NULL;
> > >   	struct pci_dev *pdev = test->pdev;
> > > +	int j, buf_size, iters, remain;
> > > +	resource_size_t bar_size;
> 
> Fix resource_size_t to u64 bar_size.
> u64 bar_size;
> 
> > >   	if (!test->bar[barno])
> > >   		return false;
> > > @@ -307,13 +308,18 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
> > >   	if (!read_buf)
> > >   		return false;
> > > -	iters = bar_size / buf_size;
> > > +	if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT)) {
> > > +		remain = do_div(bar_size, buf_size);
> > > +		iters = bar_size;
> > > +	} else {
> > > +		iters = bar_size / buf_size;
> > > +		remain = bar_size % buf_size;
> > > +	}
> 
> Removed IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT), Execute the following code.
> 
> remain = do_div(bar_size, buf_size);
> iters = bar_size;

Perhaps keep it as resource_size_t and then cast it to u64 in the do_div()
call?


Kind regards,
Niklas

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Hans Zhang]

On 2025/1/7 18:32, Niklas Cassel wrote:
>>>> ---
>>>>    drivers/misc/pci_endpoint_test.c | 12 +++++++++---
>>>>    1 file changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
>>>> index 3aaaf47fa4ee..50d4616119af 100644
>>>> --- a/drivers/misc/pci_endpoint_test.c
>>>> +++ b/drivers/misc/pci_endpoint_test.c
>>>> @@ -280,10 +280,11 @@ static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
>>>>    static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>>>>    				  enum pci_barno barno)
>>>>    {
>>>> -	int j, bar_size, buf_size, iters, remain;
>>>>    	void *write_buf __free(kfree) = NULL;
>>>>    	void *read_buf __free(kfree) = NULL;
>>>>    	struct pci_dev *pdev = test->pdev;
>>>> +	int j, buf_size, iters, remain;
>>>> +	resource_size_t bar_size;
>>
>> Fix resource_size_t to u64 bar_size.
>> u64 bar_size;
>>
>>>>    	if (!test->bar[barno])
>>>>    		return false;
>>>> @@ -307,13 +308,18 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>>>>    	if (!read_buf)
>>>>    		return false;
>>>> -	iters = bar_size / buf_size;
>>>> +	if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT)) {
>>>> +		remain = do_div(bar_size, buf_size);
>>>> +		iters = bar_size;
>>>> +	} else {
>>>> +		iters = bar_size / buf_size;
>>>> +		remain = bar_size % buf_size;
>>>> +	}
>>
>> Removed IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT), Execute the following code.
>>
>> remain = do_div(bar_size, buf_size);
>> iters = bar_size;
> 
> Perhaps keep it as resource_size_t and then cast it to u64 in the do_div()
> call?


Hi Niklas,

resource_size_t bar_size;
remain = do_div((u64)bar_size, buf_size);

It works for the arm platform.

arch/arm/include/asm/div64.h
static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
{
	register unsigned int __base      asm("r4") = base;
	register unsigned long long __n   asm("r0") = *n;
	register unsigned long long __res asm("r2");
	unsigned int __rem;
	asm(	__asmeq("%0", "r0")
		__asmeq("%1", "r2")
		__asmeq("%2", "r4")
		"bl	__do_div64"
		: "+r" (__n), "=r" (__res)
		: "r" (__base)
		: "ip", "lr", "cc");
	__rem = __n >> 32;
	*n = __res;
	return __rem;
}
#define __div64_32 __div64_32

#define do_div(n, base) __div64_32(&(n), base)


For X86 platforms, do_div is a macro definition, and the first parameter 
does not define its type. If the macro definition is replaced directly, 
an error will be reported in the ubuntu20.04 release.

resource_size_t bar_size;
remain = do_div((u64)bar_size, buf_size);

arch/x86/include/asm/div64.h
#define do_div(n, base)						\
({								\
	unsigned long __upper, __low, __high, __mod, __base;	\
	__base = (base);					\
	if (__builtin_constant_p(__base) && is_power_of_2(__base)) { \
		__mod = n & (__base - 1);			\
		n >>= ilog2(__base);				\
	} else {						\
		asm("" : "=a" (__low), "=d" (__high) : "A" (n));\
		__upper = __high;				\
		if (__high) {					\
			__upper = __high % (__base);		\
			__high = __high / (__base);		\
		}						\
		asm("divl %2" : "=a" (__low), "=d" (__mod)	\
			: "rm" (__base), "0" (__low), "1" (__upper));	\
		asm("" : "=A" (n) : "a" (__low), "d" (__high));	\
	}							\
	__mod;							\
})


Best regards
Hans

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Niklas Cassel]

On Tue, Jan 07, 2025 at 07:27:24PM +0800, Hans Zhang wrote:
> 
> 
> On 2025/1/7 18:32, Niklas Cassel wrote:
> > > > > ---
> > > > >    drivers/misc/pci_endpoint_test.c | 12 +++++++++---
> > > > >    1 file changed, 9 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> > > > > index 3aaaf47fa4ee..50d4616119af 100644
> > > > > --- a/drivers/misc/pci_endpoint_test.c
> > > > > +++ b/drivers/misc/pci_endpoint_test.c
> > > > > @@ -280,10 +280,11 @@ static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
> > > > >    static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
> > > > >    				  enum pci_barno barno)
> > > > >    {
> > > > > -	int j, bar_size, buf_size, iters, remain;
> > > > >    	void *write_buf __free(kfree) = NULL;
> > > > >    	void *read_buf __free(kfree) = NULL;
> > > > >    	struct pci_dev *pdev = test->pdev;
> > > > > +	int j, buf_size, iters, remain;
> > > > > +	resource_size_t bar_size;
> > > 
> > > Fix resource_size_t to u64 bar_size.
> > > u64 bar_size;
> > > 
> > > > >    	if (!test->bar[barno])
> > > > >    		return false;
> > > > > @@ -307,13 +308,18 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
> > > > >    	if (!read_buf)
> > > > >    		return false;
> > > > > -	iters = bar_size / buf_size;
> > > > > +	if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT)) {
> > > > > +		remain = do_div(bar_size, buf_size);
> > > > > +		iters = bar_size;
> > > > > +	} else {
> > > > > +		iters = bar_size / buf_size;
> > > > > +		remain = bar_size % buf_size;
> > > > > +	}
> > > 
> > > Removed IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT), Execute the following code.
> > > 
> > > remain = do_div(bar_size, buf_size);
> > > iters = bar_size;
> > 
> > Perhaps keep it as resource_size_t and then cast it to u64 in the do_div()
> > call?
> 
> 
> Hi Niklas,
> 
> resource_size_t bar_size;
> remain = do_div((u64)bar_size, buf_size);
> 
> It works for the arm platform.
> 
> arch/arm/include/asm/div64.h
> static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
> {
> 	register unsigned int __base      asm("r4") = base;
> 	register unsigned long long __n   asm("r0") = *n;
> 	register unsigned long long __res asm("r2");
> 	unsigned int __rem;
> 	asm(	__asmeq("%0", "r0")
> 		__asmeq("%1", "r2")
> 		__asmeq("%2", "r4")
> 		"bl	__do_div64"
> 		: "+r" (__n), "=r" (__res)
> 		: "r" (__base)
> 		: "ip", "lr", "cc");
> 	__rem = __n >> 32;
> 	*n = __res;
> 	return __rem;
> }
> #define __div64_32 __div64_32
> 
> #define do_div(n, base) __div64_32(&(n), base)
> 
> 
> For X86 platforms, do_div is a macro definition, and the first parameter
> does not define its type. If the macro definition is replaced directly, an
> error will be reported in the ubuntu20.04 release.

What is the error?

We don't need to use do_div().
The current code that does normal / and % works fine on both
32-bit and 64-bit if you just do:

 static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
                                  enum pci_barno barno)
 {
-       int j, bar_size, buf_size, iters, remain;
+       int j, buf_size, iters, remain;
        void *write_buf __free(kfree) = NULL;
        void *read_buf __free(kfree) = NULL;
        struct pci_dev *pdev = test->pdev;
+       u64 bar_size;

No?


Kind regards,
Niklas

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Hans Zhang]

On 2025/1/7 19:33, Niklas Cassel wrote:
>> Hi Niklas,
>>
>> resource_size_t bar_size;
>> remain = do_div((u64)bar_size, buf_size);
>>
>> It works for the arm platform.
>>
>> arch/arm/include/asm/div64.h
>> static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
>> {
>> 	register unsigned int __base      asm("r4") = base;
>> 	register unsigned long long __n   asm("r0") = *n;
>> 	register unsigned long long __res asm("r2");
>> 	unsigned int __rem;
>> 	asm(	__asmeq("%0", "r0")
>> 		__asmeq("%1", "r2")
>> 		__asmeq("%2", "r4")
>> 		"bl	__do_div64"
>> 		: "+r" (__n), "=r" (__res)
>> 		: "r" (__base)
>> 		: "ip", "lr", "cc");
>> 	__rem = __n >> 32;
>> 	*n = __res;
>> 	return __rem;
>> }
>> #define __div64_32 __div64_32
>>
>> #define do_div(n, base) __div64_32(&(n), base)
>>
>>
>> For X86 platforms, do_div is a macro definition, and the first parameter
>> does not define its type. If the macro definition is replaced directly, an
>> error will be reported in the ubuntu20.04 release.
> 
> What is the error?
> 
> We don't need to use do_div().
> The current code that does normal / and % works fine on both
> 32-bit and 64-bit if you just do:
> 
>   static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>                                    enum pci_barno barno)
>   {
> -       int j, bar_size, buf_size, iters, remain;
> +       int j, buf_size, iters, remain;
>          void *write_buf __free(kfree) = NULL;
>          void *read_buf __free(kfree) = NULL;
>          struct pci_dev *pdev = test->pdev;
> +       u64 bar_size;
> 
> No?


Hi Niklas,

Please look at the robot compilation error.

https://patchwork.kernel.org/project/linux-pci/patch/20241231065500.168799-1-18255117159@163.com/

drivers/misc/pci_endpoint_test.c:315: undefined reference to `__udivmoddi4'

Best regards
Hans

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Niklas Cassel]

On Tue, Jan 07, 2025 at 07:43:26PM +0800, Hans Zhang wrote:
> 
> 
> On 2025/1/7 19:33, Niklas Cassel wrote:
> > > Hi Niklas,
> > > 
> > > resource_size_t bar_size;
> > > remain = do_div((u64)bar_size, buf_size);
> > > 
> > > It works for the arm platform.
> > > 
> > > arch/arm/include/asm/div64.h
> > > static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
> > > {
> > > 	register unsigned int __base      asm("r4") = base;
> > > 	register unsigned long long __n   asm("r0") = *n;
> > > 	register unsigned long long __res asm("r2");
> > > 	unsigned int __rem;
> > > 	asm(	__asmeq("%0", "r0")
> > > 		__asmeq("%1", "r2")
> > > 		__asmeq("%2", "r4")
> > > 		"bl	__do_div64"
> > > 		: "+r" (__n), "=r" (__res)
> > > 		: "r" (__base)
> > > 		: "ip", "lr", "cc");
> > > 	__rem = __n >> 32;
> > > 	*n = __res;
> > > 	return __rem;
> > > }
> > > #define __div64_32 __div64_32
> > > 
> > > #define do_div(n, base) __div64_32(&(n), base)
> > > 
> > > 
> > > For X86 platforms, do_div is a macro definition, and the first parameter
> > > does not define its type. If the macro definition is replaced directly, an
> > > error will be reported in the ubuntu20.04 release.
> > 
> > What is the error?
> > 
> > We don't need to use do_div().
> > The current code that does normal / and % works fine on both
> > 32-bit and 64-bit if you just do:
> > 
> >   static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
> >                                    enum pci_barno barno)
> >   {
> > -       int j, bar_size, buf_size, iters, remain;
> > +       int j, buf_size, iters, remain;
> >          void *write_buf __free(kfree) = NULL;
> >          void *read_buf __free(kfree) = NULL;
> >          struct pci_dev *pdev = test->pdev;
> > +       u64 bar_size;
> > 
> > No?
> 
> 
> Hi Niklas,
> 
> Please look at the robot compilation error.
> 
> https://patchwork.kernel.org/project/linux-pci/patch/20241231065500.168799-1-18255117159@163.com/
> 
> drivers/misc/pci_endpoint_test.c:315: undefined reference to `__udivmoddi4'

That error was for your patch that changed bar_size to resource_size_t,
which is typedefed to phys_addr_t, which can be either 32-bits or 64-bits
on 32-bit systems (depending on CONFIG_X86_PAE).

I was suggesting to just change it to u64, so it will unconditionally be
64-bits.


Kind regards,
Niklas

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Hans Zhang]

On 2025/1/7 19:47, Niklas Cassel wrote:
> On Tue, Jan 07, 2025 at 07:43:26PM +0800, Hans Zhang wrote:
>>
>>
>> On 2025/1/7 19:33, Niklas Cassel wrote:
>>>> Hi Niklas,
>>>>
>>>> resource_size_t bar_size;
>>>> remain = do_div((u64)bar_size, buf_size);
>>>>
>>>> It works for the arm platform.
>>>>
>>>> arch/arm/include/asm/div64.h
>>>> static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
>>>> {
>>>> 	register unsigned int __base      asm("r4") = base;
>>>> 	register unsigned long long __n   asm("r0") = *n;
>>>> 	register unsigned long long __res asm("r2");
>>>> 	unsigned int __rem;
>>>> 	asm(	__asmeq("%0", "r0")
>>>> 		__asmeq("%1", "r2")
>>>> 		__asmeq("%2", "r4")
>>>> 		"bl	__do_div64"
>>>> 		: "+r" (__n), "=r" (__res)
>>>> 		: "r" (__base)
>>>> 		: "ip", "lr", "cc");
>>>> 	__rem = __n >> 32;
>>>> 	*n = __res;
>>>> 	return __rem;
>>>> }
>>>> #define __div64_32 __div64_32
>>>>
>>>> #define do_div(n, base) __div64_32(&(n), base)
>>>>
>>>>
>>>> For X86 platforms, do_div is a macro definition, and the first parameter
>>>> does not define its type. If the macro definition is replaced directly, an
>>>> error will be reported in the ubuntu20.04 release.
>>>
>>> What is the error?
>>>
>>> We don't need to use do_div().
>>> The current code that does normal / and % works fine on both
>>> 32-bit and 64-bit if you just do:
>>>
>>>    static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>>>                                     enum pci_barno barno)
>>>    {
>>> -       int j, bar_size, buf_size, iters, remain;
>>> +       int j, buf_size, iters, remain;
>>>           void *write_buf __free(kfree) = NULL;
>>>           void *read_buf __free(kfree) = NULL;
>>>           struct pci_dev *pdev = test->pdev;
>>> +       u64 bar_size;
>>>
>>> No?
>>
>>
>> Hi Niklas,
>>
>> Please look at the robot compilation error.
>>
>> https://patchwork.kernel.org/project/linux-pci/patch/20241231065500.168799-1-18255117159@163.com/
>>
>> drivers/misc/pci_endpoint_test.c:315: undefined reference to `__udivmoddi4'
> 
> That error was for your patch that changed bar_size to resource_size_t,
> which is typedefed to phys_addr_t, which can be either 32-bits or 64-bits
> on 32-bit systems (depending on CONFIG_X86_PAE).
> 
> I was suggesting to just change it to u64, so it will unconditionally be
> 64-bits.

Hi Niklas,

The robot has been compiled with CONFIG_PHYS_ADDR_T_64BIT=y, so 
resource_size_t=u64


include/linux/types.h

#ifdef CONFIG_PHYS_ADDR_T_64BIT
typedef u64 phys_addr_t;
#else
typedef u32 phys_addr_t;
#endif

typedef phys_addr_t resource_size_t;


Is my understanding wrong? Could you correct me, please? Thank you very 
much.

config: i386-randconfig-003-20250101 
(https://download.01.org/0day-ci/archive/20250101/202501011917.ugP1ywJV-lkp@intel.com/config)


I compiled it as a KO module for an experiment.
__umoddi3 and __udivdi3 is similar to __udivmoddi4.

u64 bar_size;

iters = bar_size / buf_size;
remain = bar_size % buf_size;

zhb@zhb:/media/zhb/hans/code/kernel_org/hans$ make
make -C /media/zhb/hans/code/kernel_org/linux/ 
M=/media/zhb/hans/code/kernel_org/hans modules
make[1]: Entering directory '/media/zhb/hans/code/kernel_org/linux'
make[2]: Entering directory '/media/zhb/hans/code/kernel_org/hans'
   CC [M]  pci_endpoint_test.o
   MODPOST Module.symvers
ERROR: modpost: "__umoddi3" [pci_endpoint_test.ko] undefined!
ERROR: modpost: "__udivdi3" [pci_endpoint_test.ko] undefined!
make[4]: *** 
[/media/zhb/hans/code/kernel_org/linux/scripts/Makefile.modpost:145: 
Module.symvers] Error 1
make[3]: *** [/media/zhb/hans/code/kernel_org/linux/Makefile:1939: 
modpost] Error 2
make[2]: *** [/media/zhb/hans/code/kernel_org/linux/Makefile:251: 
__sub-make] Error 2
make[2]: Leaving directory '/media/zhb/hans/code/kernel_org/hans'
make[1]: *** [Makefile:251: __sub-make] Error 2
make[1]: Leaving directory '/media/zhb/hans/code/kernel_org/linux'
make: *** [Makefile:10: kernel_modules] Error 2


Best regards
Hans

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Niklas Cassel]

On Tue, Jan 07, 2025 at 08:09:48PM +0800, Hans Zhang wrote:
> Hi Niklas,
> 
> The robot has been compiled with CONFIG_PHYS_ADDR_T_64BIT=y, so
> resource_size_t=u64
> 
> 
> include/linux/types.h
> 
> #ifdef CONFIG_PHYS_ADDR_T_64BIT
> typedef u64 phys_addr_t;
> #else
> typedef u32 phys_addr_t;
> #endif
> 
> typedef phys_addr_t resource_size_t;
> 
> 
> Is my understanding wrong? Could you correct me, please? Thank you very
> much.

I see. That is correct.


> 
> config: i386-randconfig-003-20250101 (https://download.01.org/0day-ci/archive/20250101/202501011917.ugP1ywJV-lkp@intel.com/config)
> 
> 
> I compiled it as a KO module for an experiment.
> __umoddi3 and __udivdi3 is similar to __udivmoddi4.
> 
> u64 bar_size;
> 
> iters = bar_size / buf_size;
> remain = bar_size % buf_size;

I think that I am an idiot (I'm the one who wrote this code).

A BAR size is always a power of two.

So I don't see how there can ever be a remainer...

buf_size = min(SZ_1M, bar_size);

If the BAR size is <= 1MB, there will be 1 iteration, no remainder.

If the BAR size is > 1MB, there will be more than one iteration,
but the size will always be evenly divisible by 1MB, so no remainder.

This should probably be two patches:
patch 1/2:
@@ -316,12 +317,6 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
                                                 write_buf, read_buf, buf_size))
                        return false;
 
-       remain = bar_size % buf_size;
-       if (remain)
-               if (pci_endpoint_test_bar_memcmp(test, barno, buf_size * iters,
-                                                write_buf, read_buf, remain))
-                       return false;
-
        return true;
 }


patch 2/2:
@@ -283,10 +283,11 @@ static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
 static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
                                  enum pci_barno barno)
 {
-       int j, bar_size, buf_size, iters, remain;
+       int j, buf_size, iters;
        void *write_buf __free(kfree) = NULL;
        void *read_buf __free(kfree) = NULL;
        struct pci_dev *pdev = test->pdev;
+       resource_size_t bar_size;
 
        if (!test->bar[barno])
                return false;



The error:
drivers/misc/pci_endpoint_test.c:315: undefined reference to `__udivmoddi4'
sounds like the compiler is using a specialized instruction to do both div
and mod in one. By removing the mod in patch 1/2, I expect that patch 2/2
will no longer get this error.


Kind regards,
Niklas

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Hans Zhang]

On 2025/1/7 19:47, Niklas Cassel wrote:

Hi Niklas,

> The error:
> drivers/misc/pci_endpoint_test.c:315: undefined reference to `__udivmoddi4'
> sounds like the compiler is using a specialized instruction to do both div
> and mod in one. By removing the mod in patch 1/2, I expect that patch 2/2
> will no longer get this error.

The __udivmoddi4 may be the way div and mod are combined.

Delete remain's patch 1/2 according to your suggestion. I compiled it as 
a KO module for an experiment.

There are still __udivdi3 errors, so the do_div API must be used.

zhb@zhb:/media/zhb/hans/code/kernel_org/hans$ make
make -C /media/zhb/hans/code/kernel_org/linux/ 
M=/media/zhb/hans/code/kernel_org/hans modules
make[1]: Entering directory '/media/zhb/hans/code/kernel_org/linux'
make[2]: Entering directory '/media/zhb/hans/code/kernel_org/hans'
   CC [M]  pci_endpoint_test.o
   MODPOST Module.symvers
ERROR: modpost: "__udivdi3" [pci_endpoint_test.ko] undefined!
make[4]: *** 
[/media/zhb/hans/code/kernel_org/linux/scripts/Makefile.modpost:145: 
Module.symvers] Error 1
make[3]: *** [/media/zhb/hans/code/kernel_org/linux/Makefile:1939: 
modpost] Error 2
make[2]: *** [/media/zhb/hans/code/kernel_org/linux/Makefile:251: 
__sub-make] Error 2
make[2]: Leaving directory '/media/zhb/hans/code/kernel_org/hans'
make[1]: *** [Makefile:251: __sub-make] Error 2
make[1]: Leaving directory '/media/zhb/hans/code/kernel_org/linux'
make: *** [Makefile:10: kernel_modules] Error 2

Best regards
Hans

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Niklas Cassel]

On Tue, Jan 07, 2025 at 11:44:21PM +0800, Hans Zhang wrote:
> 
> 
> On 2025/1/7 19:47, Niklas Cassel wrote:
> 
> Hi Niklas,
> 
> > The error:
> > drivers/misc/pci_endpoint_test.c:315: undefined reference to `__udivmoddi4'
> > sounds like the compiler is using a specialized instruction to do both div
> > and mod in one. By removing the mod in patch 1/2, I expect that patch 2/2
> > will no longer get this error.
> 
> The __udivmoddi4 may be the way div and mod are combined.
> 
> Delete remain's patch 1/2 according to your suggestion. I compiled it as a
> KO module for an experiment.
> 
> There are still __udivdi3 errors, so the do_div API must be used.

Ok. Looking at do_div(), it seems to be the correct API to use
for this problem. Just change bar_size type to u64 (instead of casting)
and use do_div() ? That is how it is seems to be used in other drivers.

I still think that a patch that removes the "remainder" code is a good
cleanup, so please send it as patch 1/2, you can be the author, just add:
Suggested-by: Niklas Cassel <cassel@kernel.org>


Kind regards,
Niklas

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Hans Zhang]

On 2025/1/7 23:57, Niklas Cassel wrote:>>> The error:
>>> drivers/misc/pci_endpoint_test.c:315: undefined reference to `__udivmoddi4'
>>> sounds like the compiler is using a specialized instruction to do both div
>>> and mod in one. By removing the mod in patch 1/2, I expect that patch 2/2
>>> will no longer get this error.
>>
>> The __udivmoddi4 may be the way div and mod are combined.
>>
>> Delete remain's patch 1/2 according to your suggestion. I compiled it as a
>> KO module for an experiment.
>>
>> There are still __udivdi3 errors, so the do_div API must be used.
> 
> Ok. Looking at do_div(), it seems to be the correct API to use
> for this problem. Just change bar_size type to u64 (instead of casting)
> and use do_div() ? That is how it is seems to be used in other drivers.
> 
> I still think that a patch that removes the "remainder" code is a good
> cleanup, so please send it as patch 1/2, you can be the author, just add:
> Suggested-by: Niklas Cassel <cassel@kernel.org>
> 

Thank you very much for Niklas' discussion on this patch. I will 
resubmit two patches in the future.

Best regards
Hans

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Arnd Bergmann]

On Tue, Jan 7, 2025, at 16:57, Niklas Cassel wrote:
> On Tue, Jan 07, 2025 at 11:44:21PM +0800, Hans Zhang wrote:
>> On 2025/1/7 19:47, Niklas Cassel wrote:
>> 
>> Hi Niklas,
>> 
>> > The error:
>> > drivers/misc/pci_endpoint_test.c:315: undefined reference to `__udivmoddi4'
>> > sounds like the compiler is using a specialized instruction to do both div
>> > and mod in one. By removing the mod in patch 1/2, I expect that patch 2/2
>> > will no longer get this error.
>> 
>> The __udivmoddi4 may be the way div and mod are combined.
>> 
>> Delete remain's patch 1/2 according to your suggestion. I compiled it as a
>> KO module for an experiment.
>> 
>> There are still __udivdi3 errors, so the do_div API must be used.
>
> Ok. Looking at do_div(), it seems to be the correct API to use
> for this problem. Just change bar_size type to u64 (instead of casting)
> and use do_div() ? That is how it is seems to be used in other drivers.

I think using div_u64_rem() instead of do_div() would make this
more readable as this is always an inline function, so the type can
remain resource_size_t, and the division gets optimized well when
that is a 32-bit type.

     Arnd

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Niklas Cassel]

On Wed, Jan 08, 2025 at 03:10:03PM +0100, Arnd Bergmann wrote:
> On Tue, Jan 7, 2025, at 16:57, Niklas Cassel wrote:
> > On Tue, Jan 07, 2025 at 11:44:21PM +0800, Hans Zhang wrote:
> >> On 2025/1/7 19:47, Niklas Cassel wrote:
> >> 
> >> Hi Niklas,
> >> 
> >> > The error:
> >> > drivers/misc/pci_endpoint_test.c:315: undefined reference to `__udivmoddi4'
> >> > sounds like the compiler is using a specialized instruction to do both div
> >> > and mod in one. By removing the mod in patch 1/2, I expect that patch 2/2
> >> > will no longer get this error.
> >> 
> >> The __udivmoddi4 may be the way div and mod are combined.
> >> 
> >> Delete remain's patch 1/2 according to your suggestion. I compiled it as a
> >> KO module for an experiment.
> >> 
> >> There are still __udivdi3 errors, so the do_div API must be used.
> >
> > Ok. Looking at do_div(), it seems to be the correct API to use
> > for this problem. Just change bar_size type to u64 (instead of casting)
> > and use do_div() ? That is how it is seems to be used in other drivers.
> 
> I think using div_u64_rem() instead of do_div() would make this
> more readable as this is always an inline function, so the type can
> remain resource_size_t, and the division gets optimized well when
> that is a 32-bit type.

After patch 1/2, we no longer care about the remainder, so I guess
div64_u64() is the correct function to use then?


Kind regards,
Niklas

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Hans Zhang]

On 2025/1/8 22:13, Niklas Cassel wrote:
Hi Niklas,

>>> Ok. Looking at do_div(), it seems to be the correct API to use
>>> for this problem. Just change bar_size type to u64 (instead of casting)
>>> and use do_div() ? That is how it is seems to be used in other drivers.
>>
>> I think using div_u64_rem() instead of do_div() would make this
>> more readable as this is always an inline function, so the type can
>> remain resource_size_t, and the division gets optimized well when
>> that is a 32-bit type.
> 
> After patch 1/2, we no longer care about the remainder, so I guess
> div64_u64() is the correct function to use then?
> 

include/asm-generic/bitsperlong.h
#ifdef CONFIG_64BIT
#define BITS_PER_LONG 64
#else
#define BITS_PER_LONG 32
#endif /* CONFIG_64BIT */

include/linux/types.h
#ifdef CONFIG_PHYS_ADDR_T_64BIT
typedef u64 phys_addr_t;
#else
typedef u32 phys_addr_t;
#endif

typedef phys_addr_t resource_size_t;


include/linux/math64.h
#if BITS_PER_LONG == 64
......
static inline u64 div64_u64(u64 dividend, u64 divisor)
{
	return dividend / divisor;
}
......
#elif BITS_PER_LONG == 32
#ifndef div64_u64
extern u64 div64_u64(u64 dividend, u64 divisor);
#endif
......
#endif /* BITS_PER_LONG */

lib/math/div64.c
u64 div64_u64(u64 dividend, u64 divisor)
{
	u32 high = divisor >> 32;
	u64 quot;

	if (high == 0) {
		quot = div_u64(dividend, divisor);
	} else {
		int n = fls(high);
		quot = div_u64(dividend >> n, divisor >> n);

		if (quot != 0)
			quot--;
		if ((dividend - quot * divisor) >= divisor)
			quot++;
	}

	return quot;
}
EXPORT_SYMBOL(div64_u64);


If CONFIG_64BIT and CONFIG_PHYS_ADDR_T_64BIT are not configured. The 
resource_size_t=u32, and the first parameter of div64_u64 is u64.The 
same question is as follows:

https://patchwork.kernel.org/project/linux-pci/patch/20250102120222.1403906-1-18255117159@163.com/

config: arm-defconfig 
(https://download.01.org/0day-ci/archive/20250103/202501030414.p0DE9xNK-lkp@intel.com/config)

All error/warnings (new ones prefixed by >>):

 >> drivers/misc/pci_endpoint_test.c:311:11: warning: comparison of 
distinct pointer types ('typeof ((bar_size)) *' (aka 'unsigned int *') 
and 'uint64_t *' (aka 'unsigned long long *')) 
[-Wcompare-distinct-pointer-types]
      311 |         remain = do_div(bar_size, buf_size);
          |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~
    include/asm-generic/div64.h:183:28: note: expanded from macro 'do_div'
      183 |         (void)(((typeof((n)) *)0) == ((uint64_t *)0));  \
          |                ~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~
 >> drivers/misc/pci_endpoint_test.c:311:11: error: incompatible pointer 
types passing 'resource_size_t *' (aka 'unsigned int *') to parameter of 
type 'uint64_t *' (aka 'unsigned long long *') 
[-Werror,-Wincompatible-pointer-types]
      311 |         remain = do_div(bar_size, buf_size);
          |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~
    include/asm-generic/div64.h:199:22: note: expanded from macro 'do_div'
      199 |                 __rem = __div64_32(&(n), __base);       \
          |                                    ^~~~
    arch/arm/include/asm/div64.h:24:45: note: passing argument to 
parameter 'n' here
       24 | static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
          |                                             ^
 >> drivers/misc/pci_endpoint_test.c:311:11: warning: shift count >= 
width of type [-Wshift-count-overflow]
      311 |         remain = do_div(bar_size, buf_size);
          |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~
    include/asm-generic/div64.h:195:25: note: expanded from macro 'do_div'
      195 |         } else if (likely(((n) >> 32) == 0)) {          \
          |                                ^  ~~
    include/linux/compiler.h:76:40: note: expanded from macro 'likely'
       76 | # define likely(x)      __builtin_expect(!!(x), 1)
          |                                             ^
    2 warnings and 1 error generated.

Best regards
Hans

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Arnd Bergmann]

On Thu, Jan 9, 2025, at 03:59, Hans Zhang wrote:
> On 2025/1/8 22:13, Niklas Cassel wrote:
>>>> Ok. Looking at do_div(), it seems to be the correct API to use
>>>> for this problem. Just change bar_size type to u64 (instead of casting)
>>>> and use do_div() ? That is how it is seems to be used in other drivers.
>>>
>>> I think using div_u64_rem() instead of do_div() would make this
>>> more readable as this is always an inline function, so the type can
>>> remain resource_size_t, and the division gets optimized well when
>>> that is a 32-bit type.
>> 
>> After patch 1/2, we no longer care about the remainder, so I guess
>> div64_u64() is the correct function to use then?

div_u64() is the correct interface here, div64_u64() is the
even slower version where both arguments are 64-bit wide.

>  >> drivers/misc/pci_endpoint_test.c:311:11: warning: comparison of 
> distinct pointer types ('typeof ((bar_size)) *' (aka 'unsigned int *') 
> and 'uint64_t *' (aka 'unsigned long long *')) 
> [-Wcompare-distinct-pointer-types]
>       311 |         remain = do_div(bar_size, buf_size);
>           |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~

You don't use div_u64() or div64_u64() here, do_div() is the macro
version that must be called with a 64-bit argument.

    Arnd

Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

[Hans Zhang]

On 2025/1/9 14:29, Arnd Bergmann wrote:
>> On 2025/1/8 22:13, Niklas Cassel wrote:
>>>>> Ok. Looking at do_div(), it seems to be the correct API to use
>>>>> for this problem. Just change bar_size type to u64 (instead of casting)
>>>>> and use do_div() ? That is how it is seems to be used in other drivers.
>>>>
>>>> I think using div_u64_rem() instead of do_div() would make this
>>>> more readable as this is always an inline function, so the type can
>>>> remain resource_size_t, and the division gets optimized well when
>>>> that is a 32-bit type.
>>>
>>> After patch 1/2, we no longer care about the remainder, so I guess
>>> div64_u64() is the correct function to use then?
> 
> div_u64() is the correct interface here, div64_u64() is the
> even slower version where both arguments are 64-bit wide.
> 
>>   >> drivers/misc/pci_endpoint_test.c:311:11: warning: comparison of
>> distinct pointer types ('typeof ((bar_size)) *' (aka 'unsigned int *')
>> and 'uint64_t *' (aka 'unsigned long long *'))
>> [-Wcompare-distinct-pointer-types]
>>        311 |         remain = do_div(bar_size, buf_size);
>>            |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> You don't use div_u64() or div64_u64() here, do_div() is the macro
> version that must be called with a 64-bit argument.
> 
>      Arnd

Thank you so much Niklas and Arnd.

Best regards
Hans