[kvm-unit-tests v4 03/12] powerpc: Abstract H_CEDE calls into a sleep functions
Thomas Huth
thuth at redhat.com
Mon Jul 3 23:13:09 AEST 2023
On 08/06/2023 09.58, Nicholas Piggin wrote:
> This consolidates several implementations, and it no longer leaves
> MSR[EE] enabled after the decrementer interrupt is handled, but
> rather disables it on return.
>
> The handler no longer allows a continuous ticking, but rather dec
> has to be re-armed and EE re-enabled (e.g., via H_CEDE hcall) each
> time.
>
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> ---
> lib/powerpc/asm/handlers.h | 2 +-
> lib/powerpc/asm/ppc_asm.h | 1 +
> lib/powerpc/asm/processor.h | 7 ++++++
> lib/powerpc/handlers.c | 10 ++++-----
> lib/powerpc/processor.c | 43 +++++++++++++++++++++++++++++++++++++
> powerpc/sprs.c | 6 +-----
> powerpc/tm.c | 20 +----------------
> 7 files changed, 58 insertions(+), 31 deletions(-)
>
> diff --git a/lib/powerpc/asm/handlers.h b/lib/powerpc/asm/handlers.h
> index 64ba727a..e4a0cd45 100644
> --- a/lib/powerpc/asm/handlers.h
> +++ b/lib/powerpc/asm/handlers.h
> @@ -3,6 +3,6 @@
>
> #include <asm/ptrace.h>
>
> -void dec_except_handler(struct pt_regs *regs, void *data);
> +void dec_handler_oneshot(struct pt_regs *regs, void *data);
>
> #endif /* _ASMPOWERPC_HANDLERS_H_ */
> diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h
> index 1b85f6bb..6299ff53 100644
> --- a/lib/powerpc/asm/ppc_asm.h
> +++ b/lib/powerpc/asm/ppc_asm.h
> @@ -36,6 +36,7 @@
> #endif /* __BYTE_ORDER__ */
>
> /* Machine State Register definitions: */
> +#define MSR_EE_BIT 15 /* External Interrupts Enable */
> #define MSR_SF_BIT 63 /* 64-bit mode */
>
> #endif /* _ASMPOWERPC_PPC_ASM_H */
> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
> index ac001e18..ebfeff2b 100644
> --- a/lib/powerpc/asm/processor.h
> +++ b/lib/powerpc/asm/processor.h
> @@ -20,6 +20,8 @@ static inline uint64_t get_tb(void)
>
> extern void delay(uint64_t cycles);
> extern void udelay(uint64_t us);
> +extern void sleep_tb(uint64_t cycles);
> +extern void usleep(uint64_t us);
>
> static inline void mdelay(uint64_t ms)
> {
> @@ -27,4 +29,9 @@ static inline void mdelay(uint64_t ms)
> udelay(1000);
> }
>
> +static inline void msleep(uint64_t ms)
> +{
> + usleep(ms * 1000);
> +}
> +
> #endif /* _ASMPOWERPC_PROCESSOR_H_ */
> diff --git a/lib/powerpc/handlers.c b/lib/powerpc/handlers.c
> index c8721e0a..296f14ff 100644
> --- a/lib/powerpc/handlers.c
> +++ b/lib/powerpc/handlers.c
> @@ -9,15 +9,13 @@
> #include <libcflat.h>
> #include <asm/handlers.h>
> #include <asm/ptrace.h>
> +#include <asm/ppc_asm.h>
>
> /*
> * Generic handler for decrementer exceptions (0x900)
> - * Just reset the decrementer back to the value specified when registering the
> - * handler
> + * Return with MSR[EE] disabled.
> */
> -void dec_except_handler(struct pt_regs *regs __unused, void *data)
> +void dec_handler_oneshot(struct pt_regs *regs, void *data)
> {
> - uint64_t dec = *((uint64_t *) data);
> -
> - asm volatile ("mtdec %0" : : "r" (dec));
> + regs->msr &= ~(1UL << MSR_EE_BIT);
> }
> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> index 0550e4fc..aaf45b68 100644
> --- a/lib/powerpc/processor.c
> +++ b/lib/powerpc/processor.c
> @@ -10,6 +10,8 @@
> #include <asm/ptrace.h>
> #include <asm/setup.h>
> #include <asm/barrier.h>
> +#include <asm/hcall.h>
> +#include <asm/handlers.h>
>
> static struct {
> void (*func)(struct pt_regs *, void *data);
> @@ -58,3 +60,44 @@ void udelay(uint64_t us)
> {
> delay((us * tb_hz) / 1000000);
> }
> +
> +void sleep_tb(uint64_t cycles)
> +{
> + uint64_t start, end, now;
> +
> + start = now = get_tb();
> + end = start + cycles;
> +
> + while (end > now) {
> + uint64_t left = end - now;
> +
> + /* TODO: Could support large decrementer */
> + if (left > 0x7fffffff)
> + left = 0x7fffffff;
> +
> + /* DEC won't fire until H_CEDE is called because EE=0 */
> + asm volatile ("mtdec %0" : : "r" (left));
> + handle_exception(0x900, &dec_handler_oneshot, NULL);
> + /*
> + * H_CEDE is called with MSR[EE] clear and enables it as part
> + * of the hcall, returning with EE enabled. The dec interrupt
> + * is then taken immediately and the handler disables EE.
> + *
> + * If H_CEDE returned for any other interrupt than dec
> + * expiring, that is considered an unhandled interrupt and
> + * the test case would be stopped.
> + */
> + if (hcall(H_CEDE) != H_SUCCESS) {
> + printf("H_CEDE failed\n");
> + abort();
> + }
> + handle_exception(0x900, NULL, NULL);
> +
> + now = get_tb();
> + }
> +}
> +
> +void usleep(uint64_t us)
> +{
> + sleep_tb((us * tb_hz) / 1000000);
> +}
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index 5cc1cd16..ba4ddee4 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -254,7 +254,6 @@ int main(int argc, char **argv)
> 0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL,
> -1ULL,
> };
> - static uint64_t decr = 0x7FFFFFFF; /* Max value */
>
> for (i = 1; i < argc; i++) {
> if (!strcmp(argv[i], "-w")) {
> @@ -288,10 +287,7 @@ int main(int argc, char **argv)
> if (pause) {
> migrate_once();
> } else {
> - puts("Sleeping...\n");
> - handle_exception(0x900, &dec_except_handler, &decr);
> - asm volatile ("mtdec %0" : : "r" (0x3FFFFFFF));
> - hcall(H_CEDE);
> + msleep(2000);
> }
>
> get_sprs(after);
> diff --git a/powerpc/tm.c b/powerpc/tm.c
> index 65cacdf5..7fa91636 100644
> --- a/powerpc/tm.c
> +++ b/powerpc/tm.c
> @@ -48,17 +48,6 @@ static int count_cpus_with_tm(void)
> return available;
> }
>
> -static int h_cede(void)
> -{
> - register uint64_t r3 asm("r3") = H_CEDE;
> -
> - asm volatile ("sc 1" : "+r"(r3) :
> - : "r0", "r4", "r5", "r6", "r7", "r8", "r9",
> - "r10", "r11", "r12", "xer", "ctr", "cc");
> -
> - return r3;
> -}
> -
> /*
> * Enable transactional memory
> * Returns: FALSE - Failure
> @@ -95,14 +84,10 @@ static bool enable_tm(void)
> static void test_h_cede_tm(int argc, char **argv)
> {
> int i;
> - static uint64_t decr = 0x3FFFFF; /* ~10ms */
>
> if (argc > 2)
> report_abort("Unsupported argument: '%s'", argv[2]);
>
> - handle_exception(0x900, &dec_except_handler, &decr);
> - asm volatile ("mtdec %0" : : "r" (decr));
> -
> if (!start_all_cpus(halt, 0))
> report_abort("Failed to start secondary cpus");
>
> @@ -120,10 +105,7 @@ static void test_h_cede_tm(int argc, char **argv)
> "bf 2,1b" : : : "cr0");
>
> for (i = 0; i < 500; i++) {
> - uint64_t rval = h_cede();
> -
> - if (rval != H_SUCCESS)
> - break;
> + msleep(10);
> mdelay(5);
msleep() directly followed by an mdelay() looks weird now. Do we still need
the mdelay()? Or could we maybe at least get a comment here why there are
now two different delaying calls here?
Thomas
> }
>
More information about the Linuxppc-dev
mailing list