Re: [Qemu-devel] [PATCH v7 04/13] s390x/css: realize css_sch_build_schib

[Dong Jia Shi]

* Cornelia Huck <address@hidden> [2017-05-05 14:04:20 +0200]:

> On Fri,  5 May 2017 04:03:43 +0200
> Dong Jia Shi <address@hidden> wrote:
> 
> > From: Xiao Feng Ren <address@hidden>
> > 
> > The S390 virtual css support already has a mechanism to build virtual
> > Sub-Channel Information Block and provide virtual subchannels to the
> 
> "to build a virtual subchannel information block (schib) and provide..."
> 
Ok.

> > guest. However, to pass-through subchannels to a guest, we need to
> > introduce a new mechanism to build its schib according to the real
> > device information. Thus we realize a new css_sch_build_schib function
> > to extract the path_masks, chpids, chpid type from sysfs. To reuse
> > the existing code, we refactor css_add_virtual_chpid to css_add_chpid.
> > 
> > Reviewed-by: Pierre Morel <address@hidden>
> > Signed-off-by: Xiao Feng Ren <address@hidden>
> > Signed-off-by: Dong Jia Shi <address@hidden>
> > ---
> >  hw/s390x/css.c         | 153 
> > ++++++++++++++++++++++++++++++++++++++++++++++++-
> >  include/hw/s390x/css.h |  36 ++++++------
> >  2 files changed, 169 insertions(+), 20 deletions(-)
> > 
> 
> > +/*
> > + * We currently retrieve the real device information from sysfs to build 
> > the
> > + * guest subchannel information block without considering the migration 
> > feature.
> > + * If migrate, it won't be sure to use the real device information 
> > directly,
> > + * this point will be handled in the future.
> 
> Let's make the second sentence: "We need to revisit this problem when
> we want to add migration support."
> 
Ok.

> > + */
> > +int css_sch_build_schib(SubchDev *sch, CssDevId *dev_id)
> > +{
> > +    CssImage *css = channel_subsys.css[sch->cssid];
> > +    PMCW *p = &sch->curr_status.pmcw;
> > +    SCSW *s = &sch->curr_status.scsw;
> > +    uint32_t type;
> > +    int i, ret;
> > +
> > +    /* We are dealing with I/O subchannels only. */
> 
> Let's move this comment directly before setting dnv; it's a bit
> confusing here.
> 
Ok. I will move it to ...

> > +    assert(css != NULL);
> > +    memset(p, 0, sizeof(PMCW));
> > +    p->flags |= PMCW_FLAGS_MASK_DNV;
... here.

> > +    p->devno = sch->devno;
> > +
> > +    /* Grab path mask from sysfs. */
> > +    ret = css_sch_get_path_masks(sch, dev_id);
> > +    if (ret) {
> > +        return ret;
> > +    }
> > +
> > +    /* Grab chpids from sysfs. */
> > +    ret = css_sch_get_chpids(sch, dev_id);
> > +    if (ret) {
> > +        return ret;
> > +    }
> > +
> > +   /* Build chpid type. */
> > +    for (i = 0; i < ARRAY_SIZE(p->chpid); i++) {
> > +        if (p->chpid[i] && !css->chpids[p->chpid[i]].in_use) {
> > +            ret = css_sch_get_chpid_type(p->chpid[i], &type, dev_id);
> > +            if (ret) {
> > +                return ret;
> > +            }
> > +            css_add_chpid(sch->cssid, p->chpid[i], type, false);
> > +        }
> > +    }
> > +
> > +    memset(s, 0, sizeof(SCSW));
> > +    sch->curr_status.mba = 0;
> > +    for (i = 0; i < ARRAY_SIZE(sch->curr_status.mda); i++) {
> > +        sch->curr_status.mda[i] = 0;
> > +    }
> > +
> > +    return 0;
> > +}
[...]

-- 
Dong Jia Shi