Hi,
On 11/1/22 20:10, David E. Box wrote:
> The structure and content of the On Demand registers is based on the GUID
> which is read from hardware through sysfs. Add support for decoding the
> registers of a new GUID 0xF210D9EF.
>
> Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
> ---
> tools/arch/x86/intel_sdsi/intel_sdsi.c | 32 ++++++++++++++++++--------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> index 01b5f9994e11..0680eda78b1a 100644
> --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
> +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> @@ -35,7 +35,8 @@
> #define SDSI_DEV "intel_vsec.sdsi"
> #define AUX_DEV_PATH "/sys/bus/auxiliary/devices/"
> #define SDSI_PATH (AUX_DEV_DIR SDSI_DEV)
> -#define GUID 0x6dd191
> +#define GUID_V1 0x6dd191
> +#define GUID_V2 0xF210D9EF
> #define REGISTERS_MIN_SIZE 72
> #define STATE_CERT_MAX_SIZE 4096
> #define STATE_MAX_NUM_LICENSES 16
> @@ -100,9 +101,17 @@ struct sdsi_regs {
> struct availability prov_avail;
> struct nvram_update_limit limits;
> uint64_t pcu_cr3_capid_cfg;
> - uint64_t socket_id;
> + union {
> + struct {
> + uint64_t socket_id;
> + } v1;
> + struct {
> + uint64_t reserved;
> + uint64_t socket_id;
> + uint64_t reserved2;
> + } v2;
> + } extra;
> };
> -
> #define CONTENT_TYPE_LK_ENC 0xD
> #define CONTENT_TYPE_LK_BLOB_ENC 0xE
>
> @@ -146,7 +155,7 @@ struct sdsi_dev {
> struct state_certificate sc;
> char *dev_name;
> char *dev_path;
> - int guid;
> + uint32_t guid;
> };
>
> enum command {
> @@ -199,7 +208,7 @@ static int sdsi_update_registers(struct sdsi_dev *s)
> return -1;
> }
>
> - if (s->guid != GUID) {
> + if (s->guid != GUID_V1 && s->guid != GUID_V2) {
> fprintf(stderr, "Unrecognized guid, 0x%x\n", s->guid);
> fclose(regs_ptr);
> return -1;
> @@ -207,7 +216,7 @@ static int sdsi_update_registers(struct sdsi_dev *s)
>
> /* Update register info for this guid */
> ret = fread(&s->regs, sizeof(uint8_t), sizeof(s->regs), regs_ptr);
> - if (ret != sizeof(s->regs)) {
> + if (ret > (int)sizeof(s->regs)) { /* FIXME: Check size by guid */
This is wrong, fread will never return more then requested, that
would lead to buffer overflows. But it may return 0 on errors, or
a short read on an error.
So you need to fix the FIXME comment you added here as now you
have just disabled all error checking.
Regards,
Hans
> fprintf(stderr, "Could not read 'registers' file\n");
> fclose(regs_ptr);
> return -1;
> @@ -252,10 +261,13 @@ static int sdsi_read_reg(struct sdsi_dev *s)
> printf(" Updates Available: %d\n", s->regs.prov_avail.available);
> printf(" Updates Threshold: %d\n", s->regs.prov_avail.threshold);
> printf("NVRAM Udate Limit\n");
> - printf(" 50%% Limit Reached: %s\n", !!s->regs.limits.sdsi_50_pct ? "Yes" : "No");
> - printf(" 75%% Limit Reached: %s\n", !!s->regs.limits.sdsi_75_pct ? "Yes" : "No");
> - printf(" 90%% Limit Reached: %s\n", !!s->regs.limits.sdsi_90_pct ? "Yes" : "No");
> - printf("Socket ID: %ld\n", s->regs.socket_id & 0xF);
> + printf(" 50%% Limit Reached: %s\n", !!s->regs.limits.sdsi_50_pct ? "Yes" : "No");
> + printf(" 75%% Limit Reached: %s\n", !!s->regs.limits.sdsi_75_pct ? "Yes" : "No");
> + printf(" 90%% Limit Reached: %s\n", !!s->regs.limits.sdsi_90_pct ? "Yes" : "No");
> + if (s->guid == GUID_V1)
> + printf("Socket ID: %ld\n", s->regs.extra.v1.socket_id & 0xF);
> + else
> + printf("Socket ID: %ld\n", s->regs.extra.v2.socket_id & 0xF);
>
> return 0;
> }
