| Message ID | 20240814095954.2359863-6-akshay.gupta@amd.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | misc: Add AMD side band interface(SBI) functionality | expand |
On Wed, Aug 14, 2024, at 11:59, Akshay Gupta wrote: > +/* input for bulk write to CPUID protocol */ > +struct cpu_msr_indata { > + u8 wr_len; /* const value */ > + u8 rd_len; /* const value */ > + u8 proto_cmd; /* const value */ > + u8 thread; /* thread number */ > + union { > + u8 reg_offset[4]; /* input value */ > + u32 value; > + }; > + u8 ext; /* extended function */ > +} __packed; You cannot have a fully aligned union inside of a misaligned struct. Since the only member is the inner 'u32 value', I think it would make more sense to make that one __packed instead of the structure. > +/* output for bulk read from CPUID protocol */ > +struct cpu_msr_outdata { > + u8 num_bytes; /* number of bytes return */ > + u8 status; /* Protocol status code */ > + union { > + u64 value; > + u8 reg_data[8]; > + }; > +} __packed; Same here. > +#define prepare_cpuid_input_message(input, thread_id, func, ext_func) \ > + input.rd_len = CPUID_RD_DATA_LEN, \ > + input.wr_len = CPUID_WR_DATA_LEN, \ > + input.proto_cmd = RD_CPUID_CMD, \ > + input.thread = thread_id << 1, \ > + input.value = func, \ > + input.ext = ext_func This can be an inline function. > +/* > + * For Mailbox command software alert status bit is set by firmware > + * to indicate command completion > + * For RMI Rev 0x20, new h/w status bit is introduced. which is used > + * by firmware to indicate completion of commands (0x71, 0x72, 0x73). > + * wait for the status bit to be set by the firmware before > + * reading the data out. > + */ > +static int sbrmi_wait_status(struct sbrmi_data *data, > + int *status, int mask) > +{ > + int ret, retry = 100; > + > + do { > + ret = regmap_read(data->regmap, SBRMI_STATUS, status); > + if (ret < 0) > + return ret; > + > + if (*status & mask) > + break; > + > + /* Wait 1~2 second for firmware to return data out */ > + if (retry > 95) > + usleep_range(50, 100); > + else > + usleep_range(10000, 20000); > + } while (retry--); This loop is likely to take much longer than 2 seconds if it times out because of all the rounding etc. You can probably change this to regmap_read_poll_timeout(), which handles timeouts correctly. > +/* command ID to identify CPUID protocol */ > +#define APML_CPUID 0x1000 > /* These are byte indexes into data_in and data_out arrays */ > #define AMD_SBI_RD_WR_DATA_INDEX 0 > #define AMD_SBI_REG_OFF_INDEX 0 > #define AMD_SBI_REG_VAL_INDEX 4 > #define AMD_SBI_RD_FLAG_INDEX 7 > +#define AMD_SBI_THREAD_LOW_INDEX 4 > +#define AMD_SBI_THREAD_HI_INDEX 5 > +#define AMD_SBI_EXT_FUNC_INDEX 6 > > #define AMD_SBI_MB_DATA_SIZE 4 > > struct apml_message { > /* message ids: > * Mailbox Messages: 0x0 ... 0x999 > + * APML_CPUID: 0x1000 > */ > __u32 cmd; > > /* > * 8 bit data for reg read, > * 32 bit data in case of mailbox, > + * up to 64 bit in case of cpuid > */ > union { > + __u64 cpu_msr_out; > __u32 mb_out[2]; > __u8 reg_out[8]; > } data_out; > > /* > * [0]...[3] mailbox 32bit input > + * cpuid, > + * [4][5] cpuid: thread > + * [6] cpuid: ext function & read eax/ebx or ecx/edx > + * [7:0] -> bits [7:4] -> ext function & > + * bit [0] read eax/ebx or ecx/edx > * [7] read/write functionality > */ > union { > + __u64 cpu_msr_in; > __u32 mb_in[2]; > __u8 reg_in[8]; > } data_in; > /* > + * Status code is returned in case of CPUID access > * Error code is returned in case of soft mailbox > */ > __u32 fw_ret_code; Low-level protocols like this are rarely a good idea to be exported to userspace. I can't see what the exact data is that you can get in and out, but you probably want higher-level interfaces for the individual things the platform interface can do, either hooking up to existing kernel subsystems or as separate user space interfaces. Arnd
On 8/19/2024 3:57 PM, Arnd Bergmann wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On Wed, Aug 14, 2024, at 11:59, Akshay Gupta wrote: > >> +/* input for bulk write to CPUID protocol */ >> +struct cpu_msr_indata { >> + u8 wr_len; /* const value */ >> + u8 rd_len; /* const value */ >> + u8 proto_cmd; /* const value */ >> + u8 thread; /* thread number */ >> + union { >> + u8 reg_offset[4]; /* input value */ >> + u32 value; >> + }; >> + u8 ext; /* extended function */ >> +} __packed; > You cannot have a fully aligned union inside of a misaligned > struct. Since the only member is the inner 'u32 value', > I think it would make more sense to make that one > __packed instead of the structure. Thank you for suggestion, will update this. >> +/* output for bulk read from CPUID protocol */ >> +struct cpu_msr_outdata { >> + u8 num_bytes; /* number of bytes return */ >> + u8 status; /* Protocol status code */ >> + union { >> + u64 value; >> + u8 reg_data[8]; >> + }; >> +} __packed; > Same here. Thank you for suggestion, will update this. > >> +#define prepare_cpuid_input_message(input, thread_id, func, ext_func) \ >> + input.rd_len = CPUID_RD_DATA_LEN, \ >> + input.wr_len = CPUID_WR_DATA_LEN, \ >> + input.proto_cmd = RD_CPUID_CMD, \ >> + input.thread = thread_id << 1, \ >> + input.value = func, \ >> + input.ext = ext_func > This can be an inline function. Will change to inline function. > >> +/* >> + * For Mailbox command software alert status bit is set by firmware >> + * to indicate command completion >> + * For RMI Rev 0x20, new h/w status bit is introduced. which is used >> + * by firmware to indicate completion of commands (0x71, 0x72, 0x73). >> + * wait for the status bit to be set by the firmware before >> + * reading the data out. >> + */ >> +static int sbrmi_wait_status(struct sbrmi_data *data, >> + int *status, int mask) >> +{ >> + int ret, retry = 100; >> + >> + do { >> + ret = regmap_read(data->regmap, SBRMI_STATUS, status); >> + if (ret < 0) >> + return ret; >> + >> + if (*status & mask) >> + break; >> + >> + /* Wait 1~2 second for firmware to return data out */ >> + if (retry > 95) >> + usleep_range(50, 100); >> + else >> + usleep_range(10000, 20000); >> + } while (retry--); > This loop is likely to take much longer than 2 seconds if it > times out because of all the rounding etc. > > You can probably change this to regmap_read_poll_timeout(), > which handles timeouts correctly. Thank you for suggestion, will use the regmap_read_poll_timeout() > >> +/* command ID to identify CPUID protocol */ >> +#define APML_CPUID 0x1000 >> /* These are byte indexes into data_in and data_out arrays */ >> #define AMD_SBI_RD_WR_DATA_INDEX 0 >> #define AMD_SBI_REG_OFF_INDEX 0 >> #define AMD_SBI_REG_VAL_INDEX 4 >> #define AMD_SBI_RD_FLAG_INDEX 7 >> +#define AMD_SBI_THREAD_LOW_INDEX 4 >> +#define AMD_SBI_THREAD_HI_INDEX 5 >> +#define AMD_SBI_EXT_FUNC_INDEX 6 >> >> #define AMD_SBI_MB_DATA_SIZE 4 >> >> struct apml_message { >> /* message ids: >> * Mailbox Messages: 0x0 ... 0x999 >> + * APML_CPUID: 0x1000 >> */ >> __u32 cmd; >> >> /* >> * 8 bit data for reg read, >> * 32 bit data in case of mailbox, >> + * up to 64 bit in case of cpuid >> */ >> union { >> + __u64 cpu_msr_out; >> __u32 mb_out[2]; >> __u8 reg_out[8]; >> } data_out; >> >> /* >> * [0]...[3] mailbox 32bit input >> + * cpuid, >> + * [4][5] cpuid: thread >> + * [6] cpuid: ext function & read eax/ebx or ecx/edx >> + * [7:0] -> bits [7:4] -> ext function & >> + * bit [0] read eax/ebx or ecx/edx >> * [7] read/write functionality >> */ >> union { >> + __u64 cpu_msr_in; >> __u32 mb_in[2]; >> __u8 reg_in[8]; >> } data_in; >> /* >> + * Status code is returned in case of CPUID access >> * Error code is returned in case of soft mailbox >> */ >> __u32 fw_ret_code; > Low-level protocols like this are rarely a good idea to be > exported to userspace. I can't see what the exact data is > that you can get in and out, but you probably want higher-level > interfaces for the individual things the platform interface > can do, either hooking up to existing kernel subsystems or > as separate user space interfaces. > > Arnd we have opensource C library and user space tool. All the platform level support is handled by the library. IOCTL is the single entry point to the Linux kernel which communicates with the SMU, using the protocols and handles the synchronization. Library is hosted at: https://github.com/amd/esmi_oob_library
diff --git a/drivers/misc/amd-sbi/rmi-core.c b/drivers/misc/amd-sbi/rmi-core.c index a6364337dc5e..fdac6848ae9a 100644 --- a/drivers/misc/amd-sbi/rmi-core.c +++ b/drivers/misc/amd-sbi/rmi-core.c @@ -17,6 +17,8 @@ /* Mask for Status Register bit[1] */ #define SW_ALERT_MASK 0x2 +/* Mask to check H/W Alert status bit */ +#define HW_ALERT_MASK 0x80 /* Do not allow setting negative power limit */ #define SBRMI_PWR_MIN 0 @@ -24,6 +26,171 @@ #define START_CMD 0x80 #define TRIGGER_MAILBOX 0x01 +/* Default message lengths as per APML command protocol */ +/* CPUID */ +#define CPUID_RD_DATA_LEN 0x8 +#define CPUID_WR_DATA_LEN 0x8 +#define CPUID_RD_REG_LEN 0xa +#define CPUID_WR_REG_LEN 0x9 + +/* CPUID MSR Command Ids */ +#define CPUID_MCA_CMD 0x73 +#define RD_CPUID_CMD 0x91 + +/* input for bulk write to CPUID protocol */ +struct cpu_msr_indata { + u8 wr_len; /* const value */ + u8 rd_len; /* const value */ + u8 proto_cmd; /* const value */ + u8 thread; /* thread number */ + union { + u8 reg_offset[4]; /* input value */ + u32 value; + }; + u8 ext; /* extended function */ +} __packed; + +/* output for bulk read from CPUID protocol */ +struct cpu_msr_outdata { + u8 num_bytes; /* number of bytes return */ + u8 status; /* Protocol status code */ + union { + u64 value; + u8 reg_data[8]; + }; +} __packed; + +#define prepare_cpuid_input_message(input, thread_id, func, ext_func) \ + input.rd_len = CPUID_RD_DATA_LEN, \ + input.wr_len = CPUID_WR_DATA_LEN, \ + input.proto_cmd = RD_CPUID_CMD, \ + input.thread = thread_id << 1, \ + input.value = func, \ + input.ext = ext_func + +/* + * For Mailbox command software alert status bit is set by firmware + * to indicate command completion + * For RMI Rev 0x20, new h/w status bit is introduced. which is used + * by firmware to indicate completion of commands (0x71, 0x72, 0x73). + * wait for the status bit to be set by the firmware before + * reading the data out. + */ +static int sbrmi_wait_status(struct sbrmi_data *data, + int *status, int mask) +{ + int ret, retry = 100; + + do { + ret = regmap_read(data->regmap, SBRMI_STATUS, status); + if (ret < 0) + return ret; + + if (*status & mask) + break; + + /* Wait 1~2 second for firmware to return data out */ + if (retry > 95) + usleep_range(50, 100); + else + usleep_range(10000, 20000); + } while (retry--); + + if (retry < 0) + ret = -ETIMEDOUT; + return ret; +} + +static int sbrmi_get_rev(struct sbrmi_data *data) +{ + struct apml_message msg = { 0 }; + int ret; + + msg.data_in.reg_in[AMD_SBI_REG_OFF_INDEX] = SBRMI_REV; + msg.data_in.reg_in[AMD_SBI_RD_FLAG_INDEX] = 1; + ret = regmap_read(data->regmap, + msg.data_in.reg_in[AMD_SBI_REG_OFF_INDEX], + &msg.data_out.mb_out[AMD_SBI_RD_WR_DATA_INDEX]); + if (ret < 0) + return ret; + + data->rev = msg.data_out.reg_out[AMD_SBI_RD_WR_DATA_INDEX]; + return 0; +} + +/* Read CPUID function protocol */ +static int rmi_cpuid_read(struct sbrmi_data *data, + struct apml_message *msg) +{ + struct cpu_msr_indata input = {0}; + struct cpu_msr_outdata output = {0}; + int val = 0; + int ret, hw_status; + u16 thread; + + mutex_lock(&data->lock); + /* cache the rev value to identify if protocol is supported or not */ + if (!data->rev) { + ret = sbrmi_get_rev(data); + if (ret < 0) + goto exit_unlock; + } + /* CPUID protocol for REV 0x10 is not supported*/ + if (data->rev == 0x10) { + ret = -EOPNOTSUPP; + goto exit_unlock; + } + + thread = msg->data_in.reg_in[AMD_SBI_THREAD_LOW_INDEX] | + msg->data_in.reg_in[AMD_SBI_THREAD_HI_INDEX] << 8; + + /* Thread > 127, Thread128 CS register, 1'b1 needs to be set to 1 */ + if (thread > 127) { + thread -= 128; + val = 1; + } + ret = regmap_write(data->regmap, SBRMI_THREAD128CS, val); + if (ret < 0) + goto exit_unlock; + + prepare_cpuid_input_message(input, thread, + msg->data_in.mb_in[AMD_SBI_RD_WR_DATA_INDEX], + msg->data_in.reg_in[AMD_SBI_EXT_FUNC_INDEX]); + + ret = regmap_bulk_write(data->regmap, CPUID_MCA_CMD, + &input, CPUID_WR_REG_LEN); + if (ret < 0) + goto exit_unlock; + + ret = sbrmi_wait_status(data, &hw_status, HW_ALERT_MASK); + if (ret < 0) + goto exit_unlock; + + ret = regmap_bulk_read(data->regmap, CPUID_MCA_CMD, + &output, CPUID_RD_REG_LEN); + if (ret < 0) + goto exit_unlock; + + ret = regmap_write(data->regmap, SBRMI_STATUS, + HW_ALERT_MASK); + if (ret < 0) + goto exit_unlock; + + if (output.num_bytes != CPUID_RD_REG_LEN - 1) { + ret = -EMSGSIZE; + goto exit_unlock; + } + if (output.status) { + ret = -EPROTOTYPE; + msg->fw_ret_code = output.status; + goto exit_unlock; + } + msg->data_out.cpu_msr_out = output.value; +exit_unlock: + mutex_unlock(&data->lock); + return ret; +} + int rmi_mailbox_xfer(struct sbrmi_data *data, struct apml_message *msg) { @@ -147,6 +314,9 @@ static long sbrmi_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) /* Mailbox protocol */ ret = rmi_mailbox_xfer(data, &msg); break; + case APML_CPUID: + ret = rmi_cpuid_read(data, &msg); + break; default: pr_err("Command:0x%x not recognized\n", msg.cmd); break; diff --git a/drivers/misc/amd-sbi/rmi-core.h b/drivers/misc/amd-sbi/rmi-core.h index b728f5582256..529c8284dec4 100644 --- a/drivers/misc/amd-sbi/rmi-core.h +++ b/drivers/misc/amd-sbi/rmi-core.h @@ -15,7 +15,8 @@ /* SB-RMI registers */ enum sbrmi_reg { - SBRMI_CTRL = 0x01, + SBRMI_REV = 0x00, + SBRMI_CTRL, SBRMI_STATUS, SBRMI_OUTBNDMSG0 = 0x30, SBRMI_OUTBNDMSG1, @@ -34,6 +35,7 @@ enum sbrmi_reg { SBRMI_INBNDMSG6, SBRMI_INBNDMSG7, SBRMI_SW_INTERRUPT, + SBRMI_THREAD128CS = 0x4b, }; /* @@ -56,6 +58,7 @@ struct sbrmi_data { struct mutex lock; u32 pwr_limit_max; u8 dev_static_addr; + u8 rev; }; int rmi_mailbox_xfer(struct sbrmi_data *data, struct apml_message *msg); diff --git a/include/uapi/misc/amd-apml.h b/include/uapi/misc/amd-apml.h index b2d711d422cb..30bb6307993b 100644 --- a/include/uapi/misc/amd-apml.h +++ b/include/uapi/misc/amd-apml.h @@ -7,38 +7,53 @@ #include <linux/types.h> +/* command ID to identify CPUID protocol */ +#define APML_CPUID 0x1000 /* These are byte indexes into data_in and data_out arrays */ #define AMD_SBI_RD_WR_DATA_INDEX 0 #define AMD_SBI_REG_OFF_INDEX 0 #define AMD_SBI_REG_VAL_INDEX 4 #define AMD_SBI_RD_FLAG_INDEX 7 +#define AMD_SBI_THREAD_LOW_INDEX 4 +#define AMD_SBI_THREAD_HI_INDEX 5 +#define AMD_SBI_EXT_FUNC_INDEX 6 #define AMD_SBI_MB_DATA_SIZE 4 struct apml_message { /* message ids: * Mailbox Messages: 0x0 ... 0x999 + * APML_CPUID: 0x1000 */ __u32 cmd; /* * 8 bit data for reg read, * 32 bit data in case of mailbox, + * up to 64 bit in case of cpuid */ union { + __u64 cpu_msr_out; __u32 mb_out[2]; __u8 reg_out[8]; } data_out; /* * [0]...[3] mailbox 32bit input + * cpuid, + * [4][5] cpuid: thread + * [6] cpuid: ext function & read eax/ebx or ecx/edx + * [7:0] -> bits [7:4] -> ext function & + * bit [0] read eax/ebx or ecx/edx * [7] read/write functionality */ union { + __u64 cpu_msr_in; __u32 mb_in[2]; __u8 reg_in[8]; } data_in; /* + * Status code is returned in case of CPUID access * Error code is returned in case of soft mailbox */ __u32 fw_ret_code;