Re: [PATCH for-10.0 03/11] hw/riscv/riscv-iommu: add riscv-iommu-hpm file

[Alistair Francis <alistair23@gmail.com>]

On Thu, Dec 5, 2024 at 11:35 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> From: Tomasz Jeznach <tjeznach@rivosinc.com>
>
> The HPM (Hardware Performance Monitor) support consists of almost 7
> hundred lines that would be put on top of the base riscv-iommu
> emulation.
>
> To avoid clogging riscv-iommu.c, add a separated riscv-iommu-hpm file
> that will contain HPM specific code.
>
> We'll start by adding riscv_iommu_hpmcycle_read(), a helper that will be
> called during the riscv_iommu_mmio_read() callback.
>
> This change will have no effect on the existing emulation since we're
> not declaring HPM feature support.
>
> Signed-off-by: Tomasz Jeznach <tjeznach@rivosinc.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/meson.build       |  3 ++-
>  hw/riscv/riscv-iommu-hpm.c | 54 ++++++++++++++++++++++++++++++++++++++
>  hw/riscv/riscv-iommu-hpm.h | 27 +++++++++++++++++++
>  hw/riscv/riscv-iommu.c     | 24 ++++++++++++++++-
>  hw/riscv/riscv-iommu.h     |  4 +++
>  5 files changed, 110 insertions(+), 2 deletions(-)
>  create mode 100644 hw/riscv/riscv-iommu-hpm.c
>  create mode 100644 hw/riscv/riscv-iommu-hpm.h
>
> diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build
> index 3c7e083aca..c22f3a7216 100644
> --- a/hw/riscv/meson.build
> +++ b/hw/riscv/meson.build
> @@ -10,7 +10,8 @@ riscv_ss.add(when: 'CONFIG_SIFIVE_U', if_true: files('sifive_u.c'))
>  riscv_ss.add(when: 'CONFIG_SPIKE', if_true: files('spike.c'))
>  riscv_ss.add(when: 'CONFIG_MICROCHIP_PFSOC', if_true: files('microchip_pfsoc.c'))
>  riscv_ss.add(when: 'CONFIG_ACPI', if_true: files('virt-acpi-build.c'))
> -riscv_ss.add(when: 'CONFIG_RISCV_IOMMU', if_true: files('riscv-iommu.c', 'riscv-iommu-pci.c', 'riscv-iommu-sys.c'))
> +riscv_ss.add(when: 'CONFIG_RISCV_IOMMU', if_true: files(
> +       'riscv-iommu.c', 'riscv-iommu-pci.c', 'riscv-iommu-sys.c', 'riscv-iommu-hpm.c'))
>  riscv_ss.add(when: 'CONFIG_MICROBLAZE_V', if_true: files('microblaze-v-generic.c'))
>
>  hw_arch += {'riscv': riscv_ss}
> diff --git a/hw/riscv/riscv-iommu-hpm.c b/hw/riscv/riscv-iommu-hpm.c
> new file mode 100644
> index 0000000000..5833ab8956
> --- /dev/null
> +++ b/hw/riscv/riscv-iommu-hpm.c
> @@ -0,0 +1,54 @@
> +/*
> + * RISC-V IOMMU - Hardware Performance Monitor (HPM) helpers
> + *
> + * Copyright (C) 2022-2023 Rivos Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/timer.h"
> +#include "cpu_bits.h"
> +#include "riscv-iommu-hpm.h"
> +#include "riscv-iommu.h"
> +#include "riscv-iommu-bits.h"
> +#include "trace.h"
> +
> +/* For now we assume IOMMU HPM frequency to be 1GHz so 1-cycle is of 1-ns. */
> +static inline uint64_t get_cycles(void)
> +{
> +    return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +}
> +
> +uint64_t riscv_iommu_hpmcycle_read(RISCVIOMMUState *s)
> +{
> +    const uint64_t cycle = riscv_iommu_reg_get64(
> +        s, RISCV_IOMMU_REG_IOHPMCYCLES);
> +    const uint32_t inhibit = riscv_iommu_reg_get32(
> +        s, RISCV_IOMMU_REG_IOCOUNTINH);
> +    const uint64_t ctr_prev = s->hpmcycle_prev;
> +    const uint64_t ctr_val = s->hpmcycle_val;
> +
> +    if (get_field(inhibit, RISCV_IOMMU_IOCOUNTINH_CY)) {
> +        /*
> +         * Counter should not increment if inhibit bit is set. We can't really
> +         * stop the QEMU_CLOCK_VIRTUAL, so we just return the last updated
> +         * counter value to indicate that counter was not incremented.
> +         */
> +        return (ctr_val & RISCV_IOMMU_IOHPMCYCLES_COUNTER) |
> +               (cycle & RISCV_IOMMU_IOHPMCYCLES_OVF);
> +    }
> +
> +    return (ctr_val + get_cycles() - ctr_prev) |
> +        (cycle & RISCV_IOMMU_IOHPMCYCLES_OVF);
> +}
> diff --git a/hw/riscv/riscv-iommu-hpm.h b/hw/riscv/riscv-iommu-hpm.h
> new file mode 100644
> index 0000000000..231c110ff2
> --- /dev/null
> +++ b/hw/riscv/riscv-iommu-hpm.h
> @@ -0,0 +1,27 @@
> +/*
> + * RISC-V IOMMU - Hardware Performance Monitor (HPM) helpers
> + *
> + * Copyright (C) 2022-2023 Rivos Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef HW_RISCV_IOMMU_HPM_H
> +#define HW_RISCV_IOMMU_HPM_H
> +
> +#include "qom/object.h"
> +#include "hw/riscv/riscv-iommu.h"
> +
> +uint64_t riscv_iommu_hpmcycle_read(RISCVIOMMUState *s);
> +
> +#endif
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index e9a0775d6e..01df25418c 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -29,6 +29,7 @@
>  #include "cpu_bits.h"
>  #include "riscv-iommu.h"
>  #include "riscv-iommu-bits.h"
> +#include "riscv-iommu-hpm.h"
>  #include "trace.h"
>
>  #define LIMIT_CACHE_CTX               (1U << 7)
> @@ -2052,7 +2053,28 @@ static MemTxResult riscv_iommu_mmio_read(void *opaque, hwaddr addr,
>          return MEMTX_ACCESS_ERROR;
>      }
>
> -    ptr = &s->regs_rw[addr];
> +    /* Compute cycle register value. */
> +    if ((addr & ~7) == RISCV_IOMMU_REG_IOHPMCYCLES) {
> +        val = riscv_iommu_hpmcycle_read(s);
> +        ptr = (uint8_t *)&val + (addr & 7);
> +    } else if ((addr & ~3) == RISCV_IOMMU_REG_IOCOUNTOVF) {
> +        /*
> +         * Software can read RISCV_IOMMU_REG_IOCOUNTOVF before timer
> +         * callback completes. In which case CY_OF bit in
> +         * RISCV_IOMMU_IOHPMCYCLES_OVF would be 0. Here we take the
> +         * CY_OF bit state from RISCV_IOMMU_REG_IOHPMCYCLES register as
> +         * it's not dependent over the timer callback and is computed
> +         * from cycle overflow.
> +         */
> +        val = ldq_le_p(&s->regs_rw[addr]);
> +        val |= (riscv_iommu_hpmcycle_read(s) & RISCV_IOMMU_IOHPMCYCLES_OVF)
> +                   ? RISCV_IOMMU_IOCOUNTOVF_CY
> +                   : 0;
> +        ptr = (uint8_t *)&val + (addr & 3);
> +    } else {
> +        ptr = &s->regs_rw[addr];
> +    }
> +
>      val = ldn_le_p(ptr, size);
>
>      *data = val;
> diff --git a/hw/riscv/riscv-iommu.h b/hw/riscv/riscv-iommu.h
> index 2de0cdfc56..380f7e81d1 100644
> --- a/hw/riscv/riscv-iommu.h
> +++ b/hw/riscv/riscv-iommu.h
> @@ -86,6 +86,10 @@ struct RISCVIOMMUState {
>
>      QLIST_ENTRY(RISCVIOMMUState) iommus;
>      QLIST_HEAD(, RISCVIOMMUSpace) spaces;
> +
> +    /* HPM cycle counter */
> +    uint64_t hpmcycle_val;      /* Current value of cycle register */
> +    uint64_t hpmcycle_prev;     /* Saved value of QEMU_CLOCK_VIRTUAL clock */
>  };
>
>  void riscv_iommu_pci_setup_iommu(RISCVIOMMUState *iommu, PCIBus *bus,
> --
> 2.47.1
>
>