[PATCH v2 0/4] hw/ppc & net: Simplify LD/ST API uses

[PATCH v2 0/4] hw/ppc & net: Simplify LD/ST API uses

[Philippe Mathieu-Daudé]

Use ldN / stN methods to access variable lengths.

v2:
- Include unstaged change in vof.c

Philippe Mathieu-Daudé (4):
  hw/ppc/spapr_nvdimm: Simplify LD/ST API uses
  hw/ppc/spapr_vof: Simplify LD/ST API uses
  hw/ppc/vof: Simplify LD/ST API uses
  net/l2tpv3: Simplify LD/ST API uses

 hw/ppc/spapr_nvdimm.c | 47 ++++---------------------------------------
 hw/ppc/spapr_vof.c    | 27 +++++++++----------------
 hw/ppc/vof.c          | 26 +++++++++---------------
 net/l2tpv3.c          |  6 +-----
 4 files changed, 23 insertions(+), 83 deletions(-)

-- 
2.45.2

[PATCH v2 1/4] hw/ppc/spapr_nvdimm: Simplify LD/ST API uses

[Philippe Mathieu-Daudé]

ldn/stn methods handle the access size, no need for the switch case.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/spapr_nvdimm.c | 47 ++++---------------------------------------
 1 file changed, 4 insertions(+), 43 deletions(-)

diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index 7d2dfe5e3d..5af0b13370 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -250,7 +250,6 @@ static target_ulong h_scm_read_metadata(PowerPCCPU *cpu,
     SpaprDrc *drc = spapr_drc_by_index(drc_index);
     NVDIMMDevice *nvdimm;
     NVDIMMClass *ddc;
-    uint64_t data = 0;
     uint8_t buf[8] = { 0 };
 
     if (!drc || !drc->dev ||
@@ -272,24 +271,7 @@ static target_ulong h_scm_read_metadata(PowerPCCPU *cpu,
     ddc = NVDIMM_GET_CLASS(nvdimm);
     ddc->read_label_data(nvdimm, buf, len, offset);
 
-    switch (len) {
-    case 1:
-        data = ldub_p(buf);
-        break;
-    case 2:
-        data = lduw_be_p(buf);
-        break;
-    case 4:
-        data = ldl_be_p(buf);
-        break;
-    case 8:
-        data = ldq_be_p(buf);
-        break;
-    default:
-        g_assert_not_reached();
-    }
-
-    args[0] = data;
+    args[0] = ldn_be_p(buf, len);
 
     return H_SUCCESS;
 }
@@ -325,31 +307,10 @@ static target_ulong h_scm_write_metadata(PowerPCCPU *cpu,
         return H_P2;
     }
 
-    switch (len) {
-    case 1:
-        if (data & 0xffffffffffffff00) {
-            return H_P2;
-        }
-        stb_p(buf, data);
-        break;
-    case 2:
-        if (data & 0xffffffffffff0000) {
-            return H_P2;
-        }
-        stw_be_p(buf, data);
-        break;
-    case 4:
-        if (data & 0xffffffff00000000) {
-            return H_P2;
-        }
-        stl_be_p(buf, data);
-        break;
-    case 8:
-        stq_be_p(buf, data);
-        break;
-    default:
-            g_assert_not_reached();
+    if (len < 8 && extract64(data, 8 * len, 64 - 8 * len)) {
+        return H_P2;
     }
+    stn_be_p(buf, data, len);
 
     ddc = NVDIMM_GET_CLASS(nvdimm);
     ddc->write_label_data(nvdimm, buf, len, offset);
-- 
2.45.2

Re: [PATCH v2 1/4] hw/ppc/spapr_nvdimm: Simplify LD/ST API uses

[Richard Henderson]

On 9/27/24 14:50, Philippe Mathieu-Daudé wrote:
> ldn/stn methods handle the access size, no need for the switch case.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/ppc/spapr_nvdimm.c | 47 ++++---------------------------------------
>   1 file changed, 4 insertions(+), 43 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v2 2/4] hw/ppc/spapr_vof: Simplify LD/ST API uses

[Philippe Mathieu-Daudé]

Directly call ldn_be_p once instead of ldl_be_p / ldq_be_p.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/spapr_vof.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/hw/ppc/spapr_vof.c b/hw/ppc/spapr_vof.c
index c02eaacfed..d238a44d88 100644
--- a/hw/ppc/spapr_vof.c
+++ b/hw/ppc/spapr_vof.c
@@ -136,26 +136,17 @@ bool spapr_vof_setprop(MachineState *ms, const char *path, const char *propname,
             vof->bootargs = g_strndup(val, vallen);
             return true;
         }
-        if (strcmp(propname, "linux,initrd-start") == 0) {
-            if (vallen == sizeof(uint32_t)) {
-                spapr->initrd_base = ldl_be_p(val);
-                return true;
+        switch (vallen) {
+        case 4:
+        case 8:
+            if (strcmp(propname, "linux,initrd-start") == 0) {
+                spapr->initrd_base = ldn_be_p(val, vallen);
             }
-            if (vallen == sizeof(uint64_t)) {
-                spapr->initrd_base = ldq_be_p(val);
-                return true;
-            }
-            return false;
-        }
-        if (strcmp(propname, "linux,initrd-end") == 0) {
-            if (vallen == sizeof(uint32_t)) {
-                spapr->initrd_size = ldl_be_p(val) - spapr->initrd_base;
-                return true;
-            }
-            if (vallen == sizeof(uint64_t)) {
-                spapr->initrd_size = ldq_be_p(val) - spapr->initrd_base;
-                return true;
+            if (strcmp(propname, "linux,initrd-end") == 0) {
+                spapr->initrd_size = ldn_be_p(val, vallen);
             }
+            return true;
+        default:
             return false;
         }
     }
-- 
2.45.2

Re: [PATCH v2 2/4] hw/ppc/spapr_vof: Simplify LD/ST API uses

[Richard Henderson]

On 9/27/24 14:50, Philippe Mathieu-Daudé wrote:
> Directly call ldn_be_p once instead of ldl_be_p / ldq_be_p.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/ppc/spapr_vof.c | 27 +++++++++------------------
>   1 file changed, 9 insertions(+), 18 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH v2 3/4] hw/ppc/vof: Simplify LD/ST API uses

[Philippe Mathieu-Daudé]

Instead of be32_to_cpu (equivalent of ldl_be_p) and ldq_be_p,
use ldn_be_p(). Similarly instead of cpu_to_be32 (equiv. stl_be_p)
and cpu_to_be64 (equiv. stq_be_p), use stn_be_p().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/vof.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
index b5b6514d79..e2549ab786 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -628,6 +628,7 @@ static void vof_dt_memory_available(void *fdt, GArray *claimed, uint64_t base)
     const uint8_t *mem0_reg;
     g_autofree uint8_t *avail = NULL;
     uint8_t *availcur;
+    size_t elsz;
 
     if (!fdt || !claimed) {
         return;
@@ -645,11 +646,8 @@ static void vof_dt_memory_available(void *fdt, GArray *claimed, uint64_t base)
 
     mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen);
     g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc));
-    if (sc == 2) {
-        mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac);
-    } else {
-        mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg + sizeof(uint32_t) * ac));
-    }
+    elsz = sc * sizeof(uint32_t);
+    mem0_end = ldn_be_p(mem0_reg + sizeof(uint32_t) * ac, elsz);
 
     g_array_sort(claimed, of_claimed_compare_func);
     vof_claimed_dump(claimed);
