Re: [kvm-unit-tests v2 03/10] powerpc: abstract H_CEDE calls into a sleep functions
On Thu Mar 23, 2023 at 10:12 PM AEST, Thomas Huth wrote:
> On 20/03/2023 08.03, 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
> > ---
> > 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 | 42 +
> > powerpc/sprs.c | 6 +-
> > powerpc/tm.c| 20 +-
> > 7 files changed, 57 insertions(+), 31 deletions(-)
> >
> > diff --git a/lib/powerpc/asm/handlers.h b/lib/powerpc/asm/handlers.h
> > index 64ba727..e4a0cd4 100644
> > --- a/lib/powerpc/asm/handlers.h
> > +++ b/lib/powerpc/asm/handlers.h
> > @@ -3,6 +3,6 @@
> >
> > #include
> >
> > -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 1b85f6b..6299ff5 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_BIT63 /* 64-bit mode */
> >
> > #endif /* _ASMPOWERPC_PPC_ASM_H */
> > diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
> > index ac001e1..ebfeff2 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 c8721e0..296f14f 100644
> > --- a/lib/powerpc/handlers.c
> > +++ b/lib/powerpc/handlers.c
> > @@ -9,15 +9,13 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > /*
> >* 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 ec85b9d..e77a240 100644
> > --- a/lib/powerpc/processor.c
> > +++ b/lib/powerpc/processor.c
> > @@ -10,6 +10,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> >
> > static struct {
> > void (*func)(struct pt_regs *, void *data);
> > @@ -54,3 +56,43 @@ void udelay(uint64_t us)
> > {
> > delay((us * tb_hz) / 100);
> > }
> > +
> > +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;
> > +
> > + /* Could support large decrementer */
> > + if (left > 0x7fff)
> > + left = 0x7fff;
> > +
> > + asm volatile ("mtdec %0" : : "r" (left));
> > + handle_exception(0x900, _handler_oneshot, NULL);
>
> Wouldn't it be better to first call handle_exception() before moving
> something into the decrementer?
It shouldn't really matter, the drecrementer could be anything here
(probably it's already -ve here if it starts at zero). It only matters
when MSR[EE]=1 or we make the H_CEDE call.
I'll add a comment at least.
Thanks,
Nick
Re: [kvm-unit-tests v2 03/10] powerpc: abstract H_CEDE calls into a sleep functions
On 20/03/2023 08.03, 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
---
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 | 42 +
powerpc/sprs.c | 6 +-
powerpc/tm.c| 20 +-
7 files changed, 57 insertions(+), 31 deletions(-)
diff --git a/lib/powerpc/asm/handlers.h b/lib/powerpc/asm/handlers.h
index 64ba727..e4a0cd4 100644
--- a/lib/powerpc/asm/handlers.h
+++ b/lib/powerpc/asm/handlers.h
@@ -3,6 +3,6 @@
#include
-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 1b85f6b..6299ff5 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_BIT63 /* 64-bit mode */
#endif /* _ASMPOWERPC_PPC_ASM_H */
diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
index ac001e1..ebfeff2 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 c8721e0..296f14f 100644
--- a/lib/powerpc/handlers.c
+++ b/lib/powerpc/handlers.c
@@ -9,15 +9,13 @@
#include
#include
#include
+#include
/*
* 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 ec85b9d..e77a240 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c
@@ -10,6 +10,8 @@
#include
#include
#include
+#include
+#include
static struct {
void (*func)(struct pt_regs *, void *data);
@@ -54,3 +56,43 @@ void udelay(uint64_t us)
{
delay((us * tb_hz) / 100);
}
+
+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;
+
+ /* Could support large decrementer */
+ if (left > 0x7fff)
+ left = 0x7fff;
+
+ asm volatile ("mtdec %0" : : "r" (left));
+ handle_exception(0x900, _handler_oneshot, NULL);
Wouldn't it be better to first call handle_exception() before moving
something into the decrementer?
+ /*
+* 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();
+ }
+}
Thomas
[kvm-unit-tests v2 03/10] powerpc: abstract H_CEDE calls into a sleep functions
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
---
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 | 42 +
powerpc/sprs.c | 6 +-
powerpc/tm.c| 20 +-
7 files changed, 57 insertions(+), 31 deletions(-)
diff --git a/lib/powerpc/asm/handlers.h b/lib/powerpc/asm/handlers.h
index 64ba727..e4a0cd4 100644
--- a/lib/powerpc/asm/handlers.h
+++ b/lib/powerpc/asm/handlers.h
@@ -3,6 +3,6 @@
#include
-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 1b85f6b..6299ff5 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 ac001e1..ebfeff2 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 c8721e0..296f14f 100644
--- a/lib/powerpc/handlers.c
+++ b/lib/powerpc/handlers.c
@@ -9,15 +9,13 @@
#include
#include
#include
+#include
/*
* 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 ec85b9d..e77a240 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c
@@ -10,6 +10,8 @@
#include
#include
#include
+#include
+#include
static struct {
void (*func)(struct pt_regs *, void *data);
@@ -54,3 +56,43 @@ void udelay(uint64_t us)
{
delay((us * tb_hz) / 100);
}
+
+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;
+
+ /* Could support large decrementer */
+ if (left > 0x7fff)
+ left = 0x7fff;
+
+ asm volatile ("mtdec %0" : : "r" (left));
+ handle_exception(0x900, _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) / 100);
+}
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 5cc1cd1..ba4ddee 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 = 0x7FFF; /* 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();
