| Message ID | 20241009-pmu_event_machine-v1-0-dcbd7a60e3ba@rivosinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Allow platform specific PMU event encoding | expand |
On 10.10.2024 02:08, Atish Patra wrote: > Currently, the pmu implementation is virt machine specific that > implements the SBI PMU encodings. In the future, individual machines > would want to implement their own custom event encodings as there > is no standard event encoding defined by the ISA. There is a performance > events TG which is working on defining standard set of events but not > encodings. That allows flexibility for platforms to choose whatever > encoding scheme they want. However, that means the generic pmu code > should be flexible enough to allow that in Qemu as well. > > This series aims to solve that problem by first disassociating the > common pmu implementation and event encoding. The event encoding is > specific to a platform and should be defined in the platform specific > machine or cpu implementation. The newly defined callbacks can be invoked > from machine specific cpu implementation or machine code itself depending > on the requirement. > > The first 5 patches in the series are generic improvements and cleanups > where as the last 5 actually implements the disassociation for the virt > machine. The current series can also be found at[2]. > > I recently found that Alexei has done a similar effort for SiFive FU740[1] > but the implementation differs from this one based on how the cpu callbacks > are invoked. For example, Alexei's series implements a single callback for > all the events and has defined machine specific counter read/write callbacks. > However, it just defaults to get_ticks() for every event. IMO, that is > confusing to the users unless we can actually emulate more generic events or > machine specific events. > > I have separate callbacks for each type of events that we currently support > in Qemu (cycle, instruction, TLB events). Separate callbacks seems a better > approach to avoid ambiguity as we have very limited event capability in qemu. > I am open to converging them to one callback as well if we think we will > be extending set of events in the future. > > Once we converge on the approaches, we can consolidate the patches > so that both SiFive FU740 and virt machine can use the same abstraction. > > Cc: alexei.filippov@syntacore.com > Thanks for CCing me and your patch. Your done a great work, but still I do not think this approach with per event callback are scalable enough. I'll suggest to collaborate to work your and mine solution to unite them to one patch set. Let me know what do you think. > [1] https://lore.kernel.org/all/20240910174747.148141-1-alexei.filippov@syntacore.com/T/ > [2] https://github.com/atishp04/qemu/tree/b4/pmu_event_machine > > Signed-off-by: Atish Patra <atishp@rivosinc.com> > --- > Atish Patra (10): > target/riscv: Fix the hpmevent mask > target/riscv: Introduce helper functions for pmu hashtable lookup > target/riscv: Protect the hashtable modifications with a lock > target/riscv: Use uint64 instead of uint as key > target/riscv: Rename the PMU events > target/riscv: Define PMU event related structures > hw/riscv/virt.c : Disassociate virt PMU events > target/riscv: Update event mapping hashtable for invalid events > target/riscv : Use the new tlb fill event functions > hw/riscv/virt.c: Generate the PMU node from the machine > > hw/riscv/virt.c | 102 +++++++++++++++++++++++- > target/riscv/cpu.h | 52 ++++++++++-- > target/riscv/cpu_bits.h | 4 +- > target/riscv/cpu_helper.c | 21 ++--- > target/riscv/pmu.c | 198 +++++++++++++++++++++------------------------- > target/riscv/pmu.h | 3 +- > 6 files changed, 246 insertions(+), 134 deletions(-) > --- > base-commit: 19a9809808a51291008f72d051d0f9b3499fc223 > change-id: 20241008-pmu_event_machine-b87c58104e61 > -- > Regards, > Atish patra >
On Thu, Oct 10, 2024 at 5:51 AM Alexei Filippov <alexei.filippov@yadro.com> wrote: > > > > On 10.10.2024 02:08, Atish Patra wrote: > > Currently, the pmu implementation is virt machine specific that > > implements the SBI PMU encodings. In the future, individual machines > > would want to implement their own custom event encodings as there > > is no standard event encoding defined by the ISA. There is a performance > > events TG which is working on defining standard set of events but not > > encodings. That allows flexibility for platforms to choose whatever > > encoding scheme they want. However, that means the generic pmu code > > should be flexible enough to allow that in Qemu as well. > > > > This series aims to solve that problem by first disassociating the > > common pmu implementation and event encoding. The event encoding is > > specific to a platform and should be defined in the platform specific > > machine or cpu implementation. The newly defined callbacks can be invoked > > from machine specific cpu implementation or machine code itself depending > > on the requirement. > > > > The first 5 patches in the series are generic improvements and cleanups > > where as the last 5 actually implements the disassociation for the virt > > machine. The current series can also be found at[2]. > > > > I recently found that Alexei has done a similar effort for SiFive FU740[1] > > but the implementation differs from this one based on how the cpu callbacks > > are invoked. For example, Alexei's series implements a single callback for > > all the events and has defined machine specific counter read/write callbacks. > > However, it just defaults to get_ticks() for every event. IMO, that is > > confusing to the users unless we can actually emulate more generic events or > > machine specific events. > > > > I have separate callbacks for each type of events that we currently support > > in Qemu (cycle, instruction, TLB events). Separate callbacks seems a better > > approach to avoid ambiguity as we have very limited event capability in qemu. > > I am open to converging them to one callback as well if we think we will > > be extending set of events in the future. > > > > Once we converge on the approaches, we can consolidate the patches > > so that both SiFive FU740 and virt machine can use the same abstraction. > > > > Cc: alexei.filippov@syntacore.com > > > Thanks for CCing me and your patch. Your done a great work, but still I > do not think this approach with per event callback are scalable enough. > I'll suggest to collaborate to work your and mine solution to unite them > to one patch set. Let me know what do you think. Yes. We should definitely collaborate and send a single series to support both virt and sifive machines. You had a question about widening the hpmevent in your series. (Answering here to keep the discussion in 1 place) As per the section 18.1. Count Overflow Control, only top 8 bits are reserved. Thus, a platform can implement their event encoding to upto 56 bit wide. > > [1] https://lore.kernel.org/all/20240910174747.148141-1-alexei.filippov@syntacore.com/T/ > > [2] https://github.com/atishp04/qemu/tree/b4/pmu_event_machine > > > > Signed-off-by: Atish Patra <atishp@rivosinc.com> > > --- > > Atish Patra (10): > > target/riscv: Fix the hpmevent mask > > target/riscv: Introduce helper functions for pmu hashtable lookup > > target/riscv: Protect the hashtable modifications with a lock > > target/riscv: Use uint64 instead of uint as key > > target/riscv: Rename the PMU events > > target/riscv: Define PMU event related structures > > hw/riscv/virt.c : Disassociate virt PMU events > > target/riscv: Update event mapping hashtable for invalid events > > target/riscv : Use the new tlb fill event functions > > hw/riscv/virt.c: Generate the PMU node from the machine > > > > hw/riscv/virt.c | 102 +++++++++++++++++++++++- > > target/riscv/cpu.h | 52 ++++++++++-- > > target/riscv/cpu_bits.h | 4 +- > > target/riscv/cpu_helper.c | 21 ++--- > > target/riscv/pmu.c | 198 +++++++++++++++++++++------------------------- > > target/riscv/pmu.h | 3 +- > > 6 files changed, 246 insertions(+), 134 deletions(-) > > --- > > base-commit: 19a9809808a51291008f72d051d0f9b3499fc223 > > change-id: 20241008-pmu_event_machine-b87c58104e61 > > -- > > Regards, > > Atish patra > >
Currently, the pmu implementation is virt machine specific that implements the SBI PMU encodings. In the future, individual machines would want to implement their own custom event encodings as there is no standard event encoding defined by the ISA. There is a performance events TG which is working on defining standard set of events but not encodings. That allows flexibility for platforms to choose whatever encoding scheme they want. However, that means the generic pmu code should be flexible enough to allow that in Qemu as well. This series aims to solve that problem by first disassociating the common pmu implementation and event encoding. The event encoding is specific to a platform and should be defined in the platform specific machine or cpu implementation. The newly defined callbacks can be invoked from machine specific cpu implementation or machine code itself depending on the requirement. The first 5 patches in the series are generic improvements and cleanups where as the last 5 actually implements the disassociation for the virt machine. The current series can also be found at[2]. I recently found that Alexei has done a similar effort for SiFive FU740[1] but the implementation differs from this one based on how the cpu callbacks are invoked. For example, Alexei's series implements a single callback for all the events and has defined machine specific counter read/write callbacks. However, it just defaults to get_ticks() for every event. IMO, that is confusing to the users unless we can actually emulate more generic events or machine specific events. I have separate callbacks for each type of events that we currently support in Qemu (cycle, instruction, TLB events). Separate callbacks seems a better approach to avoid ambiguity as we have very limited event capability in qemu. I am open to converging them to one callback as well if we think we will be extending set of events in the future. Once we converge on the approaches, we can consolidate the patches so that both SiFive FU740 and virt machine can use the same abstraction. Cc: alexei.filippov@syntacore.com [1] https://lore.kernel.org/all/20240910174747.148141-1-alexei.filippov@syntacore.com/T/ [2] https://github.com/atishp04/qemu/tree/b4/pmu_event_machine Signed-off-by: Atish Patra <atishp@rivosinc.com> --- Atish Patra (10): target/riscv: Fix the hpmevent mask target/riscv: Introduce helper functions for pmu hashtable lookup target/riscv: Protect the hashtable modifications with a lock target/riscv: Use uint64 instead of uint as key target/riscv: Rename the PMU events target/riscv: Define PMU event related structures hw/riscv/virt.c : Disassociate virt PMU events target/riscv: Update event mapping hashtable for invalid events target/riscv : Use the new tlb fill event functions hw/riscv/virt.c: Generate the PMU node from the machine hw/riscv/virt.c | 102 +++++++++++++++++++++++- target/riscv/cpu.h | 52 ++++++++++-- target/riscv/cpu_bits.h | 4 +- target/riscv/cpu_helper.c | 21 ++--- target/riscv/pmu.c | 198 +++++++++++++++++++++------------------------- target/riscv/pmu.h | 3 +- 6 files changed, 246 insertions(+), 134 deletions(-) --- base-commit: 19a9809808a51291008f72d051d0f9b3499fc223 change-id: 20241008-pmu_event_machine-b87c58104e61 -- Regards, Atish patra