[Skiboot] [PATCH 10/12] occ sensors: make endian-clean
Oliver O'Halloran
oohall at gmail.com
Tue Oct 1 15:50:06 AEST 2019
On Sun, 2019-09-29 at 17:46 +1000, Nicholas Piggin wrote:
> Convert occ sensors dt construction and in-memory hardware tables to use
> explicit endian conversions.
Looks ok. I'm not sure who's doing power managment stuff in OPAL
nowdays that should be reviewing this though.
Vaidy, do you know?
>
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> ---
> hw/occ-sensor.c | 100 ++++++++++++++++++++++++++----------------------
> hw/occ.c | 24 ++++++------
> include/occ.h | 50 ++++++++++++------------
> 3 files changed, 92 insertions(+), 82 deletions(-)
>
> diff --git a/hw/occ-sensor.c b/hw/occ-sensor.c
> index d06ca725b..6a01f6f55 100644
> --- a/hw/occ-sensor.c
> +++ b/hw/occ-sensor.c
> @@ -116,7 +116,7 @@ struct occ_sensor_data_header *get_sensor_header_block(int occ_num)
> static inline
> struct occ_sensor_name *get_names_block(struct occ_sensor_data_header *hb)
> {
> - return ((struct occ_sensor_name *)((u64)hb + hb->names_offset));
> + return ((struct occ_sensor_name *)((u64)hb + be32_to_cpu(hb->names_offset)));
> }
>
> static inline u32 sensor_handler(int occ_num, int sensor_id, int attr)
> @@ -131,11 +131,11 @@ static inline u32 sensor_handler(int occ_num, int sensor_id, int attr)
> */
> static void scale_sensor(struct occ_sensor_name *md, u64 *sensor)
> {
> - u32 factor = md->scale_factor;
> + u32 factor = be32_to_cpu(md->scale_factor);
> int i;
> s8 exp;
>
> - if (md->type == OCC_SENSOR_TYPE_CURRENT)
> + if (be16_to_cpu(md->type) == OCC_SENSOR_TYPE_CURRENT)
> *sensor *= 1000; //convert to mA
>
> *sensor *= factor >> 8;
> @@ -152,7 +152,7 @@ static void scale_sensor(struct occ_sensor_name *md, u64 *sensor)
>
> static void scale_energy(struct occ_sensor_name *md, u64 *sensor)
> {
> - u32 factor = md->freq;
> + u32 factor = be32_to_cpu(md->freq);
> int i;
> s8 exp;
>
> @@ -174,17 +174,17 @@ static u64 read_sensor(struct occ_sensor_record *sensor, int attr)
> {
> switch (attr) {
> case SENSOR_SAMPLE:
> - return sensor->sample;
> + return be16_to_cpu(sensor->sample);
> case SENSOR_SAMPLE_MIN:
> - return sensor->sample_min;
> + return be16_to_cpu(sensor->sample_min);
> case SENSOR_SAMPLE_MAX:
> - return sensor->sample_max;
> + return be16_to_cpu(sensor->sample_max);
> case SENSOR_CSM_MIN:
> - return sensor->csm_min;
> + return be16_to_cpu(sensor->csm_min);
> case SENSOR_CSM_MAX:
> - return sensor->csm_max;
> + return be16_to_cpu(sensor->csm_max);
> case SENSOR_ACCUMULATOR:
> - return sensor->accumulator;
> + return be64_to_cpu(sensor->accumulator);
> default:
> break;
> }
> @@ -197,14 +197,16 @@ static void *select_sensor_buffer(struct occ_sensor_data_header *hb, int id)
> struct occ_sensor_name *md;
> u8 *ping, *pong;
> void *buffer = NULL;
> + u32 reading_offset;
>
> if (!hb)
> return NULL;
>
> md = get_names_block(hb);
>
> - ping = (u8 *)((u64)hb + hb->reading_ping_offset);
> - pong = (u8 *)((u64)hb + hb->reading_pong_offset);
> + ping = (u8 *)((u64)hb + be32_to_cpu(hb->reading_ping_offset));
> + pong = (u8 *)((u64)hb + be32_to_cpu(hb->reading_pong_offset));
> + reading_offset = be32_to_cpu(md[id].reading_offset);
>
> /* Check which buffer is valid and read the data from that.
> * Ping Pong Action
> @@ -216,11 +218,11 @@ static void *select_sensor_buffer(struct occ_sensor_data_header *hb, int id)
>
> if (*ping && *pong) {
> u64 tping, tpong;
> - u64 ping_buf = (u64)ping + md[id].reading_offset;
> - u64 pong_buf = (u64)pong + md[id].reading_offset;
> + u64 ping_buf = (u64)ping + reading_offset;
> + u64 pong_buf = (u64)pong + reading_offset;
>
> - tping = ((struct occ_sensor_record *)ping_buf)->timestamp;
> - tpong = ((struct occ_sensor_record *)pong_buf)->timestamp;
> + tping = be64_to_cpu(((struct occ_sensor_record *)ping_buf)->timestamp);
> + tpong = be64_to_cpu(((struct occ_sensor_record *)pong_buf)->timestamp);
>
> if (tping > tpong)
> buffer = ping;
> @@ -236,7 +238,7 @@ static void *select_sensor_buffer(struct occ_sensor_data_header *hb, int id)
> }
>
> assert(buffer);
> - buffer = (void *)((u64)buffer + md[id].reading_offset);
> + buffer = (void *)((u64)buffer + reading_offset);
>
> return buffer;
> }
> @@ -264,7 +266,7 @@ int occ_sensor_read(u32 handle, u64 *data)
> if (hb->valid != 1)
> return OPAL_HARDWARE;
>
> - if (id > hb->nr_sensors)
> + if (id > be16_to_cpu(hb->nr_sensors))
> return OPAL_PARAMETER;
>
> buff = select_sensor_buffer(hb, id);
> @@ -276,7 +278,7 @@ int occ_sensor_read(u32 handle, u64 *data)
> return OPAL_SUCCESS;
>
> md = get_names_block(hb);
> - if (md[id].type == OCC_SENSOR_TYPE_POWER && attr == SENSOR_ACCUMULATOR)
> + if (be16_to_cpu(md[id].type) == OCC_SENSOR_TYPE_POWER && attr == SENSOR_ACCUMULATOR)
> scale_energy(&md[id], data);
> else
> scale_sensor(&md[id], data);
> @@ -320,7 +322,8 @@ static bool occ_sensor_sanity(struct occ_sensor_data_header *hb, int chipid)
> return false;
> }
>
> - if (!hb->names_offset || !hb->reading_ping_offset ||
> + if (!hb->names_offset ||
> + !hb->reading_ping_offset ||
> !hb->reading_pong_offset) {
> prerror("OCC: Chip %d Invalid sensor buffer pointers\n",
> chipid);
> @@ -357,9 +360,10 @@ static void add_sensor_label(struct dt_node *node, struct occ_sensor_name *md,
> {
> char sname[30] = "";
> char prefix[30] = "";
> + uint16_t location = be16_to_cpu(md->location);
> int i;
>
> - if (md->location != OCC_SENSOR_LOC_SYSTEM)
> + if (location != OCC_SENSOR_LOC_SYSTEM)
> snprintf(prefix, sizeof(prefix), "%s %d ", "Chip", chipid);
>
> for (i = 0; i < ARRAY_SIZE(str_maps); i++)
> @@ -368,7 +372,7 @@ static void add_sensor_label(struct dt_node *node, struct occ_sensor_name *md,
> char *end;
> int num = -1;
>
> - if (md->location != OCC_SENSOR_LOC_CORE)
> + if (location != OCC_SENSOR_LOC_CORE)
> num = parse_entity(md->name, &end);
>
> if (num != -1) {
> @@ -384,7 +388,7 @@ static void add_sensor_label(struct dt_node *node, struct occ_sensor_name *md,
> }
>
> /* Fallback to OCC literal if mapping is not found */
> - if (md->location == OCC_SENSOR_LOC_SYSTEM) {
> + if (location == OCC_SENSOR_LOC_SYSTEM) {
> dt_add_property_string(node, "label", md->name);
> } else {
> snprintf(sname, sizeof(sname), "%s%s", prefix, md->name);
> @@ -444,15 +448,15 @@ static bool check_sensor_sample(struct occ_sensor_data_header *hb, u32 offset)
> {
> struct occ_sensor_record *ping, *pong;
>
> - ping = (struct occ_sensor_record *)((u64)hb + hb->reading_ping_offset
> - + offset);
> - pong = (struct occ_sensor_record *)((u64)hb + hb->reading_pong_offset
> - + offset);
> + ping = (struct occ_sensor_record *)((u64)hb
> + + be32_to_cpu(hb->reading_ping_offset) + offset);
> + pong = (struct occ_sensor_record *)((u64)hb
> + + be32_to_cpu(hb->reading_pong_offset) + offset);
> return ping->sample || pong->sample;
> }
>
> static void add_sensor_node(const char *loc, const char *type, int i, int attr,
> - struct occ_sensor_name *md, u32 *phandle, u32 *ptype,
> + struct occ_sensor_name *md, __be32 *phandle, u32 *ptype,
> u32 pir, u32 occ_num, u32 chipid)
> {
> char name[30];
> @@ -468,10 +472,10 @@ static void add_sensor_node(const char *loc, const char *type, int i, int attr,
> dt_add_property_string(node, "occ_label", md->name);
> add_sensor_label(node, md, chipid);
>
> - if (md->location == OCC_SENSOR_LOC_CORE)
> + if (be16_to_cpu(md->location) == OCC_SENSOR_LOC_CORE)
> dt_add_property_cells(node, "ibm,pir", pir);
>
> - *ptype = md->type;
> + *ptype = be16_to_cpu(md->type);
>
> if (attr == SENSOR_SAMPLE) {
> handler = sensor_handler(occ_num, i, SENSOR_CSM_MAX);
> @@ -482,7 +486,7 @@ static void add_sensor_node(const char *loc, const char *type, int i, int attr,
> }
>
> dt_add_property_string(node, "compatible", "ibm,opal-sensor");
> - *phandle = node->phandle;
> + *phandle = cpu_to_be32(node->phandle);
> }
>
> bool occ_sensors_init(void)
> @@ -520,7 +524,9 @@ bool occ_sensors_init(void)
> for_each_chip(chip) {
> struct occ_sensor_data_header *hb;
> struct occ_sensor_name *md;
> - u32 *phandles, *ptype, phcount = 0;
> + __be32 *phandles;
> + u32 *ptype, phcount = 0;
> + unsigned int nr_sensors;
>
> hb = get_sensor_header_block(occ_num);
> md = get_names_block(hb);
> @@ -529,30 +535,34 @@ bool occ_sensors_init(void)
> if (!occ_sensor_sanity(hb, chip->id))
> continue;
>
> - phandles = malloc(hb->nr_sensors * sizeof(u32));
> + nr_sensors = be16_to_cpu(hb->nr_sensors);
> +
> + phandles = malloc(nr_sensors * sizeof(__be32));
> assert(phandles);
> - ptype = malloc(hb->nr_sensors * sizeof(u32));
> + ptype = malloc(nr_sensors * sizeof(u32));
> assert(ptype);
>
> - for (i = 0; i < hb->nr_sensors; i++) {
> - const char *type, *loc;
> + for (i = 0; i < nr_sensors; i++) {
> + const char *type_name, *loc;
> struct cpu_thread *c = NULL;
> uint32_t pir = 0;
> + uint16_t type = be16_to_cpu(md[i].type);
> + uint16_t location = be16_to_cpu(md[i].location);
>
> if (md[i].structure_type != OCC_SENSOR_READING_FULL)
> continue;
>
> - if (!(md[i].type & HWMON_SENSORS_MASK))
> + if (!(type & HWMON_SENSORS_MASK))
> continue;
>
> - if (md[i].location == OCC_SENSOR_LOC_GPU && !has_gpu)
> + if (location == OCC_SENSOR_LOC_GPU && !has_gpu)
> continue;
>
> - if (md[i].type == OCC_SENSOR_TYPE_POWER &&
> - !check_sensor_sample(hb, md[i].reading_offset))
> + if (type == OCC_SENSOR_TYPE_POWER &&
> + !check_sensor_sample(hb, be32_to_cpu(md[i].reading_offset)))
> continue;
>
> - if (md[i].location == OCC_SENSOR_LOC_CORE) {
> + if (location == OCC_SENSOR_LOC_CORE) {
> int num = parse_entity(md[i].name, NULL);
>
> for_each_available_core_in_chip(c, chip->id)
> @@ -563,16 +573,16 @@ bool occ_sensors_init(void)
> pir = c->pir;
> }
>
> - type = get_sensor_type_string(md[i].type);
> - loc = get_sensor_loc_string(md[i].location);
> + type_name = get_sensor_type_string(type);
> + loc = get_sensor_loc_string(location);
>
> - add_sensor_node(loc, type, i, SENSOR_SAMPLE, &md[i],
> + add_sensor_node(loc, type_name, i, SENSOR_SAMPLE, &md[i],
> &phandles[phcount], &ptype[phcount],
> pir, occ_num, chip->id);
> phcount++;
>
> /* Add energy sensors */
> - if (md[i].type == OCC_SENSOR_TYPE_POWER &&
> + if (type == OCC_SENSOR_TYPE_POWER &&
> md[i].structure_type == OCC_SENSOR_READING_FULL) {
> add_sensor_node(loc, "energy", i,
> SENSOR_ACCUMULATOR, &md[i],
> diff --git a/hw/occ.c b/hw/occ.c
> index db2744ff7..4a0a11f58 100644
> --- a/hw/occ.c
> +++ b/hw/occ.c
> @@ -97,7 +97,7 @@ struct occ_pstate_table {
> u8 flags;
> u8 vdd;
> u8 vcs;
> - u32 freq_khz;
> + __be32 freq_khz;
> } pstates[MAX_PSTATES];
> s8 core_max[MAX_P8_CORES];
> u8 pad[100];
> @@ -115,7 +115,7 @@ struct occ_pstate_table {
> u8 id;
> u8 flags;
> u16 reserved;
> - u32 freq_khz;
> + __be32 freq_khz;
> } pstates[MAX_PSTATES];
> u8 core_max[MAX_P9_CORES];
> u8 pad[56];
> @@ -375,7 +375,7 @@ static bool wait_for_all_occ_init(void)
> chip->occ_functional = true;
>
> prlog(PR_DEBUG, "OCC: Chip %02x Data (%016llx) = %016llx\n",
> - chip->id, (uint64_t)occ_data, *(uint64_t *)occ_data);
> + chip->id, (uint64_t)occ_data, be64_to_cpu(*(__be64 *)occ_data));
> }
> end_time = mftb();
> prlog(PR_NOTICE, "OCC: All Chip Rdy after %lu ms\n",
> @@ -407,8 +407,8 @@ static void parse_pstates_v2(struct occ_pstate_table *data, u32 *dt_id,
> if (cmp_pstates(data->v2.pstates[i].id, pmax) > 0)
> continue;
>
> - dt_id[j] = data->v2.pstates[i].id;
> - dt_freq[j] = data->v2.pstates[i].freq_khz / 1000;
> + dt_id[j] = cpu_to_be32(data->v2.pstates[i].id);
> + dt_freq[j] = cpu_to_be32(be32_to_cpu(data->v2.pstates[i].freq_khz) / 1000);
> j++;
>
> if (data->v2.pstates[i].id == pmin)
> @@ -429,8 +429,8 @@ static void parse_pstates_v9(struct occ_pstate_table *data, u32 *dt_id,
> if (cmp_pstates(data->v9.pstates[i].id, pmax) > 0)
> continue;
>
> - dt_id[j] = data->v9.pstates[i].id;
> - dt_freq[j] = data->v9.pstates[i].freq_khz / 1000;
> + dt_id[j] = cpu_to_be32(data->v9.pstates[i].id);
> + dt_freq[j] = cpu_to_be32(be32_to_cpu(data->v9.pstates[i].freq_khz) / 1000);
> j++;
>
> if (data->v9.pstates[i].id == pmin)
> @@ -500,8 +500,8 @@ static bool add_cpu_pstate_properties(struct dt_node *power_mgt,
> occ_data_area = (uint64_t)occ_data;
> prlog(PR_DEBUG, "OCC: Data (%16llx) = %16llx %16llx\n",
> occ_data_area,
> - *(uint64_t *)occ_data_area,
> - *(uint64_t *)(occ_data_area + 8));
> + be64_to_cpu(*(__be64 *)occ_data_area),
> + be64_to_cpu(*(__be64 *)(occ_data_area + 8)));
>
> if (!occ_data->valid) {
> /**
> @@ -676,13 +676,13 @@ static bool add_cpu_pstate_properties(struct dt_node *power_mgt,
> pturbo = occ_data->v2.pstate_turbo;
> pultra_turbo = occ_data->v2.pstate_ultra_turbo;
> for (i = 0; i < nr_cores; i++)
> - dt_cmax[i] = occ_data->v2.core_max[i];
> + dt_cmax[i] = cpu_to_be32(occ_data->v2.core_max[i]);
> break;
> case 0x9:
> pturbo = occ_data->v9.pstate_turbo;
> pultra_turbo = occ_data->v9.pstate_ultra_turbo;
> for (i = 0; i < nr_cores; i++)
> - dt_cmax[i] = occ_data->v9.core_max[i];
> + dt_cmax[i] = cpu_to_be32(occ_data->v9.core_max[i]);
> break;
> default:
> return false;
> @@ -1600,7 +1600,7 @@ int occ_sensor_group_enable(u32 group_hndl, int token, bool enable)
> return opal_occ_command(&chips[i], token, &sensor_mask_data);
> }
>
> -void occ_add_sensor_groups(struct dt_node *sg, u32 *phandles, u32 *ptype,
> +void occ_add_sensor_groups(struct dt_node *sg, __be32 *phandles, u32 *ptype,
> int nr_phandles, int chipid)
> {
> struct group_info {
> diff --git a/include/occ.h b/include/occ.h
> index 0030af5ae..f3b8f6a9a 100644
> --- a/include/occ.h
> +++ b/include/occ.h
> @@ -34,7 +34,7 @@ bool occ_get_gpu_presence(struct proc_chip *chip, int gpu_num);
> extern bool occ_sensors_init(void);
> extern int occ_sensor_read(u32 handle, u64 *data);
> extern int occ_sensor_group_clear(u32 group_hndl, int token);
> -extern void occ_add_sensor_groups(struct dt_node *sg, u32 *phandles,
> +extern void occ_add_sensor_groups(struct dt_node *sg, __be32 *phandles,
> u32 *ptype, int nr_phandles, int chipid);
>
> extern int occ_sensor_group_enable(u32 group_hndl, int token, bool enable);
> @@ -186,15 +186,15 @@ enum sensor_struct_type {
> struct occ_sensor_data_header {
> u8 valid;
> u8 version;
> - u16 nr_sensors;
> + __be16 nr_sensors;
> u8 reading_version;
> u8 pad[3];
> - u32 names_offset;
> + __be32 names_offset;
> u8 names_version;
> u8 name_length;
> u16 reserved;
> - u32 reading_ping_offset;
> - u32 reading_pong_offset;
> + __be32 reading_ping_offset;
> + __be32 reading_pong_offset;
> } __attribute__((__packed__));
>
> /**
> @@ -220,13 +220,13 @@ struct occ_sensor_data_header {
> struct occ_sensor_name {
> char name[MAX_CHARS_SENSOR_NAME];
> char units[MAX_CHARS_SENSOR_UNIT];
> - u16 gsid;
> - u32 freq;
> - u32 scale_factor;
> - u16 type;
> - u16 location;
> + __be16 gsid;
> + __be32 freq;
> + __be32 scale_factor;
> + __be16 type;
> + __be16 location;
> u8 structure_type;
> - u32 reading_offset;
> + __be32 reading_offset;
> u8 sensor_data;
> u8 pad[8];
> } __attribute__((__packed__));
> @@ -258,18 +258,18 @@ struct occ_sensor_name {
> */
> struct occ_sensor_record {
> u16 gsid;
> - u64 timestamp;
> - u16 sample;
> - u16 sample_min;
> - u16 sample_max;
> - u16 csm_min;
> - u16 csm_max;
> - u16 profiler_min;
> - u16 profiler_max;
> - u16 job_scheduler_min;
> - u16 job_scheduler_max;
> - u64 accumulator;
> - u32 update_tag;
> + __be64 timestamp;
> + __be16 sample;
> + __be16 sample_min;
> + __be16 sample_max;
> + __be16 csm_min;
> + __be16 csm_max;
> + __be16 profiler_min;
> + __be16 profiler_max;
> + __be16 job_scheduler_min;
> + __be16 job_scheduler_max;
> + __be64 accumulator;
> + __be32 update_tag;
> u8 pad[8];
> } __attribute__((__packed__));
>
> @@ -284,8 +284,8 @@ struct occ_sensor_record {
> */
> struct occ_sensor_counter {
> u16 gsid;
> - u64 timestamp;
> - u64 accumulator;
> + __be64 timestamp;
> + __be64 accumulator;
> u8 sample;
> u8 pad[5];
> } __attribute__((__packed__));
More information about the Skiboot
mailing list