Re: [PATCH v3] tpm: Disable RNG for all AMD fTPMs

["Jason A. Donenfeld" <Jason@zx2c4.com>]

On Mon, Aug 07, 2023 at 08:26:03PM -0700, Linus Torvalds wrote:
> On Mon, 7 Aug 2023 at 17:39, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > I'm not sure what's best or what Linus prefers. Linus - Jarkko sent you
> > the wrong version patch. Do you want a fixup patch that accounts for the
> > difference, and then I'll address the stable@ metadata deficiency
> > manually by talking to Greg, or would you rather some merge commit
> > magic, or something else?
> 
> Either works for me, whatever ends up being easiest.
> 
> However, looking at that v3 patch, that "should we enable/disable the
> hwrng" is now repeated *three* times, and that first one is
> 
>   if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
> -     tpm_amd_is_rng_defective(chip))
> +     chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
> 
> and wants fixing anyway: you want parenthesis around the '&'.
> 
> Yes, yes, it works (because bitwise ops have higher precedence than
> logical ones), but let's not do that.
> 
> But more importantly, can we just have a single helper inline function
> for this and *not* repeat the same multi-line expression three times
> (just in negated and then 2x non-negated format)?
> 
> That test is ugly anyway. Why is "tpm_is_firmware_upgrade()" a wrapper
> function around testing "chip->flags", but then right next to it it
> tests them explicitly.
> 
> So if we have to re-do this all, let's re-do it properly. Ok?
> 
> Thinking about it, I do guess that makes it easier to just send an
> incremental patch on top.

Alright, looks like Mario took care of that:
https://lore.kernel.org/all/20230808041229.22514-1-mario.limonciello@amd.com/
Once this is in your tree I'll ping Greg about the right stable versions
to make up for the wrong tag.

Jason