[dpdk-dev] [PATCH v3] Patch introducing API to read/write Intel Architecture Model Specific Registers (MSR)...

[Panu Matilainen]

On 01/21/2016 12:51 PM, Ananyev, Konstantin wrote:
> Hi Panu,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Panu Matilainen
>> Sent: Thursday, January 21, 2016 10:39 AM
>> To: Andralojc, WojciechX
>> Cc: dev at dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v3] Patch introducing API to read/write Intel Architecture Model Specific Registers (MSR)...
>>
>> On 01/21/2016 10:18 AM, Wojciech Andralojc wrote:
>>> Patch reworked.
>>>
>>> Signed-off-by: Wojciech Andralojc <wojciechx.andralojc at intel.com>
>>> ---
>>>    lib/librte_eal/common/include/arch/x86/rte_msr.h |  88 +++++++++++++++++
>>>    lib/librte_eal/linuxapp/eal/Makefile             |   1 +
>>>    lib/librte_eal/linuxapp/eal/arch/x86/rte_msr.c   | 116 +++++++++++++++++++++++
>>>    3 files changed, 205 insertions(+)
>>>    create mode 100644 lib/librte_eal/common/include/arch/x86/rte_msr.h
>>>    create mode 100644 lib/librte_eal/linuxapp/eal/arch/x86/rte_msr.c
>>
>> This creates a new arch-specific public API, with rte_msr.h installed as
>> a public header and implementation in the library (as opposed to
>> inline), and so the new functions would have to be added to
>> rte_eal_version.map.
>>
>> However that is a bit of a problem since it only exists on IA
>> architectures, so it'd mean dummy entries in the version map for all
>> other architectures. All the other arch-specific APIs are inline code so
>> this is the first of its kind.
>
> My thought was:
> 1. implementation is linux specific (as I know not supposed to work under freebsd).
> 2. they are not supposed to be used at run-time cide-path, so no need to be inlined.

Speed is not the only interesting attribute of inlining, inlined code 
also effectively escapes the library ABI so it does not need versioning 
/ exporting.

> 3. As I understand we plan to  have a library that will use these functions anyway.

Is this library going to be a generic or specific to Intel CPUs?
Also, if there are no other uses for the MSR API at the moment, perhaps 
the best place for this code would be within that library anyway?

> About dummy entries in the .map file: if we'll create a 'weak' generic implementation,
> that would just return an error - would it solve the issue?

Sure it'd solve the issue of dummy entries in .map but people seemed 
opposed to having a highly arch-specific API exposed to all architectures.

	- Panu -