On Thu, 7 Dec 2017 07:33:19 +0100
Thomas Huth <th...@redhat.com> wrote:
> On 08.11.2017 17:54, Halil Pasic wrote:
> > +static ccw_cb_t get_ccw_cb(CcwTestDevOpMode op_mode)
> > +{
> > + switch (op_mode) {
> > + case OP_MODE_FIB:
> > + return ccw_testdev_ccw_cb_mode_fib;
> > + case OP_MODE_NOP:
> > + default:
> > + return ccw_testdev_ccw_cb_mode_nop;
>
> Do we really want to use "nop" for unknown modes? Or should there rather
> be a ccw_testdev_ccw_cb_mode_error instead, too?
I like the idea of an error mode.
Related: Should the device have a mechanism to report the supported
modes?
>
> > + }
> > +}
(...)
> > +/* TODO This is the out-of-band variant. We may want to get rid of it */
>
> I agree, this should rather go away in the final version.
I'm not sure that the in-band variant with its magic buffer value is
superior to this version using a dedicated hypercall.
>
> > +static int set_mode_diag(const uint64_t *args)
> > +{
> > + uint64_t subch_id = args[0];
> > + uint64_t op_mode = args[1];
> > + SubchDev *sch;
> > + CcwTestDevDevice *dev;
> > + int cssid, ssid, schid, m;
> > +
> > + if (ioinst_disassemble_sch_ident(subch_id, &m, &cssid, &ssid, &schid))
> > {
> > + return -EINVAL;
> > + }
> > + sch = css_find_subch(m, cssid, ssid, schid);
> > + if (!sch || !css_subch_visible(sch)) {
> > + return -EINVAL;
> > + }
> > + dev = CCW_TESTDEV(sch->driver_data);
> > + if (dev->op_mode_locked) {
> > + return op_mode == dev->op_mode ? 0 : -EINVAL;
> > + }
> > + dev->op_mode = op_mode;
> > + sch->ccw_cb = get_ccw_cb(dev->op_mode);
> > + return 0;
