| Message ID | 1587725152-25569-9-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Testing the Channel Subsystem I/O | expand |
On 4/24/20 12:45 PM, Pierre Morel wrote: > wfi(irq_mask) allows the programm to wait for an interrupt. s/programm/program/ > The interrupt handler is in charge to remove the WAIT bit > when it finished handling interrupt. ...finished handling the interrupt. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > lib/s390x/asm/arch_def.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h > index a0d2362..e04866c 100644 > --- a/lib/s390x/asm/arch_def.h > +++ b/lib/s390x/asm/arch_def.h > @@ -13,6 +13,7 @@ > #define PSW_MASK_EXT 0x0100000000000000UL > #define PSW_MASK_DAT 0x0400000000000000UL > #define PSW_MASK_SHORT_PSW 0x0008000000000000UL > +#define PSW_MASK_WAIT 0x0002000000000000UL > #define PSW_MASK_PSTATE 0x0001000000000000UL > #define PSW_MASK_BA 0x0000000080000000UL > #define PSW_MASK_EA 0x0000000100000000UL > @@ -254,6 +255,16 @@ static inline void load_psw_mask(uint64_t mask) > : "+r" (tmp) : "a" (&psw) : "memory", "cc" ); > } > > +static inline void wfi(uint64_t irq_mask) enabled_wait() > +{ > + uint64_t psw_mask; You can directly initialize this variable. > + > + psw_mask = extract_psw_mask(); > + load_psw_mask(psw_mask | irq_mask | PSW_MASK_WAIT); Maybe add a comment here: /* * After being woken and having processed the interrupt, let's restore the PSW mask. */ > + load_psw_mask(psw_mask); > +} > + > + > static inline void enter_pstate(void) > { > uint64_t mask; >
On 2020-04-27 14:59, Janosch Frank wrote: > On 4/24/20 12:45 PM, Pierre Morel wrote: >> wfi(irq_mask) allows the programm to wait for an interrupt. > > s/programm/program/ Thx, > >> The interrupt handler is in charge to remove the WAIT bit >> when it finished handling interrupt. > > ...finished handling the interrupt. OK, thx > >> } >> >> +static inline void wfi(uint64_t irq_mask) > > enabled_wait() I do not like enabled_wait(), we do not know what is enabled and we do not know what we are waiting for. What about wait_for_interrupt() > >> +{ >> + uint64_t psw_mask; > > You can directly initialize this variable. > >> + >> + psw_mask = extract_psw_mask(); >> + load_psw_mask(psw_mask | irq_mask | PSW_MASK_WAIT); > > Maybe add a comment here: > > /* > * After being woken and having processed the interrupt, let's restore > the PSW mask. > */ > >> + load_psw_mask(psw_mask); >> +} >> + I can do this, but wasn't it obvious? Regards, Pierre
On 4/28/20 10:44 AM, Pierre Morel wrote: > > > On 2020-04-27 14:59, Janosch Frank wrote: >> On 4/24/20 12:45 PM, Pierre Morel wrote: >>> wfi(irq_mask) allows the programm to wait for an interrupt. >> >> s/programm/program/ > > Thx, > >> >>> The interrupt handler is in charge to remove the WAIT bit >>> when it finished handling interrupt. >> >> ...finished handling the interrupt. > > OK, thx > >> > >>> } >>> >>> +static inline void wfi(uint64_t irq_mask) >> >> enabled_wait() > > > I do not like enabled_wait(), we do not know what is enabled and we do > not know what we are waiting for. > > What about wait_for_interrupt() As long as it's not called wfi... > >> >>> +{ >>> + uint64_t psw_mask; >> >> You can directly initialize this variable. >> >>> + >>> + psw_mask = extract_psw_mask(); >>> + load_psw_mask(psw_mask | irq_mask | PSW_MASK_WAIT); >> >> Maybe add a comment here: >> >> /* >> * After being woken and having processed the interrupt, let's restore >> the PSW mask. >> */ >> >>> + load_psw_mask(psw_mask); >>> +} >>> + > > I can do this, but wasn't it obvious? It took me a minute, so it will take even longer for developers that are not yet familiar with s390 kernel development. > > > Regards, > Pierre >
On 2020-04-28 11:20, Janosch Frank wrote: > On 4/28/20 10:44 AM, Pierre Morel wrote: >> >> >> On 2020-04-27 14:59, Janosch Frank wrote: >>> On 4/24/20 12:45 PM, Pierre Morel wrote: >>>> wfi(irq_mask) allows the programm to wait for an interrupt. >>> >>> s/programm/program/ >> >> Thx, >> >>> >>>> The interrupt handler is in charge to remove the WAIT bit >>>> when it finished handling interrupt. >>> >>> ...finished handling the interrupt. >> >> OK, thx >> >>> >> >>>> } >>>> >>>> +static inline void wfi(uint64_t irq_mask) >>> >>> enabled_wait() >> >> >> I do not like enabled_wait(), we do not know what is enabled and we do >> not know what we are waiting for. >> >> What about wait_for_interrupt() > > As long as it's not called wfi... > >> >>> >>>> +{ >>>> + uint64_t psw_mask; >>> >>> You can directly initialize this variable. >>> >>>> + >>>> + psw_mask = extract_psw_mask(); >>>> + load_psw_mask(psw_mask | irq_mask | PSW_MASK_WAIT); >>> >>> Maybe add a comment here: >>> >>> /* >>> * After being woken and having processed the interrupt, let's restore >>> the PSW mask. >>> */ >>> >>>> + load_psw_mask(psw_mask); >>>> +} >>>> + >> >> I can do this, but wasn't it obvious? > > It took me a minute, so it will take even longer for developers that are > not yet familiar with s390 kernel development. > >> >> >> Regards, >> Pierre >> > > OK, will do Thx Pierre
diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h index a0d2362..e04866c 100644 --- a/lib/s390x/asm/arch_def.h +++ b/lib/s390x/asm/arch_def.h @@ -13,6 +13,7 @@ #define PSW_MASK_EXT 0x0100000000000000UL #define PSW_MASK_DAT 0x0400000000000000UL #define PSW_MASK_SHORT_PSW 0x0008000000000000UL +#define PSW_MASK_WAIT 0x0002000000000000UL #define PSW_MASK_PSTATE 0x0001000000000000UL #define PSW_MASK_BA 0x0000000080000000UL #define PSW_MASK_EA 0x0000000100000000UL @@ -254,6 +255,16 @@ static inline void load_psw_mask(uint64_t mask) : "+r" (tmp) : "a" (&psw) : "memory", "cc" ); } +static inline void wfi(uint64_t irq_mask) +{ + uint64_t psw_mask; + + psw_mask = extract_psw_mask(); + load_psw_mask(psw_mask | irq_mask | PSW_MASK_WAIT); + load_psw_mask(psw_mask); +} + + static inline void enter_pstate(void) { uint64_t mask;
wfi(irq_mask) allows the programm to wait for an interrupt. The interrupt handler is in charge to remove the WAIT bit when it finished handling interrupt. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- lib/s390x/asm/arch_def.h | 11 +++++++++++ 1 file changed, 11 insertions(+)