public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Anthony PERARD" <anthony.perard@citrix.com>
To: <devel@edk2.groups.io>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Julien Grall <julien.grall@arm.com>,
	Jordan Justen <jordan.l.justen@intel.com>,
	<xen-devel@lists.xenproject.org>,
	Anthony Perard <anthony.perard@citrix.com>,
	Laszlo Ersek <lersek@redhat.com>
Subject: [PATCH 05/11] OvmfPkg/XenBusDxe: Construct paths without allocation
Date: Fri, 13 Sep 2019 15:50:54 +0100	[thread overview]
Message-ID: <20190913145100.303433-6-anthony.perard@citrix.com> (raw)
In-Reply-To: <20190913145100.303433-1-anthony.perard@citrix.com>

When doing an action with a path and subpath in the xenstore,
XenStoreJoin is called to generate "$path/$subpath". But this function
do an allocation of memory which isn't necessary. Instead we will
construct the path with WRITE_REQUEST and data used to generate the
path will be copied directly to the xenstore shared ring.

Also change WRITE_REQUEST.Len type, it only contain sizes and doesn't
need to be exactly 32bits.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 OvmfPkg/XenBusDxe/XenStore.c | 78 +++++++++++++++++++++---------------
 1 file changed, 46 insertions(+), 32 deletions(-)

diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c
index 7b71dc156d..ca7be12d68 100644
--- a/OvmfPkg/XenBusDxe/XenStore.c
+++ b/OvmfPkg/XenBusDxe/XenStore.c
@@ -53,7 +53,7 @@
 
 typedef struct {
   CONST VOID  *Data;
-  UINT32      Len;
+  UINTN       Len;
 } WRITE_REQUEST;
 
 /* Register callback to watch subtree (node) in the XenStore. */
@@ -260,6 +260,35 @@ XenStoreFindWatch (
   return NULL;
 }
 
+/**
+  Fill the first three slots of a WRITE_REQUEST array.
+
+  When those three slots are concatenated to generate a string, the resulting
+  string will be "$Path\0" or "$Path/$SubPath\0" if SubPath is provided.
+**/
+STATIC
+VOID
+XenStorePrepareWriteRequest (
+  IN OUT WRITE_REQUEST *WriteRequest,
+  IN     CONST CHAR8   *Path,
+  IN     CONST CHAR8   *SubPath OPTIONAL
+  )
+{
+  SetMem(WriteRequest, 3 * sizeof (WRITE_REQUEST), 0);
+  WriteRequest[0].Data = Path;
+  WriteRequest[0].Len = AsciiStrSize (Path);
+  if (SubPath != NULL && SubPath[0] != '\0') {
+    //
+    // Remove the \0 from the first part of the request.
+    //
+    WriteRequest[0].Len--;
+    WriteRequest[1].Data = "/";
+    WriteRequest[1].Len = 1;
+    WriteRequest[2].Data = SubPath;
+    WriteRequest[2].Len = AsciiStrSize (SubPath);
+  }
+}
+
 //
 // Public Utility Functions
 // API comments for these methods can be found in XenStore.h
