| Message ID | 20181205153918.29480-6-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | 390x: Add cross hypervisor and disk boot | expand |
On 05.12.18 16:39, Janosch Frank wrote: > We need to properly implement interrupt handling for SCLP, because on > z/VM and LPAR SCLP calls are not synchronous! > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > lib/s390x/asm/arch_def.h | 1 + > lib/s390x/asm/interrupt.h | 2 ++ > lib/s390x/interrupt.c | 12 ++++++++++-- > lib/s390x/io.c | 2 +- > lib/s390x/sclp-console.c | 29 +++++++++-------------------- > lib/s390x/sclp.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > lib/s390x/sclp.h | 4 +++- > 7 files changed, 68 insertions(+), 24 deletions(-) > > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h > index d2d6e02..27c6b85 100644 > --- a/lib/s390x/asm/arch_def.h > +++ b/lib/s390x/asm/arch_def.h > @@ -33,6 +33,7 @@ struct psw { > uint64_t addr; > }; > > +#define PSW_MASK_EXT 0x0100000000000000UL > #define PSW_MASK_DAT 0x0400000000000000UL > #define PSW_MASK_PSTATE 0x0001000000000000UL > > diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h > index 013709f..de15d9e 100644 > --- a/lib/s390x/asm/interrupt.h > +++ b/lib/s390x/asm/interrupt.h > @@ -11,6 +11,8 @@ > #define _ASMS390X_IRQ_H_ > #include <asm/arch_def.h> > > +#define EXT_IRQ_SERVICE_SIG 0x2401 > + > void handle_pgm_int(void); > void handle_ext_int(void); > void handle_mcck_int(void); > diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c > index cf0a794..7118577 100644 > --- a/lib/s390x/interrupt.c > +++ b/lib/s390x/interrupt.c > @@ -12,6 +12,7 @@ > #include <libcflat.h> > #include <asm/interrupt.h> > #include <asm/barrier.h> > +#include <sclp.h> > > static bool pgm_int_expected; > static struct lowcore *lc; > @@ -107,8 +108,15 @@ void handle_pgm_int(void) > > void handle_ext_int(void) > { > - report_abort("Unexpected external call interrupt: at %#lx", > - lc->ext_old_psw.addr); > + if (lc->ext_int_code != EXT_IRQ_SERVICE_SIG) > + report_abort("Unexpected external call interrupt: at %#lx", > + lc->ext_old_psw.addr); > + else { > + lc->ext_old_psw.mask &= ~PSW_MASK_EXT; > + lc->sw_int_cr0 &= ~(1UL << 9); > + sclp_handle_ext(); > + lc->ext_int_code = 0; > + } > } > > void handle_mcck_int(void) > diff --git a/lib/s390x/io.c b/lib/s390x/io.c > index 05a0765..7294165 100644 > --- a/lib/s390x/io.c > +++ b/lib/s390x/io.c > @@ -45,8 +45,8 @@ void setup(void) > { > setup_args_progname(ipl_args); > setup_facilities(); > - sclp_console_setup(); > sclp_memory_setup(); > + sclp_console_setup(); > } > > void exit(int code) > diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c > index deacbde..13ab03d 100644 > --- a/lib/s390x/sclp-console.c > +++ b/lib/s390x/sclp-console.c > @@ -13,30 +13,13 @@ > #include <asm/page.h> > #include "sclp.h" > > -char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096))); > - > -/* Perform service call. Return 0 on success, non-zero otherwise. */ > -int sclp_service_call(unsigned int command, void *sccb) > -{ > - int cc; > - > - asm volatile( > - " .insn rre,0xb2200000,%1,%2\n" /* servc %1,%2 */ > - " ipm %0\n" > - " srl %0,28" > - : "=&d" (cc) : "d" (command), "a" (__pa(sccb)) > - : "cc", "memory"); > - if (cc == 3) > - return -1; > - if (cc == 2) > - return -1; > - return 0; > -} > - > static void sclp_set_write_mask(void) > { > WriteEventMask *sccb = (void *)_sccb; > > + while (sclp_busy) > + /* Wait for SCLP request to complete */; should we use barrier(); here to make this clearer? > + sclp_busy = true; > sccb->h.length = sizeof(WriteEventMask); > sccb->mask_length = sizeof(unsigned int); > sccb->receive_mask = SCLP_EVENT_MASK_MSG_ASCII; > @@ -57,6 +40,9 @@ void sclp_print(const char *str) > int len = strlen(str); > WriteEventData *sccb = (void *)_sccb; > > + while (sclp_busy) > + /* Wait for SCLP request to complete */; dito (I guess do not we need barriers between setting sclp_busy and testing it, because we're using volatile and we don't have multi-threading messing with the value) > + sclp_busy = true; > sccb->h.length = sizeof(WriteEventData) + len; > sccb->h.function_code = SCLP_FC_NORMAL_WRITE; > sccb->ebh.length = sizeof(EventBufferHeader) + len; > @@ -65,4 +51,7 @@ void sclp_print(const char *str) > memcpy(sccb->data, str, len); > > sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb); > + while (sclp_busy) > + /* Wait for SCLP request to complete */; dito > + > } > diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c > index 1d4a010..cd0e5e5 100644 > --- a/lib/s390x/sclp.c > +++ b/lib/s390x/sclp.c > @@ -23,6 +23,9 @@ static uint64_t storage_increment_size; > static uint64_t max_ram_size; > static uint64_t ram_size; > > +char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096))); > +volatile bool sclp_busy = false; false should be the default value already. > + > static void mem_init(phys_addr_t mem_end) > { > phys_addr_t freemem_start = (phys_addr_t)&stacktop; > @@ -30,6 +33,42 @@ static void mem_init(phys_addr_t mem_end) > phys_alloc_init(freemem_start, mem_end - freemem_start); > } > > +static void sclp_setup_int(void) > +{ > + uint64_t mask; > + > + ctl_set_bit(0, 9); > + > + mask = extract_psw_mask(); > + mask |= PSW_MASK_EXT; > + load_psw_mask(mask); > +} > + > +void sclp_handle_ext(void) > +{ Error out if !sclp_busy but we get an interrupt? > + ctl_clear_bit(0, 9); > + sclp_busy = false; > +} > + > +/* Perform service call. Return 0 on success, non-zero otherwise. */ > +int sclp_service_call(unsigned int command, void *sccb) > +{ > + int cc; > + > + sclp_setup_int(); > + asm volatile( > + " .insn rre,0xb2200000,%1,%2\n" /* servc %1,%2 */ > + " ipm %0\n" > + " srl %0,28" > + : "=&d" (cc) : "d" (command), "a" (__pa(sccb)) > + : "cc", "memory"); > + if (cc == 3) > + return -1; > + if (cc == 2) > + return -1; > + return 0; > +} Guess moving that (including _sccb) could also be a separate patch ;) > + > void sclp_memory_setup(void) > { > ReadInfo *ri = (void *)_sccb; > @@ -37,7 +76,10 @@ void sclp_memory_setup(void) > int cc; > > ri->h.length = SCCB_SIZE; > + sclp_busy = true; > sclp_service_call(SCLP_CMDW_READ_SCP_INFO_FORCED, ri); > + while (sclp_busy) > + /* Wait for SCLP request to complete */; Should we factor that out into something like sclp_start_request() { sclp_busy = true; } sclp_wait_response() { while (sclp_busy) barrier(); } > > /* calculate the storage increment size */ > rnsize = ri->rnsize; > diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h > index e008932..a91aad1 100644 > --- a/lib/s390x/sclp.h > +++ b/lib/s390x/sclp.h > @@ -207,9 +207,11 @@ typedef struct ReadEventData { > uint32_t mask; > } __attribute__((packed)) ReadEventData; > > +extern char _sccb[]; > +volatile bool sclp_busy; > +void sclp_handle_ext(void); > void sclp_console_setup(void); > void sclp_print(const char *str); > -extern char _sccb[]; > int sclp_service_call(unsigned int command, void *sccb); > void sclp_memory_setup(void); > >
On 05.12.18 20:27, David Hildenbrand wrote: > On 05.12.18 16:39, Janosch Frank wrote: >> We need to properly implement interrupt handling for SCLP, because on >> z/VM and LPAR SCLP calls are not synchronous! >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- [] >> - >> static void sclp_set_write_mask(void) >> { >> WriteEventMask *sccb = (void *)_sccb; >> >> + while (sclp_busy) >> + /* Wait for SCLP request to complete */; > > should we use barrier(); here to make this clearer? I could factor that out in sclp_wait_busy(), to make it one place to change. I thought about using spinlocks, but whatever we do, we have to be careful to not print when holding the lock. Maybe we need a sclp spinlock/busy bust error path for exit with > 0, like we do in the kernel on a panic. > >> + sclp_busy = true; >> sccb->h.length = sizeof(WriteEventMask); >> sccb->mask_length = sizeof(unsigned int); >> sccb->receive_mask = SCLP_EVENT_MASK_MSG_ASCII; >> @@ -57,6 +40,9 @@ void sclp_print(const char *str) >> int len = strlen(str); >> WriteEventData *sccb = (void *)_sccb; >> >> + while (sclp_busy) >> + /* Wait for SCLP request to complete */; > > dito > > (I guess do not we need barriers between setting sclp_busy and testing > it, because we're using volatile and we don't have multi-threading > messing with the value) > >> + sclp_busy = true; >> sccb->h.length = sizeof(WriteEventData) + len; >> sccb->h.function_code = SCLP_FC_NORMAL_WRITE; >> sccb->ebh.length = sizeof(EventBufferHeader) + len; >> @@ -65,4 +51,7 @@ void sclp_print(const char *str) >> memcpy(sccb->data, str, len); >> >> sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb); >> + while (sclp_busy) >> + /* Wait for SCLP request to complete */; > > dito > >> + >> } >> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c >> index 1d4a010..cd0e5e5 100644 >> --- a/lib/s390x/sclp.c >> +++ b/lib/s390x/sclp.c >> @@ -23,6 +23,9 @@ static uint64_t storage_increment_size; >> static uint64_t max_ram_size; >> static uint64_t ram_size; >> >> +char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096))); >> +volatile bool sclp_busy = false; > > false should be the default value already. Yes, that's a leftover from the bss clearing problem, where it sometimes was > 0... > >> + >> static void mem_init(phys_addr_t mem_end) >> { >> phys_addr_t freemem_start = (phys_addr_t)&stacktop; >> @@ -30,6 +33,42 @@ static void mem_init(phys_addr_t mem_end) >> phys_alloc_init(freemem_start, mem_end - freemem_start); >> } >> >> +static void sclp_setup_int(void) >> +{ >> + uint64_t mask; >> + >> + ctl_set_bit(0, 9); >> + >> + mask = extract_psw_mask(); >> + mask |= PSW_MASK_EXT; >> + load_psw_mask(mask); >> +} >> + >> +void sclp_handle_ext(void) >> +{ > > Error out if !sclp_busy but we get an interrupt? Yes, but then I need to reset sclp_busy in the progress and we need to hope that we get sclp output. > >> + ctl_clear_bit(0, 9); >> + sclp_busy = false; >> +} >> + >> +/* Perform service call. Return 0 on success, non-zero otherwise. */ >> +int sclp_service_call(unsigned int command, void *sccb) >> +{ >> + int cc; >> + >> + sclp_setup_int(); >> + asm volatile( >> + " .insn rre,0xb2200000,%1,%2\n" /* servc %1,%2 */ >> + " ipm %0\n" >> + " srl %0,28" >> + : "=&d" (cc) : "d" (command), "a" (__pa(sccb)) >> + : "cc", "memory"); >> + if (cc == 3) >> + return -1; >> + if (cc == 2) >> + return -1; >> + return 0; >> +} > > Guess moving that (including _sccb) could also be a separate patch ;) Will do > >> + >> void sclp_memory_setup(void) >> { >> ReadInfo *ri = (void *)_sccb; >> @@ -37,7 +76,10 @@ void sclp_memory_setup(void) >> int cc; >> >> ri->h.length = SCCB_SIZE; >> + sclp_busy = true; >> sclp_service_call(SCLP_CMDW_READ_SCP_INFO_FORCED, ri); >> + while (sclp_busy) >> + /* Wait for SCLP request to complete */; > > Should we factor that out into something like As written above, I'm all for the second one, but I dislike the start one liner. > > sclp_start_request() > { > sclp_busy = true; > } > > sclp_wait_response() > { > while (sclp_busy) > barrier(); > } > >> >> /* calculate the storage increment size */ >> rnsize = ri->rnsize; >> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h >> index e008932..a91aad1 100644 >> --- a/lib/s390x/sclp.h >> +++ b/lib/s390x/sclp.h >> @@ -207,9 +207,11 @@ typedef struct ReadEventData { >> uint32_t mask; >> } __attribute__((packed)) ReadEventData; >> >> +extern char _sccb[]; >> +volatile bool sclp_busy; >> +void sclp_handle_ext(void); >> void sclp_console_setup(void); >> void sclp_print(const char *str); >> -extern char _sccb[]; >> int sclp_service_call(unsigned int command, void *sccb); >> void sclp_memory_setup(void); >> >> > >
> >> >>> + >>> void sclp_memory_setup(void) >>> { >>> ReadInfo *ri = (void *)_sccb; >>> @@ -37,7 +76,10 @@ void sclp_memory_setup(void) >>> int cc; >>> >>> ri->h.length = SCCB_SIZE; >>> + sclp_busy = true; >>> sclp_service_call(SCLP_CMDW_READ_SCP_INFO_FORCED, ri); >>> + while (sclp_busy) >>> + /* Wait for SCLP request to complete */; >> >> Should we factor that out into something like > > As written above, I'm all for the second one, but I dislike the start > one liner. > Sure, fine with me!
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h index d2d6e02..27c6b85 100644 --- a/lib/s390x/asm/arch_def.h +++ b/lib/s390x/asm/arch_def.h @@ -33,6 +33,7 @@ struct psw { uint64_t addr; }; +#define PSW_MASK_EXT 0x0100000000000000UL #define PSW_MASK_DAT 0x0400000000000000UL #define PSW_MASK_PSTATE 0x0001000000000000UL diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h index 013709f..de15d9e 100644 --- a/lib/s390x/asm/interrupt.h +++ b/lib/s390x/asm/interrupt.h @@ -11,6 +11,8 @@ #define _ASMS390X_IRQ_H_ #include <asm/arch_def.h> +#define EXT_IRQ_SERVICE_SIG 0x2401 + void handle_pgm_int(void); void handle_ext_int(void); void handle_mcck_int(void); diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c index cf0a794..7118577 100644 --- a/lib/s390x/interrupt.c +++ b/lib/s390x/interrupt.c @@ -12,6 +12,7 @@ #include <libcflat.h> #include <asm/interrupt.h> #include <asm/barrier.h> +#include <sclp.h> static bool pgm_int_expected; static struct lowcore *lc; @@ -107,8 +108,15 @@ void handle_pgm_int(void) void handle_ext_int(void) { - report_abort("Unexpected external call interrupt: at %#lx", - lc->ext_old_psw.addr); + if (lc->ext_int_code != EXT_IRQ_SERVICE_SIG) + report_abort("Unexpected external call interrupt: at %#lx", + lc->ext_old_psw.addr); + else { + lc->ext_old_psw.mask &= ~PSW_MASK_EXT; + lc->sw_int_cr0 &= ~(1UL << 9); + sclp_handle_ext(); + lc->ext_int_code = 0; + } } void handle_mcck_int(void) diff --git a/lib/s390x/io.c b/lib/s390x/io.c index 05a0765..7294165 100644 --- a/lib/s390x/io.c +++ b/lib/s390x/io.c @@ -45,8 +45,8 @@ void setup(void) { setup_args_progname(ipl_args); setup_facilities(); - sclp_console_setup(); sclp_memory_setup(); + sclp_console_setup(); } void exit(int code) diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c index deacbde..13ab03d 100644 --- a/lib/s390x/sclp-console.c +++ b/lib/s390x/sclp-console.c @@ -13,30 +13,13 @@ #include <asm/page.h> #include "sclp.h" -char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096))); - -/* Perform service call. Return 0 on success, non-zero otherwise. */ -int sclp_service_call(unsigned int command, void *sccb) -{ - int cc; - - asm volatile( - " .insn rre,0xb2200000,%1,%2\n" /* servc %1,%2 */ - " ipm %0\n" - " srl %0,28" - : "=&d" (cc) : "d" (command), "a" (__pa(sccb)) - : "cc", "memory"); - if (cc == 3) - return -1; - if (cc == 2) - return -1; - return 0; -} - static void sclp_set_write_mask(void) { WriteEventMask *sccb = (void *)_sccb; + while (sclp_busy) + /* Wait for SCLP request to complete */; + sclp_busy = true; sccb->h.length = sizeof(WriteEventMask); sccb->mask_length = sizeof(unsigned int); sccb->receive_mask = SCLP_EVENT_MASK_MSG_ASCII; @@ -57,6 +40,9 @@ void sclp_print(const char *str) int len = strlen(str); WriteEventData *sccb = (void *)_sccb; + while (sclp_busy) + /* Wait for SCLP request to complete */; + sclp_busy = true; sccb->h.length = sizeof(WriteEventData) + len; sccb->h.function_code = SCLP_FC_NORMAL_WRITE; sccb->ebh.length = sizeof(EventBufferHeader) + len; @@ -65,4 +51,7 @@ void sclp_print(const char *str) memcpy(sccb->data, str, len); sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb); + while (sclp_busy) + /* Wait for SCLP request to complete */; + } diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c index 1d4a010..cd0e5e5 100644 --- a/lib/s390x/sclp.c +++ b/lib/s390x/sclp.c @@ -23,6 +23,9 @@ static uint64_t storage_increment_size; static uint64_t max_ram_size; static uint64_t ram_size; +char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096))); +volatile bool sclp_busy = false; + static void mem_init(phys_addr_t mem_end) { phys_addr_t freemem_start = (phys_addr_t)&stacktop; @@ -30,6 +33,42 @@ static void mem_init(phys_addr_t mem_end) phys_alloc_init(freemem_start, mem_end - freemem_start); } +static void sclp_setup_int(void) +{ + uint64_t mask; + + ctl_set_bit(0, 9); + + mask = extract_psw_mask(); + mask |= PSW_MASK_EXT; + load_psw_mask(mask); +} + +void sclp_handle_ext(void) +{ + ctl_clear_bit(0, 9); + sclp_busy = false; +} + +/* Perform service call. Return 0 on success, non-zero otherwise. */ +int sclp_service_call(unsigned int command, void *sccb) +{ + int cc; + + sclp_setup_int(); + asm volatile( + " .insn rre,0xb2200000,%1,%2\n" /* servc %1,%2 */ + " ipm %0\n" + " srl %0,28" + : "=&d" (cc) : "d" (command), "a" (__pa(sccb)) + : "cc", "memory"); + if (cc == 3) + return -1; + if (cc == 2) + return -1; + return 0; +} + void sclp_memory_setup(void) { ReadInfo *ri = (void *)_sccb; @@ -37,7 +76,10 @@ void sclp_memory_setup(void) int cc; ri->h.length = SCCB_SIZE; + sclp_busy = true; sclp_service_call(SCLP_CMDW_READ_SCP_INFO_FORCED, ri); + while (sclp_busy) + /* Wait for SCLP request to complete */; /* calculate the storage increment size */ rnsize = ri->rnsize; diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h index e008932..a91aad1 100644 --- a/lib/s390x/sclp.h +++ b/lib/s390x/sclp.h @@ -207,9 +207,11 @@ typedef struct ReadEventData { uint32_t mask; } __attribute__((packed)) ReadEventData; +extern char _sccb[]; +volatile bool sclp_busy; +void sclp_handle_ext(void); void sclp_console_setup(void); void sclp_print(const char *str); -extern char _sccb[]; int sclp_service_call(unsigned int command, void *sccb); void sclp_memory_setup(void);
We need to properly implement interrupt handling for SCLP, because on z/VM and LPAR SCLP calls are not synchronous! Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- lib/s390x/asm/arch_def.h | 1 + lib/s390x/asm/interrupt.h | 2 ++ lib/s390x/interrupt.c | 12 ++++++++++-- lib/s390x/io.c | 2 +- lib/s390x/sclp-console.c | 29 +++++++++-------------------- lib/s390x/sclp.c | 42 ++++++++++++++++++++++++++++++++++++++++++ lib/s390x/sclp.h | 4 +++- 7 files changed, 68 insertions(+), 24 deletions(-)