lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAhSdy1EO9K-dFt545=27hu8ZXwszUE2J5EtBxgwhAyJpQ3zDA@mail.gmail.com> Date: Sat, 30 Dec 2023 13:35:59 +0530 From: Anup Patel <anup@...infault.org> To: Atish Patra <atishp@...osinc.com> Cc: linux-kernel@...r.kernel.org, Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alexghiti@...osinc.com>, Andrew Jones <ajones@...tanamicro.com>, Atish Patra <atishp@...shpatra.org>, Conor Dooley <conor.dooley@...rochip.com>, Guo Ren <guoren@...nel.org>, Heiko Stuebner <heiko@...ech.de>, kvm-riscv@...ts.infradead.org, kvm@...r.kernel.org, linux-riscv@...ts.infradead.org, Mark Rutland <mark.rutland@....com>, Palmer Dabbelt <palmer@...belt.com>, Paul Walmsley <paul.walmsley@...ive.com>, Will Deacon <will@...nel.org> Subject: Re: [v2 08/10] RISC-V: KVM: Implement SBI PMU Snapshot feature On Sat, Dec 30, 2023 at 3:20 AM Atish Patra <atishp@...osinc.com> wrote: > > PMU Snapshot function allows to minimize the number of traps when the > guest access configures/access the hpmcounters. If the snapshot feature > is enabled, the hypervisor updates the shared memory with counter > data and state of overflown counters. The guest can just read the > shared memory instead of trap & emulate done by the hypervisor. > > This patch doesn't implement the counter overflow yet. > > Signed-off-by: Atish Patra <atishp@...osinc.com> LGTM. Reviewed-by: Anup Patel <anup@...infault.org> Regards, Anup > --- > arch/riscv/include/asm/kvm_vcpu_pmu.h | 9 ++ > arch/riscv/kvm/aia.c | 5 ++ > arch/riscv/kvm/vcpu_onereg.c | 7 +- > arch/riscv/kvm/vcpu_pmu.c | 120 +++++++++++++++++++++++++- > arch/riscv/kvm/vcpu_sbi_pmu.c | 3 + > 5 files changed, 140 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/include/asm/kvm_vcpu_pmu.h b/arch/riscv/include/asm/kvm_vcpu_pmu.h > index 395518a1664e..d56b901a61fc 100644 > --- a/arch/riscv/include/asm/kvm_vcpu_pmu.h > +++ b/arch/riscv/include/asm/kvm_vcpu_pmu.h > @@ -50,6 +50,12 @@ struct kvm_pmu { > bool init_done; > /* Bit map of all the virtual counter used */ > DECLARE_BITMAP(pmc_in_use, RISCV_KVM_MAX_COUNTERS); > + /* Bit map of all the virtual counter overflown */ > + DECLARE_BITMAP(pmc_overflown, RISCV_KVM_MAX_COUNTERS); > + /* The address of the counter snapshot area (guest physical address) */ > + gpa_t snapshot_addr; > + /* The actual data of the snapshot */ > + struct riscv_pmu_snapshot_data *sdata; > }; > > #define vcpu_to_pmu(vcpu) (&(vcpu)->arch.pmu_context) > @@ -85,6 +91,9 @@ int kvm_riscv_vcpu_pmu_ctr_cfg_match(struct kvm_vcpu *vcpu, unsigned long ctr_ba > int kvm_riscv_vcpu_pmu_ctr_read(struct kvm_vcpu *vcpu, unsigned long cidx, > struct kvm_vcpu_sbi_return *retdata); > void kvm_riscv_vcpu_pmu_init(struct kvm_vcpu *vcpu); > +int kvm_riscv_vcpu_pmu_setup_snapshot(struct kvm_vcpu *vcpu, unsigned long saddr_low, > + unsigned long saddr_high, unsigned long flags, > + struct kvm_vcpu_sbi_return *retdata); > void kvm_riscv_vcpu_pmu_deinit(struct kvm_vcpu *vcpu); > void kvm_riscv_vcpu_pmu_reset(struct kvm_vcpu *vcpu); > > diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c > index a944294f6f23..71d161d7430d 100644 > --- a/arch/riscv/kvm/aia.c > +++ b/arch/riscv/kvm/aia.c > @@ -545,6 +545,9 @@ void kvm_riscv_aia_enable(void) > enable_percpu_irq(hgei_parent_irq, > irq_get_trigger_type(hgei_parent_irq)); > csr_set(CSR_HIE, BIT(IRQ_S_GEXT)); > + /* Enable IRQ filtering for overflow interrupt only if sscofpmf is present */ > + if (__riscv_isa_extension_available(NULL, RISCV_ISA_EXT_SSCOFPMF)) > + csr_write(CSR_HVIEN, BIT(IRQ_PMU_OVF)); > } > > void kvm_riscv_aia_disable(void) > @@ -560,6 +563,8 @@ void kvm_riscv_aia_disable(void) > > /* Disable per-CPU SGEI interrupt */ > csr_clear(CSR_HIE, BIT(IRQ_S_GEXT)); > + if (__riscv_isa_extension_available(NULL, RISCV_ISA_EXT_SSCOFPMF)) > + csr_clear(CSR_HVIEN, BIT(IRQ_PMU_OVF)); > disable_percpu_irq(hgei_parent_irq); > > aia_set_hvictl(false); > diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c > index fc34557f5356..581568847910 100644 > --- a/arch/riscv/kvm/vcpu_onereg.c > +++ b/arch/riscv/kvm/vcpu_onereg.c > @@ -117,8 +117,13 @@ void kvm_riscv_vcpu_setup_isa(struct kvm_vcpu *vcpu) > for (i = 0; i < ARRAY_SIZE(kvm_isa_ext_arr); i++) { > host_isa = kvm_isa_ext_arr[i]; > if (__riscv_isa_extension_available(NULL, host_isa) && > - kvm_riscv_vcpu_isa_enable_allowed(i)) > + kvm_riscv_vcpu_isa_enable_allowed(i)) { > + /* Sscofpmf depends on interrupt filtering defined in ssaia */ > + if (host_isa == RISCV_ISA_EXT_SSCOFPMF && > + !__riscv_isa_extension_available(NULL, RISCV_ISA_EXT_SSAIA)) > + continue; > set_bit(host_isa, vcpu->arch.isa); > + } > } > } > > diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c > index 08f561998611..e980235b8436 100644 > --- a/arch/riscv/kvm/vcpu_pmu.c > +++ b/arch/riscv/kvm/vcpu_pmu.c > @@ -311,6 +311,81 @@ int kvm_riscv_vcpu_pmu_read_hpm(struct kvm_vcpu *vcpu, unsigned int csr_num, > return ret; > } > > +static void kvm_pmu_clear_snapshot_area(struct kvm_vcpu *vcpu) > +{ > + struct kvm_pmu *kvpmu = vcpu_to_pmu(vcpu); > + int snapshot_area_size = sizeof(struct riscv_pmu_snapshot_data); > + > + if (kvpmu->sdata) { > + memset(kvpmu->sdata, 0, snapshot_area_size); > + if (kvpmu->snapshot_addr != INVALID_GPA) > + kvm_vcpu_write_guest(vcpu, kvpmu->snapshot_addr, > + kvpmu->sdata, snapshot_area_size); > + kfree(kvpmu->sdata); > + kvpmu->sdata = NULL; > + } > + kvpmu->snapshot_addr = INVALID_GPA; > +} > + > +int kvm_riscv_vcpu_pmu_setup_snapshot(struct kvm_vcpu *vcpu, unsigned long saddr_low, > + unsigned long saddr_high, unsigned long flags, > + struct kvm_vcpu_sbi_return *retdata) > +{ > + struct kvm_pmu *kvpmu = vcpu_to_pmu(vcpu); > + int snapshot_area_size = sizeof(struct riscv_pmu_snapshot_data); > + int sbiret = 0; > + gpa_t saddr; > + unsigned long hva; > + bool writable; > + > + if (!kvpmu) { > + sbiret = SBI_ERR_INVALID_PARAM; > + goto out; > + } > + > + if (saddr_low == -1 && saddr_high == -1) { > + kvm_pmu_clear_snapshot_area(vcpu); > + return 0; > + } > + > + saddr = saddr_low; > + > + if (saddr_high != 0) { > + if (IS_ENABLED(CONFIG_32BIT)) > + saddr |= ((gpa_t)saddr << 32); > + else > + sbiret = SBI_ERR_INVALID_ADDRESS; > + goto out; > + } > + > + if (kvm_is_error_gpa(vcpu->kvm, saddr)) { > + sbiret = SBI_ERR_INVALID_PARAM; > + goto out; > + } > + > + hva = kvm_vcpu_gfn_to_hva_prot(vcpu, saddr >> PAGE_SHIFT, &writable); > + if (kvm_is_error_hva(hva) || !writable) { > + sbiret = SBI_ERR_INVALID_ADDRESS; > + goto out; > + } > + > + kvpmu->snapshot_addr = saddr; > + kvpmu->sdata = kzalloc(snapshot_area_size, GFP_ATOMIC); > + if (!kvpmu->sdata) > + return -ENOMEM; > + > + if (kvm_vcpu_write_guest(vcpu, saddr, kvpmu->sdata, snapshot_area_size)) { > + kfree(kvpmu->sdata); > + kvpmu->snapshot_addr = INVALID_GPA; > + sbiret = SBI_ERR_FAILURE; > + } > + > +out: > + retdata->err_val = sbiret; > + > + return 0; > +} > + > int kvm_riscv_vcpu_pmu_num_ctrs(struct kvm_vcpu *vcpu, > struct kvm_vcpu_sbi_return *retdata) > { > @@ -344,20 +419,32 @@ int kvm_riscv_vcpu_pmu_ctr_start(struct kvm_vcpu *vcpu, unsigned long ctr_base, > int i, pmc_index, sbiret = 0; > struct kvm_pmc *pmc; > int fevent_code; > + bool snap_flag_set = flags & SBI_PMU_START_FLAG_INIT_FROM_SNAPSHOT; > > - if (kvm_pmu_validate_counter_mask(kvpmu, ctr_base, ctr_mask) < 0) { > + if ((kvm_pmu_validate_counter_mask(kvpmu, ctr_base, ctr_mask) < 0)) { > sbiret = SBI_ERR_INVALID_PARAM; > goto out; > } > > + if (snap_flag_set && kvpmu->snapshot_addr == INVALID_GPA) { > + sbiret = SBI_ERR_NO_SHMEM; > + goto out; > + } > + > /* Start the counters that have been configured and requested by the guest */ > for_each_set_bit(i, &ctr_mask, RISCV_MAX_COUNTERS) { > pmc_index = i + ctr_base; > if (!test_bit(pmc_index, kvpmu->pmc_in_use)) > continue; > pmc = &kvpmu->pmc[pmc_index]; > - if (flags & SBI_PMU_START_FLAG_SET_INIT_VALUE) > + if (flags & SBI_PMU_START_FLAG_SET_INIT_VALUE) { > pmc->counter_val = ival; > + } else if (snap_flag_set) { > + kvm_vcpu_read_guest(vcpu, kvpmu->snapshot_addr, kvpmu->sdata, > + sizeof(struct riscv_pmu_snapshot_data)); > + pmc->counter_val = kvpmu->sdata->ctr_values[pmc_index]; > + } > + > if (pmc->cinfo.type == SBI_PMU_CTR_TYPE_FW) { > fevent_code = get_event_code(pmc->event_idx); > if (fevent_code >= SBI_PMU_FW_MAX) { > @@ -401,12 +488,18 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base, > u64 enabled, running; > struct kvm_pmc *pmc; > int fevent_code; > + bool snap_flag_set = flags & SBI_PMU_STOP_FLAG_TAKE_SNAPSHOT; > > - if (kvm_pmu_validate_counter_mask(kvpmu, ctr_base, ctr_mask) < 0) { > + if ((kvm_pmu_validate_counter_mask(kvpmu, ctr_base, ctr_mask) < 0)) { > sbiret = SBI_ERR_INVALID_PARAM; > goto out; > } > > + if (snap_flag_set && kvpmu->snapshot_addr == INVALID_GPA) { > + sbiret = SBI_ERR_NO_SHMEM; > + goto out; > + } > + > /* Stop the counters that have been configured and requested by the guest */ > for_each_set_bit(i, &ctr_mask, RISCV_MAX_COUNTERS) { > pmc_index = i + ctr_base; > @@ -439,9 +532,28 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base, > } else { > sbiret = SBI_ERR_INVALID_PARAM; > } > + > + if (snap_flag_set && !sbiret) { > + if (pmc->cinfo.type == SBI_PMU_CTR_TYPE_FW) > + pmc->counter_val = kvpmu->fw_event[fevent_code].value; > + else if (pmc->perf_event) > + pmc->counter_val += perf_event_read_value(pmc->perf_event, > + &enabled, &running); > + /* TODO: Add counter overflow support when sscofpmf support is added */ > + kvpmu->sdata->ctr_values[i] = pmc->counter_val; > + kvm_vcpu_write_guest(vcpu, kvpmu->snapshot_addr, kvpmu->sdata, > + sizeof(struct riscv_pmu_snapshot_data)); > + } > + > if (flags & SBI_PMU_STOP_FLAG_RESET) { > pmc->event_idx = SBI_PMU_EVENT_IDX_INVALID; > clear_bit(pmc_index, kvpmu->pmc_in_use); > + if (snap_flag_set) { > + /* Clear the snapshot area for the upcoming deletion event */ > + kvpmu->sdata->ctr_values[i] = 0; > + kvm_vcpu_write_guest(vcpu, kvpmu->snapshot_addr, kvpmu->sdata, > + sizeof(struct riscv_pmu_snapshot_data)); > + } > } > } > > @@ -567,6 +679,7 @@ void kvm_riscv_vcpu_pmu_init(struct kvm_vcpu *vcpu) > kvpmu->num_hw_ctrs = num_hw_ctrs + 1; > kvpmu->num_fw_ctrs = SBI_PMU_FW_MAX; > memset(&kvpmu->fw_event, 0, SBI_PMU_FW_MAX * sizeof(struct kvm_fw_event)); > + kvpmu->snapshot_addr = INVALID_GPA; > > if (kvpmu->num_hw_ctrs > RISCV_KVM_MAX_HW_CTRS) { > pr_warn_once("Limiting the hardware counters to 32 as specified by the ISA"); > @@ -626,6 +739,7 @@ void kvm_riscv_vcpu_pmu_deinit(struct kvm_vcpu *vcpu) > } > bitmap_zero(kvpmu->pmc_in_use, RISCV_MAX_COUNTERS); > memset(&kvpmu->fw_event, 0, SBI_PMU_FW_MAX * sizeof(struct kvm_fw_event)); > + kvm_pmu_clear_snapshot_area(vcpu); > } > > void kvm_riscv_vcpu_pmu_reset(struct kvm_vcpu *vcpu) > diff --git a/arch/riscv/kvm/vcpu_sbi_pmu.c b/arch/riscv/kvm/vcpu_sbi_pmu.c > index b70179e9e875..9f61136e4bb1 100644 > --- a/arch/riscv/kvm/vcpu_sbi_pmu.c > +++ b/arch/riscv/kvm/vcpu_sbi_pmu.c > @@ -64,6 +64,9 @@ static int kvm_sbi_ext_pmu_handler(struct kvm_vcpu *vcpu, struct kvm_run *run, > case SBI_EXT_PMU_COUNTER_FW_READ: > ret = kvm_riscv_vcpu_pmu_ctr_read(vcpu, cp->a0, retdata); > break; > + case SBI_EXT_PMU_SNAPSHOT_SET_SHMEM: > + ret = kvm_riscv_vcpu_pmu_setup_snapshot(vcpu, cp->a0, cp->a1, cp->a2, retdata); > + break; > default: > retdata->err_val = SBI_ERR_NOT_SUPPORTED; > } > -- > 2.34.1 >
Powered by blists - more mailing lists