Message-ID: <50fa4eca-ce74-431f-8497-273d2c5956f2@infradead.org>
Date:   Mon, 3 Jan 2022 17:00:53 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.1
Subject: Re: [PATCH 2/2] IB/rdmavt: modify rdmavt/qp.c for UML
Content-Language: en-US
To:     Jason Gunthorpe <jgg@nvidia.com>
Cc:     linux-kernel@vger.kernel.org, Jeff Dike <jdike@addtoit.com>,
        Richard Weinberger <richard@nod.at>,
        Anton Ivanov <anton.ivanov@cambridgegreys.com>,
        Johannes Berg <johannes@sipsolutions.net>,
        linux-rdma@vger.kernel.org, linux-um@lists.infradead.org
References: <20220102070623.24009-1-rdunlap@infradead.org>
 <20220103230445.GA2592848@nvidia.com>
From:   Randy Dunlap <rdunlap@infradead.org>
In-Reply-To: <20220103230445.GA2592848@nvidia.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk


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