On Fri, 17 Apr 2020 04:34:39 +0200
Eric Farman <far...@linux.ibm.com> wrote:
> We have a use case (vfio-ccw) where a CRW is already built and
> ready to use. Rather than teasing out the components just to
> reassemble it later, let's rework this code so we can queue a
> fully-qualified CRW directly.
>
> Signed-off-by: Eric Farman <far...@linux.ibm.com>
> ---
> hw/s390x/css.c | 44 ++++++++++++++++++++++++++++--------------
> include/hw/s390x/css.h | 1 +
> 2 files changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index a44faa3549..a72c09adbe 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -2170,30 +2170,23 @@ void css_subch_assign(uint8_t cssid, uint8_t ssid,
> uint16_t schid,
> }
> }
>
> -void css_queue_crw(uint8_t rsc, uint8_t erc, int solicited,
> - int chain, uint16_t rsid)
> +void css_queue_crw_cont(CRW crw)
Don't really like this name, as it makes me think of 'continuation'
instead of 'container'.
css_queue_crw_container?
css_crw_add_to_queue?
Naming is hard :(
> {
> CrwContainer *crw_cont;
>
> - trace_css_crw(rsc, erc, rsid, chain ? "(chained)" : "");
> + trace_css_crw((crw.flags & CRW_FLAGS_MASK_RSC) >> 8,
> + crw.flags & CRW_FLAGS_MASK_ERC,
> + crw.rsid,
> + (crw.flags & CRW_FLAGS_MASK_C) ? "(chained)" : "");
> +
> /* TODO: Maybe use a static crw pool? */
> crw_cont = g_try_new0(CrwContainer, 1);
> if (!crw_cont) {
> channel_subsys.crws_lost = true;
> return;
Now that we actually pass something in, do we want to inform the caller
whether the crw was queued or not?
> }
> - 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;
> - }
> - crw_cont->crw.rsid = rsid;
> - if (channel_subsys.crws_lost) {
> - crw_cont->crw.flags |= CRW_FLAGS_MASK_R;
> - channel_subsys.crws_lost = false;
> - }
> +
> + crw_cont->crw = crw;
>
> QTAILQ_INSERT_TAIL(&channel_subsys.pending_crws, crw_cont, sibling);
>
Generally, looks sane to me.
