From: Thomas Huth <thuth@redhat.com>
To: Nicholas Piggin <npiggin@gmail.com>, kvm@vger.kernel.org
Cc: Laurent Vivier <lvivier@redhat.com>, linuxppc-dev@lists.ozlabs.org
Subject: Re: [kvm-unit-tests v2 07/10] powerpc/spapr_vpa: Add basic VPA tests
Date: Thu, 23 Mar 2023 15:07:15 +0100 [thread overview]
Message-ID: <e10767db-95c2-18a2-aa9a-a055844570ac@redhat.com> (raw)
In-Reply-To: <20230320070339.915172-8-npiggin@gmail.com>
On 20/03/2023 08.03, Nicholas Piggin wrote:
> The VPA is a(n optional) memory structure shared between the hypervisor
> and operating system, defined by PAPR. This test defines the structure
> and adds registration, deregistration, and a few simple sanity tests.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> lib/linux/compiler.h | 2 +
> lib/powerpc/asm/hcall.h | 1 +
> lib/ppc64/asm/vpa.h | 62 ++++++++++++++++++++++++++++
> powerpc/Makefile.ppc64 | 2 +-
> powerpc/spapr_vpa.c | 90 +++++++++++++++++++++++++++++++++++++++++
Please add the new test to powerpc/unittests.cfg, otherwise it won't get
picked up by the run_tests.sh script.
> diff --git a/lib/linux/compiler.h b/lib/linux/compiler.h
> index 6f565e4..c9d205e 100644
> --- a/lib/linux/compiler.h
> +++ b/lib/linux/compiler.h
> @@ -45,7 +45,9 @@
>
> #define barrier() asm volatile("" : : : "memory")
>
> +#ifndef __always_inline
> #define __always_inline inline __attribute__((always_inline))
> +#endif
What's this change good for? ... it doesn't seem to be related to this patch?
> diff --git a/lib/ppc64/asm/vpa.h b/lib/ppc64/asm/vpa.h
> new file mode 100644
> index 0000000..11dde01
> --- /dev/null
> +++ b/lib/ppc64/asm/vpa.h
> @@ -0,0 +1,62 @@
> +#ifndef _ASMPOWERPC_VPA_H_
> +#define _ASMPOWERPC_VPA_H_
> +/*
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +
> +#ifndef __ASSEMBLY__
> +
> +struct vpa {
> + uint32_t descriptor;
> + uint16_t size;
> + uint8_t reserved1[3];
> + uint8_t status;
Where does this status field come from? ... My LoPAPR only says that there
are 18 "reserved" bytes in total here.
> + uint8_t reserved2[14];
> + uint32_t fru_node_id;
> + uint32_t fru_proc_id;
> + uint8_t reserved3[56];
> + uint8_t vhpn_change_counters[8];
> + uint8_t reserved4[80];
> + uint8_t cede_latency;
> + uint8_t maintain_ebb;
> + uint8_t reserved5[6];
> + uint8_t dtl_enable_mask;
> + uint8_t dedicated_cpu_donate;
> + uint8_t maintain_fpr;
> + uint8_t maintain_pmc;
> + uint8_t reserved6[28];
> + uint64_t idle_estimate_purr;
> + uint8_t reserved7[28];
> + uint16_t maintain_nr_slb;
> + uint8_t idle;
> + uint8_t maintain_vmx;
> + uint32_t vp_dispatch_count;
> + uint32_t vp_dispatch_dispersion;
> + uint64_t vp_fault_count;
> + uint64_t vp_fault_tb;
> + uint64_t purr_exprop_idle;
> + uint64_t spurr_exprop_idle;
> + uint64_t purr_exprop_busy;
> + uint64_t spurr_exprop_busy;
> + uint64_t purr_donate_idle;
> + uint64_t spurr_donate_idle;
> + uint64_t purr_donate_busy;
> + uint64_t spurr_donate_busy;
> + uint64_t vp_wait3_tb;
> + uint64_t vp_wait2_tb;
> + uint64_t vp_wait1_tb;
> + uint64_t purr_exprop_adjunct_busy;
> + uint64_t spurr_exprop_adjunct_busy;
The above two fields are also marked as "reserved" in my LoPAPR ... which
version did you use?
> + uint32_t supervisor_pagein_count;
> + uint8_t reserved8[4];
> + uint64_t purr_exprop_adjunct_idle;
> + uint64_t spurr_exprop_adjunct_idle;
> + uint64_t adjunct_insns_executed;
dito for the above three lines... I guess my LoPAPR is too old...
> + uint8_t reserved9[120];
> + uint64_t dtl_index;
> + uint8_t reserved10[96];
> +};
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* _ASMPOWERPC_VPA_H_ */
> diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64
> index ea68447..b0ed2b1 100644
> --- a/powerpc/Makefile.ppc64
> +++ b/powerpc/Makefile.ppc64
> @@ -19,7 +19,7 @@ reloc.o = $(TEST_DIR)/reloc64.o
> OBJDIRS += lib/ppc64
>
> # ppc64 specific tests
> -tests =
> +tests = $(TEST_DIR)/spapr_vpa.elf
>
> include $(SRCDIR)/$(TEST_DIR)/Makefile.common
>
> diff --git a/powerpc/spapr_vpa.c b/powerpc/spapr_vpa.c
> new file mode 100644
> index 0000000..45688fe
> --- /dev/null
> +++ b/powerpc/spapr_vpa.c
> @@ -0,0 +1,90 @@
> +/*
> + * Test sPAPR hypervisor calls (aka. h-calls)
Adjust to "Test sPAPR H_REGISTER_VPA hypervisor call" ?
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include <libfdt/libfdt.h>
> +#include <devicetree.h>
> +#include <libcflat.h>
> +#include <util.h>
> +#include <alloc.h>
> +#include <asm/processor.h>
> +#include <asm/hcall.h>
> +#include <asm/vpa.h>
> +#include <asm/io.h> /* for endian accessors */
> +
> +static void print_vpa(struct vpa *vpa)
> +{
> + printf("VPA\n");
> + printf("descriptor: 0x%08x\n", be32_to_cpu(vpa->descriptor));
> + printf("size: 0x%04x\n", be16_to_cpu(vpa->size));
> + printf("status: 0x%02x\n", vpa->status);
> + printf("fru_node_id: 0x%08x\n", be32_to_cpu(vpa->fru_node_id));
> + printf("fru_proc_id: 0x%08x\n", be32_to_cpu(vpa->fru_proc_id));
> + printf("vhpn_change_counters: 0x%02x %02x %02x %02x %02x %02x %02x %02x\n", vpa->vhpn_change_counters[0], vpa->vhpn_change_counters[1], vpa->vhpn_change_counters[2], vpa->vhpn_change_counters[3], vpa->vhpn_change_counters[4], vpa->vhpn_change_counters[5], vpa->vhpn_change_counters[6], vpa->vhpn_change_counters[7]);
> + printf("vp_dispatch_count: 0x%08x\n", be32_to_cpu(vpa->vp_dispatch_count));
> + printf("vp_dispatch_dispersion: 0x%08x\n", be32_to_cpu(vpa->vp_dispatch_dispersion));
> + printf("vp_fault_count: 0x%08lx\n", be64_to_cpu(vpa->vp_fault_count));
> + printf("vp_fault_tb: 0x%08lx\n", be64_to_cpu(vpa->vp_fault_tb));
> + printf("purr_exprop_idle: 0x%08lx\n", be64_to_cpu(vpa->purr_exprop_idle));
> + printf("spurr_exprop_idle: 0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_idle));
> + printf("purr_exprop_busy: 0x%08lx\n", be64_to_cpu(vpa->purr_exprop_busy));
> + printf("spurr_exprop_busy: 0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_busy));
> + printf("purr_donate_idle: 0x%08lx\n", be64_to_cpu(vpa->purr_donate_idle));
> + printf("spurr_donate_idle: 0x%08lx\n", be64_to_cpu(vpa->spurr_donate_idle));
> + printf("purr_donate_busy: 0x%08lx\n", be64_to_cpu(vpa->purr_donate_busy));
> + printf("spurr_donate_busy: 0x%08lx\n", be64_to_cpu(vpa->spurr_donate_busy));
> + printf("vp_wait3_tb: 0x%08lx\n", be64_to_cpu(vpa->vp_wait3_tb));
> + printf("vp_wait2_tb: 0x%08lx\n", be64_to_cpu(vpa->vp_wait2_tb));
> + printf("vp_wait1_tb: 0x%08lx\n", be64_to_cpu(vpa->vp_wait1_tb));
> + printf("purr_exprop_adjunct_busy: 0x%08lx\n", be64_to_cpu(vpa->purr_exprop_adjunct_busy));
> + printf("spurr_exprop_adjunct_busy: 0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_adjunct_busy));
> + printf("purr_exprop_adjunct_idle: 0x%08lx\n", be64_to_cpu(vpa->purr_exprop_adjunct_idle));
> + printf("spurr_exprop_adjunct_idle: 0x%08lx\n", be64_to_cpu(vpa->spurr_exprop_adjunct_idle));
> + printf("adjunct_insns_executed: 0x%08lx\n", be64_to_cpu(vpa->adjunct_insns_executed));
> + printf("dtl_index: 0x%08lx\n", be64_to_cpu(vpa->dtl_index));
> +}
> +
> +/**
> + * Test the H_REGISTER_VPA h-call register/deregister.
> + */
> +static void register_vpa(struct vpa *vpa)
> +{
> + uint32_t cpuid = fdt_boot_cpuid_phys(dt_fdt());
> + int disp_count1, disp_count2;
> + int rc;
> +
> + rc = hcall(H_REGISTER_VPA, 1ULL << 45, cpuid, vpa);
> + report(rc == H_SUCCESS, "VPA registered");
> +
> + print_vpa(vpa);
> +
> + disp_count1 = be32_to_cpu(vpa->vp_dispatch_count);
> + report(disp_count1 % 2 == 0, "Dispatch count is even while running");
> + msleep(100);
> + disp_count2 = be32_to_cpu(vpa->vp_dispatch_count);
> + report(disp_count1 != disp_count2, "Dispatch count increments over H_CEDE");
> +
> + rc = hcall(H_REGISTER_VPA, 5ULL << 45, cpuid, vpa);
> + report(rc == H_SUCCESS, "VPA deregistered");
> +
> + disp_count1 = be32_to_cpu(vpa->vp_dispatch_count);
> + report(disp_count1 % 2 == 1, "Dispatch count is odd after deregister");
> +}
Now that was a very tame amount of tests ;-)
I'd suggest to add some more:
- Check hcall(H_REGISTER_VPA, 0, ...);
- Check hcall(H_REGISTER_VPA, ..., bad-cpu-id, ...)
- Check hcall(H_REGISTER_VPA, ..., ..., unaligned-address)
- Check hcall(H_REGISTER_VPA, ..., ..., illegal-address)
- Check registration with vpa->size being too small
- Check registration where the vpa crosses the 4k boundary
What do you think?
> +int main(int argc, char **argv)
> +{
> + struct vpa *vpa;
> +
> + vpa = memalign(4096, sizeof(*vpa));
> +
> + memset(vpa, 0, sizeof(*vpa));
> +
> + vpa->size = cpu_to_be16(sizeof(*vpa));
> +
> + report_prefix_push("vpa");
This lacks the corresponding report_prefix_pop() later.
> + register_vpa(vpa);
> +
> + return report_summary();
> +}
Thomas
next prev parent reply other threads:[~2023-03-23 14:08 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-20 7:03 [kvm-unit-tests v2 00/10] powerpc: updates, P10, PNV support Nicholas Piggin
2023-03-20 7:03 ` [kvm-unit-tests v2 01/10] MAINTAINERS: Update powerpc list Nicholas Piggin
2023-03-23 11:23 ` Thomas Huth
2023-03-20 7:03 ` [kvm-unit-tests v2 02/10] powerpc: add local variant of SPR test Nicholas Piggin
2023-03-23 11:26 ` Thomas Huth
2023-03-27 5:37 ` Nicholas Piggin
2023-03-20 7:03 ` [kvm-unit-tests v2 03/10] powerpc: abstract H_CEDE calls into a sleep functions Nicholas Piggin
2023-03-23 12:12 ` Thomas Huth
2023-03-27 5:39 ` Nicholas Piggin
2023-03-20 7:03 ` [kvm-unit-tests v2 04/10] powerpc: Add ISA v3.1 (POWER10) support to SPR test Nicholas Piggin
2023-03-23 12:01 ` Thomas Huth
2023-03-20 7:03 ` [kvm-unit-tests v2 05/10] powerpc: Indirect SPR accessor functions Nicholas Piggin
2023-03-20 7:03 ` [kvm-unit-tests v2 06/10] powerpc/sprs: Specify SPRs with data rather than code Nicholas Piggin
2023-03-23 12:36 ` Thomas Huth
2023-03-27 11:59 ` Nicholas Piggin
2023-03-20 7:03 ` [kvm-unit-tests v2 07/10] powerpc/spapr_vpa: Add basic VPA tests Nicholas Piggin
2023-03-23 14:07 ` Thomas Huth [this message]
2023-03-27 6:27 ` Nicholas Piggin
2023-03-20 7:03 ` [kvm-unit-tests v2 08/10] powerpc: Discover runtime load address dynamically Nicholas Piggin
2023-03-20 7:03 ` [kvm-unit-tests v2 09/10] powerpc: Support powernv machine with QEMU TCG Nicholas Piggin
2023-03-20 9:47 ` Cédric Le Goater
2023-03-21 0:38 ` Nicholas Piggin
2023-03-23 14:14 ` Thomas Huth
2023-03-20 7:03 ` [kvm-unit-tests v2 10/10] powerpc/sprs: Test hypervisor registers on powernv machine Nicholas Piggin
2023-03-23 14:16 ` Thomas Huth
2023-03-21 6:14 ` [kvm-unit-tests v2 00/10] powerpc: updates, P10, PNV support Nicholas Piggin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e10767db-95c2-18a2-aa9a-a055844570ac@redhat.com \
--to=thuth@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=lvivier@redhat.com \
--cc=npiggin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).