[PATCH 2/2] IB/rdmavt: modify rdmavt/qp.c for UML

[PATCH 2/2] IB/rdmavt: modify rdmavt/qp.c for UML

[Randy Dunlap]

When building rdmavt for ARCH=um, qp.c has a build error on a reference
to the x86-specific cpuinfo field 'x86_cache_size'. This value is then
used to determine whether to use cacheless_memcpy() or not.
Provide a fake value to LLC for CONFIG_UML. Then provide a separate
verison of cacheless_memcpy() for CONFIG_UML that is just a plain
memcpy(), like the calling code uses.

Prevents these build errors:

../drivers/infiniband/sw/rdmavt/qp.c: In function ‘rvt_wss_llc_size’:
../drivers/infiniband/sw/rdmavt/qp.c:88:23: error: ‘struct cpuinfo_um’ has no member named ‘x86_cache_size’; did you mean ‘x86_capability’?
  return boot_cpu_data.x86_cache_size;

../drivers/infiniband/sw/rdmavt/qp.c: In function ‘cacheless_memcpy’:
../drivers/infiniband/sw/rdmavt/qp.c:100:2: error: implicit declaration of function ‘__copy_user_nocache’; did you mean ‘copy_user_page’? [-Werror=implicit-function-declaration]
  __copy_user_nocache(dst, (void __user *)src, n, 0);

Fixes: 68f5d3f3b654 ("um: add PCI over virtio emulation driver")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
 drivers/infiniband/sw/rdmavt/qp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

--- linux-next-20211224.orig/drivers/infiniband/sw/rdmavt/qp.c
+++ linux-next-20211224/drivers/infiniband/sw/rdmavt/qp.c
@@ -84,10 +84,15 @@ EXPORT_SYMBOL(ib_rvt_state_ops);
 /* platform specific: return the last level cache (llc) size, in KiB */
 static int rvt_wss_llc_size(void)
 {
+#if !defined(CONFIG_UML)
 	/* assume that the boot CPU value is universal for all CPUs */
 	return boot_cpu_data.x86_cache_size;
+#else /* CONFIG_UML */
+	return 1024;	/* fake 1 MB LLC size */
+#endif
 }
 
