[PATCH v2 0/2] optionrom/pvh: use memcmp() to find the RSDP signature

[PATCH v2 0/2] optionrom/pvh: use memcmp() to find the RSDP signature

[Stefano Garzarella]

In this version I added a memcmp() implementation. In v1 I tried with
__builtin_memcmp() but CI failed with clang [1] and gcc 4.8.5.

v2:
- added patch 1 to provide a memcmp() implementation
- made 'rsdp_signature' static const [Phil]
- uses memcmp() instead of __builtin_memcmp()
- added pvh.bin binary with the changes applied

v1: https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg07425.html

Thanks,
Stefano

[1] https://gitlab.com/sgarzarella/qemu/-/jobs/1117036403#L385

Stefano Garzarella (2):
  optionrom: add memcmp() implementation
  optionrom/pvh: use memcmp() to find the RSDP signature

 pc-bios/optionrom/optrom.h   |  15 +++++++++++++++
 pc-bios/optionrom/pvh_main.c |  12 +++++++-----
 pc-bios/pvh.bin              | Bin 1536 -> 1536 bytes
 3 files changed, 22 insertions(+), 5 deletions(-)

-- 
2.30.2

[PATCH v2 1/2] optionrom: add memcmp() implementation

[Stefano Garzarella]

Provide memcmp() implementation that can be used by optionroms.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 pc-bios/optionrom/optrom.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/pc-bios/optionrom/optrom.h b/pc-bios/optionrom/optrom.h
index 357819259a..b98436e413 100644
--- a/pc-bios/optionrom/optrom.h
+++ b/pc-bios/optionrom/optrom.h
@@ -25,6 +25,7 @@
 #define OPTROM_H
 
 #include <stdint.h>
+#include <stddef.h>
 #include "../../include/standard-headers/linux/qemu_fw_cfg.h"
 
 #define barrier() asm("" : : : "memory")
@@ -107,4 +108,18 @@ static inline uint32_t be32_to_cpu(uint32_t x)
     return bswap32(x);
 }
 
+static inline int memcmp(const void *str1, const void *str2, size_t count)
+{
+    const unsigned char *c1 = str1, *c2 = str2;
+    int ret = 0;
+
+    while (count--) {
+        ret = *c1++ - *c2++;
+        if (ret) {
+            break;
+        }
+    }
+
+    return ret;
+}
 #endif /* OPTROM_H */
-- 
2.30.2

[PATCH v2 2/2] optionrom/pvh: use memcmp() to find the RSDP signature

[Stefano Garzarella]

