| Message ID | 20230913183348.1349409-4-jithu.joseph@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | IFS support for GNR and SRF | expand |
On Wed, 13 Sep 2023, Jithu Joseph wrote: > Scan image loading flow for newer IFS generations (1 and 2) are slightly > different from that of current generation (0). In newer schemes, > loading need not be done once for each socket as was done in gen0. > > Also the width of CHUNK related bitfields in SCAN_HASHES_STATUS MSR has > increased from 8 -> 16 bits. Similarly there are width differences > for CHUNK_AUTHENTICATION_STATUS too. > > Further the parameter to AUTHENTICATE_AND_COPY_CHUNK is passed > differently in newer generations. > > Signed-off-by: Jithu Joseph <jithu.joseph@intel.com> > Reviewed-by: Tony Luck <tony.luck@intel.com> > Tested-by: Pengfei Xu <pengfei.xu@intel.com> > --- > drivers/platform/x86/intel/ifs/ifs.h | 27 ++++++ > drivers/platform/x86/intel/ifs/load.c | 113 +++++++++++++++++++++++++- > 2 files changed, 138 insertions(+), 2 deletions(-) > > diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h > index d666aeed20fc..886dc74de57d 100644 > --- a/drivers/platform/x86/intel/ifs/ifs.h > +++ b/drivers/platform/x86/intel/ifs/ifs.h > @@ -137,6 +137,8 @@ > #define MSR_CHUNKS_AUTHENTICATION_STATUS 0x000002c5 > #define MSR_ACTIVATE_SCAN 0x000002c6 > #define MSR_SCAN_STATUS 0x000002c7 > +#define MSR_SAF_CTRL 0x000004f0 > + > #define SCAN_NOT_TESTED 0 > #define SCAN_TEST_PASS 1 > #define SCAN_TEST_FAIL 2 > @@ -158,6 +160,19 @@ union ifs_scan_hashes_status { > }; > }; > > +union ifs_scan_hashes_status_gen2 { > + u64 data; > + struct { > + u16 chunk_size; > + u16 num_chunks; > + u8 error_code; > + u32 chunks_in_stride :9; > + u32 rsvd :2; > + u32 max_core_limit :12; > + u32 valid :1; This doesn't look it would be guaranteed to provide the alignment you seem to want for the fields. > + }; > +}; > + > /* MSR_CHUNKS_AUTH_STATUS bit fields */ > union ifs_chunks_auth_status { > u64 data; > @@ -170,6 +185,16 @@ union ifs_chunks_auth_status { > }; > }; > > +union ifs_chunks_auth_status_gen2 { > + u64 data; > + struct { > + u16 valid_chunks; > + u16 total_chunks; > + u8 error_code; > + u32 rsvd :24; Ditto. > + }; > +}; > + > /* MSR_ACTIVATE_SCAN bit fields */ > union ifs_scan { > u64 data; > @@ -230,6 +255,7 @@ struct ifs_test_caps { > * @scan_details: opaque scan status code from h/w > * @cur_batch: number indicating the currently loaded test file > * @generation: IFS test generation enumerated by hardware > + * @chunk_size: size of a test chunk > */ > struct ifs_data { > int loaded_version; > @@ -240,6 +266,7 @@ struct ifs_data { > u64 scan_details; > u32 cur_batch; > u32 generation; > + u32 chunk_size; > }; > > struct ifs_work { > diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c > index 851c97cc6a6b..e8fb03dd8bcf 100644 > --- a/drivers/platform/x86/intel/ifs/load.c > +++ b/drivers/platform/x86/intel/ifs/load.c > @@ -26,6 +26,11 @@ union meta_data { > > #define IFS_HEADER_SIZE (sizeof(struct microcode_header_intel)) > #define META_TYPE_IFS 1 > +#define INVALIDATE_STRIDE (0x1UL) Unnecessary parenthesis. Align. > +#define IFS_GEN_STRIDE_AWARE 2 > +#define AUTH_INTERRUPTED_ERROR 5 > +#define IFS_AUTH_RETRY_CT 10 > + > static struct microcode_header_intel *ifs_header_ptr; /* pointer to the ifs image header */ > static u64 ifs_hash_ptr; /* Address of ifs metadata (hash) */ > static u64 ifs_test_image_ptr; /* 256B aligned address of test pattern */ > @@ -44,7 +49,10 @@ static const char * const scan_hash_status[] = { > static const char * const scan_authentication_status[] = { > [0] = "No error reported", > [1] = "Attempt to authenticate a chunk which is already marked as authentic", > - [2] = "Chunk authentication error. The hash of chunk did not match expected value" > + [2] = "Chunk authentication error. The hash of chunk did not match expected value", > + [3] = "Reserved", > + [4] = "Chunk outside the current stride", > + [5] = "Authentication flow interrupted" Add the trailing comma to avoid the need to touch the line later if more entries are added. > }; > > #define MC_HEADER_META_TYPE_END (0) > @@ -154,6 +162,104 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work) > complete(&ifs_done); > } > > +static int get_num_chunks(int gen, union ifs_scan_hashes_status_gen2 status) > +{ > + return gen >= IFS_GEN_STRIDE_AWARE ? status.chunks_in_stride : status.num_chunks; Remove extra space. > +} > + > +static bool need_copy_scan_hashes(struct ifs_data *ifsd) > +{ > + if (!ifsd->loaded || ifsd->generation < IFS_GEN_STRIDE_AWARE || > + ifsd->loaded_version != ifs_header_ptr->rev) { > + return true; > + } > + return false; IMO, this would be easier to read: return !ifsd->loaded || ifsd->generation < IFS_GEN_STRIDE_AWARE || ifsd->loaded_version != ifs_header_ptr->rev; > +} > + > +static int copy_hashes_authenticate_chunks_gen2(struct device *dev) > +{ > + union ifs_scan_hashes_status_gen2 hashes_status; > + union ifs_chunks_auth_status_gen2 chunk_status; > + u32 err_code, valid_chunks, total_chunks; > + int i, num_chunks, chunk_size; > + union meta_data *ifs_meta; > + int starting_chunk_nr; > + struct ifs_data *ifsd; > + u64 linear_addr, base; > + u64 chunk_table[2]; > + int retry_count; > + > + ifsd = ifs_get_data(dev); > + > + if (need_copy_scan_hashes(ifsd)) { > + wrmsrl(MSR_COPY_SCAN_HASHES, ifs_hash_ptr); > + rdmsrl(MSR_SCAN_HASHES_STATUS, hashes_status.data); > + > + /* enumerate the scan image information */ > + chunk_size = hashes_status.chunk_size * 1024; SZ_1K ? > + err_code = hashes_status.error_code; > + > + num_chunks = get_num_chunks(ifsd->generation, hashes_status); > + > + if (!hashes_status.valid) { > + hashcopy_err_message(dev, err_code); > + return -EIO; > + } > + ifsd->loaded_version = ifs_header_ptr->rev; > + ifsd->chunk_size = chunk_size; > + } else { > + num_chunks = ifsd->valid_chunks; > + chunk_size = ifsd->chunk_size; > + } > + > + if (ifsd->generation >= IFS_GEN_STRIDE_AWARE) { > + wrmsrl(MSR_SAF_CTRL, INVALIDATE_STRIDE); > + rdmsrl(MSR_CHUNKS_AUTHENTICATION_STATUS, chunk_status.data); > + if (chunk_status.valid_chunks != 0) { > + dev_err(dev, "Couldn't invalidate installed stride - %d\n", > + chunk_status.valid_chunks); > + return -EIO; > + } > + } > + > + base = ifs_test_image_ptr; > + ifs_meta = (union meta_data *)find_meta_data(ifs_header_ptr, META_TYPE_IFS); > + starting_chunk_nr = ifs_meta->starting_chunk; > + > + /* scan data authentication and copy chunks to secured memory */ > + for (i = 0; i < num_chunks; i++) { > + retry_count = IFS_AUTH_RETRY_CT; > + linear_addr = base + i * chunk_size; > + > + chunk_table[0] = starting_chunk_nr + i; > + chunk_table[1] = linear_addr; > +auth_retry: > + wrmsrl(MSR_AUTHENTICATE_AND_COPY_CHUNK, (u64)chunk_table); > + rdmsrl(MSR_CHUNKS_AUTHENTICATION_STATUS, chunk_status.data); > + err_code = chunk_status.error_code; > + if (err_code == AUTH_INTERRUPTED_ERROR && --retry_count) > + goto auth_retry; do { } while (); > + if (err_code) { > + ifsd->loading_error = true; > + auth_err_message(dev, err_code); > + return -EIO; > + } > + } > + > + valid_chunks = chunk_status.valid_chunks; > + total_chunks = chunk_status.total_chunks; > + > + if (valid_chunks != total_chunks) { > + ifsd->loading_error = true; > + dev_err(dev, "Couldn't authenticate all the chunks.Authenticated %d total %d.\n", Missing whitespace. > + valid_chunks, total_chunks); > + return -EIO; > + } > + ifsd->valid_chunks = valid_chunks; > + > + return 0; > +} > + > static int validate_ifs_metadata(struct device *dev) > { > struct ifs_data *ifsd = ifs_get_data(dev); > @@ -206,7 +312,9 @@ static int scan_chunks_sanity_check(struct device *dev) > return ret; > > ifsd->loading_error = false; > - ifsd->loaded_version = ifs_header_ptr->rev; > + > + if (ifsd->generation > 0) > + return copy_hashes_authenticate_chunks_gen2(dev); > > /* copy the scan hash and authenticate per package */ > cpus_read_lock(); > @@ -226,6 +334,7 @@ static int scan_chunks_sanity_check(struct device *dev) > ifs_pkg_auth[curr_pkg] = 1; > } > ret = 0; > + ifsd->loaded_version = ifs_header_ptr->rev; > out: > cpus_read_unlock(); > >
On 9/15/2023 9:46 AM, Ilpo Järvinen wrote: > On Wed, 13 Sep 2023, Jithu Joseph wrote: > >> Scan image loading flow for newer IFS generations (1 and 2) are slightly >> different from that of current generation (0). In newer schemes, >> loading need not be done once for each socket as was done in gen0. >> >> Also the width of CHUNK related bitfields in SCAN_HASHES_STATUS MSR has >> increased from 8 -> 16 bits. Similarly there are width differences >> for CHUNK_AUTHENTICATION_STATUS too. >> >> Further the parameter to AUTHENTICATE_AND_COPY_CHUNK is passed >> differently in newer generations. >> >> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com> >> Reviewed-by: Tony Luck <tony.luck@intel.com> >> Tested-by: Pengfei Xu <pengfei.xu@intel.com> >> --- >> drivers/platform/x86/intel/ifs/ifs.h | 27 ++++++ >> drivers/platform/x86/intel/ifs/load.c | 113 +++++++++++++++++++++++++- >> 2 files changed, 138 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h >> index d666aeed20fc..886dc74de57d 100644 >> --- a/drivers/platform/x86/intel/ifs/ifs.h >> +++ b/drivers/platform/x86/intel/ifs/ifs.h >> @@ -137,6 +137,8 @@ >> #define MSR_CHUNKS_AUTHENTICATION_STATUS 0x000002c5 >> #define MSR_ACTIVATE_SCAN 0x000002c6 >> #define MSR_SCAN_STATUS 0x000002c7 >> +#define MSR_SAF_CTRL 0x000004f0 >> + >> #define SCAN_NOT_TESTED 0 >> #define SCAN_TEST_PASS 1 >> #define SCAN_TEST_FAIL 2 >> @@ -158,6 +160,19 @@ union ifs_scan_hashes_status { >> }; >> }; >> >> +union ifs_scan_hashes_status_gen2 { >> + u64 data; >> + struct { >> + u16 chunk_size; >> + u16 num_chunks; >> + u8 error_code; >> + u32 chunks_in_stride :9; >> + u32 rsvd :2; >> + u32 max_core_limit :12; >> + u32 valid :1; > > This doesn't look it would be guaranteed to provide the alignment you seem > to want for the fields. To Quote Tony from an earlier response to a similar query[1] "This driver is X86_64 specific (and it seems incredibly unlikely that some other architecture will copy this h/w interface so closely that they want to re-use this driver. There's an x86_64 ABI that says how bitfields in C are allocated." [1] https://lore.kernel.org/lkml/SJ1PR11MB6083EBD2D2826E0A247AF242FCD19@SJ1PR11MB6083.namprd11.prod.outlook.com/ Agree to the rest of your comments ... will revise them as per your suggestion. Jithu
On Fri, 15 Sep 2023, Joseph, Jithu wrote: > On 9/15/2023 9:46 AM, Ilpo Järvinen wrote: > > On Wed, 13 Sep 2023, Jithu Joseph wrote: > > > >> Scan image loading flow for newer IFS generations (1 and 2) are slightly > >> different from that of current generation (0). In newer schemes, > >> loading need not be done once for each socket as was done in gen0. > >> > >> Also the width of CHUNK related bitfields in SCAN_HASHES_STATUS MSR has > >> increased from 8 -> 16 bits. Similarly there are width differences > >> for CHUNK_AUTHENTICATION_STATUS too. > >> > >> Further the parameter to AUTHENTICATE_AND_COPY_CHUNK is passed > >> differently in newer generations. > >> > >> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com> > >> Reviewed-by: Tony Luck <tony.luck@intel.com> > >> Tested-by: Pengfei Xu <pengfei.xu@intel.com> > >> --- > >> drivers/platform/x86/intel/ifs/ifs.h | 27 ++++++ > >> drivers/platform/x86/intel/ifs/load.c | 113 +++++++++++++++++++++++++- > >> 2 files changed, 138 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h > >> index d666aeed20fc..886dc74de57d 100644 > >> --- a/drivers/platform/x86/intel/ifs/ifs.h > >> +++ b/drivers/platform/x86/intel/ifs/ifs.h > >> @@ -137,6 +137,8 @@ > >> #define MSR_CHUNKS_AUTHENTICATION_STATUS 0x000002c5 > >> #define MSR_ACTIVATE_SCAN 0x000002c6 > >> #define MSR_SCAN_STATUS 0x000002c7 > >> +#define MSR_SAF_CTRL 0x000004f0 > >> + > >> #define SCAN_NOT_TESTED 0 > >> #define SCAN_TEST_PASS 1 > >> #define SCAN_TEST_FAIL 2 > >> @@ -158,6 +160,19 @@ union ifs_scan_hashes_status { > >> }; > >> }; > >> > >> +union ifs_scan_hashes_status_gen2 { > >> + u64 data; > >> + struct { > >> + u16 chunk_size; > >> + u16 num_chunks; > >> + u8 error_code; > >> + u32 chunks_in_stride :9; > >> + u32 rsvd :2; > >> + u32 max_core_limit :12; > >> + u32 valid :1; > > > > This doesn't look it would be guaranteed to provide the alignment you seem > > to want for the fields. > > To Quote Tony from an earlier response to a similar query[1] > > "This driver is X86_64 specific (and it seems > incredibly unlikely that some other architecture will copy this h/w > interface so closely that they want to re-use this driver. There's an x86_64 > ABI that says how bitfields in C are allocated." > > > > [1] https://lore.kernel.org/lkml/SJ1PR11MB6083EBD2D2826E0A247AF242FCD19@SJ1PR11MB6083.namprd11.prod.outlook.com/ Hi, I was actually not that worried about this from portability perspective but from placing u32 bitfield after u8 which according to some info I read about this topic way back would not get the alignment you're after. As I could not find anything concrete which "says" (does somebody have some reference for something which actually documents this?) something about x86_64 I ended up using pahole and checked that gcc did not leave hole there so it seems to be fine after all. I think Tony's "proof" is pretty invalid. He doesn't differentiate HW interface related bitfields from those which are not HW interface related (to the extent that in fact most of those bitfields likely are not HW interface related).
> I think Tony's "proof" is pretty invalid. He doesn't differentiate > HW interface related bitfields from those which are not HW interface > related (to the extent that in fact most of those bitfields likely are not > HW interface related). When I made that comment it was about a patch series that used bitfields to decode the subfields in Intel model specific MSRs. I think that's true of use in this series too. I think most of these are for MSR decode. The one mentioned in this thread: "union ifs_scan_hashes_status_gen2 {" definitely is. Are there any that are not for MSRs? I'd also claim "Intel specific" if there are some decoding parts of the Intel scan file format. -Tony
On Mon, 18 Sep 2023, Luck, Tony wrote: > > I think Tony's "proof" is pretty invalid. He doesn't differentiate > > HW interface related bitfields from those which are not HW interface > > related (to the extent that in fact most of those bitfields likely are not > > HW interface related). > > When I made that comment it was about a patch series that used > bitfields to decode the subfields in Intel model specific MSRs. I > think that's true of use in this series too. But your grep in [1] was not limited to such cases nor to HW interface related ones in general. What I meant with your proof being invalid is that the argument against bitfields have been related to using them with HW interfaces, not just generic use of the bitfields (even if there have been some performance issues in that area as well). Simply grepping through include/ directly is not going to tell anything if the bitfield in question is related to HW interfaces or not. > I think most of these are for MSR decode. The one mentioned in > this thread: "union ifs_scan_hashes_status_gen2 {" definitely is. > > Are there any that are not for MSRs? I'd also claim "Intel > specific" if there are some decoding parts of the Intel scan > file format. First of all, I already checked myself that the alignment is not incorrect so I don't find it as problematic as I thought it was (I did not even flag all bitfield addition in the patches, just the cases were u8 was followed by u32 bitfield which I thought is not going to work because of something I read about this topic some time ago claimed if the type changes the bitfield does not carry over). Since you replied, would you happen to have a pointer something that tells (in writing) how the bitfields in C are allocated in case of x86_64? I spent a bit of time trying to find something but came up nothing. [1] https://lore.kernel.org/lkml/SJ1PR11MB6083EBD2D2826E0A247AF242FCD19@SJ1PR11MB6083.namprd11.prod.outlook.com/
> Since you replied, would you happen to have a pointer something that tells > (in writing) how the bitfields in C are allocated in case of x86_64? I > spent a bit of time trying to find something but came up nothing. Search engines don't seem to be as good as they used to be (or I'm not as good at finding the right query). There's a bit on page 14 of: https://refspecs.linuxbase.org/elf/x86_64-abi-0.99.pdf that says bit fields are allocated right to left (which is a good start). But I thought there was a doc somewhere that gave more detail about alignment of bitfields. -Tony
On Mon, 18 Sep 2023, Luck, Tony wrote: > > Since you replied, would you happen to have a pointer something that tells > > (in writing) how the bitfields in C are allocated in case of x86_64? I > > spent a bit of time trying to find something but came up nothing. > > Search engines don't seem to be as good as they used to be (or I'm not as > good at finding the right query). > > There's a bit on page 14 of: > > https://refspecs.linuxbase.org/elf/x86_64-abi-0.99.pdf > > that says bit fields are allocated right to left (which is a good > start). But I thought there was a doc somewhere that gave > more detail about alignment of bitfields. Thanks, appreciated. In this case it is not just about the bitfield itself nor the bit allocation order but sharing the storage unit with another member, and to further complicate things, members have different alignment requirement too (32-bit aligned u8 followed by u32 bitfield). The document states: "Bit-fields obey the same size and alignment rules as other structure and union members." which seems to contradict my test that found that the u32 bitfield won't be 32-bit aligned but gets combined with the 32-bit aligned u8. Perhaps it's because the total number of bits still fits to 32 bits so the bitfield doesn't cross the 32-bit boundary even when combined with the preceeding u8.
On 9/18/2023 9:29 AM, Ilpo Järvinen wrote: > > In this case it is not just about the bitfield itself nor the bit > allocation order but sharing the storage unit with another member, and to > further complicate things, members have different alignment requirement > too (32-bit aligned u8 followed by u32 bitfield). > I too verified that the size of the whole structure matches that of MSR 64 bits (8 bytes). Initially when IFS scan was added the all MSR structure members were bit-fields, later there was a suggestion to use basic C types if applicable during subsequent Array BIST patch series. I followed this approach with the current patch series . I will change the current series to use all bit-field MSR structures in v2, given mixing basic types and bitfields is a a source of confusion Jithu
On 9/18/23 09:51, Joseph, Jithu wrote: > > On 9/18/2023 9:29 AM, Ilpo Järvinen wrote: > >> In this case it is not just about the bitfield itself nor the bit >> allocation order but sharing the storage unit with another member, and to >> further complicate things, members have different alignment requirement >> too (32-bit aligned u8 followed by u32 bitfield). >> > I too verified that the size of the whole structure matches that of MSR 64 bits (8 bytes). > > Initially when IFS scan was added the all MSR structure members were bit-fields, later there was a suggestion to > use basic C types if applicable during subsequent Array BIST patch series. I followed this approach with the current patch series . > > I will change the current series to use all bit-field MSR structures in v2, given mixing basic types and bitfields is a a source of confusion That's the wrong direction. :) What is more obviously correct. This: struct { u16 valid_chunks; u16 total_chunks; u8 error_code; u8 rsvd1; u8 rsvd2; u8 rsvd3; }; or this: struct { u16 valid_chunks; u16 total_chunks; u8 error_code; u32 error_code :8; u32 rsvd :24; }; ?
On 9/18/2023 9:58 AM, Dave Hansen wrote: > On 9/18/23 09:51, Joseph, Jithu wrote: >> >> On 9/18/2023 9:29 AM, Ilpo Järvinen wrote: >> >>> In this case it is not just about the bitfield itself nor the bit >>> allocation order but sharing the storage unit with another member, and to >>> further complicate things, members have different alignment requirement >>> too (32-bit aligned u8 followed by u32 bitfield). >>> >> I too verified that the size of the whole structure matches that of MSR 64 bits (8 bytes). >> >> Initially when IFS scan was added the all MSR structure members were bit-fields, later there was a suggestion to >> use basic C types if applicable during subsequent Array BIST patch series. I followed this approach with the current patch series . >> >> I will change the current series to use all bit-field MSR structures in v2, given mixing basic types and bitfields is a a source of confusion > > That's the wrong direction. :) > > What is more obviously correct. This: > > struct { > u16 valid_chunks; > u16 total_chunks; > u8 error_code; > u8 rsvd1; > u8 rsvd2; > u8 rsvd3; > }; > > or this: > > struct { > u16 valid_chunks; > u16 total_chunks; > u32 error_code :8; > u32 rsvd :24; > }; I will go with the second pattern above, given that pattern can be followed for other MSR structures too, where fields doesn't split as evenly Jithu
diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h index d666aeed20fc..886dc74de57d 100644 --- a/drivers/platform/x86/intel/ifs/ifs.h +++ b/drivers/platform/x86/intel/ifs/ifs.h @@ -137,6 +137,8 @@ #define MSR_CHUNKS_AUTHENTICATION_STATUS 0x000002c5 #define MSR_ACTIVATE_SCAN 0x000002c6 #define MSR_SCAN_STATUS 0x000002c7 +#define MSR_SAF_CTRL 0x000004f0 + #define SCAN_NOT_TESTED 0 #define SCAN_TEST_PASS 1 #define SCAN_TEST_FAIL 2 @@ -158,6 +160,19 @@ union ifs_scan_hashes_status { }; }; +union ifs_scan_hashes_status_gen2 { + u64 data; + struct { + u16 chunk_size; + u16 num_chunks; + u8 error_code; + u32 chunks_in_stride :9; + u32 rsvd :2; + u32 max_core_limit :12; + u32 valid :1; + }; +}; + /* MSR_CHUNKS_AUTH_STATUS bit fields */ union ifs_chunks_auth_status { u64 data; @@ -170,6 +185,16 @@ union ifs_chunks_auth_status { }; }; +union ifs_chunks_auth_status_gen2 { + u64 data; + struct { + u16 valid_chunks; + u16 total_chunks; + u8 error_code; + u32 rsvd :24; + }; +}; + /* MSR_ACTIVATE_SCAN bit fields */ union ifs_scan { u64 data; @@ -230,6 +255,7 @@ struct ifs_test_caps { * @scan_details: opaque scan status code from h/w * @cur_batch: number indicating the currently loaded test file * @generation: IFS test generation enumerated by hardware + * @chunk_size: size of a test chunk */ struct ifs_data { int loaded_version; @@ -240,6 +266,7 @@ struct ifs_data { u64 scan_details; u32 cur_batch; u32 generation; + u32 chunk_size; }; struct ifs_work { diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c index 851c97cc6a6b..e8fb03dd8bcf 100644 --- a/drivers/platform/x86/intel/ifs/load.c +++ b/drivers/platform/x86/intel/ifs/load.c @@ -26,6 +26,11 @@ union meta_data { #define IFS_HEADER_SIZE (sizeof(struct microcode_header_intel)) #define META_TYPE_IFS 1 +#define INVALIDATE_STRIDE (0x1UL) +#define IFS_GEN_STRIDE_AWARE 2 +#define AUTH_INTERRUPTED_ERROR 5 +#define IFS_AUTH_RETRY_CT 10 + static struct microcode_header_intel *ifs_header_ptr; /* pointer to the ifs image header */ static u64 ifs_hash_ptr; /* Address of ifs metadata (hash) */ static u64 ifs_test_image_ptr; /* 256B aligned address of test pattern */ @@ -44,7 +49,10 @@ static const char * const scan_hash_status[] = { static const char * const scan_authentication_status[] = { [0] = "No error reported", [1] = "Attempt to authenticate a chunk which is already marked as authentic", - [2] = "Chunk authentication error. The hash of chunk did not match expected value" + [2] = "Chunk authentication error. The hash of chunk did not match expected value", + [3] = "Reserved", + [4] = "Chunk outside the current stride", + [5] = "Authentication flow interrupted" }; #define MC_HEADER_META_TYPE_END (0) @@ -154,6 +162,104 @@ static void copy_hashes_authenticate_chunks(struct work_struct *work) complete(&ifs_done); } +static int get_num_chunks(int gen, union ifs_scan_hashes_status_gen2 status) +{ + return gen >= IFS_GEN_STRIDE_AWARE ? status.chunks_in_stride : status.num_chunks; +} + +static bool need_copy_scan_hashes(struct ifs_data *ifsd) +{ + if (!ifsd->loaded || ifsd->generation < IFS_GEN_STRIDE_AWARE || + ifsd->loaded_version != ifs_header_ptr->rev) { + return true; + } + return false; +} + +static int copy_hashes_authenticate_chunks_gen2(struct device *dev) +{ + union ifs_scan_hashes_status_gen2 hashes_status; + union ifs_chunks_auth_status_gen2 chunk_status; + u32 err_code, valid_chunks, total_chunks; + int i, num_chunks, chunk_size; + union meta_data *ifs_meta; + int starting_chunk_nr; + struct ifs_data *ifsd; + u64 linear_addr, base; + u64 chunk_table[2]; + int retry_count; + + ifsd = ifs_get_data(dev); + + if (need_copy_scan_hashes(ifsd)) { + wrmsrl(MSR_COPY_SCAN_HASHES, ifs_hash_ptr); + rdmsrl(MSR_SCAN_HASHES_STATUS, hashes_status.data); + + /* enumerate the scan image information */ + chunk_size = hashes_status.chunk_size * 1024; + err_code = hashes_status.error_code; + + num_chunks = get_num_chunks(ifsd->generation, hashes_status); + + if (!hashes_status.valid) { + hashcopy_err_message(dev, err_code); + return -EIO; + } + ifsd->loaded_version = ifs_header_ptr->rev; + ifsd->chunk_size = chunk_size; + } else { + num_chunks = ifsd->valid_chunks; + chunk_size = ifsd->chunk_size; + } + + if (ifsd->generation >= IFS_GEN_STRIDE_AWARE) { + wrmsrl(MSR_SAF_CTRL, INVALIDATE_STRIDE); + rdmsrl(MSR_CHUNKS_AUTHENTICATION_STATUS, chunk_status.data); + if (chunk_status.valid_chunks != 0) { + dev_err(dev, "Couldn't invalidate installed stride - %d\n", + chunk_status.valid_chunks); + return -EIO; + } + } + + base = ifs_test_image_ptr; + ifs_meta = (union meta_data *)find_meta_data(ifs_header_ptr, META_TYPE_IFS); + starting_chunk_nr = ifs_meta->starting_chunk; + + /* scan data authentication and copy chunks to secured memory */ + for (i = 0; i < num_chunks; i++) { + retry_count = IFS_AUTH_RETRY_CT; + linear_addr = base + i * chunk_size; + + chunk_table[0] = starting_chunk_nr + i; + chunk_table[1] = linear_addr; +auth_retry: + wrmsrl(MSR_AUTHENTICATE_AND_COPY_CHUNK, (u64)chunk_table); + rdmsrl(MSR_CHUNKS_AUTHENTICATION_STATUS, chunk_status.data); + err_code = chunk_status.error_code; + if (err_code == AUTH_INTERRUPTED_ERROR && --retry_count) + goto auth_retry; + if (err_code) { + ifsd->loading_error = true; + auth_err_message(dev, err_code); + return -EIO; + } + } + + valid_chunks = chunk_status.valid_chunks; + total_chunks = chunk_status.total_chunks; + + if (valid_chunks != total_chunks) { + ifsd->loading_error = true; + dev_err(dev, "Couldn't authenticate all the chunks.Authenticated %d total %d.\n", + valid_chunks, total_chunks); + return -EIO; + } + ifsd->valid_chunks = valid_chunks; + + return 0; +} + static int validate_ifs_metadata(struct device *dev) { struct ifs_data *ifsd = ifs_get_data(dev); @@ -206,7 +312,9 @@ static int scan_chunks_sanity_check(struct device *dev) return ret; ifsd->loading_error = false; - ifsd->loaded_version = ifs_header_ptr->rev; + + if (ifsd->generation > 0) + return copy_hashes_authenticate_chunks_gen2(dev); /* copy the scan hash and authenticate per package */ cpus_read_lock(); @@ -226,6 +334,7 @@ static int scan_chunks_sanity_check(struct device *dev) ifs_pkg_auth[curr_pkg] = 1; } ret = 0; + ifsd->loaded_version = ifs_header_ptr->rev; out: cpus_read_unlock();