@@ -674,18 +672,12 @@ static void vof_dt_memory_available(void *fdt, GArray *claimed, uint64_t base)
             size = mem0_end - start;
         }
 
-        if (ac == 2) {
-            *(uint64_t *) availcur = cpu_to_be64(start);
-        } else {
-            *(uint32_t *) availcur = cpu_to_be32(start);
-        }
-        availcur += sizeof(uint32_t) * ac;
-        if (sc == 2) {
-            *(uint64_t *) availcur = cpu_to_be64(size);
-        } else {
-            *(uint32_t *) availcur = cpu_to_be32(size);
-        }
-        availcur += sizeof(uint32_t) * sc;
+        elsz = ac * sizeof(uint32_t);
+        stn_be_p(&availcur, elsz, start);
+        availcur += elsz;
+        elsz = sc * sizeof(uint32_t);
+        stn_be_p(&availcur, elsz, size);
+        availcur += elsz;
 
         if (size) {
             trace_vof_avail(c.start + c.size, c.start + c.size + size, size);
-- 
2.45.2

[PATCH v2 4/4] net/l2tpv3: Simplify LD/ST API uses

[Philippe Mathieu-Daudé]

Directly call ldn_be_p once instead of ldl_be_p / ldq_be_p.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 net/l2tpv3.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index b5547cb917..7a0d5dcfe9 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -318,11 +318,7 @@ static int l2tpv3_verify_header(NetL2TPV3State *s, uint8_t *buf)
     */
 
     if (s->cookie) {
-        if (s->cookie_is_64) {
-            cookie = ldq_be_p(buf + s->cookie_offset);
-        } else {
-            cookie = ldl_be_p(buf + s->cookie_offset) & 0xffffffffULL;
-        }
+        cookie = ldn_be_p(buf + s->cookie_offset, s->cookie_is_64 ? 8 : 4);
         if (cookie != s->rx_cookie) {
             if (!s->header_mismatch) {
                 error_report("unknown cookie id");
-- 
2.45.2

Re: [PATCH v2 4/4] net/l2tpv3: Simplify LD/ST API uses

[Richard Henderson]

On 9/27/24 14:50, Philippe Mathieu-Daudé wrote:
> Directly call ldn_be_p once instead of ldl_be_p / ldq_be_p.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   net/l2tpv3.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/net/l2tpv3.c b/net/l2tpv3.c
> index b5547cb917..7a0d5dcfe9 100644
> --- a/net/l2tpv3.c
> +++ b/net/l2tpv3.c
> @@ -318,11 +318,7 @@ static int l2tpv3_verify_header(NetL2TPV3State *s, uint8_t *buf)
>       */
>   
>       if (s->cookie) {
> -        if (s->cookie_is_64) {
> -            cookie = ldq_be_p(buf + s->cookie_offset);
> -        } else {
> -            cookie = ldl_be_p(buf + s->cookie_offset) & 0xffffffffULL;
> -        }
> +        cookie = ldn_be_p(buf + s->cookie_offset, s->cookie_is_64 ? 8 : 4);
>           if (cookie != s->rx_cookie) {
>               if (!s->header_mismatch) {
>                   error_report("unknown cookie id");

Is this really an improvement?  I don't see it.


r~