* [Qemu-devel] [PATCH v2 0/2] ERC cleanup and CRW bugfix (was: Channel Path realted CRW generation) @ 2017-08-01 7:57 Dong Jia Shi 2017-08-01 7:57 ` [Qemu-devel] [PATCH v2 1/2] s390x/css: use macro for event-information pending error recover code Dong Jia Shi 2017-08-01 7:57 ` [Qemu-devel] [PATCH v2 2/2] s390x/css: generate solicited crw for rchp completion signaling Dong Jia Shi 0 siblings, 2 replies; 9+ messages in thread From: Dong Jia Shi @ 2017-08-01 7:57 UTC (permalink / raw) To: qemu-devel; +Cc: cohuck, borntraeger, agraf, rth, bjsdjshi, pasic, pmorel This series is trying to: 1. clear up ERC related code 2. bugfix for channel path related CRW generation Change log ---------- v1->v2: Patch #1: Add all ERCs. Commit message update. Patch #2: Rename the new added parameter to more speaking name -- solicited. Dong Jia Shi (2): s390x/css: use macro for event-information pending error recover code s390x/css: generate solicited crw for rchp completion signaling hw/s390x/css.c | 16 ++++++++++------ include/hw/s390x/css.h | 3 ++- include/hw/s390x/ioinst.h | 11 +++++++++-- 3 files changed, 21 insertions(+), 9 deletions(-) -- 2.11.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] s390x/css: use macro for event-information pending error recover code 2017-08-01 7:57 [Qemu-devel] [PATCH v2 0/2] ERC cleanup and CRW bugfix (was: Channel Path realted CRW generation) Dong Jia Shi @ 2017-08-01 7:57 ` Dong Jia Shi 2017-08-01 15:24 ` Halil Pasic 2017-08-01 7:57 ` [Qemu-devel] [PATCH v2 2/2] s390x/css: generate solicited crw for rchp completion signaling Dong Jia Shi 1 sibling, 1 reply; 9+ messages in thread From: Dong Jia Shi @ 2017-08-01 7:57 UTC (permalink / raw) To: qemu-devel; +Cc: cohuck, borntraeger, agraf, rth, bjsdjshi, pasic, pmorel Let's use a macro for the ERC (error recover code) when generating a Channel Subsystem Event-information pending CRW (channel report word). While we are at it, let's also add all other ERCs. Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> --- hw/s390x/css.c | 2 +- include/hw/s390x/ioinst.h | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 6a42b95cee..5321ca016b 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -2103,7 +2103,7 @@ void css_generate_chp_crws(uint8_t cssid, uint8_t chpid) void css_generate_css_crws(uint8_t cssid) { if (!channel_subsys.sei_pending) { - css_queue_crw(CRW_RSC_CSS, 0, 0, cssid); + css_queue_crw(CRW_RSC_CSS, CRW_ERC_EVENT, 0, cssid); } channel_subsys.sei_pending = true; } diff --git a/include/hw/s390x/ioinst.h b/include/hw/s390x/ioinst.h index 92d15655e4..f89019f78f 100644 --- a/include/hw/s390x/ioinst.h +++ b/include/hw/s390x/ioinst.h @@ -201,8 +201,15 @@ typedef struct CRW { #define CRW_FLAGS_MASK_A 0x0080 #define CRW_FLAGS_MASK_ERC 0x003f -#define CRW_ERC_INIT 0x02 -#define CRW_ERC_IPI 0x04 +#define CRW_ERC_EVENT 0x00 /* event information pending */ +#define CRW_ERC_AVAIL 0x01 /* available */ +#define CRW_ERC_INIT 0x02 /* initialized */ +#define CRW_ERC_TERROR 0x03 /* temporary error */ +#define CRW_ERC_IPI 0x04 /* installed parm initialized */ +#define CRW_ERC_TERM 0x05 /* terminal */ +#define CRW_ERC_PERRN 0x06 /* perm. error, facility not init */ +#define CRW_ERC_PERRI 0x07 /* perm. error, facility init */ +#define CRW_ERC_PMOD 0x08 /* installed parameters modified */ #define CRW_RSC_SUBCH 0x3 #define CRW_RSC_CHP 0x4 -- 2.11.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] s390x/css: use macro for event-information pending error recover code 2017-08-01 7:57 ` [Qemu-devel] [PATCH v2 1/2] s390x/css: use macro for event-information pending error recover code Dong Jia Shi @ 2017-08-01 15:24 ` Halil Pasic 2017-08-02 1:15 ` Dong Jia Shi 0 siblings, 1 reply; 9+ messages in thread From: Halil Pasic @ 2017-08-01 15:24 UTC (permalink / raw) To: Dong Jia Shi, qemu-devel; +Cc: cohuck, borntraeger, agraf, rth, pmorel On 08/01/2017 09:57 AM, Dong Jia Shi wrote: > Let's use a macro for the ERC (error recover code) when generating a > Channel Subsystem Event-information pending CRW (channel report word). > > While we are at it, let's also add all other ERCs. > > Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> > --- > hw/s390x/css.c | 2 +- > include/hw/s390x/ioinst.h | 11 +++++++++-- > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 6a42b95cee..5321ca016b 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -2103,7 +2103,7 @@ void css_generate_chp_crws(uint8_t cssid, uint8_t chpid) > void css_generate_css_crws(uint8_t cssid) > { > if (!channel_subsys.sei_pending) { > - css_queue_crw(CRW_RSC_CSS, 0, 0, cssid); > + css_queue_crw(CRW_RSC_CSS, CRW_ERC_EVENT, 0, cssid); > } > channel_subsys.sei_pending = true; > } > diff --git a/include/hw/s390x/ioinst.h b/include/hw/s390x/ioinst.h > index 92d15655e4..f89019f78f 100644 > --- a/include/hw/s390x/ioinst.h > +++ b/include/hw/s390x/ioinst.h > @@ -201,8 +201,15 @@ typedef struct CRW { > #define CRW_FLAGS_MASK_A 0x0080 > #define CRW_FLAGS_MASK_ERC 0x003f > > -#define CRW_ERC_INIT 0x02 > -#define CRW_ERC_IPI 0x04 > +#define CRW_ERC_EVENT 0x00 /* event information pending */ > +#define CRW_ERC_AVAIL 0x01 /* available */ > +#define CRW_ERC_INIT 0x02 /* initialized */ > +#define CRW_ERC_TERROR 0x03 /* temporary error */ > +#define CRW_ERC_IPI 0x04 /* installed parm initialized */ > +#define CRW_ERC_TERM 0x05 /* terminal */ > +#define CRW_ERC_PERRN 0x06 /* perm. error, facility not init */ > +#define CRW_ERC_PERRI 0x07 /* perm. error, facility init */ > +#define CRW_ERC_PMOD 0x08 /* installed parameters modified */ You have missed installed parameters restored from the PoP (above you say add all other). Other than that. LGTM > > #define CRW_RSC_SUBCH 0x3 > #define CRW_RSC_CHP 0x4 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] s390x/css: use macro for event-information pending error recover code 2017-08-01 15:24 ` Halil Pasic @ 2017-08-02 1:15 ` Dong Jia Shi 2017-08-02 9:57 ` Cornelia Huck 0 siblings, 1 reply; 9+ messages in thread From: Dong Jia Shi @ 2017-08-02 1:15 UTC (permalink / raw) To: Halil Pasic Cc: Dong Jia Shi, qemu-devel, cohuck, borntraeger, agraf, rth, pmorel * Halil Pasic <pasic@linux.vnet.ibm.com> [2017-08-01 17:24:10 +0200]: > > > On 08/01/2017 09:57 AM, Dong Jia Shi wrote: > > Let's use a macro for the ERC (error recover code) when generating a > > Channel Subsystem Event-information pending CRW (channel report word). > > > > While we are at it, let's also add all other ERCs. > > > > Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> > > --- > > hw/s390x/css.c | 2 +- > > include/hw/s390x/ioinst.h | 11 +++++++++-- > > 2 files changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > > index 6a42b95cee..5321ca016b 100644 > > --- a/hw/s390x/css.c > > +++ b/hw/s390x/css.c > > @@ -2103,7 +2103,7 @@ void css_generate_chp_crws(uint8_t cssid, uint8_t chpid) > > void css_generate_css_crws(uint8_t cssid) > > { > > if (!channel_subsys.sei_pending) { > > - css_queue_crw(CRW_RSC_CSS, 0, 0, cssid); > > + css_queue_crw(CRW_RSC_CSS, CRW_ERC_EVENT, 0, cssid); > > } > > channel_subsys.sei_pending = true; > > } > > diff --git a/include/hw/s390x/ioinst.h b/include/hw/s390x/ioinst.h > > index 92d15655e4..f89019f78f 100644 > > --- a/include/hw/s390x/ioinst.h > > +++ b/include/hw/s390x/ioinst.h > > @@ -201,8 +201,15 @@ typedef struct CRW { > > #define CRW_FLAGS_MASK_A 0x0080 > > #define CRW_FLAGS_MASK_ERC 0x003f > > > > -#define CRW_ERC_INIT 0x02 > > -#define CRW_ERC_IPI 0x04 > > +#define CRW_ERC_EVENT 0x00 /* event information pending */ > > +#define CRW_ERC_AVAIL 0x01 /* available */ > > +#define CRW_ERC_INIT 0x02 /* initialized */ > > +#define CRW_ERC_TERROR 0x03 /* temporary error */ > > +#define CRW_ERC_IPI 0x04 /* installed parm initialized */ > > +#define CRW_ERC_TERM 0x05 /* terminal */ > > +#define CRW_ERC_PERRN 0x06 /* perm. error, facility not init */ > > +#define CRW_ERC_PERRI 0x07 /* perm. error, facility init */ > > +#define CRW_ERC_PMOD 0x08 /* installed parameters modified */ > > You have missed installed parameters restored from the PoP (above > you say add all other). Ok. Will add: #define CRW_ERC_IPR 0x0A /* installed parameters restored */ > > Other than that. > > LGTM > > > > > #define CRW_RSC_SUBCH 0x3 > > #define CRW_RSC_CHP 0x4 > > -- Dong Jia Shi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] s390x/css: use macro for event-information pending error recover code 2017-08-02 1:15 ` Dong Jia Shi @ 2017-08-02 9:57 ` Cornelia Huck 0 siblings, 0 replies; 9+ messages in thread From: Cornelia Huck @ 2017-08-02 9:57 UTC (permalink / raw) To: Dong Jia Shi; +Cc: Halil Pasic, qemu-devel, borntraeger, agraf, rth, pmorel On Wed, 2 Aug 2017 09:15:18 +0800 Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> wrote: > * Halil Pasic <pasic@linux.vnet.ibm.com> [2017-08-01 17:24:10 +0200]: > > > > > > > On 08/01/2017 09:57 AM, Dong Jia Shi wrote: > > > Let's use a macro for the ERC (error recover code) when generating a > > > Channel Subsystem Event-information pending CRW (channel report word). > > > > > > While we are at it, let's also add all other ERCs. > > > > > > Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> > > > --- > > > hw/s390x/css.c | 2 +- > > > include/hw/s390x/ioinst.h | 11 +++++++++-- > > > 2 files changed, 10 insertions(+), 3 deletions(-) (...) > > > diff --git a/include/hw/s390x/ioinst.h b/include/hw/s390x/ioinst.h > > > index 92d15655e4..f89019f78f 100644 > > > --- a/include/hw/s390x/ioinst.h > > > +++ b/include/hw/s390x/ioinst.h > > > @@ -201,8 +201,15 @@ typedef struct CRW { > > > #define CRW_FLAGS_MASK_A 0x0080 > > > #define CRW_FLAGS_MASK_ERC 0x003f > > > > > > -#define CRW_ERC_INIT 0x02 > > > -#define CRW_ERC_IPI 0x04 > > > +#define CRW_ERC_EVENT 0x00 /* event information pending */ > > > +#define CRW_ERC_AVAIL 0x01 /* available */ > > > +#define CRW_ERC_INIT 0x02 /* initialized */ > > > +#define CRW_ERC_TERROR 0x03 /* temporary error */ > > > +#define CRW_ERC_IPI 0x04 /* installed parm initialized */ > > > +#define CRW_ERC_TERM 0x05 /* terminal */ > > > +#define CRW_ERC_PERRN 0x06 /* perm. error, facility not init */ > > > +#define CRW_ERC_PERRI 0x07 /* perm. error, facility init */ > > > +#define CRW_ERC_PMOD 0x08 /* installed parameters modified */ > > > > You have missed installed parameters restored from the PoP (above > > you say add all other). > Ok. Will add: > #define CRW_ERC_IPR 0x0A /* installed parameters restored */ Makes sense. > > > > > Other than that. > > > > LGTM > > > > > > > > #define CRW_RSC_SUBCH 0x3 > > > #define CRW_RSC_CHP 0x4 > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] s390x/css: generate solicited crw for rchp completion signaling 2017-08-01 7:57 [Qemu-devel] [PATCH v2 0/2] ERC cleanup and CRW bugfix (was: Channel Path realted CRW generation) Dong Jia Shi 2017-08-01 7:57 ` [Qemu-devel] [PATCH v2 1/2] s390x/css: use macro for event-information pending error recover code Dong Jia Shi @ 2017-08-01 7:57 ` Dong Jia Shi 2017-08-01 15:16 ` Halil Pasic 1 sibling, 1 reply; 9+ messages in thread From: Dong Jia Shi @ 2017-08-01 7:57 UTC (permalink / raw) To: qemu-devel; +Cc: cohuck, borntraeger, agraf, rth, bjsdjshi, pasic, pmorel A successful completion of rchp should signal a solicited channel path initialized CRW (channel report word), while the current implementation always generates an un-solicited one. Let's fix this. Reported-by: Halil Pasic <pasic@linux.vnet.ibm.com> Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> --- hw/s390x/css.c | 16 ++++++++++------ include/hw/s390x/css.h | 3 ++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 5321ca016b..bfa56f4b12 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -1745,10 +1745,10 @@ int css_do_rchp(uint8_t cssid, uint8_t chpid) } /* We don't really use a channel path, so we're done here. */ - css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, + css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1, channel_subsys.max_cssid > 0 ? 1 : 0, chpid); if (channel_subsys.max_cssid > 0) { - css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 0, real_cssid << 8); + css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1, 0, real_cssid << 8); } return 0; } @@ -2028,7 +2028,8 @@ void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, } } -void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid) +void css_queue_crw(uint8_t rsc, uint8_t erc, int solicited, + int chain, uint16_t rsid) { CrwContainer *crw_cont; @@ -2040,6 +2041,9 @@ void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid) return; } crw_cont->crw.flags = (rsc << 8) | erc; + if (solicited) { + crw_cont->crw.flags |= CRW_FLAGS_MASK_S; + } if (chain) { crw_cont->crw.flags |= CRW_FLAGS_MASK_C; } @@ -2086,9 +2090,9 @@ void css_generate_sch_crws(uint8_t cssid, uint8_t ssid, uint16_t schid, } chain_crw = (channel_subsys.max_ssid > 0) || (channel_subsys.max_cssid > 0); - css_queue_crw(CRW_RSC_SUBCH, CRW_ERC_IPI, chain_crw ? 1 : 0, schid); + css_queue_crw(CRW_RSC_SUBCH, CRW_ERC_IPI, 0, chain_crw ? 1 : 0, schid); if (chain_crw) { - css_queue_crw(CRW_RSC_SUBCH, CRW_ERC_IPI, 0, + css_queue_crw(CRW_RSC_SUBCH, CRW_ERC_IPI, 0, 0, (guest_cssid << 8) | (ssid << 4)); } /* RW_ERC_IPI --> clear pending interrupts */ @@ -2103,7 +2107,7 @@ void css_generate_chp_crws(uint8_t cssid, uint8_t chpid) void css_generate_css_crws(uint8_t cssid) { if (!channel_subsys.sei_pending) { - css_queue_crw(CRW_RSC_CSS, CRW_ERC_EVENT, 0, cssid); + css_queue_crw(CRW_RSC_CSS, CRW_ERC_EVENT, 0, 0, cssid); } channel_subsys.sei_pending = true; } diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h index 5c5fe6b202..5b017e1fc3 100644 --- a/include/hw/s390x/css.h +++ b/include/hw/s390x/css.h @@ -150,7 +150,8 @@ void copy_scsw_to_guest(SCSW *dest, const SCSW *src); void css_inject_io_interrupt(SubchDev *sch); void css_reset(void); void css_reset_sch(SubchDev *sch); -void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid); +void css_queue_crw(uint8_t rsc, uint8_t erc, int solicited, + int chain, uint16_t rsid); void css_generate_sch_crws(uint8_t cssid, uint8_t ssid, uint16_t schid, int hotplugged, int add); void css_generate_chp_crws(uint8_t cssid, uint8_t chpid); -- 2.11.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] s390x/css: generate solicited crw for rchp completion signaling 2017-08-01 7:57 ` [Qemu-devel] [PATCH v2 2/2] s390x/css: generate solicited crw for rchp completion signaling Dong Jia Shi @ 2017-08-01 15:16 ` Halil Pasic 2017-08-02 1:20 ` Dong Jia Shi 2017-08-02 10:01 ` Cornelia Huck 0 siblings, 2 replies; 9+ messages in thread From: Halil Pasic @ 2017-08-01 15:16 UTC (permalink / raw) To: Dong Jia Shi, qemu-devel; +Cc: cohuck, borntraeger, agraf, rth, pmorel On 08/01/2017 09:57 AM, Dong Jia Shi wrote: [..] > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -1745,10 +1745,10 @@ int css_do_rchp(uint8_t cssid, uint8_t chpid) > } > > /* We don't really use a channel path, so we're done here. */ > - css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, > + css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1, > channel_subsys.max_cssid > 0 ? 1 : 0, chpid); > if (channel_subsys.max_cssid > 0) { > - css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 0, real_cssid << 8); > + css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1, 0, real_cssid << 8); > } > return 0; > } > @@ -2028,7 +2028,8 @@ void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, > } > } > > -void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid) > +void css_queue_crw(uint8_t rsc, uint8_t erc, int solicited, > + int chain, uint16_t rsid) I think you could make the parameters solicited and chain bool (AFAIU they are conceptually bool) for clearer semantic. If you go with that you could also get rid of the superfluous ternary operator ( we have stuff like some_cond ? 1 : 0 for the chain parameter in more than one place. Btw. I find bool flags easy to mix up and difficult to read. I have no better idea how to write this (in C) though. I was considering throwing chain and solicited together into a single flags parameter, but looking at the client code it does not look like a good idea. Besides the cosmetic considerations above LGTM ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] s390x/css: generate solicited crw for rchp completion signaling 2017-08-01 15:16 ` Halil Pasic @ 2017-08-02 1:20 ` Dong Jia Shi 2017-08-02 10:01 ` Cornelia Huck 1 sibling, 0 replies; 9+ messages in thread From: Dong Jia Shi @ 2017-08-02 1:20 UTC (permalink / raw) To: Halil Pasic Cc: Dong Jia Shi, qemu-devel, cohuck, borntraeger, agraf, rth, pmorel * Halil Pasic <pasic@linux.vnet.ibm.com> [2017-08-01 17:16:37 +0200]: > > > On 08/01/2017 09:57 AM, Dong Jia Shi wrote: > [..] > > --- a/hw/s390x/css.c > > +++ b/hw/s390x/css.c > > @@ -1745,10 +1745,10 @@ int css_do_rchp(uint8_t cssid, uint8_t chpid) > > } > > > > /* We don't really use a channel path, so we're done here. */ > > - css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, > > + css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1, > > channel_subsys.max_cssid > 0 ? 1 : 0, chpid); > > if (channel_subsys.max_cssid > 0) { > > - css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 0, real_cssid << 8); > > + css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1, 0, real_cssid << 8); > > } > > return 0; > > } > > @@ -2028,7 +2028,8 @@ void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, > > } > > } > > > > -void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid) > > +void css_queue_crw(uint8_t rsc, uint8_t erc, int solicited, > > + int chain, uint16_t rsid) > > I think you could make the parameters solicited and chain bool (AFAIU > they are conceptually bool) for clearer semantic. If you go with that > you could also get rid of the superfluous ternary operator ( we have > stuff like some_cond ? 1 : 0 for the chain parameter in more than > one place. > > Btw. I find bool flags easy to mix up and difficult to read. I have no better > idea how to write this (in C) though. I was considering throwing chain and > solicited together into a single flags parameter, but looking at the client code > it does not look like a good idea. I think I just need to get used to differet tastes. ;) > > Besides the cosmetic considerations above LGTM Thanks! -- Dong Jia Shi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] s390x/css: generate solicited crw for rchp completion signaling 2017-08-01 15:16 ` Halil Pasic 2017-08-02 1:20 ` Dong Jia Shi @ 2017-08-02 10:01 ` Cornelia Huck 1 sibling, 0 replies; 9+ messages in thread From: Cornelia Huck @ 2017-08-02 10:01 UTC (permalink / raw) To: Halil Pasic; +Cc: Dong Jia Shi, qemu-devel, borntraeger, agraf, rth, pmorel On Tue, 1 Aug 2017 17:16:37 +0200 Halil Pasic <pasic@linux.vnet.ibm.com> wrote: > On 08/01/2017 09:57 AM, Dong Jia Shi wrote: > [..] > > --- a/hw/s390x/css.c > > +++ b/hw/s390x/css.c > > @@ -1745,10 +1745,10 @@ int css_do_rchp(uint8_t cssid, uint8_t chpid) > > } > > > > /* We don't really use a channel path, so we're done here. */ > > - css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, > > + css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1, > > channel_subsys.max_cssid > 0 ? 1 : 0, chpid); > > if (channel_subsys.max_cssid > 0) { > > - css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 0, real_cssid << 8); > > + css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1, 0, real_cssid << 8); > > } > > return 0; > > } > > @@ -2028,7 +2028,8 @@ void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid, > > } > > } > > > > -void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid) > > +void css_queue_crw(uint8_t rsc, uint8_t erc, int solicited, > > + int chain, uint16_t rsid) > > I think you could make the parameters solicited and chain bool (AFAIU > they are conceptually bool) for clearer semantic. If you go with that > you could also get rid of the superfluous ternary operator ( we have > stuff like some_cond ? 1 : 0 for the chain parameter in more than > one place. Just adding the new parameter is the minimum change, and we should keep it consistent. I'm not convinced about converting to bool yet. > > Btw. I find bool flags easy to mix up and difficult to read. I have no better > idea how to write this (in C) though. I was considering throwing chain and > solicited together into a single flags parameter, but looking at the client code > it does not look like a good idea. Yes, combining them would do nothing for the code. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-08-02 10:01 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-01 7:57 [Qemu-devel] [PATCH v2 0/2] ERC cleanup and CRW bugfix (was: Channel Path realted CRW generation) Dong Jia Shi 2017-08-01 7:57 ` [Qemu-devel] [PATCH v2 1/2] s390x/css: use macro for event-information pending error recover code Dong Jia Shi 2017-08-01 15:24 ` Halil Pasic 2017-08-02 1:15 ` Dong Jia Shi 2017-08-02 9:57 ` Cornelia Huck 2017-08-01 7:57 ` [Qemu-devel] [PATCH v2 2/2] s390x/css: generate solicited crw for rchp completion signaling Dong Jia Shi 2017-08-01 15:16 ` Halil Pasic 2017-08-02 1:20 ` Dong Jia Shi 2017-08-02 10:01 ` Cornelia Huck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).