From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-74.mimecast.com ([63.128.21.74]:54841 "EHLO us-smtp-delivery-74.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728513AbgCXP7E (ORCPT ); Tue, 24 Mar 2020 11:59:04 -0400 Date: Tue, 24 Mar 2020 16:58:54 +0100 From: Cornelia Huck Subject: Re: [RFC PATCH v2 2/9] vfio-ccw: Register a chp_event callback for vfio-ccw Message-ID: <20200324165854.3d862d5b.cohuck@redhat.com> In-Reply-To: <459a60d1-699d-2f16-bb59-23f11b817b81@linux.ibm.com> References: <20200206213825.11444-1-farman@linux.ibm.com> <20200206213825.11444-3-farman@linux.ibm.com> <20200214131147.0a98dd7d.cohuck@redhat.com> <459a60d1-699d-2f16-bb59-23f11b817b81@linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-s390-owner@vger.kernel.org List-ID: To: Eric Farman Cc: Halil Pasic , Jason Herne , Jared Rossi , linux-s390@vger.kernel.org, kvm@vger.kernel.org On Fri, 14 Feb 2020 11:35:21 -0500 Eric Farman wrote: > On 2/14/20 7:11 AM, Cornelia Huck wrote: > > On Thu, 6 Feb 2020 22:38:18 +0100 > > Eric Farman wrote: > > (...) > >> @@ -257,6 +258,48 @@ static int vfio_ccw_sch_event(struct subchannel *sch, int process) > >> return rc; > >> } > >> > >> +static int vfio_ccw_chp_event(struct subchannel *sch, > >> + struct chp_link *link, int event) > >> +{ > >> + struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev); > >> + int mask = chp_ssd_get_mask(&sch->ssd_info, link); > >> + int retry = 255; > >> + > >> + if (!private || !mask) > >> + return 0; > >> + > >> + VFIO_CCW_MSG_EVENT(2, "%pUl (%x.%x.%04x): mask=0x%x event=%d\n", > >> + mdev_uuid(private->mdev), sch->schid.cssid, > >> + sch->schid.ssid, sch->schid.sch_no, > >> + mask, event); > >> + > >> + if (cio_update_schib(sch)) > >> + return -ENODEV; > >> + > >> + switch (event) { > >> + case CHP_VARY_OFF: > >> + /* Path logically turned off */ > >> + sch->opm &= ~mask; > >> + sch->lpm &= ~mask; > >> + break; > >> + case CHP_OFFLINE: > >> + /* Path is gone */ > >> + cio_cancel_halt_clear(sch, &retry); > > > > Any reason you do this only for CHP_OFFLINE and not for CHP_VARY_OFF? > > Hrm... No reason that I can think of. I can fix this. > > > > >> + break; > >> + case CHP_VARY_ON: > >> + /* Path logically turned on */ > >> + sch->opm |= mask; > >> + sch->lpm |= mask; > >> + break; > >> + case CHP_ONLINE: > >> + /* Path became available */ > >> + sch->lpm |= mask & sch->opm; > > > > If I'm not mistaken, this patch introduces the first usage of sch->opm > > in the vfio-ccw code. > > Correct. > > > Are we missing something? > > Maybe? :) > > >Or am I missing > > something? :) > > > > Since it's only used in this code, for acting as a step between > vary/config off/on, maybe this only needs to be dealing with the lpm > field itself? Ok, I went over this again and also looked at what the standard I/O subchannel driver does, and I think this is fine, as the lpm basically factors in the opm already. (Will need to keep this in mind for the following patches.) > > >> + break; > >> + } > >> + > >> + return 0; > >> +} > >> + > >> static struct css_device_id vfio_ccw_sch_ids[] = { > >> { .match_flags = 0x1, .type = SUBCHANNEL_TYPE_IO, }, > >> { /* end of list */ }, > > (...) > > >