Re: [Qemu-devel] [PATCH v2 4/5] tpm: add CRB device

[Stefan Berger <stefanb@linux.vnet.ibm.com>]

On 01/19/2018 01:42 PM, Eduardo Habkost wrote:
> On Fri, Jan 19, 2018 at 12:10:03PM -0500, Stefan Berger wrote:
>> On 01/19/2018 09:11 AM, Marc-André Lureau wrote:
>>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB)
>>> Interface as defined in TCG PC Client Platform TPM Profile (PTP)
>>> Specification Family “2.0” Level 00 Revision 01.03 v22.
>>>
>>> The PTP allows device implementation to switch between TIS and CRB
>>> model at run time, but given that CRB is a simpler device to
>>> implement, I chose to implement it as a different device.
>>>
>>> The device doesn't implement other locality than 0 for now (my laptop
>>> TPM doesn't either, so I assume this isn't so bad)
>>>
>>> The command/reply memory region is statically allocated after the CRB
>>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I
>>> wonder if the BIOS could or should allocate it instead, or what size
>>> to use, again this seems to fit well expectations)
>> I removed this last sentence now. It's at the right location.
>>
>>> The PTP doesn't specify a particular bus to put the device. So I added
>>> it on the system bus directly, so it could hopefully be used easily on
>>> a different platform than x86. Currently, it fails to init on piix,
>>> because error_on_sysbus_device() check. The check may be changed in a
>>> near future, see discussion on the qemu-devel ML.
>> I think this has to be solved. So I remove these last 2 sentences. I'll have
>> to wait until that other patch series from Eduard is merged since it doesn't
>> start yet.
> The series was just merged to master.  It's possible to make a
> machine accept the new device using
> machine_class_allow_dynamic_sysbus_dev(), now.

I saw that.
>
> However, is it really necessary to make it a sysbus device?
> Having bus-less devices was not possible in the past, but it is
> possible today.
>
What I don't like about it is the fact that I would have to use q35 if 
we only extend that machine type to allow this sysbus device. What is 
the reason that dynamic sysbus devices have to explicitly be allowed? If 
we don't need to limit this device to a certain machine type that may be 
more user friendly.