[perso_tlv] improve accessor macros robustness and consolidate marsha…

…ling

Make sure that TLV accessor macros' arguments are evaluated once and
expressions passed as arguments are evaluated properly.

Also let the macros deal with big/little endian swapping of the header
values.

End to end Cros personalization post still succeeds.

Signed-off-by: Vadim Bendebury <[email protected]>

sw/device/silicon_creator/manuf/base/ft_personalize.c

@@ -553,7 +553,6 @@ static status_t personalize_endorse_certificates(ujson_t *uj) {
       // We just prepared the set of wrapped certificates, let's trust that the
       // data is correct and does not need more validation.
       memcpy(&crth, certs, sizeof(crth));
-      crth = __builtin_bswap16(crth);
       certs += sizeof(crth);
       PERSO_TLV_GET_FIELD(Crth, NameSize, crth, &name_len);
       memcpy(name, certs, name_len);

sw/device/silicon_creator/manuf/base/perso_tlv_data.c

@@ -30,7 +30,6 @@ status_t perso_tlv_set_cert_block(const uint8_t *buf, size_t max_room,
   perso_tlv_object_header_t obj_type;
 
   memcpy(&objh, buf, sizeof(objh));
-  objh = __builtin_bswap16(objh);
   PERSO_TLV_GET_FIELD(Objh, Size, objh, &obj_size);
 
   if (obj_size > max_room)
@@ -53,7 +52,6 @@ status_t perso_tlv_set_cert_block(const uint8_t *buf, size_t max_room,
   // Let's retrieve cert wrapper header.
   block->wrapped_cert_p = buf;
   memcpy(&crth, buf, sizeof(crth));
-  crth = __builtin_bswap16(crth);
   max_room -= sizeof(crth);
   buf += sizeof(crth);
   PERSO_TLV_GET_FIELD(Crth, Size, crth, &wrapped_cert_size);
@@ -148,10 +146,8 @@ status_t perso_tlv_prepare_cert_for_shipping(const char *name,
   PERSO_TLV_SET_FIELD(Crth, Size, cert_header, wrapped_len);
   PERSO_TLV_SET_FIELD(Crth, NameSize, cert_header, name_len);
 
-  uint16_t swapped = __builtin_bswap16(obj_header);
-  TRY(perso_tlv_push_to_blob(&swapped, sizeof(obj_header), pb));
-  swapped = __builtin_bswap16(cert_header);
-  TRY(perso_tlv_push_to_blob(&swapped, sizeof(cert_header), pb));
+  TRY(perso_tlv_push_to_blob(&obj_header, sizeof(obj_header), pb));
+  TRY(perso_tlv_push_to_blob(&cert_header, sizeof(cert_header), pb));
   TRY(perso_tlv_push_to_blob(name, name_len, pb));
   TRY(perso_tlv_push_to_blob(cert_body, cert_size, pb));
   pb->num_objs++;

sw/device/silicon_creator/manuf/base/perso_tlv_data.h

@@ -73,23 +73,25 @@ typedef enum perso_tlv_cert_header_fields {
   kCrthNameSizeFieldMask = (1 << kCrthNameSizeFieldWidth) - 1,
 } perso_tlv_cert_header_fields_t;
 
-// Helper macros allowing set or get various header fields.
-#define PERSO_TLV_SET_FIELD(type_name, field_name, full_value, field_value)   \
-  {                                                                           \
-    uint16_t mask = k##type_name##field_name##FieldMask;                      \
-    uint16_t shift = k##type_name##field_name##FieldShift;                    \
-    uint16_t fieldv = (uint16_t)field_value;                                  \
-    uint16_t fullv = (uint16_t)full_value;                                    \
-    fieldv = field_value & mask;                                              \
-    mask = (uint16_t)(mask << shift);                                         \
-    full_value = (uint16_t)((fullv & ~mask) | (((uint16_t)fieldv) << shift)); \
+// Helper macros allowing set or get various object and certificate header
+// fields. Operate on objects in big endian representation, as they are
+// transferred over wire.
+#define PERSO_TLV_SET_FIELD(type_name, field_name, full_value, field_value) \
+  {                                                                         \
+    uint16_t mask = k##type_name##field_name##FieldMask;                    \
+    uint16_t shift = k##type_name##field_name##FieldShift;                  \
+    uint16_t fieldv = (uint16_t)(field_value)&mask;                         \
+    uint16_t fullv = __builtin_bswap16((uint16_t)(full_value));             \
+    mask = (uint16_t)(mask << shift);                                       \
+    (full_value) = __builtin_bswap16(                                       \
+        (uint16_t)((fullv & ~mask) | (((uint16_t)fieldv) << shift)));       \
   }
 
 #define PERSO_TLV_GET_FIELD(type_name, field_name, full_value, field_value) \
   {                                                                         \
     uint16_t mask = k##type_name##field_name##FieldMask;                    \
     uint16_t shift = k##type_name##field_name##FieldShift;                  \
-    *field_value = (full_value >> shift) & mask;                            \
+    *(field_value) = (__builtin_bswap16(full_value) >> shift) & mask;       \
   }
 
 /**