Add a caching layer for the TPM established flag so that we don't
need to go to the emulator every time the flag is read by accessing
the REG_ACCESS register.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
v1->v2:
- move the caching to the backend layer since detecting the
TPM 1.2 TSC_ResetEstablishmentBit() command is easier to do
here.
---
hw/tpm/tpm_emulator.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
index e1a6810..b293db7 100644
--- a/hw/tpm/tpm_emulator.c
+++ b/hw/tpm/tpm_emulator.c
@@ -73,6 +73,9 @@ typedef struct TPMEmulator {
Error *migration_blocker;
QemuMutex mutex;
+
+ unsigned int established_flag:1;
+ unsigned int established_flag_cached:1;
} TPMEmulator;
@@ -287,16 +290,22 @@ static bool tpm_emulator_get_tpm_established_flag(TPMBackend *tb)
TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
ptm_est est;
- DPRINTF("%s", __func__);
+ if (tpm_emu->established_flag_cached) {
+ return tpm_emu->established_flag;
+ }
+
if (tpm_emulator_ctrlcmd(tpm_emu, CMD_GET_TPMESTABLISHED, &est,
0, sizeof(est)) < 0) {
error_report("tpm-emulator: Could not get the TPM established flag: %s",
strerror(errno));
return false;
}
- DPRINTF("established flag: %0x", est.u.resp.bit);
+ DPRINTF("got established flag: %0x", est.u.resp.bit);
+
+ tpm_emu->established_flag_cached = 1;
+ tpm_emu->established_flag = (est.u.resp.bit != 0);
- return (est.u.resp.bit != 0);
+ return tpm_emu->established_flag;
}
static int tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
@@ -327,6 +336,8 @@ static int tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
return -1;
}
+ tpm_emu->established_flag_cached = 0;
+
return 0;
}
--
2.5.5
Hi
On Tue, Nov 14, 2017 at 10:52 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> Add a caching layer for the TPM established flag so that we don't
> need to go to the emulator every time the flag is read by accessing
> the REG_ACCESS register.
What's the impact? Isn't this just a "small" optimization? Iotw, why
is this for-2.11?
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>
> v1->v2:
> - move the caching to the backend layer since detecting the
> TPM 1.2 TSC_ResetEstablishmentBit() command is easier to do
> here.
> ---
> hw/tpm/tpm_emulator.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> index e1a6810..b293db7 100644
> --- a/hw/tpm/tpm_emulator.c
> +++ b/hw/tpm/tpm_emulator.c
> @@ -73,6 +73,9 @@ typedef struct TPMEmulator {
> Error *migration_blocker;
>
> QemuMutex mutex;
> +
> + unsigned int established_flag:1;
> + unsigned int established_flag_cached:1;
> } TPMEmulator;
>
>
> @@ -287,16 +290,22 @@ static bool tpm_emulator_get_tpm_established_flag(TPMBackend *tb)
> TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
> ptm_est est;
>
> - DPRINTF("%s", __func__);
> + if (tpm_emu->established_flag_cached) {
> + return tpm_emu->established_flag;
> + }
> +
> if (tpm_emulator_ctrlcmd(tpm_emu, CMD_GET_TPMESTABLISHED, &est,
> 0, sizeof(est)) < 0) {
> error_report("tpm-emulator: Could not get the TPM established flag: %s",
> strerror(errno));
> return false;
> }
> - DPRINTF("established flag: %0x", est.u.resp.bit);
> + DPRINTF("got established flag: %0x", est.u.resp.bit);
> +
> + tpm_emu->established_flag_cached = 1;
> + tpm_emu->established_flag = (est.u.resp.bit != 0);
>
> - return (est.u.resp.bit != 0);
> + return tpm_emu->established_flag;
> }
>
> static int tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
> @@ -327,6 +336,8 @@ static int tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
> return -1;
> }
>
> + tpm_emu->established_flag_cached = 0;
> +
> return 0;
> }
>
> --
> 2.5.5
>
--
Marc-André Lureau
On 11/14/2017 06:40 PM, Marc-André Lureau wrote:
> Hi
>
> On Tue, Nov 14, 2017 at 10:52 PM, Stefan Berger
> <stefanb@linux.vnet.ibm.com> wrote:
>> Add a caching layer for the TPM established flag so that we don't
>> need to go to the emulator every time the flag is read by accessing
>> the REG_ACCESS register.
> What's the impact? Isn't this just a "small" optimization? Iotw, why
> is this for-2.11?
The TIS has a register that contains this flag and that's being polled
quite frequently. So it generates a lot of traffic to the emulator. This
caching layer gets rid of most of the traffic.
Stefan
>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>
>> v1->v2:
>> - move the caching to the backend layer since detecting the
>> TPM 1.2 TSC_ResetEstablishmentBit() command is easier to do
>> here.
>> ---
>> hw/tpm/tpm_emulator.c | 17 ++++++++++++++---
>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
>> index e1a6810..b293db7 100644
>> --- a/hw/tpm/tpm_emulator.c
>> +++ b/hw/tpm/tpm_emulator.c
>> @@ -73,6 +73,9 @@ typedef struct TPMEmulator {
>> Error *migration_blocker;
>>
>> QemuMutex mutex;
>> +
>> + unsigned int established_flag:1;
>> + unsigned int established_flag_cached:1;
>> } TPMEmulator;
>>
>>
>> @@ -287,16 +290,22 @@ static bool tpm_emulator_get_tpm_established_flag(TPMBackend *tb)
>> TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
>> ptm_est est;
>>
>> - DPRINTF("%s", __func__);
>> + if (tpm_emu->established_flag_cached) {
>> + return tpm_emu->established_flag;
>> + }
>> +
>> if (tpm_emulator_ctrlcmd(tpm_emu, CMD_GET_TPMESTABLISHED, &est,
>> 0, sizeof(est)) < 0) {
>> error_report("tpm-emulator: Could not get the TPM established flag: %s",
>> strerror(errno));
>> return false;
>> }
>> - DPRINTF("established flag: %0x", est.u.resp.bit);
>> + DPRINTF("got established flag: %0x", est.u.resp.bit);
>> +
>> + tpm_emu->established_flag_cached = 1;
>> + tpm_emu->established_flag = (est.u.resp.bit != 0);
>>
>> - return (est.u.resp.bit != 0);
>> + return tpm_emu->established_flag;
>> }
>>
>> static int tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
>> @@ -327,6 +336,8 @@ static int tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
>> return -1;
>> }
>>
>> + tpm_emu->established_flag_cached = 0;
>> +
>> return 0;
>> }
>>
>> --
>> 2.5.5
>>
>
>
Hi
On Wed, Nov 15, 2017 at 2:16 AM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> On 11/14/2017 06:40 PM, Marc-André Lureau wrote:
>>
>> Hi
>>
>> On Tue, Nov 14, 2017 at 10:52 PM, Stefan Berger
>> <stefanb@linux.vnet.ibm.com> wrote:
>>>
>>> Add a caching layer for the TPM established flag so that we don't
>>> need to go to the emulator every time the flag is read by accessing
>>> the REG_ACCESS register.
>>
>> What's the impact? Isn't this just a "small" optimization? Iotw, why
>> is this for-2.11?
>
>
> The TIS has a register that contains this flag and that's being polled quite
> frequently. So it generates a lot of traffic to the emulator. This caching
> layer gets rid of most of the traffic.
I didn't notice any problem when doing my tests, I guess Amarnath
niether. perhaps it's best to delay for after 2.11.
> Stefan
>
>
>>
>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>>
>>> v1->v2:
>>> - move the caching to the backend layer since detecting the
>>> TPM 1.2 TSC_ResetEstablishmentBit() command is easier to do
>>> here.
>>> ---
>>> hw/tpm/tpm_emulator.c | 17 ++++++++++++++---
>>> 1 file changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
>>> index e1a6810..b293db7 100644
>>> --- a/hw/tpm/tpm_emulator.c
>>> +++ b/hw/tpm/tpm_emulator.c
>>> @@ -73,6 +73,9 @@ typedef struct TPMEmulator {
>>> Error *migration_blocker;
>>>
>>> QemuMutex mutex;
>>> +
>>> + unsigned int established_flag:1;
>>> + unsigned int established_flag_cached:1;
>>> } TPMEmulator;
>>>
>>>
>>> @@ -287,16 +290,22 @@ static bool
>>> tpm_emulator_get_tpm_established_flag(TPMBackend *tb)
>>> TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
>>> ptm_est est;
>>>
>>> - DPRINTF("%s", __func__);
>>> + if (tpm_emu->established_flag_cached) {
>>> + return tpm_emu->established_flag;
>>> + }
>>> +
>>> if (tpm_emulator_ctrlcmd(tpm_emu, CMD_GET_TPMESTABLISHED, &est,
>>> 0, sizeof(est)) < 0) {
>>> error_report("tpm-emulator: Could not get the TPM established
>>> flag: %s",
>>> strerror(errno));
>>> return false;
>>> }
>>> - DPRINTF("established flag: %0x", est.u.resp.bit);
>>> + DPRINTF("got established flag: %0x", est.u.resp.bit);
>>> +
>>> + tpm_emu->established_flag_cached = 1;
>>> + tpm_emu->established_flag = (est.u.resp.bit != 0);
>>>
>>> - return (est.u.resp.bit != 0);
>>> + return tpm_emu->established_flag;
>>> }
>>>
>>> static int tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
>>> @@ -327,6 +336,8 @@ static int
>>> tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
>>> return -1;
>>> }
>>>
>>> + tpm_emu->established_flag_cached = 0;
>>> +
>>> return 0;
>>> }
>>>
>>> --
>>> 2.5.5
>>>
>>
>>
>
--
Marc-André Lureau
On Wed, 2017-11-15 at 04:47 +0100, Marc-André Lureau wrote: > Hi > > On Wed, Nov 15, 2017 at 2:16 AM, Stefan Berger > <stefanb@linux.vnet.ibm.com> wrote: > > > > On 11/14/2017 06:40 PM, Marc-André Lureau wrote: > > > > > > > > > Hi > > > > > > On Tue, Nov 14, 2017 at 10:52 PM, Stefan Berger > > > <stefanb@linux.vnet.ibm.com> wrote: > > > > > > > > > > > > Add a caching layer for the TPM established flag so that we > > > > don't > > > > need to go to the emulator every time the flag is read by > > > > accessing > > > > the REG_ACCESS register. > > > What's the impact? Isn't this just a "small" optimization? Iotw, > > > why > > > is this for-2.11? > > > > The TIS has a register that contains this flag and that's being > > polled quite > > frequently. So it generates a lot of traffic to the emulator. This > > caching > > layer gets rid of most of the traffic. > I didn't notice any problem when doing my tests, I guess Amarnath > niether. perhaps it's best to delay for after 2.11. Yes, I could see there is little frequent(21 calls to backend) to get establish flag, and this patch will help to avoid them. But i still agree with Marc and tag this as "small" optimization. - Amarnath
On 11/15/2017 04:05 AM, Valluri, Amarnath wrote:
> On Wed, 2017-11-15 at 04:47 +0100, Marc-André Lureau wrote:
>> Hi
>>
>> On Wed, Nov 15, 2017 at 2:16 AM, Stefan Berger
>> <stefanb@linux.vnet.ibm.com> wrote:
>>> On 11/14/2017 06:40 PM, Marc-André Lureau wrote:
>>>>
>>>> Hi
>>>>
>>>> On Tue, Nov 14, 2017 at 10:52 PM, Stefan Berger
>>>> <stefanb@linux.vnet.ibm.com> wrote:
>>>>>
>>>>> Add a caching layer for the TPM established flag so that we
>>>>> don't
>>>>> need to go to the emulator every time the flag is read by
>>>>> accessing
>>>>> the REG_ACCESS register.
>>>> What's the impact? Isn't this just a "small" optimization? Iotw,
>>>> why
>>>> is this for-2.11?
>>> The TIS has a register that contains this flag and that's being
>>> polled quite
>>> frequently. So it generates a lot of traffic to the emulator. This
>>> caching
>>> layer gets rid of most of the traffic.
>> I didn't notice any problem when doing my tests, I guess Amarnath
>> niether. perhaps it's best to delay for after 2.11.
> Yes, I could see there is little frequent(21 calls to backend) to get
> establish flag, and this patch will help to avoid them. But i still
> agree with Marc and tag this as "small" optimization.
Can you review it so we have it ready for 2.12?
Thanks,
Stefan
>
> - Amarnath
On 11/14/2017 10:47 PM, Marc-André Lureau wrote:
> Hi
>
> On Wed, Nov 15, 2017 at 2:16 AM, Stefan Berger
> <stefanb@linux.vnet.ibm.com> wrote:
>> On 11/14/2017 06:40 PM, Marc-André Lureau wrote:
>>> Hi
>>>
>>> On Tue, Nov 14, 2017 at 10:52 PM, Stefan Berger
>>> <stefanb@linux.vnet.ibm.com> wrote:
>>>> Add a caching layer for the TPM established flag so that we don't
>>>> need to go to the emulator every time the flag is read by accessing
>>>> the REG_ACCESS register.
>>> What's the impact? Isn't this just a "small" optimization? Iotw, why
>>> is this for-2.11?
>>
>> The TIS has a register that contains this flag and that's being polled quite
>> frequently. So it generates a lot of traffic to the emulator. This caching
>> layer gets rid of most of the traffic.
> I didn't notice any problem when doing my tests, I guess Amarnath
> niether. perhaps it's best to delay for after 2.11.
So then let's delay it. Following a Reviewed-by I'd put it in the
tpm-next queue then.
Stefan
Hi
On Tue, Nov 14, 2017 at 10:52 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> Add a caching layer for the TPM established flag so that we don't
> need to go to the emulator every time the flag is read by accessing
> the REG_ACCESS register.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
looks good,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> v1->v2:
> - move the caching to the backend layer since detecting the
> TPM 1.2 TSC_ResetEstablishmentBit() command is easier to do
> here.
> ---
> hw/tpm/tpm_emulator.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> index e1a6810..b293db7 100644
> --- a/hw/tpm/tpm_emulator.c
> +++ b/hw/tpm/tpm_emulator.c
> @@ -73,6 +73,9 @@ typedef struct TPMEmulator {
> Error *migration_blocker;
>
> QemuMutex mutex;
> +
> + unsigned int established_flag:1;
> + unsigned int established_flag_cached:1;
> } TPMEmulator;
>
>
> @@ -287,16 +290,22 @@ static bool tpm_emulator_get_tpm_established_flag(TPMBackend *tb)
> TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
> ptm_est est;
>
> - DPRINTF("%s", __func__);
> + if (tpm_emu->established_flag_cached) {
> + return tpm_emu->established_flag;
> + }
> +
> if (tpm_emulator_ctrlcmd(tpm_emu, CMD_GET_TPMESTABLISHED, &est,
> 0, sizeof(est)) < 0) {
> error_report("tpm-emulator: Could not get the TPM established flag: %s",
> strerror(errno));
> return false;
> }
> - DPRINTF("established flag: %0x", est.u.resp.bit);
> + DPRINTF("got established flag: %0x", est.u.resp.bit);
> +
> + tpm_emu->established_flag_cached = 1;
> + tpm_emu->established_flag = (est.u.resp.bit != 0);
>
> - return (est.u.resp.bit != 0);
> + return tpm_emu->established_flag;
> }
>
> static int tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
> @@ -327,6 +336,8 @@ static int tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
> return -1;
> }
>
> + tpm_emu->established_flag_cached = 0;
> +
> return 0;
> }
>
> --
> 2.5.5
>
--
Marc-André Lureau
© 2016 - 2026 Red Hat, Inc.