Re: [EXT] Re: [PATCH] common/cnxk: use cas with release semantics for batch alloc

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 1/12/2022 6:18 AM, Ruifeng Wang wrote:
>> -----Original Message-----
>> From: Ruifeng Wang <Ruifeng.Wang@arm.com>
>> Sent: Wednesday, January 12, 2022 11:01 AM
>> To: Ferruh Yigit <ferruh.yigit@intel.com>; Ashwin Sekhar Thalakalath
>> Kottilveetil <asekhar@marvell.com>; dev@dpdk.org; Honnappa Nagarahalli
>> <Honnappa.Nagarahalli@arm.com>
>> Cc: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>;
>> jerinj@marvell.com; Sunil Kumar Kori <skori@marvell.com>; Satha
>> Koteswara Rao Kottidi <skoteshwar@marvell.com>; Pavan Nikhilesh
>> Bhagavatula <pbhagavatula@marvell.com>; Kiran Kumar Kokkilagadda
>> <kirankumark@marvell.com>; Satheesh Paul <psatheesh@marvell.com>;
>> Anoob Joseph <anoobj@marvell.com>; Akhil Goyal <gakhil@marvell.com>;
>> nd <nd@arm.com>
>> Subject: RE: [EXT] Re: [PATCH] common/cnxk: use cas with release semantics
>> for batch alloc
>>
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>> Sent: Tuesday, January 11, 2022 9:46 PM
>>> To: Ashwin Sekhar Thalakalath Kottilveetil <asekhar@marvell.com>;
>>> dev@dpdk.org; Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
>>> Ruifeng Wang <Ruifeng.Wang@arm.com>
>>> Cc: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>;
>>> jerinj@marvell.com; Sunil Kumar Kori <skori@marvell.com>; Satha
>>> Koteswara Rao Kottidi <skoteshwar@marvell.com>; Pavan Nikhilesh
>>> Bhagavatula <pbhagavatula@marvell.com>; Kiran Kumar Kokkilagadda
>>> <kirankumark@marvell.com>; Satheesh Paul <psatheesh@marvell.com>;
>>> Anoob Joseph <anoobj@marvell.com>; Akhil Goyal <gakhil@marvell.com>
>>> Subject: Re: [EXT] Re: [PATCH] common/cnxk: use cas with release
>>> semantics for batch alloc
>>>
>>> On 1/11/2022 12:26 PM, Ashwin Sekhar Thalakalath Kottilveetil wrote:
>>>> CAS is compare and swap. CASL is compare and swap with release
>>> semantics.
>>>>
>>>
>>> What does 'release semantics' mean? What is functional difference in both?
>>
>> 'release semantics' is semantics in memory ordering for store operations.
>> It ensures store-store ordering.
>>
>> And some comments below.
>>>
>>>> But on CNXK platform, the functionality of CAS* instructions is
>>>> completely
>>> different when it is done to specific addresses. These APIs are meant
>>> for use for such special cases. These cannot be made ARM generic.
>>>>
>>>> Ashwin Sekhar T K
>>>>
>>>>> -----Original Message-----
>>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>> Sent: Tuesday, January 11, 2022 5:42 PM
>>>>> To: Ashwin Sekhar Thalakalath Kottilveetil <asekhar@marvell.com>;
>>>>> dev@dpdk.org; Honnappa Nagarahalli
>> <Honnappa.Nagarahalli@arm.com>;
>>>>> Ruifeng Wang (Arm Technology China) <Ruifeng.Wang@arm.com>
>>>>> Cc: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; Jerin Jacob
>>>>> Kollanukkaran <jerinj@marvell.com>; Sunil Kumar Kori
>>>>> <skori@marvell.com>; Satha Koteswara Rao Kottidi
>>>>> <skoteshwar@marvell.com>; Pavan Nikhilesh Bhagavatula
>>>>> <pbhagavatula@marvell.com>; Kiran Kumar Kokkilagadda
>>>>> <kirankumark@marvell.com>; Satheesh Paul
>> <psatheesh@marvell.com>;
>>>>> Anoob Joseph <anoobj@marvell.com>; Akhil Goyal
>> <gakhil@marvell.com>
>>>>> Subject: [EXT] Re: [PATCH] common/cnxk: use cas with release
>>>>> semantics for batch alloc
>>>>>
>>>>> External Email
>>>>>
>>>>> -------------------------------------------------------------------
>>>>> --
>>>>> - On 1/11/2022 12:08 PM, Ferruh Yigit wrote:
>>>>>> On 11/30/2021 5:45 AM, Ashwin Sekhar T K wrote:
>>>>>>> Before issuing the batch alloc, we clear the first word of cache
>>>>>>> lines so that NPA can update the status. Make sure that this line
>>>>>>> clear is flushed before the batch alloc is issued.
>>>>>>>
>>>>>>> Signed-off-by: Ashwin Sekhar T K <asekhar@marvell.com>
>>>>>>> ---
>>>>>>>     drivers/common/cnxk/roc_io.h         | 12 ++++++++++++
>>>>>>>     drivers/common/cnxk/roc_io_generic.h |  9 +++++++++
>>>>>>>     drivers/common/cnxk/roc_npa.h        |  2 +-
>>>>>>>     3 files changed, 22 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/common/cnxk/roc_io.h
>>>>>>> b/drivers/common/cnxk/roc_io.h index fe5f7f46d0..4f15503c29
>>>>>>> 100644
>>>>>>> --- a/drivers/common/cnxk/roc_io.h
>>>>>>> +++ b/drivers/common/cnxk/roc_io.h
>>>>>>> @@ -78,6 +78,18 @@ roc_atomic64_cas(uint64_t compare, uint64_t
>>>>> swap,
>>>>>>> int64_t *ptr)
>>>>>>>         return compare;
>>>>>>>     }
>>>>>>> +static __plt_always_inline uint64_t roc_atomic64_casl(uint64_t
>>>>>>> +compare, uint64_t swap, int64_t *ptr) {
>>>>>>> +    asm volatile(PLT_CPU_FEATURE_PREAMBLE
>>>>>>> +             "casl %[compare], %[swap], [%[ptr]]\n"
>>>>>>> +             : [compare] "+r"(compare)
>>>>>>> +             : [swap] "r"(swap), [ptr] "r"(ptr)
>>>>>>> +             : "memory");
>>>>>>> +
>>>>>>
>>>>>> out of curiosity, what is the "cas with release semantics"?
>>>>>> briefly, what is the difference between 'cas' and 'casl'?
>>>>>
>>>>> + Honnappa & Ruifeng,
>>
>> Thanks Ferruh for adding me in this loop.
>>>>>
>>>>> Isn't this API Arm wide, instead of being cnxk specific?
>>>>> Does it make sense to make this API for arm and cnxk use from there?
>>
>> Yes, CAS operation can be used Arm wide.
>> Generally, CAS is available via __atomic_compare_exchange/_n() compiler
>> built-ins. This is the way we use atomic in DPDK. So there is no need to add
>> another generic API.
> 
> Just to make my comment more clear.
> For generic CAS operations, compiler built-ins can be used. No more API needed.
> Given the special usage of the instructions in CNXK, the inline assembly here is not intended to be a wrapper of
> generic CAS operation but rather an interface to other hardware function. It doesn't make sense to make it Arm wide.
> 

Got it. Thanks for clarification, I will continue with the set.