On Wed, 13 Sep 2023, Jithu Joseph wrote:
> Make changes to scan test flow such that MSRs are populated
> appropriately based on the generation supported by hardware.
>
> Width of chunk related bitfields is ACTIVATE_SCAN and SCAN_STATUS MSRs
> are different in newer IFS generation compared to gen0.
>
> Signed-off-by: Jithu Joseph <jithu.joseph@xxxxxxxxx>
> Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>
> Tested-by: Pengfei Xu <pengfei.xu@xxxxxxxxx>
> ---
> drivers/platform/x86/intel/ifs/ifs.h | 14 ++++++++++++++
> drivers/platform/x86/intel/ifs/runtest.c | 23 ++++++++++++++++++-----
> 2 files changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index 886dc74de57d..3265a6d8a6f3 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -205,6 +205,12 @@ union ifs_scan {
> u32 delay :31;
> u32 sigmce :1;
> };
> + struct {
> + u16 start;
> + u16 stop;
> + u32 delay :31;
> + u32 sigmce :1;
> + } gen2;
I don't like the way old struct is left without genx naming. It makes the
code below more confusing as is.
> };
>
> /* MSR_SCAN_STATUS bit fields */
> @@ -219,6 +225,14 @@ union ifs_status {
> u32 control_error :1;
> u32 signature_error :1;
> };
> + struct {
> + u16 chunk_num;
> + u16 chunk_stop_index;
> + u8 error_code;
> + u32 rsvd1 :22;
> + u32 control_error :1;
> + u32 signature_error :1;
Again, I don't think the alignment will be correct in this case.
> + } gen2;
> };
>
> /* MSR_ARRAY_BIST bit fields */
> diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
> index 1061eb7ec399..4bbab6be2fa2 100644
> --- a/drivers/platform/x86/intel/ifs/runtest.c
> +++ b/drivers/platform/x86/intel/ifs/runtest.c
> @@ -171,6 +171,8 @@ static void ifs_test_core(int cpu, struct device *dev)
> union ifs_status status;
> unsigned long timeout;
> struct ifs_data *ifsd;
> + int to_start, to_stop;
> + int status_chunk;
> u64 msrvals[2];
> int retries;
>
> @@ -179,13 +181,21 @@ static void ifs_test_core(int cpu, struct device *dev)
> activate.rsvd = 0;
> activate.delay = IFS_THREAD_WAIT;
> activate.sigmce = 0;
> - activate.start = 0;
> - activate.stop = ifsd->valid_chunks - 1;
> + to_start = 0;
> + to_stop = ifsd->valid_chunks - 1;
> +
> + if (ifsd->generation) {
> + activate.gen2.start = to_start;
> + activate.gen2.stop = to_stop;
> + } else {
> + activate.start = to_start;
> + activate.stop = to_stop;
> + }
>
> timeout = jiffies + HZ / 2;
> retries = MAX_IFS_RETRIES;
>
> - while (activate.start <= activate.stop) {
> + while (to_start <= to_stop) {
> if (time_after(jiffies, timeout)) {
> status.error_code = IFS_SW_TIMEOUT;
> break;
> @@ -202,7 +212,8 @@ static void ifs_test_core(int cpu, struct device *dev)
> if (!can_restart(status))
> break;
>
> - if (status.chunk_num == activate.start) {
> + status_chunk = ifsd->generation ? status.gen2.chunk_num : status.chunk_num;
> + if (status_chunk == to_start) {
> /* Check for forward progress */
> if (--retries == 0) {
> if (status.error_code == IFS_NO_ERROR)
> @@ -211,7 +222,9 @@ static void ifs_test_core(int cpu, struct device *dev)
> }
> } else {
> retries = MAX_IFS_RETRIES;
> - activate.start = status.chunk_num;
> + ifsd->generation ? (activate.gen2.start = status_chunk) :
> + (activate.start = status_chunk);
Misaligned.
> + to_start = status_chunk;
> }
> }
>
>
--
i.
