[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] tpm: add backend for mssim
From: |
James Bottomley |
Subject: |
Re: [PATCH] tpm: add backend for mssim |
Date: |
Wed, 14 Dec 2022 07:47:28 -0500 |
User-agent: |
Evolution 3.42.4 |
On Wed, 2022-12-14 at 11:31 +0000, Daniel P. Berrangé wrote:
> On Mon, Dec 12, 2022 at 05:06:05PM -0500, James Bottomley wrote:
> > On Mon, 2022-12-12 at 15:47 +0000, Daniel P. Berrangé wrote:
> > > Copy'ing Markus for QAPI design feedback.
> > >
> > > On Sat, Dec 10, 2022 at 12:10:18PM -0500, James Bottomley wrote:
> > [...]
> > > > +##
> > > > +# @TPMmssimOptions:
> > > > +#
> > > > +# Information for the mssim emulator connection
> > > > +#
> > > > +# @host: host name or IP address to connect to
> > > > +# @port: port for the standard TPM commands
> > > > +# @ctrl: control port for TPM state changes
> > > > +#
> > > > +# Since: 7.2.0
> > > > +##
> > > > +{ 'struct': 'TPMmssimOptions',
> > > > + 'data': {
> > > > + 'host': 'str',
> > > > + 'port': 'str',
> > > > + 'ctrl': 'str' },
> > > > + 'if': 'CONFIG_TPM' }
> > >
> > > We don't want to be adding new code using plain host/port combos,
> > > as that misses extra functionality for controlling IPv4 vs IPv6
> > > usage.
> > >
> > > The existing 'emulator' backend references a chardev, but I'm
> > > not especially in favour of using the chardev indirection either,
> > > when all we should really need is a SocketAddress
> > >
> > > IOW, from a QAPI design POV, IMHO the best practice would be
> > >
> > > { 'struct': 'TPMmssimOptions',
> > > 'data': {
> > > 'command': 'SocketAddress',
> > > 'control': 'SocketAddress' },
> > > 'if': 'CONFIG_TPM' }
> > >
> > >
> > > The main wrinkle with this is that exprssing nested struct fields
> > > with QemuOpts is a disaster zone, and -tpmdev doesn't yet support
> > > JSON syntax.
> > >
> > > IMHO we should just fix the latter problem, as I don't think it
> > > ought to be too hard. Probably a cut+paste / search/replace job
> > > on the chanmge we did for -device in:
> > >
> > > commit 5dacda5167560b3af8eadbce5814f60ba44b467e
> > > Author: Kevin Wolf <kwolf@redhat.com>
> > > Date: Fri Oct 8 15:34:42 2021 +0200
> > >
> > > vl: Enable JSON syntax for -device
> > >
> > > This would mean we could use plain -tpmdev for a local instance
> > >
> > > -tpmdev mssim,id=tpm0 \
> > > -device tpm-crb,tpmdev=tpm0 \
> > >
> > > but to use a remote emulator we would use
> > >
> > > -tpmdev "{'backend': 'mssim', 'id': 'tpm0',
> > > 'command': {
> > > 'type': 'inet',
> > > 'host': 'remote',
> > > 'port': '4455'
> > > },
> > > 'control': {
> > > 'type': 'inet',
> > > 'host': 'remote',
> > > 'port': '4456'
> > > }}"
> > >
> > > (without the whitepace/newlines, which i just used for sake of
> > > clarity)
> >
> > Just on this, might it not be easier for the commandline to do what
> > gluster does? just use the '.' as a separator and subqdict
> > extraction, so you'd specify
> >
> > -tpmdev
> > mssim,id=tpm0,command.type=inet,command.host=remote,command.port=44
> > 55,control.type=inet,control.host=remote,control.port=4456
> >
> > With the added bonus that X.type could be defaulted to inet and
> > control.host could follow command.host and so on?
>
> These days, we have a policy of not tyring to map nested data onto
> the flat QemuOpts. This has been done in several areas and we've
> ended up with a mess of ever so slightly different impls each with
> their own flaws. This is why our preferred approach these days is to
> add support for JSON syntax to enable non-flat config.
Well, OK, but I've got to say on behalf of shell script writers
everywhere that using json for command line arguments is a textbook
definition of cruel and unusual punishment.
James
Re: [PATCH] tpm: add backend for mssim, James Bottomley, 2022/12/12