On 7/13/22 15:07, Claudio Imbrenda wrote:
On Wed, 13 Jul 2022 14:24:57 +0200
Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:
On 7/13/22 12:45, Claudio Imbrenda wrote:
Use per-CPU flags and callbacks for Program and Extern interrupts,
instead of global variables.
This allows for more accurate error handling; a CPU waiting for an
interrupt will not have it "stolen" by a different CPU that was not
supposed to wait for one, and now two CPUs can wait for interrupts at
the same time.
This will significantly improve error reporting and debugging when
things go wrong.
Both program interrupts and external interrupts are now CPU-bound, even
though some external interrupts are floating (notably, the SCLP
interrupt). In those cases, the testcases should mask interrupts and/or
expect them appropriately according to need.
Signed-off-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
---
lib/s390x/asm/arch_def.h | 16 ++++++++++-
lib/s390x/smp.h | 8 +-----
lib/s390x/interrupt.c | 57 +++++++++++++++++++++++++++++-----------
lib/s390x/smp.c | 11 ++++++++
4 files changed, 69 insertions(+), 23 deletions(-)
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index b3282367..03578277 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -41,6 +41,17 @@ struct psw {
uint64_t addr;
};
+struct cpu {
+ struct lowcore *lowcore;
+ uint64_t *stack;
+ void (*pgm_cleanup_func)(void);
We should change the parameter to include the stack frame for easier
manipulation of the pre-exception registers, especially the CRs.
will do
+ uint16_t addr;
+ uint16_t idx;
+ bool active;
+ bool pgm_int_expected;
+ bool ext_int_expected;
+};
And I'd opt for also integrating the io handling function and getting
rid of the unset function to make them all look the same.
I/O is usually floating, though, I don't think it makes sense to have
it per-cpu
Right, it just bugs me that it's handled so differently.
I'll find a solution for that if my eyes stumble over it too often.
Looking at Nico's patches the external handler will follow soon anyway.
should I add the external handler here?
Discuss that with Nico, I don't have a strong opinion on that
I'm not 100% happy with having this struct in this file, what kept you
from including smp.h?
smp.h depends on arch_def.h, which then would depend on smp.h
+struct lowcore *smp_get_lowcore(uint16_t idx)
+{
+ if (THIS_CPU->idx == idx)
+ return &lowcore;
+
+ check_idx(idx);
+ return cpus[idx].lowcore;
+}
I'm waiting for the moment where we need locking in the struct cpu.
+
int smp_sigp(uint16_t idx, uint8_t order, unsigned long parm, uint32_t *status)
{
check_idx(idx);
@@ -253,6 +262,7 @@ static int smp_cpu_setup_nolock(uint16_t idx, struct psw psw)
/* Copy all exception psws. */
memcpy(lc, cpus[0].lowcore, 512);
+ lc->this_cpu = &cpus[idx];
/* Setup stack */
cpus[idx].stack = (uint64_t *)alloc_pages(2);
@@ -325,6 +335,7 @@ void smp_setup(void)
for (i = 0; i < num; i++) {
cpus[i].addr = entry[i].address;
cpus[i].active = false;
+ cpus[i].idx = i;
/*
* Fill in the boot CPU. If the boot CPU is not at index 0,
* swap it with the one at index 0. This guarantees that the
