From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43987) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRPgh-0002Os-Gx for qemu-devel@nongnu.org; Fri, 27 Feb 2015 13:31:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YRPgc-0005I3-2H for qemu-devel@nongnu.org; Fri, 27 Feb 2015 13:30:59 -0500 Received: from e7.ny.us.ibm.com ([32.97.182.137]:43618) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRPgb-0005Hp-R4 for qemu-devel@nongnu.org; Fri, 27 Feb 2015 13:30:53 -0500 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 27 Feb 2015 13:30:53 -0500 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: References: <1425006675-19976-1-git-send-email-mdroth@linux.vnet.ibm.com> <1425006675-19976-3-git-send-email-mdroth@linux.vnet.ibm.com> Message-ID: <20150227183033.433.28805@loki> Date: Fri, 27 Feb 2015 12:30:33 -0600 Subject: Re: [Qemu-devel] [PATCH v6 02/15] spapr_drc: initial implementation of sPAPRDRConnector device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: aik@ozlabs.ru, Alexander Graf , "qemu-devel@nongnu.org" , ncmike@ncultra.org, "qemu-ppc@nongnu.org" , tyreld@linux.vnet.ibm.com, Nathan Fontenot , David Gibson Quoting Bharata B Rao (2015-02-27 03:52:09) > On Fri, Feb 27, 2015 at 8:41 AM, Michael Roth = wrote: > > +static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt, > > + int fdt_start_offset, bool coldplug, Error **errp) > > +{ > > + DPRINTFN("drc: %x, attach", get_index(drc)); > > + > > + if (drc->isolation_state !=3D SPAPR_DR_ISOLATION_STATE_ISOLATED) { > > + error_setg(errp, "an attached device is still awaiting release= "); > > + return; > > + } > > + g_assert(drc->allocation_state =3D=3D SPAPR_DR_ALLOCATION_STATE_UN= USABLE); > > + g_assert(fdt || coldplug); > > + > > + /* NOTE: setting initial isolation state to UNISOLATED means we ca= n't > > + * detach unless guest has a userspace/kernel that moves this state > > + * back to ISOLATED in response to an unplug event, or this is done > > + * manually by the admin prior. if we force things while the guest > > + * may be accessing the device, we can easily crash the guest, so = we > > + * we defer completion of removal in such cases to the reset() hoo= k. > > + */ > > + drc->isolation_state =3D SPAPR_DR_ISOLATION_STATE_UNISOLATED; > > + drc->allocation_state =3D SPAPR_DR_ALLOCATION_STATE_USABLE; > = > BTW shouldn't isolation_state and allocation_state be set to > UNISOLATED and USABLE in response to guest's rtas-set-indicator calls > ? At least, that is how it is done/expected for CPU hotplug case. Ref: Well, for PCI we always start in USABLE (state diagram in PAPR+ 13.4), and stay there. So set_allocation_state() should actually be a no-op for PCI; there's no valid transition to UNUSABLE (or EXCHANGE/RECOVER). For non-PCI, it looks like we should start in UNUSABLE, and let RTAS take it from there. It also looks like the resource removal shouldn't be finalized until there's a transition from ISOLATED/USABLE to ISOLATED/UNUSABLE. I just pushed a patch here that I'm hoping will fix the non-PCI cases. Can you give it a spin and let me know? https://github.com/mdroth/qemu/commit/d91b0c6d6f794292e384cf129368aaae90294= f5b > kernel code arch/powerpc/platforms/pseries/dlpar.c: > dlpar_acquire[release]_drc(). > = > Same question for detach(). > = > Regards, > Bharata.