<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p><br>
    </p>
    <br>
    <div class="moz-cite-prefix">On 05/04/2018 09:32 AM, Samuel
      Mendoza-Jonas wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:27fbdbe910bf6c2b1970caeb1e6fbc6fc976fb1b.camel@mendozajonas.com">
      <pre wrap="">On Wed, 2018-05-02 at 18:35 +0800, Ge Song wrote:
</pre>
      <blockquote type="cite">
        <pre wrap="">From: Ge Song <a class="moz-txt-link-rfc2396E" href="mailto:ge.song@hxt-semitech.com"><ge.song@hxt-semitech.com></a>

Provide methods to load/store petitboot's configuration on efi-based
platforms. A test case is also provided.

Signed-off-by: Ge Song <a class="moz-txt-link-rfc2396E" href="mailto:ge.song@hxt-semitech.com"><ge.song@hxt-semitech.com></a>
---
</pre>
      </blockquote>
      <pre wrap="">Hi Ge Song, thanks for the updates. In future revisions please add a line here
under the '---' describing what changed since the last version, it helps to
keep track of changes :)
</pre>
    </blockquote>
    <font face="Times New Roman, Times, serif">Hi Sam, sorry for the
      inconvenience. I have got little experience in submitting code to<br>
      open source community. Thanks for your instructions:) </font><br>
    <blockquote type="cite"
cite="mid:27fbdbe910bf6c2b1970caeb1e6fbc6fc976fb1b.camel@mendozajonas.com">
      <pre wrap="">
</pre>
      <blockquote type="cite">
        <pre wrap=""> lib/Makefile.am        |   2 +
 test/lib/Makefile.am   |   3 +-
 lib/efi/efivar.h       |  46 +++++
 lib/efi/efivar.c       | 179 ++++++++++++++++++++
 test/lib/test-efivar.c |  93 ++++++++++
 5 files changed, 322 insertions(+), 1 deletion(-)

diff --git a/lib/Makefile.am b/lib/Makefile.am
index bb7dfe489132..50fb9d14df49 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -58,6 +58,8 @@ lib_libpbcore_la_SOURCES = \
        lib/util/util.h \
        lib/flash/config.h \
        lib/flash/flash.h \
+       lib/efi/efivar.h \
+       lib/efi/efivar.c \
        $(gpg_int_SOURCES)
 
 if ENABLE_MTD
diff --git a/test/lib/Makefile.am b/test/lib/Makefile.am
index 9636b08d6a6b..22e3ac91499d 100644
--- a/test/lib/Makefile.am
+++ b/test/lib/Makefile.am
@@ -23,7 +23,8 @@ lib_TESTS = \
        test/lib/test-process-parent-stdout \
        test/lib/test-process-both \
        test/lib/test-process-stdout-eintr \
-       test/lib/test-fold
+       test/lib/test-fold \
+       test/lib/test-efivar
 
 $(lib_TESTS): LIBS += $(core_lib)
 