New versions of gcc report a potential error and there may be alignment
issues using uint64_t pointer to check the RSDP signature:

    gcc 10.2.1 "cc (Alpine 10.2.1_pre2) 10.2.1 20210313" reports:

    pc-bios/optionrom/pvh_main.c: In function 'search_rsdp':
    pc-bios/optionrom/pvh_main.c:61:21: warning: comparison is always false
    due to limited range of data type [-Wtype-limits]
       61 |         if (*rsdp_p == RSDP_SIGNATURE) {
          |                     ^~

Let's use memcmp() to get more readable code and avoid these issues.

Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---

Notes:
    v2:
    - made 'rsdp_signature' static const [Phil]
    - uses memcmp() instead of __builtin_memcmp()
    - added pvh.bin binary with the changes applied

 pc-bios/optionrom/pvh_main.c |  12 +++++++-----
 pc-bios/pvh.bin              | Bin 1536 -> 1536 bytes
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/pc-bios/optionrom/pvh_main.c b/pc-bios/optionrom/pvh_main.c
index 28e79d7fc4..f654703bad 100644
--- a/pc-bios/optionrom/pvh_main.c
+++ b/pc-bios/optionrom/pvh_main.c
@@ -27,7 +27,8 @@ asm (".code32"); /* this code will be executed in protected mode */
 #include "optrom_fw_cfg.h"
 #include "../../include/hw/xen/start_info.h"
 
-#define RSDP_SIGNATURE          0x2052545020445352LL /* "RSD PTR " */
+#define RSDP_SIGNATURE          "RSD PTR "
+#define RSDP_SIGNATURE_SIZE     8
 #define RSDP_AREA_ADDR          0x000E0000
 #define RSDP_AREA_SIZE          0x00020000
 #define EBDA_BASE_ADDR          0x0000040E
@@ -53,12 +54,13 @@ static uint8_t cmdline_buffer[CMDLINE_BUFSIZE];
 /* Search RSDP signature. */
 static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr)
 {
-    uint64_t *rsdp_p;
+    static const char rsdp_signature[RSDP_SIGNATURE_SIZE] = RSDP_SIGNATURE;
+    char *rsdp_p;
 
     /* RSDP signature is always on a 16 byte boundary */
-    for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t *)end_addr;
-         rsdp_p += 2) {
-        if (*rsdp_p == RSDP_SIGNATURE) {
+    for (rsdp_p = (char *)start_addr; rsdp_p < (char *)end_addr;
+         rsdp_p += 16) {
+        if (memcmp(rsdp_p, rsdp_signature, RSDP_SIGNATURE_SIZE) == 0) {
             return (uintptr_t)rsdp_p;
         }
     }
diff --git a/pc-bios/pvh.bin b/pc-bios/pvh.bin
index 8033080ada2db4c4613fdc3bb5a69d79c7b0c0ca..778ca261447bed794ecb98bb29138a09568aca67 100644
GIT binary patch
delta 656
zcmZqRY2cYKnJIyF;?!q6oi`3X5NJNZ#ebrcWpX>CZ@rI-$Z^&MiVO@DNArx2vn~O$
z7#SEC7ACMVFf6_R<n71+u}%P4$63z+Nf6k<z`)1wny>kQfGq=q00V!^C7@uVMNqJd
zLg}l<3YLJ7AcfM0iw}S`9f4}v*C5Ei&}pJ_+(m^2WJvQvCjM5&&dJ-D>^OD+)pUyP
zQJnmTNsf)bbrDc{vJA6qJ;-6*J}MmlA2uJ6iH?hpJ=|LcbO*>_5ID{XGu-e%+Cn~{
zv7LtwK4NM9;lSUf^8f$;Zl>M|3_wxK3q_(YRT&u=dP~$8CIFpnd4a!K6v&6V`7O-N
zPYf@CUEL`PGWaFygMa`3r%ldhHmq*|@{hB&07($&c2N;o+yNHqfeP&asaU)LEVczI
zcARw$ki`hp@bVSVq~0xRAkAP$>;#3uE)fvdMMdC6?q8s5eN+T?i2_ABKfH(q@mW;5
zLsVE6g94`>6gmtH5MA65T_P_mK&nDiL|QME8bG83AW|GJWPwsICm}4IV0hr=R|Ge0
z0z^F*L_N=ow|~G65ZNUJ6ajjV=f!Q12#~QD<S_?I{GJ4iip?Kc_!uX#Dl@(T0NEhW
AtpET3

delta 702
zcmZqRY2cYKndyM&#Hr7C4n7v>yw!Y)i~m%o$mDiL-})F8k>jilq6`cbNArx2v$g<P
zj0_A63l9Jpi#Gs?9VbAn6+q%R>lz>l0y`KO_!!bAbPF_o3JP{n==NY~{1^}tq`-NR
zzeOFWz4?HEEdvK1L+Rb)tP7xej<YU-vJ4NTE#zZiU^w`I#kQUS$S8B~X6W4jVp$$5
z7kin;#K6$I4P+0{9Lr<;ZJ|H`EOwPXFub%FWPGP6$mrfOhJ6XFK%R-paTgV!VL<(l
znfP1HfezX$0F>Av0u<_emG*wOAdt5cq@s6n0h1EP46qq<L?_Q;l4B|jnY@ol)(jLR
z-99QD{~tCVk%^9rk3HO5qQ)=*Y#+iaFKzz+|KGbs4P+TeesVmsVSNHn>v7f;APEB9
zE-E66Gr&SQP@x?l6^kc;#il^Tj<favS)ibQX@P3y1h7kYfgIoKq9X7j_V54yK$i*Z
z0_p7h@WL0wXHn@6QDIpO@=1NC=o}yg(ZvnXCGtW8q$)&3r1ersCPa!0BE|E97h<5u
zE+L=@(78M>e*Xa~1~L|t;}tic=V5+<*dze4iQ~l*pt&#0{~>}55^@s^51_>4%RN9B
MZNA7N&B#~{07`n#VE_OC

-- 
2.30.2