linux-kernel - Re: [PATCH] tpm_tis: fix stall after iowrite*()s

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 15 Aug 2017 15:10:33 -0500
From:   Haris Okanovic <haris.okanovic@...com>
To:     Alexander Stein <alexander.stein@...tec-electronic.com>
Cc:     linux-rt-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        tpmdd-devel@...ts.sourceforge.net, harisokn@...il.com,
        julia.cartwright@...com, gratian.crisan@...com,
        scott.hartman@...com, chris.graf@...com, brad.mouring@...com,
        jonathan.david@...com, peterhuewe@....de, tpmdd@...horst.net,
        jarkko.sakkinen@...ux.intel.com, jgunthorpe@...idianresearch.com,
        eric.gardiner@...com
Subject: Re: [PATCH] tpm_tis: fix stall after iowrite*()s



On 08/15/2017 01:11 AM, Alexander Stein wrote:
> On Monday 14 August 2017 17:53:47, Haris Okanovic wrote:
>> --- a/drivers/char/tpm/tpm_tis.c
>> +++ b/drivers/char/tpm/tpm_tis.c
>> @@ -52,6 +52,22 @@ static inline struct tpm_tis_tcg_phy
>> *to_tpm_tis_tcg_phy(struct tpm_tis_data *da return container_of(data,
>> struct tpm_tis_tcg_phy, priv);
>>   }
>>
>> +static inline void tpm_tis_iowrite8(u8 b, void __iomem *iobase, u32 addr)
>> +{
>> +	iowrite8(b, iobase + addr);
>> +#ifdef CONFIG_PREEMPT_RT_FULL
>> +	ioread8(iobase + TPM_ACCESS(0));
>> +#endif
>> +}
> 
> Maybe add some comment why an iorad8 is actually requried after each write on
> RT. Currently it is rather obvious why this additional read is necessary. But
> is this still the case in a year?
> 

Sure. I re-added the tpm_tis_flush() function with comments to explain 
what's going. Calling it from tpm_tis_iowrite8() and tpm_tis_iowrite32().

Will post a PATCH v2 shortly.

>> +static inline void tpm_tis_iowrite32(u32 b, void __iomem *iobase, u32 addr)
>> +{
>> +	iowrite32(b, iobase + addr);
>> +#ifdef CONFIG_PREEMPT_RT_FULL
>> +	ioread8(iobase + TPM_ACCESS(0));
>> +#endif
>> +}
> 
> Same applies here. Or add a comment above both functions describing their
> purpose.
> 
> Just my 2 cents
> 
> Best regards,
> Alexander
>