From nobody Sat Feb 7 06:21:28 2026 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zoho.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+47216+1787277+3901457@groups.io ARC-Seal: i=1; a=rsa-sha256; t=1568386270; cv=none; d=zoho.com; s=zohoarc; b=J0eFXrJc3Pe4rk73jc4boe6Qr6NqUkdmiLYqJtsuTeBQN/7funWEPJDhNse6WJlbEsXKZwPPmFJxwG9sQ7IJpXH6isd+3m9rYYqNNqNb/dUI9pBA89hQF2Ep4fEbbvy0Fw6M1jY22gDJ4Owqyq3eSyyC8rswb5E3enA45J2jqj4= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1568386270; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To:ARC-Authentication-Results; bh=+BAoHXea8lZvNbVJajLun2fwHKwbKtalLfHvTB/X45Y=; b=JufYCv7Y7+XgChOt4wMD17BEHWPyVu5Vfr6N9ZVCPRlPcrJZYWeGorgiUZj+dzLYtUjsmDSlTccFx1bCOvXebJNKQi+fcHNcDzhdmlJ3kun/8zshm3g/eVqmIkiuHQmcbpW2YoVTGTgvV6yc1FsWBxZmF83N5W43r/UnDzZWak8= ARC-Authentication-Results: i=1; mx.zoho.com; dkim=pass; spf=pass (zoho.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+47216+1787277+3901457@groups.io Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1568386270811505.5460206815711; Fri, 13 Sep 2019 07:51:10 -0700 (PDT) Return-Path: X-Received: from esa1.hc3370-68.iphmx.com (esa1.hc3370-68.iphmx.com []) by groups.io with SMTP; Fri, 13 Sep 2019 07:51:09 -0700 Received-SPF: pass (zoho.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+47216+1787277+3901457@groups.io; helo=web01.groups.io; Received-SPF: None (esa1.hc3370-68.iphmx.com: no sender authenticity information available from domain of anthony.perard@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="anthony.perard@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa1.hc3370-68.iphmx.com: domain of anthony.perard@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="anthony.perard@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ~all" Received-SPF: None (esa1.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: 438aV6qfDMfFLKwz7+b256cgHR6jsDeUAT3aYMe98K54x5Kw+DD+JjwzgHc0JvX/ZDk39Y6COc tuIPX1b4FHKirXWaUEVlBDTuKgbOHruuqK6Cl3NaBYCr2+zodcDGmX9FnAeQQ2nwExGXQfyMot CzFPsAZMaVTYa2pR9Sv7OypMAoRznJ/lGIv3gounik3jQz8Tenu/xmIM+hAfoI4ayUDL+we9Tc in9zbAQitbvc/j2aq7/QvZJg0wYQe3E+mvgn+pPQUY3gFzlqv1/SgSwFpnijMNa4Ct6h9f2lA6 vBk= X-SBRS: 2.7 X-MesageID: 5595146 X-Ironport-Server: esa1.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.64,501,1559534400"; d="scan'208";a="5595146" From: "Anthony PERARD" To: CC: Ard Biesheuvel , Julien Grall , Jordan Justen , , Anthony Perard , Laszlo Ersek Subject: [edk2-devel] [PATCH 06/11] OvmfPkg/XenBusDxe: Rework XenStoreProcessMessage to avoid allocating memory Date: Fri, 13 Sep 2019 15:50:55 +0100 Message-ID: <20190913145100.303433-7-anthony.perard@citrix.com> In-Reply-To: <20190913145100.303433-1-anthony.perard@citrix.com> References: <20190913145100.303433-1-anthony.perard@citrix.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,anthony.perard@citrix.com Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1568386270; bh=+BAoHXea8lZvNbVJajLun2fwHKwbKtalLfHvTB/X45Y=; h=CC:Content-Type:Date:From:Reply-To:Subject:To; b=LNy9zAR+NNvpd/SmiOPlSq2knYPumVJ44SBrQ5yzad5PYlitJEF27By/lUg/kF5anF0 IeLC3aZ/siYU3qgO0s+nO1//3hu05Us4fxTqPtRoDFoEo7IKWlx0xTgHyHIc5fRg2Av3q jHWxyEDPGNxpWuxTdblKPl7hEhtDDhHEKmE= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" This patch rework XenStoreProcessMessage in order to avoid memory allocation when a reply is expected. Instead of allocating a buffer for this reply, we are going to copy to a buffer passed by the caller. For messages that aren't fully received, they will be stored in a buffer that have been allocated at the initialisation of the driver. A temporary memory allocation is made in XenStoreTalkv but that will be removed in a further patch. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2190 Signed-off-by: Anthony PERARD Acked-by: Laszlo Ersek --- OvmfPkg/XenBusDxe/XenStore.c | 297 +++++++++++++++-------------------- 1 file changed, 130 insertions(+), 167 deletions(-) diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c index ca7be12d68..004d3b6022 100644 --- a/OvmfPkg/XenBusDxe/XenStore.c +++ b/OvmfPkg/XenBusDxe/XenStore.c @@ -72,27 +72,6 @@ struct _XENSTORE_WATCH #define XENSTORE_WATCH_FROM_LINK(l) \ CR (l, XENSTORE_WATCH, Link, XENSTORE_WATCH_SIGNATURE) =20 - -/** - * Structure capturing messages received from the XenStore service. - */ -#define XENSTORE_MESSAGE_SIGNATURE SIGNATURE_32 ('X', 'S', 's', 'm') -typedef struct { - UINT32 Signature; - LIST_ENTRY Link; - - struct xsd_sockmsg Header; - - union { - /* Queued replies. */ - struct { - CHAR8 *Body; - } Reply; - } u; -} XENSTORE_MESSAGE; -#define XENSTORE_MESSAGE_FROM_LINK(r) \ - CR (r, XENSTORE_MESSAGE, Link, XENSTORE_MESSAGE_SIGNATURE) - /** * Container for all XenStore related state. */ @@ -105,21 +84,6 @@ typedef struct { =20 XENBUS_DEVICE *Dev; =20 - /** - * A list of replies to our requests. - * - * The reply list is filled by xs_rcv_thread(). It - * is consumed by the context that issued the request - * to which a reply is made. The requester blocks in - * XenStoreReadReply (). - * - * /note Only one requesting context can be active at a time. - */ - LIST_ENTRY ReplyList; - - /** Lock protecting the reply list. */ - EFI_LOCK ReplyLock; - /** * List of registered watches. */ @@ -136,6 +100,13 @@ typedef struct { =20 /** Handle for XenStore events. */ EFI_EVENT EventChannelEvent; + + /** Buffer used to copy payloads from the xenstore ring */ + // The + 1 is to allow to have a \0. + CHAR8 Buffer[XENSTORE_PAYLOAD_MAX + 1]; + + /** ID used when sending messages to xenstored */ + UINTN NextRequestId; } XENSTORE_PRIVATE; =20 // @@ -148,6 +119,12 @@ static XENSTORE_PRIVATE xs; // Private Utility Functions // =20 +STATIC +XENSTORE_STATUS +XenStoreGetError ( + CONST CHAR8 *ErrorStr + ); + /** Count and optionally record pointers to a number of NUL terminated strings in a buffer. @@ -613,70 +590,106 @@ XenStoreReadStore ( Block reading the next message from the XenStore service and process the result. =20 + @param ExpectedRequestId Block until a reply to with this ID is see= n. + @param ExpectedTransactionId Idem, but should also match this ID. + @param BufferSize IN: size of the buffer + OUT: The returned length of the reply. + @param Buffer The returned body of the reply. + @return XENSTORE_STATUS_SUCCESS on success. Otherwise an errno value indicating the type of failure encountered. **/ STATIC XENSTORE_STATUS XenStoreProcessMessage ( - VOID + IN UINT32 ExpectedRequestId, + IN UINT32 ExpectedTransactionId, + IN OUT UINTN *BufferSize OPTIONAL, + IN OUT CHAR8 *Buffer OPTIONAL ) { - XENSTORE_MESSAGE *Message; - CHAR8 *Body; - XENSTORE_STATUS Status; - - Message =3D AllocateZeroPool (sizeof (XENSTORE_MESSAGE)); - Message->Signature =3D XENSTORE_MESSAGE_SIGNATURE; - Status =3D XenStoreReadStore (&Message->Header, sizeof (Message->Header)= ); - if (Status !=3D XENSTORE_STATUS_SUCCESS) { - FreePool (Message); - DEBUG ((EFI_D_ERROR, "XenStore: Error read store (%d)\n", Status)); - return Status; - } - - Body =3D AllocatePool (Message->Header.len + 1); - Status =3D XenStoreReadStore (Body, Message->Header.len); - if (Status !=3D XENSTORE_STATUS_SUCCESS) { - FreePool (Body); - FreePool (Message); - DEBUG ((EFI_D_ERROR, "XenStore: Error read store (%d)\n", Status)); - return Status; - } - Body[Message->Header.len] =3D '\0'; + struct xsd_sockmsg Header; + CHAR8 *Payload; + XENSTORE_STATUS Status; =20 - if (Message->Header.type =3D=3D XS_WATCH_EVENT) { - CONST CHAR8 *WatchEventPath; - CONST CHAR8 *WatchEventToken; - VOID *ConvertedToken; - XENSTORE_WATCH *Watch; + while (TRUE) { =20 - // - // Parse WATCH_EVENT messages - // \0\0 - // - WatchEventPath =3D Body; - WatchEventToken =3D WatchEventPath + AsciiStrSize (WatchEventPath); + Status =3D XenStoreReadStore (&Header, sizeof (Header)); + if (Status !=3D XENSTORE_STATUS_SUCCESS) { + DEBUG ((DEBUG_ERROR, "XenStore: Error read store (%d)\n", Status)); + return Status; + } =20 - ConvertedToken =3D (VOID *) AsciiStrHexToUintn (WatchEventToken); + ASSERT (Header.len <=3D XENSTORE_PAYLOAD_MAX); + if (Header.len > XENSTORE_PAYLOAD_MAX) { + DEBUG ((DEBUG_ERROR, "XenStore: Message payload over %d (is %d)\n", + XENSTORE_PAYLOAD_MAX, Header.len)); + Header.len =3D XENSTORE_PAYLOAD_MAX; + } =20 - EfiAcquireLock (&xs.RegisteredWatchesLock); - Watch =3D XenStoreFindWatch (ConvertedToken); - DEBUG ((DEBUG_INFO, "XenStore: Watch event %a\n", WatchEventToken)); - if (Watch !=3D NULL) { - Watch->Triggered =3D TRUE; - } else { - DEBUG ((EFI_D_WARN, "XenStore: Watch handle %a not found\n", - WatchEventToken)); + Payload =3D xs.Buffer; + Status =3D XenStoreReadStore (Payload, Header.len); + if (Status !=3D XENSTORE_STATUS_SUCCESS) { + DEBUG ((DEBUG_ERROR, "XenStore: Error read store (%d)\n", Status)); + return Status; } - EfiReleaseLock (&xs.RegisteredWatchesLock); - FreePool (Message); - FreePool (Body); - } else { - Message->u.Reply.Body =3D Body; - EfiAcquireLock (&xs.ReplyLock); - InsertTailList (&xs.ReplyList, &Message->Link); - EfiReleaseLock (&xs.ReplyLock); + Payload[Header.len] =3D '\0'; + + if (Header.type =3D=3D XS_WATCH_EVENT) { + CONST CHAR8 *WatchEventPath; + CONST CHAR8 *WatchEventToken; + VOID *ConvertedToken; + XENSTORE_WATCH *Watch; + + // + // Parse WATCH_EVENT messages + // \0\0 + // + WatchEventPath =3D Payload; + WatchEventToken =3D WatchEventPath + AsciiStrSize (WatchEventPath); + + ConvertedToken =3D (VOID *) AsciiStrHexToUintn (WatchEventToken); + + EfiAcquireLock (&xs.RegisteredWatchesLock); + Watch =3D XenStoreFindWatch (ConvertedToken); + DEBUG ((DEBUG_INFO, "XenStore: Watch event %a\n", WatchEventToken)); + if (Watch !=3D NULL) { + Watch->Triggered =3D TRUE; + } else { + DEBUG ((DEBUG_WARN, "XenStore: Watch handle %a not found\n", + WatchEventToken)); + } + EfiReleaseLock (&xs.RegisteredWatchesLock); + + if (Header.req_id =3D=3D ExpectedRequestId + && Header.tx_id =3D=3D ExpectedTransactionId + && Buffer =3D=3D NULL) { + // + // We were waiting for a watch event + // + return XENSTORE_STATUS_SUCCESS; + } + } else if (Header.req_id =3D=3D ExpectedRequestId + && Header.tx_id =3D=3D ExpectedTransactionId) { + Status =3D XENSTORE_STATUS_SUCCESS; + if (Header.type =3D=3D XS_ERROR) { + Status =3D XenStoreGetError (Payload); + } else if (Buffer !=3D NULL) { + ASSERT (BufferSize !=3D NULL); + ASSERT (*BufferSize >=3D Header.len); + CopyMem (Buffer, Payload, MIN (Header.len + 1, *BufferSize)); + *BufferSize =3D Header.len; + } else { + // + // Payload should be "OK" if the function sending a request doesn't + // expect a reply. + // + ASSERT (Header.len =3D=3D 3); + ASSERT (AsciiStrCmp (Payload, "OK") =3D=3D 0); + } + return Status; + } + } =20 return XENSTORE_STATUS_SUCCESS; @@ -736,51 +749,6 @@ XenStoreGetError ( return XENSTORE_STATUS_EINVAL; } =20 -/** - Block waiting for a reply to a message request. - - @param TypePtr The returned type of the reply. - @param LenPtr The returned body length of the reply. - @param Result The returned body of the reply. -**/ -STATIC -XENSTORE_STATUS -XenStoreReadReply ( - OUT enum xsd_sockmsg_type *TypePtr, - OUT UINT32 *LenPtr OPTIONAL, - OUT VOID **Result - ) -{ - XENSTORE_MESSAGE *Message; - LIST_ENTRY *Entry; - CHAR8 *Body; - - while (IsListEmpty (&xs.ReplyList)) { - XENSTORE_STATUS Status; - Status =3D XenStoreProcessMessage (); - if (Status !=3D XENSTORE_STATUS_SUCCESS && Status !=3D XENSTORE_STATUS= _EAGAIN) { - DEBUG ((DEBUG_ERROR, "XenStore, error while reading the ring (%d).\n= ", - Status)); - return Status; - } - } - EfiAcquireLock (&xs.ReplyLock); - Entry =3D GetFirstNode (&xs.ReplyList); - Message =3D XENSTORE_MESSAGE_FROM_LINK (Entry); - RemoveEntryList (Entry); - EfiReleaseLock (&xs.ReplyLock); - - *TypePtr =3D Message->Header.type; - if (LenPtr !=3D NULL) { - *LenPtr =3D Message->Header.len; - } - Body =3D Message->u.Reply.Body; - - FreePool (Message); - *Result =3D Body; - return XENSTORE_STATUS_SUCCESS; -} - /** Send a message with an optionally muti-part body to the XenStore service. =20 @@ -806,16 +774,17 @@ XenStoreTalkv ( ) { struct xsd_sockmsg Message; - void *Return =3D NULL; - UINT32 Index; - XENSTORE_STATUS Status; + UINTN Index; + XENSTORE_STATUS Status; + VOID *Buffer; + UINTN BufferSize; =20 if (Transaction =3D=3D XST_NIL) { Message.tx_id =3D 0; } else { Message.tx_id =3D Transaction->Id; } - Message.req_id =3D 0; + Message.req_id =3D xs.NextRequestId++; Message.type =3D RequestType; Message.len =3D 0; for (Index =3D 0; Index < NumRequests; Index++) { @@ -836,29 +805,36 @@ XenStoreTalkv ( } } =20 - Status =3D XenStoreReadReply ((enum xsd_sockmsg_type *)&Message.type, Le= nPtr, &Return); - -Error: - if (Status !=3D XENSTORE_STATUS_SUCCESS) { - return Status; + if (ResultPtr) { + Buffer =3D AllocatePool (XENSTORE_PAYLOAD_MAX + 1); + BufferSize =3D XENSTORE_PAYLOAD_MAX; + } else { + Buffer =3D NULL; + BufferSize =3D 0; } =20 - if (Message.type =3D=3D XS_ERROR) { - Status =3D XenStoreGetError (Return); - FreePool (Return); + // + // Wait for a reply to our request + // + Status =3D XenStoreProcessMessage (Message.req_id, Message.tx_id, + &BufferSize, Buffer); + + if (Status !=3D XENSTORE_STATUS_SUCCESS) { + DEBUG ((DEBUG_ERROR, "XenStore, error while reading the ring (%d).\n", + Status)); + FreePool (Buffer); return Status; } =20 - /* Reply is either error or an echo of our request message type. */ - ASSERT ((enum xsd_sockmsg_type)Message.type =3D=3D RequestType); - if (ResultPtr) { - *ResultPtr =3D Return; - } else { - FreePool (Return); + *ResultPtr =3D Buffer; + if (LenPtr) { + *LenPtr =3D BufferSize; + } } =20 - return XENSTORE_STATUS_SUCCESS; +Error: + return Status; } =20 /** @@ -975,7 +951,7 @@ XenStoreWaitWatch ( return XENSTORE_STATUS_SUCCESS; } =20 - Status =3D XenStoreProcessMessage (); + Status =3D XenStoreProcessMessage (0, 0, NULL, NULL); if (Status !=3D XENSTORE_STATUS_SUCCESS && Status !=3D XENSTORE_STATUS= _EAGAIN) { return Status; } @@ -1060,12 +1036,12 @@ XenStoreInit ( DEBUG ((EFI_D_INFO, "XenBusInit: XenBus rings @%p, event channel %x\n", xs.XenStore, xs.EventChannel)); =20 - InitializeListHead (&xs.ReplyList); InitializeListHead (&xs.RegisteredWatches); =20 - EfiInitializeLock (&xs.ReplyLock, TPL_NOTIFY); EfiInitializeLock (&xs.RegisteredWatchesLock, TPL_NOTIFY); =20 + xs.NextRequestId =3D 1; + /* Initialize the shared memory rings to talk to xenstored */ Status =3D XenStoreInitComms (&xs); =20 @@ -1095,19 +1071,6 @@ XenStoreDeinit ( } } =20 - if (!IsListEmpty (&xs.ReplyList)) { - XENSTORE_MESSAGE *Message; - LIST_ENTRY *Entry; - Entry =3D GetFirstNode (&xs.ReplyList); - while (!IsNull (&xs.ReplyList, Entry)) { - Message =3D XENSTORE_MESSAGE_FROM_LINK (Entry); - Entry =3D GetNextNode (&xs.ReplyList, Entry); - RemoveEntryList (&Message->Link); - FreePool (Message->u.Reply.Body); - FreePool (Message); - } - } - gBS->CloseEvent (xs.EventChannelEvent); =20 if (xs.XenStore->server_features & XENSTORE_SERVER_FEATURE_RECONNECTION)= { --=20 Anthony PERARD -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47216): https://edk2.groups.io/g/devel/message/47216 Mute This Topic: https://groups.io/mt/34128017/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-