Re: [PATCH v2] target/riscv: Add support for machine specific pmu's events

[Aleksei Filippov <alexei.filippov@syntacore.com>]

On 08.07.2024 16:42, Philippe Mathieu-Daudé wrote:
> Hi Aleksei,
> 
> On 8/7/24 11:46, Aleksei Filippov wrote:
>> On 25.06.2024 21:18, Richard Henderson wrote:
>>> On 6/25/24 07:46, Alexei Filippov wrote:
>>>> Was added call backs for machine specific pmu events.
>>>> Simplify monitor functions by adding new hash table, which going to map
>>>> counter number and event index.
>>>> Was added read/write callbacks which going to simplify support for events,
>>>> which expected to have different behavior.
>>>>
>>>> Signed-off-by: Alexei Filippov <alexei.filippov@syntacore.com>
>>>> ---
>>>> Changes since v2:
>>>>         -rebased to latest master
>>>>   target/riscv/cpu.h |   9 +++
>>>>   target/riscv/csr.c |  43 +++++++++-----
>>>>   target/riscv/pmu.c | 139 ++++++++++++++++++++++-----------------------
>>>>   target/riscv/pmu.h |  11 ++--
>>>>   4 files changed, 115 insertions(+), 87 deletions(-)
>>>>
>>>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>>>> index 6fe0d712b4..fbf82b050b 100644
>>>> --- a/target/riscv/cpu.h
>>>> +++ b/target/riscv/cpu.h
>>>> @@ -374,6 +374,13 @@ struct CPUArchState {
>>>>       uint64_t (*rdtime_fn)(void *);
>>>>       void *rdtime_fn_arg;
>>>> +    /*machine specific pmu callback */
>>>> +    void (*pmu_ctr_write)(PMUCTRState *counter, uint32_t event_idx,
>>>> +                          target_ulong val, bool high_half);
>>>> +    target_ulong (*pmu_ctr_read)(PMUCTRState *counter, uint32_t event_idx,
>>>> +                                 bool high_half);
>>>> +    bool (*pmu_vendor_support)(uint32_t event_idx);
>>>
>>> Do these really belong in CPUArchState, rather than RISCVCPUClass?
>>>
>>> Surely there's more to this series, since these fields are never set...
>>>
>>>
>>> r~
>>
>> Initially this callbacks was added to CPUArchState just to be along with 
>> similar implementation with rdtime_fn*.
>>
>> Yes, you're right, there are more series to this, but, it can't be separated 
>> from syntacore specific parts, which is unfortunately not ready yet to be 
>> published. So, I can prepare second patch to implement PMU subsystem for virt 
>> device. What do you think about it? (I'll send it in the few days).
> 
> How can we test your patch meanwhile?
Sorry for such late response, but anyway, I created an RFC with PoC and 
description on how I tested this and resend patch with PoC all together. 
https://lore.kernel.org/all/20240910174747.148141-1-alexei.filippov@syntacore.com/

> 
> Thanks,
> 
> Phil.
> 

-- 
Sincerely,
Aleksei Filippov