<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 8, 2017, at 4:30 AM, Laszlo Ersek <<a href="mailto:lersek@redhat.com" class="">lersek@redhat.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On 02/05/17 18:09,<span class="Apple-converted-space"> </span></span><a href="mailto:ben@skyportsystems.com" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">ben@skyportsystems.com</a><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class=""><span class="Apple-converted-space"> </span>wrote:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">From: Ben Warren <<a href="mailto:ben@skyportsystems.com" class="">ben@skyportsystems.com</a>><br class=""><br class="">This command is similar to ADD_POINTER, but instead of patching<br class="">memory, it writes the pointer back to QEMU over the DMA interface.<br class=""><br class="">Signed-off-by: Ben Warren <<a href="mailto:ben@skyportsystems.com" class="">ben@skyportsystems.com</a>><br class="">---<br class="">src/fw/romfile_loader.c | 37 +++++++++++++++++++++++++++++++++++++<br class="">src/fw/romfile_loader.h | 16 ++++++++++------<br class="">2 files changed, 47 insertions(+), 6 deletions(-)<br class=""><br class="">diff --git a/src/fw/romfile_loader.c b/src/fw/romfile_loader.c<br class="">index f4b17ff..d0ae42b 100644<br class="">--- a/src/fw/romfile_loader.c<br class="">+++ b/src/fw/romfile_loader.c<br class="">@@ -5,6 +5,7 @@<br class="">#include "romfile.h" // struct romfile_s<br class="">#include "malloc.h" // Zone*, _malloc<br class="">#include "output.h" // warn_*<br class="">+#include "paravirt.h" // qemu_cfg_write_file<br class=""><br class="">struct romfile_loader_file {<br class="">    struct romfile_s *file;<br class="">@@ -98,7 +99,39 @@ static void romfile_loader_add_pointer(struct romfile_loader_entry_s *entry,<br class="">    pointer += (unsigned long)src_file->data;<br class="">    pointer = cpu_to_le64(pointer);<br class="">    memcpy(dest_file->data + offset, &pointer, entry->pointer_size);<br class="">+    return;<br class="">+err:<br class="">+    warn_internalerror();<br class="">+}<br class="">+<br class="">+static void romfile_loader_write_pointer(struct romfile_loader_entry_s *entry,<br class="">+                                       struct romfile_loader_files *files)<br class="">+{<br class="">+    struct romfile_s *dest_file;<br class="">+    struct romfile_loader_file *src_file;<br class="">+    unsigned offset = le32_to_cpu(entry->pointer_offset);<br class="">+    u64 pointer = 0;<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">There's no need to zero-init "pointer" here. It doesn't hurt of course;</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">I just like to avoid initialization if it is useless.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">+<br class="">+    /* Writing back to a file that may not be loaded in RAM */<br class="">+    dest_file = romfile_find(entry->pointer_dest_file);<br class="">+    src_file = romfile_loader_find(entry->pointer_src_file, files);<br class=""><br class="">+    if (!dest_file || !src_file || !src_file->data ||<br class="">+        offset + entry->pointer_size < offset ||<br class="">+        offset + entry->pointer_size > dest_file->size ||<br class="">+        entry->pointer_size < 1 || entry->pointer_size > 8 ||<br class="">+        entry->pointer_size & (entry->pointer_size - 1)) {<br class="">+        goto err;<br class="">+    }<br class="">+<br class="">+    pointer = (unsigned long)src_file->data;<br class="">+    pointer = cpu_to_le64(pointer);<br class="">+<br class="">+    /* Only supported on QEMU */<br class="">+    if (qemu_cfg_write_file(&pointer, dest_file, offset,<br class="">+                            entry->pointer_size) != entry->pointer_size) {<br class="">+        goto err;<br class="">+    }<br class="">    return;<br class="">err:<br class="">    warn_internalerror();<br class="">@@ -161,6 +194,10 @@ int romfile_loader_execute(const char *name)<br class="">                        break;<br class="">                case ROMFILE_LOADER_COMMAND_ADD_CHECKSUM:<br class="">                        romfile_loader_add_checksum(entry, files);<br class="">+                        break;<br class="">+                case ROMFILE_LOADER_COMMAND_WRITE_POINTER:<br class="">+                        romfile_loader_write_pointer(entry, files);<br class="">+                        break;<br class="">                default:<br class="">                        /* Skip commands that we don't recognize. */<br class="">                        break;<br class="">diff --git a/src/fw/romfile_loader.h b/src/fw/romfile_loader.h<br class="">index 15eab2a..0c0782c 100644<br class="">--- a/src/fw/romfile_loader.h<br class="">+++ b/src/fw/romfile_loader.h<br class="">@@ -25,10 +25,13 @@ struct romfile_loader_entry_s {<br class="">        };<br class=""><br class="">        /*<br class="">-         * COMMAND_ADD_POINTER - patch the table (originating from<br class="">-         * @dest_file) at @pointer_offset, by adding a pointer to the table<br class="">+         * COMMAND_ADD_POINTER &<br class="">+         * COMMAND_WRITE_POINTER - patch memory (originating from<br class="">+         * @dest_file) at @pointer.offset, by adding a pointer to the memory<br class="">         * originating from @src_file. 1,2,4 or 8 byte unsigned<br class="">-         * addition is used depending on @pointer_size.<br class="">+         * addition is used depending on @pointer.size.<br class="">+         * Instead of patching memory, COMMAND_WRITE_POINTER writes the changes<br class="">+         * to @dest_file back to the host via DMA<br class="">         */<br class="">        struct {<br class="">            char pointer_dest_file[ROMFILE_LOADER_FILESZ];<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">In general I think the patch is good, but (similarly to the QEMU case) I</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">quite dislike this kind of repurposing.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">At the least, please leave the original comment block for</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">COMMAND_ADD_POINTER intact, and add a separate, standalone comment block</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">for COMMAND_WRITE_POINTER.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Thanks!</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Laszlo</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote>OK, sometimes I take minimalism a bit far :)<br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">@@ -57,9 +60,10 @@ struct romfile_loader_entry_s {<br class="">};<br class=""><br class="">enum {<br class="">-    ROMFILE_LOADER_COMMAND_ALLOCATE     = 0x1,<br class="">-    ROMFILE_LOADER_COMMAND_ADD_POINTER  = 0x2,<br class="">-    ROMFILE_LOADER_COMMAND_ADD_CHECKSUM = 0x3,<br class="">+    ROMFILE_LOADER_COMMAND_ALLOCATE          = 0x1,<br class="">+    ROMFILE_LOADER_COMMAND_ADD_POINTER       = 0x2,<br class="">+    ROMFILE_LOADER_COMMAND_ADD_CHECKSUM      = 0x3,<br class="">+    ROMFILE_LOADER_COMMAND_WRITE_POINTER     = 0x4,<br class="">};<br class=""><br class="">enum {</blockquote></div></blockquote></div><br class=""></body></html>