+#if !defined(CONFIG_UML)
 /* platform specific: cacheless copy */
 static void cacheless_memcpy(void *dst, void *src, size_t n)
 {
@@ -99,6 +104,13 @@ static void cacheless_memcpy(void *dst,
 	 */
 	__copy_user_nocache(dst, (void __user *)src, n, 0);
 }
+#else
+/* for CONFIG_UML, this is just a plain memcpy() */
+static void cacheless_memcpy(void *dst, void *src, size_t n)
+{
+	memcpy(dst, src, n);
+}
+#endif
 
 void rvt_wss_exit(struct rvt_dev_info *rdi)
 {

Re: [PATCH 2/2] IB/rdmavt: modify rdmavt/qp.c for UML

[Jason Gunthorpe]

On Sat, Jan 01, 2022 at 11:06:23PM -0800, Randy Dunlap wrote:
> When building rdmavt for ARCH=um, qp.c has a build error on a reference
> to the x86-specific cpuinfo field 'x86_cache_size'. This value is then
> used to determine whether to use cacheless_memcpy() or not.
> Provide a fake value to LLC for CONFIG_UML. Then provide a separate
> verison of cacheless_memcpy() for CONFIG_UML that is just a plain
> memcpy(), like the calling code uses.
> 
> Prevents these build errors:
> 
> ../drivers/infiniband/sw/rdmavt/qp.c: In function ‘rvt_wss_llc_size’:
> ../drivers/infiniband/sw/rdmavt/qp.c:88:23: error: ‘struct cpuinfo_um’ has no member named ‘x86_cache_size’; did you mean ‘x86_capability’?
>   return boot_cpu_data.x86_cache_size;
> 
> ../drivers/infiniband/sw/rdmavt/qp.c: In function ‘cacheless_memcpy’:
> ../drivers/infiniband/sw/rdmavt/qp.c:100:2: error: implicit declaration of function ‘__copy_user_nocache’; did you mean ‘copy_user_page’? [-Werror=implicit-function-declaration]
>   __copy_user_nocache(dst, (void __user *)src, n, 0);
> 
> Fixes: 68f5d3f3b654 ("um: add PCI over virtio emulation driver")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>  drivers/infiniband/sw/rdmavt/qp.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> +++ linux-next-20211224/drivers/infiniband/sw/rdmavt/qp.c
> @@ -84,10 +84,15 @@ EXPORT_SYMBOL(ib_rvt_state_ops);
>  /* platform specific: return the last level cache (llc) size, in KiB */
>  static int rvt_wss_llc_size(void)
>  {
> +#if !defined(CONFIG_UML)
>  	/* assume that the boot CPU value is universal for all CPUs */
>  	return boot_cpu_data.x86_cache_size;
> +#else /* CONFIG_UML */
> +	return 1024;	/* fake 1 MB LLC size */
> +#endif
>  }
>  
> +#if !defined(CONFIG_UML)
>  /* platform specific: cacheless copy */
>  static void cacheless_memcpy(void *dst, void *src, size_t n)
>  {
> @@ -99,6 +104,13 @@ static void cacheless_memcpy(void *dst,
>  	 */
>  	__copy_user_nocache(dst, (void __user *)src, n, 0);
>  }
> +#else
> +/* for CONFIG_UML, this is just a plain memcpy() */
> +static void cacheless_memcpy(void *dst, void *src, size_t n)
> +{
> +	memcpy(dst, src, n);
> +}
> +#endif

memcpy is not the same thing as __copy_user - the hint is in the
__user cast..

It should by copy_from_user(), I think, and this is all just somehow
broken to not check the return code.

Why are you trying to make a HW driver compile on UML? Is there any
way to even use a driver like this in a UML environment?

Jason

Re: [PATCH 2/2] IB/rdmavt: modify rdmavt/qp.c for UML

[Randy Dunlap]

On 1/3/22 15:04, Jason Gunthorpe wrote:
> On Sat, Jan 01, 2022 at 11:06:23PM -0800, Randy Dunlap wrote:
>> When building rdmavt for ARCH=um, qp.c has a build error on a reference
>> to the x86-specific cpuinfo field 'x86_cache_size'. This value is then
>> used to determine whether to use cacheless_memcpy() or not.
>> Provide a fake value to LLC for CONFIG_UML. Then provide a separate
>> verison of cacheless_memcpy() for CONFIG_UML that is just a plain
>> memcpy(), like the calling code uses.
>>
>> Prevents these build errors:
>>
>> ../drivers/infiniband/sw/rdmavt/qp.c: In function ‘rvt_wss_llc_size’:
>> ../drivers/infiniband/sw/rdmavt/qp.c:88:23: error: ‘struct cpuinfo_um’ has no member named ‘x86_cache_size’; did you mean ‘x86_capability’?
>>   return boot_cpu_data.x86_cache_size;
>>
>> ../drivers/infiniband/sw/rdmavt/qp.c: In function ‘cacheless_memcpy’:
>> ../drivers/infiniband/sw/rdmavt/qp.c:100:2: error: implicit declaration of function ‘__copy_user_nocache’; did you mean ‘copy_user_page’? [-Werror=implicit-function-declaration]
>>   __copy_user_nocache(dst, (void __user *)src, n, 0);
>>
>> Fixes: 68f5d3f3b654 ("um: add PCI over virtio emulation driver")
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>  drivers/infiniband/sw/rdmavt/qp.c |   12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> +++ linux-next-20211224/drivers/infiniband/sw/rdmavt/qp.c
>> @@ -84,10 +84,15 @@ EXPORT_SYMBOL(ib_rvt_state_ops);
>>  /* platform specific: return the last level cache (llc) size, in KiB */
>>  static int rvt_wss_llc_size(void)
>>  {
>> +#if !defined(CONFIG_UML)
>>  	/* assume that the boot CPU value is universal for all CPUs */
>>  	return boot_cpu_data.x86_cache_size;
>> +#else /* CONFIG_UML */
>> +	return 1024;	/* fake 1 MB LLC size */
>> +#endif
>>  }
>>  
>> +#if !defined(CONFIG_UML)
>>  /* platform specific: cacheless copy */
>>  static void cacheless_memcpy(void *dst, void *src, size_t n)
>>  {
>> @@ -99,6 +104,13 @@ static void cacheless_memcpy(void *dst,
>>  	 */
>>  	__copy_user_nocache(dst, (void __user *)src, n, 0);
>>  }
>> +#else
>> +/* for CONFIG_UML, this is just a plain memcpy() */
>> +static void cacheless_memcpy(void *dst, void *src, size_t n)
>> +{
>> +	memcpy(dst, src, n);
>> +}
>> +#endif
> 
> memcpy is not the same thing as __copy_user - the hint is in the
> __user cast..
> 
> It should by copy_from_user(), I think, and this is all just somehow
> broken to not check the return code.

Thanks.

> Why are you trying to make a HW driver compile on UML? Is there any
> way to even use a driver like this in a UML environment?

I'm just trying to clean up lots of UML build errors.
I'm quite happy just making the driver depend on !UML.

UML maintainers, what do you think?

Thanks again.

-- 
~Randy

Re: [PATCH 2/2] IB/rdmavt: modify rdmavt/qp.c for UML

[Christoph Hellwig]

Yikes, this rdmavt code is completely fucked up and a very good example
why people should not use __user casts or random internal helpers.

The right fix is to remove this crap entirely, and if the rdmavt
maintainer think that they absolutely need a non-temporal memcpy they
need to work to add a core API for it.

Re: [PATCH 2/2] IB/rdmavt: modify rdmavt/qp.c for UML

[Anton Ivanov]

On 04/01/2022 01:00, Randy Dunlap wrote:
> 
> 
> On 1/3/22 15:04, Jason Gunthorpe wrote:
>> On Sat, Jan 01, 2022 at 11:06:23PM -0800, Randy Dunlap wrote:
>>> When building rdmavt for ARCH=um, qp.c has a build error on a reference
>>> to the x86-specific cpuinfo field 'x86_cache_size'. This value is then
>>> used to determine whether to use cacheless_memcpy() or not.
>>> Provide a fake value to LLC for CONFIG_UML. Then provide a separate
>>> verison of cacheless_memcpy() for CONFIG_UML that is just a plain
>>> memcpy(), like the calling code uses.
>>>
>>> Prevents these build errors:
>>>
>>> ../drivers/infiniband/sw/rdmavt/qp.c: In function ‘rvt_wss_llc_size’:
>>> ../drivers/infiniband/sw/rdmavt/qp.c:88:23: error: ‘struct cpuinfo_um’ has no member named ‘x86_cache_size’; did you mean ‘x86_capability’?
>>>    return boot_cpu_data.x86_cache_size;
>>>
>>> ../drivers/infiniband/sw/rdmavt/qp.c: In function ‘cacheless_memcpy’:
>>> ../drivers/infiniband/sw/rdmavt/qp.c:100:2: error: implicit declaration of function ‘__copy_user_nocache’; did you mean ‘copy_user_page’? [-Werror=implicit-function-declaration]
>>>    __copy_user_nocache(dst, (void __user *)src, n, 0);
>>>
>>> Fixes: 68f5d3f3b654 ("um: add PCI over virtio emulation driver")
>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>   drivers/infiniband/sw/rdmavt/qp.c |   12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> +++ linux-next-20211224/drivers/infiniband/sw/rdmavt/qp.c
>>> @@ -84,10 +84,15 @@ EXPORT_SYMBOL(ib_rvt_state_ops);
>>>   /* platform specific: return the last level cache (llc) size, in KiB */
>>>   static int rvt_wss_llc_size(void)
>>>   {
>>> +#if !defined(CONFIG_UML)
>>>   	/* assume that the boot CPU value is universal for all CPUs */
>>>   	return boot_cpu_data.x86_cache_size;
>>> +#else /* CONFIG_UML */
>>> +	return 1024;	/* fake 1 MB LLC size */
>>> +#endif
>>>   }
>>>   
>>> +#if !defined(CONFIG_UML)
>>>   /* platform specific: cacheless copy */
>>>   static void cacheless_memcpy(void *dst, void *src, size_t n)
>>>   {
>>> @@ -99,6 +104,13 @@ static void cacheless_memcpy(void *dst,
>>>   	 */
>>>   	__copy_user_nocache(dst, (void __user *)src, n, 0);
>>>   }
>>> +#else
>>> +/* for CONFIG_UML, this is just a plain memcpy() */
>>> +static void cacheless_memcpy(void *dst, void *src, size_t n)
>>> +{
>>> +	memcpy(dst, src, n);
>>> +}
>>> +#endif
>>
>> memcpy is not the same thing as __copy_user - the hint is in the
>> __user cast..
>>
>> It should by copy_from_user(), I think, and this is all just somehow
>> broken to not check the return code.
> 
> Thanks.
> 
>> Why are you trying to make a HW driver compile on UML? Is there any
>> way to even use a driver like this in a UML environment?
> 
> I'm just trying to clean up lots of UML build errors.
> I'm quite happy just making the driver depend on !UML.
> 
> UML maintainers, what do you think?
> 
> Thanks again.
> 

I would suggest that we just !UML this driver.

-- 
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

Re: [PATCH 2/2] IB/rdmavt: modify rdmavt/qp.c for UML

[Johannes Berg]

On Tue, 2022-01-04 at 08:03 +0000, Anton Ivanov wrote:
> > 
> > > Why are you trying to make a HW driver compile on UML? Is there any
> > > way to even use a driver like this in a UML environment?
> > 
> > I'm just trying to clean up lots of UML build errors.
> > I'm quite happy just making the driver depend on !UML.
> > 
> > UML maintainers, what do you think?
> > 
> > Thanks again.
> > 
> 
> I would suggest that we just !UML this driver.
> 
Agree, unless some of the maintainers of this driver actually wants to
build simulation for it for testing or something, it's almost certainly
completely useless.

After all, the reason I enabled PCI on UML was to be able to test - in
simulation - PCI driver code in UML... Most certainly nobody wants to do
that here, so it's pointless to let the driver be compiled.

OTOH, as Christoph points out, that seems like a band-aid for some
really strange code, but it's probably the easiest way to get the build
issue fixed in the short term.

johannes