diff --git a/lib/efi/efivar.h b/lib/efi/efivar.h
new file mode 100644
index 000000000000..3dfec02d319d
--- /dev/null
+++ b/lib/efi/efivar.h
@@ -0,0 +1,46 @@
+/*
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ *  Copyright (C) 2018 Huaxintong Semiconductor Technology Co.,Ltd. All rights
+ *  reserved.
+ *  Author: Ge Song <a class="moz-txt-link-rfc2396E" href="mailto:ge.song@hxt-semitech.com"><ge.song@hxt-semitech.com></a>
+ */
+#ifndef EFIVAR_H
+#define EFIVAR_H
+
+#include <linux/magic.h>
+#include <stdint.h>
+
+#define EFI_VARIABLE_NON_VOLATILE                           0x00000001
+#define EFI_VARIABLE_BOOTSERVICE_ACCESS                     0x00000002
+#define EFI_VARIABLE_RUNTIME_ACCESS                         0x00000004
+#define EFI_VARIABLE_HARDWARE_ERROR_RECORD                  0x00000008
+#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS             0x00000010
+#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS  0x00000020
+#define EFI_VARIABLE_APPEND_WRITE                           0x00000040
+
+#ifndef EFIVARFS_MAGIC
+#define EFIVARFS_MAGIC 0xde5e81e4
+#endif
+
+static inline const char* get_efivarfs_path(void)
+{
+       return "/sys/firmware/efi/efivars/";
+}
+int efi_get_variable(void *ctx, const char *guidstr, const char *name,
+               uint8_t **data, size_t *data_size, uint32_t *attributes);
+int efi_set_variable(void *ctx, const char *guidstr, const char *name,
+               uint8_t *data, size_t data_size, uint32_t attributes);
+int efi_del_variable(void *ctx, const char *guidstr, const char *name);
+#endif /* EFIVAR_H */
diff --git a/lib/efi/efivar.c b/lib/efi/efivar.c
new file mode 100644
index 000000000000..5ce3464b9e77
--- /dev/null
+++ b/lib/efi/efivar.c
@@ -0,0 +1,179 @@
+/*
+ *  Methods to manipulation of EFI variables on UEFI-based platforms
+ *
+ *  The library relies on pesudo file system named "efivarfs". This is the
+ *  new interface to manipulate EFI varibles and was part of the linux
+ *  kernel v3.10 release.
+ *
+ *  There is also a legacy efivars interface exported via the path
+ *  "firmware/efi/vars/" relative to the mount point of sysfs. Since it has
+ *  some drawbacks and is deprecated, we don't support this interface.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ *  Copyright (C) 2018 Huaxintong Semiconductor Technology Co.,Ltd. All rights
+ *  reserved.
+ *  Author: Ge Song <a class="moz-txt-link-rfc2396E" href="mailto:ge.song@hxt-semitech.com"><ge.song@hxt-semitech.com></a>
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/fs.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "efivar.h"
+#include "talloc/talloc.h"
+
+int efi_del_variable(void *ctx, const char *guidstr,
+               const char *name)
+{
+       int fd, flag, errno_value;
+       int rc = -1;
+       char *path;
+
+       path = talloc_asprintf(ctx, "%s%s-%s",
+                               get_efivarfs_path(), name, guidstr);
+       if (!path)
+               return ENOMEM;
+
+       fd = open(path, O_RDONLY|O_NONBLOCK);
+       if (fd == -1)
+               goto err;
+
+       rc = ioctl(fd, FS_IOC_GETFLAGS, &flag);
+       if (rc == -1)
+               goto err;
+
+       flag &= ~FS_IMMUTABLE_FL;
+       rc = ioctl(fd, FS_IOC_SETFLAGS, &flag);
+       if (rc == -1)
+               goto err;
+
+       close(fd);
+       rc = unlink(path);
+
+err:
+       errno_value = errno;
+       if (fd > 0)
+               close(fd);
+
+       errno = errno_value;
+       return rc;
+}
+
+int efi_get_variable(void *ctx, const char *guidstr, const char *name,
+               uint8_t **data, size_t *data_size, uint32_t *attributes)
+{
+       int fd, errno_value;
+       int rc = -1;
+       void *p, *buf;
+       size_t bufsize = 4096;
+       size_t filesize = 0;
+       ssize_t sz;
+       char *path;
+
+       path = talloc_asprintf(ctx, "%s%s-%s",
+                               get_efivarfs_path(), name, guidstr);
+       if (!path)
+               return ENOMEM;
+
+       fd = open(path, O_RDONLY);
+       if (fd < 0)
+               goto err;
+
+       buf = talloc_size(ctx, bufsize);
+       if (!buf)
+               goto err;
+
+       do {
+               p = buf + filesize;
+               sz = read(fd, p, bufsize);
+               if (sz < 0 && errno == EAGAIN) {
+                       continue;
+               } else if (sz == 0) {
+                       break;
+               }
+               filesize += sz;
+       } while (1);
+
+       *attributes = *(uint32_t *)buf;
+       *data = (uint8_t *)(buf + sizeof(uint32_t));
+       *data_size = strlen(buf + sizeof(uint32_t));
+       rc = 0;
+
+err:
+       errno_value = errno;
+       if (fd > 0)
+               close(fd);
+
+       errno = errno_value;
+       return rc;
+}
+
+int efi_set_variable(void *ctx, const char *guidstr, const char *name,
+               uint8_t *data, size_t data_size, uint32_t attributes)
+{
+       int rc = -1, errno_value;
+       int fd = -1;
+       ssize_t len;
+       char *path;
+       void *buf;
+       size_t bufsize;
+       mode_t mask = umask(umask(0));
+
+       path = talloc_asprintf(ctx, "%s%s-%s",
+                               get_efivarfs_path(), name, guidstr);
+       if (!path)
+               return ENOMEM;
+
+       if (!access(path, F_OK)) {
+               rc = efi_del_variable(ctx, guidstr, name);
+               if (rc < 0) {
+                       goto err;
+               }
+       }
+
+       fd = open(path, O_CREAT|O_WRONLY, mask);
+       if (fd < 0)
+               goto err;
+
+       bufsize = sizeof(uint32_t) + data_size;
+       buf = talloc_size(ctx, bufsize);
+       if (!buf)
+               goto err;
+
+       *(uint32_t *)buf = attributes;
+       memcpy(buf + sizeof(uint32_t), data, data_size);
+
+       len = write(fd, buf, bufsize);
+       if ((size_t)len != bufsize)
+               goto err;
+       else
+               rc = 0;
+
+err:
+       errno_value = errno;
+       if (fd > 0)
+               close(fd);
+
+       errno = errno_value;
+       return rc;
+}
diff --git a/test/lib/test-efivar.c b/test/lib/test-efivar.c
new file mode 100644
index 000000000000..b952ee6b3018
--- /dev/null
+++ b/test/lib/test-efivar.c
@@ -0,0 +1,93 @@
+/*
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ *  Copyright (C) 2018 Huaxintong Semiconductor Technology Co.,Ltd. All rights
+ *  reserved.
+ *  Author: Ge Song <a class="moz-txt-link-rfc2396E" href="mailto:ge.song@hxt-semitech.com"><ge.song@hxt-semitech.com></a>
+ */
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/statfs.h>
+
+#include "efi/efivar.h"
+#include "talloc/talloc.h"
+
+#define DEF_ATTR       (EFI_VARIABLE_NON_VOLATILE | \
+       EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)
+
+static const char *test_efivar_guid = "c9c07add-256e-4452-b911-f8d0d35a1ac7";
+static const char *test_varname = "efivartest";
+static const char *test_data = "petitboot";
+
+static bool probe(void)
+{
+       const char *path = "/sys/firmware/efi/efivars/";
+       int rc = 0;
+       struct statfs buf;
+
+       if (access(path, F_OK))
+               return false;
+
+       memset(&buf, '\0', sizeof(buf));
+       rc = statfs(path, &buf);
+       if (rc)
+               return false;
+
+       typeof(buf.f_type) magic = EFIVARFS_MAGIC;
+       if (!memcmp(&buf.f_type, &magic, sizeof (magic)))
+               return true;
+
+       return false;
+}
+
+int main(void)
+{
+       void *ctx = NULL;
+       int rc, errno_value;
+       size_t size;
+       uint8_t *data = NULL;
+       uint32_t attr = DEF_ATTR;
+
+       if(!probe())
+               return EXIT_SUCCESS;
+
+       talloc_new(ctx);
+       size = strlen(test_data) + 1;
+       rc = efi_set_variable(ctx, test_efivar_guid, test_varname,
+                               (uint8_t *)test_data, size, attr);
+
+       rc = efi_get_variable(ctx, test_efivar_guid, test_varname,
+                               &data, &size, &attr);
+
+       rc = strcmp((char *)data, test_data);
</pre>
      </blockquote>
      <pre wrap="">This still segfaults:

        (gdb) frame 1
        #1  0x0000555555554f94 in main () at ../test/lib/test-efivar.c:80
        80              rc = strcmp((char *)data, test_data);

Most likely because efi_get_variable() is returning an error, likely since
efi_set_variable() is failing because /sys/firmware/efi/efivars is only
writable by root on my machine.
I could just run the test as root and it would probably pass, but I don't want
to be writing random efi variables to my (or anyone elses) laptops as part of a
Petitboot test :)
</pre>
    </blockquote>
    <font face="Times New Roman, Times, serif">Yes, operations under that
      folder require root permission. I forgot to mention that since
      usually<br>
      petitboot will be executed with the root permission , so sorry for
      it.<br>
      <br>
      In fact, the temporary variable set in NVRAM will be deleted in
      the end of the function, when<br>
      it returns normally, the variable won't be found in efivarfs
      anymore.<br>
      <br>
      To be precise, the delete operation causes one bit in variable's
      attribute field(in NVRAM) transition<br>
      from 0(valid) to 1(invalid), when this happens, both firmware and
      OS will ignore this variable and<br>
      no side effect will be introduced. <br>
      <br>
      The shortcoming is a little space is occupied by this temporary
      variable, although it has been deleted.<br>
      Don't worry about it, when the variable storage is full, a reclaim
      action will be taken by firmware.<br>
    </font>
    <blockquote type="cite"
