Re: [tpmdd-devel] Regarding recently Added TPM2.0 support to the Nuvoton i2c driver
On Tue, Jul 26, 2016 at 02:17:11PM -0600, Jason Gunthorpe wrote:
> On Tue, Jul 26, 2016 at 11:44:43PM +0530, Nayna wrote:
> > I got these questions while testing some TPM2.0 stuff using the kernel
> > code from repo having this patch and am using Nuvoton TPM.
> >
> > #1. It seems that support is added only for following device-ids.
> > {.compatible = "nuvoton,npct501"},
> > {.compatible = "winbond,wpct301"},
> > {.compatible = "nuvoton,npct601", .data = OF_IS_TPM2},
> >
> > So, was wondering about why device id nuvoton,npct650 wasn't added for
> > the support ?
>
> Yes, that was the device ID list for Nuvoton.
>
> It is convention in device tree to include older device IDs if the
> device is compatible.
>
> So you might do
>
> compatible = "nuvoton,npct650", "nuvoton,npct601"
>
> Andrew, is 601 even the right name?
>
> > Was it expected to work with some wild-card type device-id as specified
> > in the first line of description comment of file i.e. npct6XX. ?
>
> No.
>
> > So, why is there hard-coded checking and not using tpm2_probe() method
> > which is itself based on direct TPM hardware response for setting the
> > TPM2 flag. ? Is there something I am missing in the design which
> > mandates to have .data set as OF_IS_TPM2.
>
> Generally speaking probing is somewhat discouraged, currently we only
> probe for PC platform tis (and even that might be a mistake), all
> other drivers are designed to be explicit.
I did probing that way originally because the dTPM I had in hand in
2014 did not give expected information from TIS registers in the force
path. I'm sorry but can't recall anymore more exact description what was
the problem but the dTPM was not a production version.
> Jason
/Jarkko
--
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] Regarding recently Added TPM2.0 support to the Nuvoton i2c driver
Thanks everyone for discussion and clarifying the design points. This helps and we can now test by changing our device-tree entries. Also, we are assuming that just updating .compatible device tree entry should do the fix. We don't have to add any additional .data property. And driver will match .data value even for nuvoton,npct650 from device driver npct601 entry, where .data is explicitly specified. Thanks & Regards, - Nayna On 07/28/2016 04:04 AM, George Wilson wrote: > On Wed, Jul 27, 2016 at 11:42:29AM -0600, Jason Gunthorpe wrote: >>> Should not the device tree tell you what is actually *there* and let the >>> driver code decide what is compatible? Rather than the firmware trying to >>> make a guess as to which Nuvoton device id to try to match in the Nuvoton >>> driver code? >> >> Yes, that is what I suggested: >> >>compatible = "nuvoton,npct650", "nuvoton,npct601" >> >> Today's kernel has no idea what 650 is, but since the DT says it is >> compatible with the 601 then today's kernel will bind to it. 601 is >> the kernel documented compatible tag for that specific I2C >> API. > > So in this case, early firmware would simply pass the additional > compatible string. > >> Perhaps we should add 6xx as the generic flavor? I'm not as clear on >> what DT maintainers think about that, but there is precedent. > > We thought about suggesting that too. However, > http://elinux.org/Device_Tree_Usage advises: > >Warning: Don't use wildcard compatible values, like >"fsl,mpc83xx-uart" or similar. Silicon vendors will invariably >make a change that breaks your wildcard assumptions the moment >it is too late to change it. Instead, choose a specific silicon >implementations and make all subsequent silicon compatible with >it. > > It looks like the driver's compatible table is on the right track and > we need to make our platform's device tree conform to its expectations. > -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] Regarding recently Added TPM2.0 support to the Nuvoton i2c driver
On Wed, Jul 27, 2016 at 10:31:52AM -0600, Jason Gunthorpe wrote: > On Wed, Jul 27, 2016 at 11:05:14AM -0500, George Wilson wrote: > > > > Yes, generally Linux expects DT to be set correctly by the boot > > > firmware. Early firmware needs to know the TPM type anyhow to do the > > > TPM setup, so this doesn't seem like a realistic scenario. > > > > A reset is required after upgrade/downgrade. But the version still > > needs to be detected by the firmware somehow. It could be configured > > manually in firmware state after the upgrade/downgrade to properly set > > the property, which seems much less desirable than a probe. > > Well, the firmware has to take care of this. If the firmware wants to > support switchable firmware it needs to be able to do probe that works > with all the firmware versions. > > ACPI and DT both expect the TPM version to be passed to the OS, it is > up to the firmware to do that correctly.. > > The fact you've already seen TPM2 probing failures makes me very > reluctant to turn it on more broadly, and maybe even think we should > get rid of it from tis too.. Cool, that clarifies it precisely: the expectation is that firmware will do any necessary probing and pass the property up in the device tree. All drivers supporting OF must use the data property as a flag for 2.0 for models capable of 2.0 function. > > Jason > -- George Wilson IBM Linux Technology Center Security Development -- ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] Regarding recently Added TPM2.0 support to the Nuvoton i2c driver
On 07/27/2016 12:24 PM, Jason Gunthorpe wrote: > On Wed, Jul 27, 2016 at 10:30:09AM -0400, Dave Heller wrote: >>> So you might do >>> >>>compatible = "nuvoton,npct650", "nuvoton,npct601" >>> >>> Andrew, is 601 even the right name? >> >> Hi Jason, I'm not sure I'm understanding your answer here. Looks like >> we will have to add a couple of lines to specifically list the npct650, >> why not just using the existing syntax, one device id per line? Or you >> are saying, since the npct601 and npct650 are similar models, it's >> better to list them on the same line? > > The above example would go in your device tree, are you using device tree? > > Jason Thanks, I get in now: you are saying we should add that to our device tree and leave the driver code alone. Here is were my understanding of the device tree entry is a bit lacking" maybe this is the convention that is normally followed. And yes, the device tree property is named "compatible". But this seems intuitively backward to me: Should not the device tree tell you what is actually *there* and let the driver code decide what is compatible? Rather than the firmware trying to make a guess as to which Nuvoton device id to try to match in the Nuvoton driver code? Put another way, should the "smarts" as to which Nuvoton part numbers are considered compatible to the driver, be done by FW? Or left to the driver? -- What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports.http://sdm.link/zohodev2dev ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] Regarding recently Added TPM2.0 support to the Nuvoton i2c driver
On Wed, Jul 27, 2016 at 11:05:14AM -0500, George Wilson wrote: > > Yes, generally Linux expects DT to be set correctly by the boot > > firmware. Early firmware needs to know the TPM type anyhow to do the > > TPM setup, so this doesn't seem like a realistic scenario. > > A reset is required after upgrade/downgrade. But the version still > needs to be detected by the firmware somehow. It could be configured > manually in firmware state after the upgrade/downgrade to properly set > the property, which seems much less desirable than a probe. Well, the firmware has to take care of this. If the firmware wants to support switchable firmware it needs to be able to do probe that works with all the firmware versions. ACPI and DT both expect the TPM version to be passed to the OS, it is up to the firmware to do that correctly.. The fact you've already seen TPM2 probing failures makes me very reluctant to turn it on more broadly, and maybe even think we should get rid of it from tis too.. Jason -- What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports.http://sdm.link/zohodev2dev ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] Regarding recently Added TPM2.0 support to the Nuvoton i2c driver
On Wed, Jul 27, 2016 at 10:30:09AM -0400, Dave Heller wrote: > > So you might do > > > > compatible = "nuvoton,npct650", "nuvoton,npct601" > > > > Andrew, is 601 even the right name? > > Hi Jason, I'm not sure I'm understanding your answer here. Looks like > we will have to add a couple of lines to specifically list the npct650, > why not just using the existing syntax, one device id per line? Or you > are saying, since the npct601 and npct650 are similar models, it's > better to list them on the same line? The above example would go in your device tree, are you using device tree? Jason -- What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports.http://sdm.link/zohodev2dev ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] Regarding recently Added TPM2.0 support to the Nuvoton i2c driver
On Tue, Jul 26, 2016 at 03:03:44PM -0600, Jason Gunthorpe wrote: > On Tue, Jul 26, 2016 at 03:39:02PM -0500, George Wilson wrote: > > > Generally speaking probing is somewhat discouraged, currently we only > > > probe for PC platform tis (and even that might be a mistake), all > > > other drivers are designed to be explicit. > > > > How should field upgradable/downgradable TPMs be handled since hardcoding > > the version in the device tree might give the wrong answer? Would early > > firmware be expected to probe nonetheless and set the right device tree > > property? > > Is that a real thing? Yep, it's a thing. I know of at least 2 parts from 2 different suppliers that are field upgradable/downgradable. > > Yes, generally Linux expects DT to be set correctly by the boot > firmware. Early firmware needs to know the TPM type anyhow to do the > TPM setup, so this doesn't seem like a realistic scenario. A reset is required after upgrade/downgrade. But the version still needs to be detected by the firmware somehow. It could be configured manually in firmware state after the upgrade/downgrade to properly set the property, which seems much less desirable than a probe. > > For TPM we made a somewhat arbitary choice that TPM2 has to be > explicit. If there are real systems that benefit from auto-probing it > could be revisited.. I think it's as necessary - at least at the firmware level - as for x86_64. > > But, to be honest, I'm not certain how robust our probe technique is, > and I think we should avoid probing, since TCG didn't design an > approved detection sequence (??). We did see issues with some older 1.2 TPMs that hung when probed by the kernel with the wrong device driver but that hasn't been an issue for some time. It looks like UEFI ultimately does probe and likely that's required for other platforms. I don't think it's safe to assume a TPM is always loaded with a particular firmware version across hard resets. > > Jason > -- George Wilson IBM Linux Technology Center Security Development -- What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports.http://sdm.link/zohodev2dev ___ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] Regarding recently Added TPM2.0 support to the Nuvoton i2c driver
On 07/26/2016 04:17 PM, Jason Gunthorpe wrote:
> On Tue, Jul 26, 2016 at 11:44:43PM +0530, Nayna wrote:
>> I got these questions while testing some TPM2.0 stuff using the kernel
>> code from repo having this patch and am using Nuvoton TPM.
>>
>> #1. It seems that support is added only for following device-ids.
>> {.compatible = "nuvoton,npct501"},
>> {.compatible = "winbond,wpct301"},
>> {.compatible = "nuvoton,npct601", .data = OF_IS_TPM2},
>>
>> So, was wondering about why device id nuvoton,npct650 wasn't added for
>> the support ?
>
> Yes, that was the device ID list for Nuvoton.
>
> It is convention in device tree to include older device IDs if the
> device is compatible.
>
> So you might do
>
> compatible = "nuvoton,npct650", "nuvoton,npct601"
>
> Andrew, is 601 even the right name?
Hi Jason, I'm not sure I'm understanding your answer here. Looks like
we will have to add a couple of lines to specifically list the npct650,
why not just using the existing syntax, one device id per line? Or you
are saying, since the npct601 and npct650 are similar models, it's
better to list them on the same line?
>> Was it expected to work with some wild-card type device-id as specified
>> in the first line of description comment of file i.e. npct6XX. ?
>
> No.
>
>> So, why is there hard-coded checking and not using tpm2_probe() method
>> which is itself based on direct TPM hardware response for setting the
>> TPM2 flag. ? Is there something I am missing in the design which
>> mandates to have .data set as OF_IS_TPM2.
>
> Generally speaking probing is somewhat discouraged, currently we only
> probe for PC platform tis (and even that might be a mistake), all
> other drivers are designed to be explicit.
>
> Jason
>
> --
> What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
> patterns at an interface-level. Reveals which users, apps, and protocols are
> consuming the most bandwidth. Provides multi-vendor support for NetFlow,
> J-Flow, sFlow and other flows. Make informed decisions using capacity planning
> reports.http://sdm.link/zohodev2dev
> ___
> tpmdd-devel mailing list
> tpmdd-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
>
--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Re: [tpmdd-devel] Regarding recently Added TPM2.0 support to the Nuvoton i2c driver
On Tue, Jul 26, 2016 at 02:17:11PM -0600, Jason Gunthorpe wrote:
> On Tue, Jul 26, 2016 at 11:44:43PM +0530, Nayna wrote:
> > I got these questions while testing some TPM2.0 stuff using the kernel
> > code from repo having this patch and am using Nuvoton TPM.
> >
> > #1. It seems that support is added only for following device-ids.
> > {.compatible = "nuvoton,npct501"},
> > {.compatible = "winbond,wpct301"},
> > {.compatible = "nuvoton,npct601", .data = OF_IS_TPM2},
> >
> > So, was wondering about why device id nuvoton,npct650 wasn't added for
> > the support ?
>
> Yes, that was the device ID list for Nuvoton.
>
> It is convention in device tree to include older device IDs if the
> device is compatible.
>
> So you might do
>
> compatible = "nuvoton,npct650", "nuvoton,npct601"
>
> Andrew, is 601 even the right name?
>
> > Was it expected to work with some wild-card type device-id as specified
> > in the first line of description comment of file i.e. npct6XX. ?
>
> No.
>
> > So, why is there hard-coded checking and not using tpm2_probe() method
> > which is itself based on direct TPM hardware response for setting the
> > TPM2 flag. ? Is there something I am missing in the design which
> > mandates to have .data set as OF_IS_TPM2.
>
> Generally speaking probing is somewhat discouraged, currently we only
> probe for PC platform tis (and even that might be a mistake), all
> other drivers are designed to be explicit.
How should field upgradable/downgradable TPMs be handled since hardcoding
the version in the device tree might give the wrong answer? Would early
firmware be expected to probe nonetheless and set the right device tree
property?
>
> Jason
>
> --
> What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
> patterns at an interface-level. Reveals which users, apps, and protocols are
> consuming the most bandwidth. Provides multi-vendor support for NetFlow,
> J-Flow, sFlow and other flows. Make informed decisions using capacity planning
> reports.http://sdm.link/zohodev2dev
> ___
> tpmdd-devel mailing list
> tpmdd-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
>
--
George Wilson
IBM Linux Technology Center
Security Development
--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
[tpmdd-devel] Regarding recently Added TPM2.0 support to the Nuvoton i2c driver
Hi
I might not be aware of some background related to TPM device driver
design and so have got few questions regarding recently added support in
Nuvoton i2c driver for TPM2.0
(http://git.infradead.org/users/jjs/linux-tpmdd.git/commit/82cc1a49b6358394938e759dc4c22b2be773bbad).
I got these questions while testing some TPM2.0 stuff using the kernel
code from repo having this patch and am using Nuvoton TPM.
#1. It seems that support is added only for following device-ids.
{.compatible = "nuvoton,npct501"},
{.compatible = "winbond,wpct301"},
{.compatible = "nuvoton,npct601", .data = OF_IS_TPM2},
So, was wondering about why device id nuvoton,npct650 wasn't added for
the support ?
Was it expected to work with some wild-card type device-id as specified
in the first line of description comment of file i.e. npct6XX. ?
/**
* Nuvoton TPM I2C Device Driver Interface for WPCT301/NPCT501/NPCT6XX,
**/
Also, the patch log message says "tpm: Add TPM 2.0 support to the
Nuvoton i2c driver (NPCT6xx family)" i.e. NPCT6xx family, so my
assumption was that it will work with any NPCT6xx device id.
#2 Was also trying to understand why the .data is hardcoded for
nuvoton,npct601, but not for other device ids ?
And I see that this device-id .data info is then used in
i2c_nuvoton_probe() method to set the tpm_chip flag as TPM2 device
if (dev->of_node) {
const struct of_device_id *of_id;
printk("dev->of_node if\n");
of_id = of_match_device(dev->driver->of_match_table, dev);
if (of_id && of_id->data == OF_IS_TPM2)
{
printk("of_id->data == OF_IS_TPM2\n");
chip->flags |= TPM_CHIP_FLAG_TPM2;
}
} else
{
printk("dev->of_node else\n");
if (id->driver_data == I2C_IS_TPM2)
{
printk("id->driver_data == I2C_IS_TPM2\n");
chip->flags |= TPM_CHIP_FLAG_TPM2;
}
}
So, why is there hard-coded checking and not using tpm2_probe() method
which is itself based on direct TPM hardware response for setting the
TPM2 flag. ? Is there something I am missing in the design which
mandates to have .data set as OF_IS_TPM2.
Please let me know.
Thanks & Regards,
- Nayna
--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
