From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=Gp66B907; spf=pass (domain: apple.com, ip: 17.171.2.72, mailfrom: afish@apple.com) Received: from ma1-aaemail-dr-lapp03.apple.com (ma1-aaemail-dr-lapp03.apple.com [17.171.2.72]) by groups.io with SMTP; Mon, 16 Sep 2019 11:37:06 -0700 Received: from pps.filterd (ma1-aaemail-dr-lapp03.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp03.apple.com (8.16.0.27/8.16.0.27) with SMTP id x8GIZuCn049813; Mon, 16 Sep 2019 11:36:59 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=sender : content-type : mime-version : subject : from : in-reply-to : date : cc : content-transfer-encoding : message-id : references : to; s=20180706; bh=mmVj1d/8VdkRJRAZhuFscXrF6JynYVyiK4xd+QYTepk=; b=Gp66B907H2J/6mxPno0pyqsbxjjNdOfzltXOj7E2sIfort6N356yasH0cikEVN8brdXC cDU+ZpkDdppsWrCBBb4XdVZVFWGtArUSQN8qed0H8H9QGwibHAZJ0o/+YdxryuGKONf3 9WeRDZqmOjorQWxSeaLa/FLLQxZvGilriVXaygZt/1Lj7KDntUdbeI13HLIN3vPgZglI PvQpDivm46lUo5iqn2mH/eoNdCt9Roi8EJ4+13CrV4Tk1J+azIwyU9JG2299EPydVYRH z5awLsfd3cUqxToaqgwPRWWRY9RWov3FdBUOHLWpC5pBRIF8KYFtRAA1Ej7rSLwVe0e6 nw== Received: from mr2-mtap-s03.rno.apple.com (mr2-mtap-s03.rno.apple.com [17.179.226.135]) by ma1-aaemail-dr-lapp03.apple.com with ESMTP id 2v0y6uw7nd-4 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Mon, 16 Sep 2019 11:36:59 -0700 Received: from nwk-mmpp-sz09.apple.com (nwk-mmpp-sz09.apple.com [17.128.115.80]) by mr2-mtap-s03.rno.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) with ESMTPS id <0PXX00G3YT1MFDF0@mr2-mtap-s03.rno.apple.com>; Mon, 16 Sep 2019 11:36:58 -0700 (PDT) Received: from process_milters-daemon.nwk-mmpp-sz09.apple.com by nwk-mmpp-sz09.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) id <0PXX00900SL39D00@nwk-mmpp-sz09.apple.com>; Mon, 16 Sep 2019 11:36:56 -0700 (PDT) X-Va-A: X-Va-T-CD: 08777febe38bb384cc57fda39d0586b7 X-Va-E-CD: a5a5e046124b1576edd6ac48f9946ad5 X-Va-R-CD: 5689ef722b0dd46b2e850591228443f8 X-Va-CD: 0 X-Va-ID: f532aa3d-acee-4954-a4da-4b99dffee240 X-V-A: X-V-T-CD: 08777febe38bb384cc57fda39d0586b7 X-V-E-CD: a5a5e046124b1576edd6ac48f9946ad5 X-V-R-CD: 5689ef722b0dd46b2e850591228443f8 X-V-CD: 0 X-V-ID: e5b5a9e4-cd01-4149-929b-a17acfbf650d X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-09-16_07:,, signatures=0 Received: from [17.235.22.77] (unknown [17.235.22.77]) by nwk-mmpp-sz09.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) with ESMTPSA id <0PXX00EUWT1IKX20@nwk-mmpp-sz09.apple.com>; Mon, 16 Sep 2019 11:36:56 -0700 (PDT) Sender: afish@apple.com MIME-version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: Re: [edk2-devel] [PATCH 09/11] OvmfPkg/XenBusDxe: Fix NotifyExitBoot to avoid Memory Allocation Services From: "Andrew Fish" In-reply-to: <26405443-8a65-5d03-dd35-1000ac3fbf0a@redhat.com> Date: Mon, 16 Sep 2019 11:36:48 -0700 Cc: Anthony Perard , Ard Biesheuvel , Julien Grall , Jordan Justen , xen-devel@lists.xenproject.org Message-id: References: <20190913145100.303433-1-anthony.perard@citrix.com> <20190913145100.303433-10-anthony.perard@citrix.com> <26405443-8a65-5d03-dd35-1000ac3fbf0a@redhat.com> To: devel@edk2.groups.io, lersek@redhat.com X-Mailer: Apple Mail (2.3445.104.11) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-09-16_07:,, signatures=0 Content-type: text/plain; charset=us-ascii Content-transfer-encoding: quoted-printable > On Sep 16, 2019, at 10:36 AM, Laszlo Ersek wrote: >=20 > On 09/13/19 16:50, Anthony PERARD wrote: >> This patch fix the EVT_SIGNAL_EXIT_BOOT_SERVICES handler to avoid >> using the Memory Allocation Services. >>=20 >> This comes with a new interface named RegisterExitCallback so that PV >> drivers can disconnect from the backend before XenBusDxe is teared >> down. >>=20 >> Instead of using Disconnect() to tear down the XenBus driver and the >> children drivers, we are going to ask every driver using >> XENBUS_PROTOCOL to disconnect from the hardware via the callback set >> with RegisterExitCallback, then reset the xenstore shared ring and >> the grant table. >=20 > I think this approach -- a lower-level bus driver calling out to > dependent device drivers -- is quite unusual. >=20 Laszlo, I agree given the timer event activity is stopped prior to calling any of = the EXIT_BOOT_SERVICES events there is usually not a requirement to call th= e drivers Stop() function. Generally Exit Boot Services events just turn of= f DMA, or any other hardware that could touch memory that is being freed ba= ck for OS usage. Since the timer activity, and thus all event activity is s= topped there is not a lot of ways for the drivers to ever have any EFI code= run again.=20 The only other exception I can think of is if the OS driver makes some kin= d of assumption about the state of the hardware. Thanks, Andrew Fish > How about the following instead: >=20 > - introduce two XenBusIo protocol member functions, AddReference() and > RemoveReference(). RemoveReference() should take a BOOLEAN called > "HandOffToOs". The device drivers should call AddReference() just before > exiting DriverBindingStart() with success, and RemoveReference(FALSE) in > DriverBindingStop(). >=20 > - these protocol member functions would increment / decrement a > reference counter in the underlying XenBus abstraction. Additionally, > RemoveReference() would store the HandOffToOs parameter to a bus-level > BOOLEAN too (regardless of previous value stored there -- a TRUE->FALSE > transition would never happen anyway; see below). >=20 > - both XenBusDxe and the Xen device drivers should register EBS > callbacks, per controller handle (in BindingStart()), and unregister > them (in BindingStop()) >=20 > - the ordering between EBS notification functions (queued at the same > TPL) is unspecified. In the device driver notification functions, the > last action should be a call to XenBusIo->RemoveReference(TRUE) -- after > the device-specific "forget me" actions have been done. >=20 > - if RemoveReference() gets a TRUE parameter, then it should check the > resultant (post-decrement) value of the refcount. If it has gone to > zero, RemoveReference() should re-set the xenbus / xenstore connection. > If the parameter is FALSE, it shouldn't do anything particular after > decrementing the refcount. >=20 > - in the XenBus EBS handler, if the refcount is positive at the time of > the call, nothing should be done. Otherwise, if HandOffToOs is TRUE, > nothing should be done, similarly. Otherwise, the xenbus/xenstore > connection should be re-set. >=20 > The idea is that normal Start/Stop should manage the refcount as > expected. At ExitBootServices(), the XenBus level handler should only > clear the connection to the hypervisor if no RemoveReference() call has > done, or will do, it. (If the counter is positive, then a later > RemoveReference() call will do it; if it's zero but HandOffToOs is TRUE, > then it's been done already. If the counter is zero and the BOOLEAN is > FALSE, then all devices have been disconnected normally with Stop() -- > or none have been connected at all --, before ExitBootServices(), so the > XenBus driver itself has to ask for being forgotten.) >=20 > Admittedly, this is more complicated (due to the unspecified ordering > between EBS notifications). I just feel it's more idiomatic to go > through normal protocol member functions in EBS notification functions, > rather than special callbacks. >=20 > (Side comment: the reference counting could normally be replaced by > gBS->OpenProtocolInformation(); however, that service itself allocates > memory, so we can't use it in EBS notification functions.) >=20 > Thanks > Laszlo >=20 >=20 >=20