On Tue, Jun 03, 2025 at 03:26:47PM -0700, Dan Williams wrote:
> Alexey Kardashevskiy wrote:
> [..]
> > >>> +static int pci_tsm_accept(struct pci_dev *pdev)
> > >>> +{
> > >>> + struct pci_tsm_pf0 *tsm = to_pci_tsm_pf0(pdev->tsm);
> > >>> + int rc;
> > >>> +
> > >>> + struct mutex *lock __free(tsm_ops_unlock) = tsm_ops_lock(tsm);
> > >>
> > >> Add an empty line.
> > >
> > > I think we, as a community, are still figuring out the coding-style
> > > around scope-based cleanup declarations, but I would argue, no empty
> > > line required after mid-function variable declarations. Now, in this
> > > case it is arguably not "mid-function", but all the other occurrences of
> > > tsm_ops_lock() are checking the result on the immediate next line.
> >
> > Do not really care as much :)
>
> Hey, what's kernel development without little side-arguments about
> whitespace? Will leave it alone for now.
I don't think places should be open coding "free" functions for
mutexes. We already have support for interruptable mutexes in
cleanup.h. The syntax is unfriendly and that seems to have been a
deliberate decision. Meaning if you can't stomache the syntax then
probably don't use cleanup.h, so don't open code it?
> My heartburn with that is that there is an indefinite amount of time
> whereby a device is MSE + BME active without any driver to deal with the
> consequences. For example, what if the device needs some form of reset /
> re-initialization to quiet an engine or silence an interrupt that
> immediately starts firing upon the LOCKED -> RUN transition. Userspace
> is not in a good position to make judgements about the state of the
> device outside of the Interface Report.
That sounds horrible, I'd expect a device to be largely reset and
quiet after entering T=1, otherwise I'd fear there is some way left
over junk from the non-secure state, programmed by the untrusted
hypervisor, could leak into the secure state.
Jason
