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@xxxxxxxxx>
> >> Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>
> >> Tested-by: Pengfei Xu <pengfei.xu@xxxxxxxxx>
> >> ---
> >> 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
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.