cite="mid:27fbdbe910bf6c2b1970caeb1e6fbc6fc976fb1b.camel@mendozajonas.com">
      <pre wrap="">What would be better would be to have a 'fake' efivar directory that we set up
as part of the test, and write and read changes to that. Then we're just
testing that our writing and parsing of the efivarfs format works, not testing
the efivarfs implementation of whatever machine we're on.

For example we could have a test/lib/efivarfs directory with some dummy files
in there in the efivarfs format. Then if we change the efivarfs path Petitboot
uses the test can write to that without affecting the system. What do you think?
</pre>
    </blockquote>
    <font face="Times New Roman, Times, serif">Sounds like writing a
      simple userspace filesystem? I agree with you, that's really a
      good solution,<br>
      and it's a interesting work, maybe more time is needed, I have to
      study how to accomplish it first:) <br>
    </font>
    <blockquote type="cite"
cite="mid:27fbdbe910bf6c2b1970caeb1e6fbc6fc976fb1b.camel@mendozajonas.com">
      <pre wrap="">
Cheers,
Sam

</pre>
      <blockquote type="cite">
        <pre wrap="">+  if (rc) {
+               talloc_free(ctx);
+               assert(0);
+       }
+
+       rc = efi_del_variable(ctx, test_efivar_guid, test_varname);
+
+       rc = efi_get_variable(ctx, test_efivar_guid, test_varname,
+                               &data, &size, &attr);
+
+       errno_value = errno;
+       talloc_free(ctx);
+       assert(errno_value == ENOENT);
+
+       return EXIT_SUCCESS;
+}
</pre>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>