On Sun, Dec 22, 2024 at 07:32:33PM +0530, Vaibhav Jain wrote:
> Implement and setup necessary structures to send a prepolulated
> Guest-State-Buffer(GSB) requesting hostwide counters to L0-PowerVM and have
> the returned GSB holding the values of these counters parsed. This is done
> via existing GSB implementation and with the newly added support of
> Hostwide elements in GSB.
>
> The request to L0-PowerVM to return Hostwide counters is done using a
> pre-allocated GSB named 'gsb_l0_stats'. To be able to populate this GSB
> with the needed Guest-State-Elements (GSIDs) a instance of 'struct
> kvmppc_gs_msg' named 'gsm_l0_stats' is introduced. The 'gsm_l0_stats' is
> tied to an instance of 'struct kvmppc_gs_msg_ops' named 'gsb_ops_l0_stats'
> which holds various callbacks to be compute the size ( hostwide_get_size()
> ), populate the GSB ( hostwide_fill_info() ) and
> refresh ( hostwide_refresh_info() ) the contents of
> 'l0_stats' that holds the Hostwide counters returned from L0-PowerVM.
>
> To protect these structures from simultaneous access a spinlock
> 'lock_l0_stats' has been introduced. The allocation and initialization of
> the above structures is done in newly introduced kvmppc_init_hostwide() and
> similarly the cleanup is performed in newly introduced
> kvmppc_cleanup_hostwide().
>
> Signed-off-by: Vaibhav Jain <vaibhav@xxxxxxxxxxxxx>
> ---
> arch/powerpc/kvm/book3s_hv_pmu.c | 189 +++++++++++++++++++++++++++++++
> 1 file changed, 189 insertions(+)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_pmu.c b/arch/powerpc/kvm/book3s_hv_pmu.c
> index e72542d5e750..f7fd5190ecf7 100644
> --- a/arch/powerpc/kvm/book3s_hv_pmu.c
> +++ b/arch/powerpc/kvm/book3s_hv_pmu.c
> @@ -27,10 +27,31 @@
> #include <asm/plpar_wrappers.h>
> #include <asm/firmware.h>
>
> +#include "asm/guest-state-buffer.h"
> +
> enum kvmppc_pmu_eventid {
> KVMPPC_EVENT_MAX,
> };
>
> +#define KVMPPC_PMU_EVENT_ATTR(_name, _id) \
> + PMU_EVENT_ATTR_ID(_name, power_events_sysfs_show, _id)
> +
> +/* Holds the hostwide stats */
> +static struct kvmppc_hostwide_stats {
> + u64 guest_heap;
> + u64 guest_heap_max;
> + u64 guest_pgtable_size;
> + u64 guest_pgtable_size_max;
> + u64 guest_pgtable_reclaim;
> +} l0_stats;
> +
> +/* Protect access to l0_stats */
> +static DEFINE_SPINLOCK(lock_l0_stats);
> +
> +/* GSB related structs needed to talk to L0 */
> +static struct kvmppc_gs_msg *gsm_l0_stats;
> +static struct kvmppc_gs_buff *gsb_l0_stats;
> +
> static struct attribute *kvmppc_pmu_events_attr[] = {
> NULL,
> };
> @@ -90,6 +111,167 @@ static void kvmppc_pmu_read(struct perf_event *event)
> {
> }
>
> +/* Return the size of the needed guest state buffer */
> +static size_t hostwide_get_size(struct kvmppc_gs_msg *gsm)
> +
> +{
> + size_t size = 0;
> + const u16 ids[] = {
> + KVMPPC_GSID_L0_GUEST_HEAP,
> + KVMPPC_GSID_L0_GUEST_HEAP_MAX,
> + KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE,
> + KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE_MAX,
> + KVMPPC_GSID_L0_GUEST_PGTABLE_RECLAIM
> + };
> +
> + for (int i = 0; i < ARRAY_SIZE(ids); i++)
> + size += kvmppc_gse_total_size(kvmppc_gsid_size(ids[i]));
> + return size;
> +}
> +
> +/* Populate the request guest state buffer */
> +static int hostwide_fill_info(struct kvmppc_gs_buff *gsb,
> + struct kvmppc_gs_msg *gsm)
> +{
> + struct kvmppc_hostwide_stats *stats = gsm->data;
> +
> + /*
> + * It doesn't matter what values are put into request buffer as
> + * they are going to be overwritten anyways. But for the sake of
> + * testcode and symmetry contents of existing stats are put
> + * populated into the request guest state buffer.
> + */
> + if (kvmppc_gsm_includes(gsm, KVMPPC_GSID_L0_GUEST_HEAP))
> + kvmppc_gse_put_u64(gsb, KVMPPC_GSID_L0_GUEST_HEAP,
> + stats->guest_heap);
> + if (kvmppc_gsm_includes(gsm, KVMPPC_GSID_L0_GUEST_HEAP_MAX))
> + kvmppc_gse_put_u64(gsb, KVMPPC_GSID_L0_GUEST_HEAP_MAX,
> + stats->guest_heap_max);
> + if (kvmppc_gsm_includes(gsm, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE))
> + kvmppc_gse_put_u64(gsb, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE,
> + stats->guest_pgtable_size);
> + if (kvmppc_gsm_includes(gsm, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE_MAX))
> + kvmppc_gse_put_u64(gsb, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE_MAX,
> + stats->guest_pgtable_size_max);
> + if (kvmppc_gsm_includes(gsm, KVMPPC_GSID_L0_GUEST_PGTABLE_RECLAIM))
> + kvmppc_gse_put_u64(gsb, KVMPPC_GSID_L0_GUEST_PGTABLE_RECLAIM,
> + stats->guest_pgtable_reclaim);
> +
> + return 0;
> +}
kvmppc_gse_put_u64() can return an error. I think we can handle it just
like gs_msg_ops_vcpu_fill_info()
> +
> +/* Parse and update the host wide stats from returned gsb */
> +static int hostwide_refresh_info(struct kvmppc_gs_msg *gsm,
> + struct kvmppc_gs_buff *gsb)
> +{
> + struct kvmppc_gs_parser gsp = { 0 };
> + struct kvmppc_hostwide_stats *stats = gsm->data;
> + struct kvmppc_gs_elem *gse;
> + int rc;
> +
> + rc = kvmppc_gse_parse(&gsp, gsb);
> + if (rc < 0)
> + return rc;
> +
> + gse = kvmppc_gsp_lookup(&gsp, KVMPPC_GSID_L0_GUEST_HEAP);
> + if (gse)
> + stats->guest_heap = kvmppc_gse_get_u64(gse);
> +
> + gse = kvmppc_gsp_lookup(&gsp, KVMPPC_GSID_L0_GUEST_HEAP_MAX);
> + if (gse)
> + stats->guest_heap_max = kvmppc_gse_get_u64(gse);
> +
> + gse = kvmppc_gsp_lookup(&gsp, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE);
> + if (gse)
> + stats->guest_pgtable_size = kvmppc_gse_get_u64(gse);
> +
> + gse = kvmppc_gsp_lookup(&gsp, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE_MAX);
> + if (gse)
> + stats->guest_pgtable_size_max = kvmppc_gse_get_u64(gse);
> +
> + gse = kvmppc_gsp_lookup(&gsp, KVMPPC_GSID_L0_GUEST_PGTABLE_RECLAIM);
> + if (gse)
> + stats->guest_pgtable_reclaim = kvmppc_gse_get_u64(gse);
> +
> + return 0;
> +}
> +
> +/* gsb-message ops for setting up/parsing */
> +static struct kvmppc_gs_msg_ops gsb_ops_l0_stats = {
> + .get_size = hostwide_get_size,
> + .fill_info = hostwide_fill_info,
> + .refresh_info = hostwide_refresh_info,
> +};
> +
> +static int kvmppc_init_hostwide(void)
> +{
> + int rc = 0;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&lock_l0_stats, flags);
> +
> + /* already registered ? */
> + if (gsm_l0_stats) {
> + rc = 0;
> + goto out;
> + }
> +
> + /* setup the Guest state message/buffer to talk to L0 */
> + gsm_l0_stats = kvmppc_gsm_new(&gsb_ops_l0_stats, &l0_stats,
> + GSM_SEND, GFP_KERNEL);
> + if (!gsm_l0_stats) {
> + rc = -ENOMEM;
> + goto out;
> + }
> +
> + /* Populate the Idents */
> + kvmppc_gsm_include(gsm_l0_stats, KVMPPC_GSID_L0_GUEST_HEAP);
> + kvmppc_gsm_include(gsm_l0_stats, KVMPPC_GSID_L0_GUEST_HEAP_MAX);
> + kvmppc_gsm_include(gsm_l0_stats, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE);
> + kvmppc_gsm_include(gsm_l0_stats, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE_MAX);
> + kvmppc_gsm_include(gsm_l0_stats, KVMPPC_GSID_L0_GUEST_PGTABLE_RECLAIM);
> +
> + /* allocate GSB. Guest/Vcpu Id is ignored */
> + gsb_l0_stats = kvmppc_gsb_new(kvmppc_gsm_size(gsm_l0_stats), 0, 0,
> + GFP_KERNEL);
> + if (!gsb_l0_stats) {
> + rc = -ENOMEM;
> + goto out;
> + }
> +
> + /* ask the ops to fill in the info */
> + rc = kvmppc_gsm_fill_info(gsm_l0_stats, gsb_l0_stats);
> + if (rc)
> + goto out;
> +out:
> + if (rc) {
> + if (gsm_l0_stats)
> + kvmppc_gsm_free(gsm_l0_stats);
> + if (gsb_l0_stats)
> + kvmppc_gsb_free(gsb_l0_stats);
> + gsm_l0_stats = NULL;
> + gsb_l0_stats = NULL;
> + }
> + spin_unlock_irqrestore(&lock_l0_stats, flags);
> + return rc;
> +}
The error handling can probably be simplified to avoid multiple ifs:
<snip>
/* allocate GSB. Guest/Vcpu Id is ignored */
gsb_l0_stats = kvmppc_gsb_new(kvmppc_gsm_size(gsm_l0_stats), 0, 0,
GFP_KERNEL);
if (!gsb_l0_stats) {
rc = -ENOMEM;
goto err_gsm;
}
/* ask the ops to fill in the info */
rc = kvmppc_gsm_fill_info(gsm_l0_stats, gsb_l0_stats);
if (!rc)
goto out;
err_gsb:
kvmppc_gsb_free(gsb_l0_stats);
gsb_l0_stats = NULL;
err_gsm:
kvmppc_gsm_free(gsm_l0_stats);
gsm_l0_stats = NULL;
out:
spin_unlock_irqrestore(&lock_l0_stats, flags);
return rc;
}
> +
> +static void kvmppc_cleanup_hostwide(void)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&lock_l0_stats, flags);
> +
> + if (gsm_l0_stats)
> + kvmppc_gsm_free(gsm_l0_stats);
> + if (gsb_l0_stats)
> + kvmppc_gsb_free(gsb_l0_stats);
> + gsm_l0_stats = NULL;
> + gsb_l0_stats = NULL;
> +
> + spin_unlock_irqrestore(&lock_l0_stats, flags);
> +}
> +
> /* L1 wide counters PMU */
> static struct pmu kvmppc_pmu = {
> .task_ctx_nr = perf_sw_context,
> @@ -108,6 +290,10 @@ int kvmppc_register_pmu(void)
>
> /* only support events for nestedv2 right now */
> if (kvmhv_is_nestedv2()) {
> + rc = kvmppc_init_hostwide();
> + if (rc)
> + goto out;
> +
> /* Setup done now register the PMU */
> pr_info("Registering kvm-hv pmu");
>
> @@ -117,6 +303,7 @@ int kvmppc_register_pmu(void)
> -1) : 0;
> }
>
> +out:
> return rc;
> }
> EXPORT_SYMBOL_GPL(kvmppc_register_pmu);
> @@ -124,6 +311,8 @@ EXPORT_SYMBOL_GPL(kvmppc_register_pmu);
> void kvmppc_unregister_pmu(void)
> {
> if (kvmhv_is_nestedv2()) {
> + kvmppc_cleanup_hostwide();
> +
> if (kvmppc_pmu.type != -1)
> perf_pmu_unregister(&kvmppc_pmu);
>
> --
> 2.47.1
>
