linux-kernel - Re: [PATCH v10 4/5] tpm: Add a driver for Loongson TPM device

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]

Message-ID: <CAGxU2F44BTLzY9T+7yYo+XqPuZky5nHd9+BtUy4QFnoVsehpDg@mail.gmail.com>
Date: Wed, 28 May 2025 11:36:38 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: Qunqin Zhao <zhaoqunqin@...ngson.cn>
Cc: lee@...nel.org, herbert@...dor.apana.org.au, jarkko@...nel.org, 
	linux-kernel@...r.kernel.org, loongarch@...ts.linux.dev, davem@...emloft.net, 
	linux-crypto@...r.kernel.org, peterhuewe@....de, jgg@...pe.ca, 
	linux-integrity@...r.kernel.org, Yinggang Gu <guyinggang@...ngson.cn>, 
	Huacai Chen <chenhuacai@...ngson.cn>
Subject: Re: [PATCH v10 4/5] tpm: Add a driver for Loongson TPM device

On Wed, 28 May 2025 at 11:25, Qunqin Zhao <zhaoqunqin@...ngson.cn> wrote:
>
>
> 在 2025/5/28 下午5:00, Stefano Garzarella 写道:
> > On Wed, May 28, 2025 at 04:42:05PM +0800, Qunqin Zhao wrote:
> >>
> >> 在 2025/5/28 下午3:57, Stefano Garzarella 写道:
> >>>> +    chip = tpmm_chip_alloc(dev, &tpm_loongson_ops);
> >>>> +    if (IS_ERR(chip))
> >>>> +        return PTR_ERR(chip);
> >>>> +    chip->flags = TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_IRQ;
> >>>
> >>> Why setting TPM_CHIP_FLAG_IRQ?
> >>
> >> When tpm_engine completes  TPM_CC* command,
> >>
> >> the hardware will indeed trigger an interrupt to the kernel.
> >
> > IIUC that is hidden by loongson_se_send_engine_cmd(), that for this
> > driver is completely synchronous, no?
> >
> >>
> >>>
> >>> IIUC this driver is similar to ftpm and svsm where the send is
> >>> synchronous so having .status, .cancel, etc. set to 0 should be
> >>> enough to call .recv() just after send() in tpm_try_transmit(). See
> >>> commit 980a573621ea ("tpm: Make chip->{status,cancel,req_canceled}
> >>> opt")
> >> The send callback would wait until the TPM_CC* command complete. We
> >> don't need a poll.
> >
> > Right, that's what I was saying too, send() is synchronous (as in ftpm
> > and svsm). The polling in tpm_try_transmit() is already skipped since
> > we are setting .status = 0, .req_complete_mask = 0, .req_complete_val
> > = 0, etc. so IMHO this is exactly the same of ftpm and svsm, so we
> > don't need to set TPM_CHIP_FLAG_IRQ.
>
> I see,  but why not skip polling directly in "if (chip->flags &
> TPM_CHIP_FLAG_IRQ)"  instead of do while?

This is exactly what I'm doing in the series I linked before [1] but
without leveraging TPM_CHIP_FLAG_IRQ's hack

>
> And TPM_CHIP_FLAG_IRQ flag can remind us this hardware is "IRQ" not "POLL".

But you're not handling IRQ through this driver, so for the tpm
interface sending tpm commands through this driver is not interrupt
based or needs polling, but it's synchronous, so this is why I'm
adding TPM_CHIP_FLAG_SYNC in [1] and I think I can include the changes
I did for ftpm and svsm also for this driver in that series.

So my suggestion in this patch is to do something similar of what we
already have for those driver, and we can set TPM_CHIP_FLAG_SYNC when
my series will be merged (or I'll include the changes if this series
will go first).

Thanks,
Stefano

[1] https://lore.kernel.org/linux-integrity/20250514134630.137621-1-sgarzare@redhat.com/

>
> Thanks,
>
> Qunqin.
>
> >
> > Thanks,
> > Stefano
>