@@ -842,6 +871,7 @@ XenStoreTalkv (
   @param Transaction    The transaction to use for this request.
   @param RequestType    The type of message to send.
   @param Body           The body of the request.
+  @param SubPath        If !NULL and not "", "/$SubPath" is append to Body.
   @param LenPtr         The returned length of the reply.
   @param Result         The returned body of the reply.
 
@@ -854,16 +884,16 @@ XenStoreSingle (
   IN  CONST XENSTORE_TRANSACTION *Transaction,
   IN  enum xsd_sockmsg_type   RequestType,
   IN  CONST CHAR8             *Body,
+  IN  CONST CHAR8             *SubPath OPTIONAL,
   OUT UINT32                  *LenPtr OPTIONAL,
   OUT VOID                    **Result OPTIONAL
   )
 {
-  WRITE_REQUEST WriteRequest;
+  WRITE_REQUEST   WriteRequest[3];
 
-  WriteRequest.Data = (VOID *) Body;
-  WriteRequest.Len = (UINT32)AsciiStrSize (Body);
+  XenStorePrepareWriteRequest (WriteRequest, Body, SubPath);
 
-  return XenStoreTalkv (Transaction, RequestType, &WriteRequest, 1,
+  return XenStoreTalkv (Transaction, RequestType, WriteRequest, 3,
                         LenPtr, Result);
 }
 
@@ -1113,15 +1143,12 @@ XenStoreListDirectory (
   OUT CONST CHAR8           ***DirectoryListPtr
   )
 {
-  CHAR8 *Path;
   CHAR8 *TempStr;
   UINT32 Len = 0;
   XENSTORE_STATUS Status;
 
-  Path = XenStoreJoin (DirectoryPath, Node);
-  Status = XenStoreSingle (Transaction, XS_DIRECTORY, Path, &Len,
+  Status = XenStoreSingle (Transaction, XS_DIRECTORY, DirectoryPath, Node, &Len,
                            (VOID **) &TempStr);
-  FreePool (Path);
   if (Status != XENSTORE_STATUS_SUCCESS) {
     return Status;
   }
@@ -1160,13 +1187,11 @@ XenStoreRead (
   OUT VOID                    **Result
   )
 {
-  CHAR8 *Path;
   VOID *Value;
   XENSTORE_STATUS Status;
 
-  Path = XenStoreJoin (DirectoryPath, Node);
-  Status = XenStoreSingle (Transaction, XS_READ, Path, LenPtr, &Value);
-  FreePool (Path);
+  Status = XenStoreSingle (Transaction, XS_READ, DirectoryPath, Node,
+    LenPtr, &Value);
   if (Status != XENSTORE_STATUS_SUCCESS) {
     return Status;
   }
@@ -1183,21 +1208,13 @@ XenStoreWrite (
   IN CONST CHAR8           *Str
   )
 {
-  CHAR8 *Path;
-  WRITE_REQUEST WriteRequest[2];
-  XENSTORE_STATUS Status;
+  WRITE_REQUEST   WriteRequest[4];
 
-  Path = XenStoreJoin (DirectoryPath, Node);
+  XenStorePrepareWriteRequest (WriteRequest, DirectoryPath, Node);
+  WriteRequest[3].Data = Str;
+  WriteRequest[3].Len = AsciiStrLen (Str);
 
-  WriteRequest[0].Data = (VOID *) Path;
-  WriteRequest[0].Len = (UINT32)AsciiStrSize (Path);
-  WriteRequest[1].Data = (VOID *) Str;
-  WriteRequest[1].Len = (UINT32)AsciiStrLen (Str);
-
-  Status = XenStoreTalkv (Transaction, XS_WRITE, WriteRequest, 2, NULL, NULL);
-  FreePool (Path);
-
-  return Status;
+  return XenStoreTalkv (Transaction, XS_WRITE, WriteRequest, 4, NULL, NULL);
 }
 
 XENSTORE_STATUS
@@ -1207,12 +1224,9 @@ XenStoreRemove (
   IN CONST CHAR8            *Node
   )
 {
-  CHAR8 *Path;
   XENSTORE_STATUS Status;
 
-  Path = XenStoreJoin (DirectoryPath, Node);
-  Status = XenStoreSingle (Transaction, XS_RM, Path, NULL, NULL);
-  FreePool (Path);
+  Status = XenStoreSingle (Transaction, XS_RM, DirectoryPath, Node, NULL, NULL);
 
   return Status;
 }
@@ -1226,7 +1240,7 @@ XenStoreTransactionStart (
   XENSTORE_STATUS Status;
 
   Status = XenStoreSingle (XST_NIL, XS_TRANSACTION_START, "", NULL,
-                           (VOID **) &IdStr);
+    NULL, (VOID **) &IdStr);
   if (Status == XENSTORE_STATUS_SUCCESS) {
     Transaction->Id = (UINT32)AsciiStrDecimalToUintn (IdStr);
     FreePool (IdStr);
@@ -1246,7 +1260,7 @@ XenStoreTransactionEnd (
   AbortStr[0] = Abort ? 'F' : 'T';
   AbortStr[1] = '\0';
 
-  return XenStoreSingle (Transaction, XS_TRANSACTION_END, AbortStr, NULL, NULL);
+  return XenStoreSingle (Transaction, XS_TRANSACTION_END, AbortStr, NULL, NULL, NULL);
 }
 
 XENSTORE_STATUS
-- 
Anthony PERARD


  parent reply	other threads:[~2019-09-13 14:51 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-13 14:50 [PATCH 00/11] OvmfPkg/XenBusDxe: Fix ExitBootServices handler to avoid allocation Anthony PERARD
2019-09-13 14:50 ` [PATCH 01/11] OvmfPkg/XenBusDxe: Fix missing \n in DEBUG messages Anthony PERARD
2019-09-16 14:24   ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` [PATCH 02/11] OvmfPkg/XenBusDxe: Have XenStoreFindWatch take a pointer Anthony PERARD
2019-09-16 14:38   ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` [PATCH 03/11] OvmfPkg/XenBusDxe: Rework watch events reception Anthony PERARD
2019-09-16 14:39   ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` [PATCH 04/11] OvmfPkg/XenBusDxe: Avoid Allocate in XenStoreVSPrint Anthony PERARD
2019-09-16 14:45   ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` Anthony PERARD [this message]
2019-09-16 15:39   ` [edk2-devel] [PATCH 05/11] OvmfPkg/XenBusDxe: Construct paths without allocation Laszlo Ersek
2019-09-16 15:43     ` Laszlo Ersek
2019-09-13 14:50 ` [PATCH 06/11] OvmfPkg/XenBusDxe: Rework XenStoreProcessMessage to avoid allocating memory Anthony PERARD
2019-09-16 15:41   ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` [PATCH 07/11] OvmfPkg/XenBusDxe: Use on stack buffer in internal functions Anthony PERARD
2019-09-16 16:11   ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` [PATCH 08/11] OvmfPkg/XenBus: Change XENBUS_PROTOCOL to not return allocated memory Anthony PERARD
2019-09-16 16:16   ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` [PATCH 09/11] OvmfPkg/XenBusDxe: Fix NotifyExitBoot to avoid Memory Allocation Services Anthony PERARD
2019-09-16 17:36   ` [edk2-devel] " Laszlo Ersek
2019-09-16 18:36     ` Andrew Fish
2019-09-16 19:31       ` Laszlo Ersek
2019-09-16 20:50         ` Andrew Fish
2019-09-13 14:50 ` [PATCH 10/11] OvmfPkg/XenPvBlkDxe: Use XenBusIo->RegisterExitCallback Anthony PERARD
2019-09-13 14:51 ` [PATCH 11/11] OvmfPkg/XenBusDxe: Fix XenStoreWaitForEvent use during EBS Anthony PERARD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190913145100.303433-6-anthony.perard@citrix.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox