backends/tpm.c | 29 +++++ hw/tpm/tpm_emulator.c | 303 +++++++++++++++++++++++++++++++++++++++++-- hw/tpm/tpm_tis.c | 216 +++++++++++++++++------------- hw/tpm/tpm_util.c | 7 + hw/tpm/tpm_util.h | 7 + include/sysemu/tpm_backend.h | 22 ++++ 6 files changed, 483 insertions(+), 101 deletions(-)
This set of patches implements support for migrating the state of the external 'swtpm' TPM emulator as well as that of the emulated device interfaces. I have primarily tested this with the TIS and TPM 1.2 so far, but it also seems to work with TPM 2. The TIS is simplified first by reducing the number of buffers and read and write offsets into these buffers. Following the state machine of the TIS, a single buffer and r/w offset is enough for all localities since only one locality can ever be active. This series applies on top of my tpm-next branch. One of the challenges that is addressed by this set of patches is the fact that the TPM emulator may be processing a command while the state serialization of the devices is supposed to happen. A necessary first step has been implemented here that ensures that a response has been received from the exernal emulator and the bottom half function, which delivers the response and adjusts device registers (TIS or CRB), has been executed, before the device's state is serialized. A subsequent extension may need to address the live migration loop and delay the serialization of devices until the response from the external TPM has been received. Though the likelihood that someone executes a long-lasting TPM command while this is occurring is certainly rare. Stefan Stefan Berger (13): tpm_tis: convert uint32_t to size_t tpm_tis: limit size of buffer from backend tpm_tis: remove TPMSizeBuffer usage tpm_tis: move buffers from localities into common location tpm_tis: merge read and write buffer into single buffer tpm_tis: move r/w_offsets to TPMState tpm_tis: merge r/w_offset into rw_offset tpm: Implement tpm_sized_buffer_reset tpm: Introduce condition to notify waiters of completed command tpm: Introduce condition in TPM backend for notification tpm: implement tpm_backend_wait_cmd_completed tpm: extend TPM emulator with state migration support tpm_tis: extend TPM TIS with state migration support backends/tpm.c | 29 +++++ hw/tpm/tpm_emulator.c | 303 +++++++++++++++++++++++++++++++++++++++++-- hw/tpm/tpm_tis.c | 216 +++++++++++++++++------------- hw/tpm/tpm_util.c | 7 + hw/tpm/tpm_util.h | 7 + include/sysemu/tpm_backend.h | 22 ++++ 6 files changed, 483 insertions(+), 101 deletions(-) -- 2.5.5
Hi On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote: > This set of patches implements support for migrating the state of the > external 'swtpm' TPM emulator as well as that of the emulated device > interfaces. I have primarily tested this with the TIS and TPM 1.2 so > far, but it also seems to work with TPM 2. > > The TIS is simplified first by reducing the number of buffers and read > and write offsets into these buffers. Following the state machine of the > TIS, a single buffer and r/w offset is enough for all localities since > only one locality can ever be active. > > This series applies on top of my tpm-next branch. > > One of the challenges that is addressed by this set of patches is the fact > that the TPM emulator may be processing a command while the state > serialization of the devices is supposed to happen. A necessary first step > has been implemented here that ensures that a response has been received > from the exernal emulator and the bottom half function, which delivers the > response and adjusts device registers (TIS or CRB), has been executed, > before the device's state is serialized. > > A subsequent extension may need to address the live migration loop and delay > the serialization of devices until the response from the external TPM has > been received. Though the likelihood that someone executes a long-lasting > TPM command while this is occurring is certainly rare. > > Stefan > > Stefan Berger (13): > tpm_tis: convert uint32_t to size_t > tpm_tis: limit size of buffer from backend > tpm_tis: remove TPMSizeBuffer usage > tpm_tis: move buffers from localities into common location > tpm_tis: merge read and write buffer into single buffer > tpm_tis: move r/w_offsets to TPMState > tpm_tis: merge r/w_offset into rw_offset > tpm: Implement tpm_sized_buffer_reset ok for the above (you could queue/pull-req this now) > tpm: Introduce condition to notify waiters of completed command > tpm: Introduce condition in TPM backend for notification > tpm: implement tpm_backend_wait_cmd_completed > tpm: extend TPM emulator with state migration support > tpm_tis: extend TPM TIS with state migration support Much of the complexity from this migration series comes with the handling & synchronization of the IO thread. I think having a seperate thread doesn't bring much today TPM thread. it is a workaround for the chardev API being mostly synchronous. Am I wrong? (yes, passthrough doesn't use chardev, but it should probably use qio or chardev internally) Other kind of devices using a seperate process suffer the same problem. Just grep for qemu_chr_fe_write and look for the preceding comment, it's a common flaw in qemu code base. Code use the synchronous API, and sometime use non-blocking write (hw/usb/redirect.c seems quite better!) I would like to get rid of the seperate thread in TPM before we add migration support. We should try to improve the chardev API to make it easier to do non-blocking IO. This should considerably simplify the above patches and benefit the rest of qemu (instead of having everyone doing its own thing with seperate thread or other kind of continuation state). What do you think? > > backends/tpm.c | 29 +++++ > hw/tpm/tpm_emulator.c | 303 +++++++++++++++++++++++++++++++++++++++++-- > hw/tpm/tpm_tis.c | 216 +++++++++++++++++------------- > hw/tpm/tpm_util.c | 7 + > hw/tpm/tpm_util.h | 7 + > include/sysemu/tpm_backend.h | 22 ++++ > 6 files changed, 483 insertions(+), 101 deletions(-) > > -- > 2.5.5 > > -- Marc-André Lureau
On 12/22/2017 07:49 AM, Marc-André Lureau wrote: > Hi > > On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger > <stefanb@linux.vnet.ibm.com> wrote: >> This set of patches implements support for migrating the state of the >> external 'swtpm' TPM emulator as well as that of the emulated device >> interfaces. I have primarily tested this with the TIS and TPM 1.2 so >> far, but it also seems to work with TPM 2. >> >> The TIS is simplified first by reducing the number of buffers and read >> and write offsets into these buffers. Following the state machine of the >> TIS, a single buffer and r/w offset is enough for all localities since >> only one locality can ever be active. >> >> This series applies on top of my tpm-next branch. >> >> One of the challenges that is addressed by this set of patches is the fact >> that the TPM emulator may be processing a command while the state >> serialization of the devices is supposed to happen. A necessary first step >> has been implemented here that ensures that a response has been received >> from the exernal emulator and the bottom half function, which delivers the >> response and adjusts device registers (TIS or CRB), has been executed, >> before the device's state is serialized. >> >> A subsequent extension may need to address the live migration loop and delay >> the serialization of devices until the response from the external TPM has >> been received. Though the likelihood that someone executes a long-lasting >> TPM command while this is occurring is certainly rare. >> >> Stefan >> >> Stefan Berger (13): >> tpm_tis: convert uint32_t to size_t >> tpm_tis: limit size of buffer from backend >> tpm_tis: remove TPMSizeBuffer usage >> tpm_tis: move buffers from localities into common location >> tpm_tis: merge read and write buffer into single buffer >> tpm_tis: move r/w_offsets to TPMState >> tpm_tis: merge r/w_offset into rw_offset >> tpm: Implement tpm_sized_buffer_reset > ok for the above (you could queue/pull-req this now) > >> tpm: Introduce condition to notify waiters of completed command >> tpm: Introduce condition in TPM backend for notification >> tpm: implement tpm_backend_wait_cmd_completed >> tpm: extend TPM emulator with state migration support >> tpm_tis: extend TPM TIS with state migration support > Much of the complexity from this migration series comes with the > handling & synchronization of the IO thread. > > I think having a seperate thread doesn't bring much today TPM thread. > it is a workaround for the chardev API being mostly synchronous. Am I > wrong? (yes, passthrough doesn't use chardev, but it should probably > use qio or chardev internally) > > Other kind of devices using a seperate process suffer the same > problem. Just grep for qemu_chr_fe_write and look for the preceding > comment, it's a common flaw in qemu code base. Code use the > synchronous API, and sometime use non-blocking write > (hw/usb/redirect.c seems quite better!) > > I would like to get rid of the seperate thread in TPM before we add > migration support. We should try to improve the chardev API to make it > easier to do non-blocking IO. This should considerably simplify the > above patches and benefit the rest of qemu (instead of having everyone > doing its own thing with seperate thread or other kind of continuation > state). > > What do you think? I am wondering whether it will help us to get rid of the conditions/notifications, like patches 9-11 of this series. I doubt 12 and 13 will change. At some point device state is supposed to be written out and in case of the TPM we have to wait for the response to have come back into the backend. We won't start listening on the file descriptor for an outstanding response, so I guess we will still wait for a notification in that case as well. So I am not sure which parts are going to be simpler... Stefan > >> backends/tpm.c | 29 +++++ >> hw/tpm/tpm_emulator.c | 303 +++++++++++++++++++++++++++++++++++++++++-- >> hw/tpm/tpm_tis.c | 216 +++++++++++++++++------------- >> hw/tpm/tpm_util.c | 7 + >> hw/tpm/tpm_util.h | 7 + >> include/sysemu/tpm_backend.h | 22 ++++ >> 6 files changed, 483 insertions(+), 101 deletions(-) >> >> -- >> 2.5.5 >> >> > >
Hi On Fri, Dec 22, 2017 at 4:59 PM, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote: > On 12/22/2017 07:49 AM, Marc-André Lureau wrote: >> >> Hi >> >> On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger >> <stefanb@linux.vnet.ibm.com> wrote: >>> >>> This set of patches implements support for migrating the state of the >>> external 'swtpm' TPM emulator as well as that of the emulated device >>> interfaces. I have primarily tested this with the TIS and TPM 1.2 so >>> far, but it also seems to work with TPM 2. >>> >>> The TIS is simplified first by reducing the number of buffers and read >>> and write offsets into these buffers. Following the state machine of the >>> TIS, a single buffer and r/w offset is enough for all localities since >>> only one locality can ever be active. >>> >>> This series applies on top of my tpm-next branch. >>> >>> One of the challenges that is addressed by this set of patches is the >>> fact >>> that the TPM emulator may be processing a command while the state >>> serialization of the devices is supposed to happen. A necessary first >>> step >>> has been implemented here that ensures that a response has been received >>> from the exernal emulator and the bottom half function, which delivers >>> the >>> response and adjusts device registers (TIS or CRB), has been executed, >>> before the device's state is serialized. >>> >>> A subsequent extension may need to address the live migration loop and >>> delay >>> the serialization of devices until the response from the external TPM has >>> been received. Though the likelihood that someone executes a long-lasting >>> TPM command while this is occurring is certainly rare. >>> >>> Stefan >>> >>> Stefan Berger (13): >>> tpm_tis: convert uint32_t to size_t >>> tpm_tis: limit size of buffer from backend >>> tpm_tis: remove TPMSizeBuffer usage >>> tpm_tis: move buffers from localities into common location >>> tpm_tis: merge read and write buffer into single buffer >>> tpm_tis: move r/w_offsets to TPMState >>> tpm_tis: merge r/w_offset into rw_offset >>> tpm: Implement tpm_sized_buffer_reset >> >> ok for the above (you could queue/pull-req this now) >> >>> tpm: Introduce condition to notify waiters of completed command >>> tpm: Introduce condition in TPM backend for notification >>> tpm: implement tpm_backend_wait_cmd_completed >>> tpm: extend TPM emulator with state migration support >>> tpm_tis: extend TPM TIS with state migration support >> >> Much of the complexity from this migration series comes with the >> handling & synchronization of the IO thread. >> >> I think having a seperate thread doesn't bring much today TPM thread. >> it is a workaround for the chardev API being mostly synchronous. Am I >> wrong? (yes, passthrough doesn't use chardev, but it should probably >> use qio or chardev internally) >> >> Other kind of devices using a seperate process suffer the same >> problem. Just grep for qemu_chr_fe_write and look for the preceding >> comment, it's a common flaw in qemu code base. Code use the >> synchronous API, and sometime use non-blocking write >> (hw/usb/redirect.c seems quite better!) >> >> I would like to get rid of the seperate thread in TPM before we add >> migration support. We should try to improve the chardev API to make it >> easier to do non-blocking IO. This should considerably simplify the >> above patches and benefit the rest of qemu (instead of having everyone >> doing its own thing with seperate thread or other kind of continuation >> state). >> >> What do you think? > > > I am wondering whether it will help us to get rid of the > conditions/notifications, like patches 9-11 of this series. I doubt 12 and > 13 will change. At some point device state is supposed to be written out and > in case of the TPM we have to wait for the response to have come back into > the backend. We won't start listening on the file descriptor for an > outstanding response, so I guess we will still wait for a notification in > that case as well. So I am not sure which parts are going to be simpler... Why would qemu need to wait for completion of emulator? Couldn't qemu & emulator save the current state, including in-flights commands? That's apparently what usbredir does. > > Stefan > > >> >>> backends/tpm.c | 29 +++++ >>> hw/tpm/tpm_emulator.c | 303 >>> +++++++++++++++++++++++++++++++++++++++++-- >>> hw/tpm/tpm_tis.c | 216 +++++++++++++++++------------- >>> hw/tpm/tpm_util.c | 7 + >>> hw/tpm/tpm_util.h | 7 + >>> include/sysemu/tpm_backend.h | 22 ++++ >>> 6 files changed, 483 insertions(+), 101 deletions(-) >>> >>> -- >>> 2.5.5 >>> >>> >> >> > -- Marc-André Lureau
On 12/22/2017 11:13 AM, Marc-André Lureau wrote: > Hi > > On Fri, Dec 22, 2017 at 4:59 PM, Stefan Berger > <stefanb@linux.vnet.ibm.com> wrote: >> On 12/22/2017 07:49 AM, Marc-André Lureau wrote: >>> Hi >>> >>> On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger >>> <stefanb@linux.vnet.ibm.com> wrote: >>>> This set of patches implements support for migrating the state of the >>>> external 'swtpm' TPM emulator as well as that of the emulated device >>>> interfaces. I have primarily tested this with the TIS and TPM 1.2 so >>>> far, but it also seems to work with TPM 2. >>>> >>>> The TIS is simplified first by reducing the number of buffers and read >>>> and write offsets into these buffers. Following the state machine of the >>>> TIS, a single buffer and r/w offset is enough for all localities since >>>> only one locality can ever be active. >>>> >>>> This series applies on top of my tpm-next branch. >>>> >>>> One of the challenges that is addressed by this set of patches is the >>>> fact >>>> that the TPM emulator may be processing a command while the state >>>> serialization of the devices is supposed to happen. A necessary first >>>> step >>>> has been implemented here that ensures that a response has been received >>>> from the exernal emulator and the bottom half function, which delivers >>>> the >>>> response and adjusts device registers (TIS or CRB), has been executed, >>>> before the device's state is serialized. >>>> >>>> A subsequent extension may need to address the live migration loop and >>>> delay >>>> the serialization of devices until the response from the external TPM has >>>> been received. Though the likelihood that someone executes a long-lasting >>>> TPM command while this is occurring is certainly rare. >>>> >>>> Stefan >>>> >>>> Stefan Berger (13): >>>> tpm_tis: convert uint32_t to size_t >>>> tpm_tis: limit size of buffer from backend >>>> tpm_tis: remove TPMSizeBuffer usage >>>> tpm_tis: move buffers from localities into common location >>>> tpm_tis: merge read and write buffer into single buffer >>>> tpm_tis: move r/w_offsets to TPMState >>>> tpm_tis: merge r/w_offset into rw_offset >>>> tpm: Implement tpm_sized_buffer_reset >>> ok for the above (you could queue/pull-req this now) >>> >>>> tpm: Introduce condition to notify waiters of completed command >>>> tpm: Introduce condition in TPM backend for notification >>>> tpm: implement tpm_backend_wait_cmd_completed >>>> tpm: extend TPM emulator with state migration support >>>> tpm_tis: extend TPM TIS with state migration support >>> Much of the complexity from this migration series comes with the >>> handling & synchronization of the IO thread. >>> >>> I think having a seperate thread doesn't bring much today TPM thread. >>> it is a workaround for the chardev API being mostly synchronous. Am I >>> wrong? (yes, passthrough doesn't use chardev, but it should probably >>> use qio or chardev internally) >>> >>> Other kind of devices using a seperate process suffer the same >>> problem. Just grep for qemu_chr_fe_write and look for the preceding >>> comment, it's a common flaw in qemu code base. Code use the >>> synchronous API, and sometime use non-blocking write >>> (hw/usb/redirect.c seems quite better!) >>> >>> I would like to get rid of the seperate thread in TPM before we add >>> migration support. We should try to improve the chardev API to make it >>> easier to do non-blocking IO. This should considerably simplify the >>> above patches and benefit the rest of qemu (instead of having everyone >>> doing its own thing with seperate thread or other kind of continuation >>> state). >>> >>> What do you think? >> >> I am wondering whether it will help us to get rid of the >> conditions/notifications, like patches 9-11 of this series. I doubt 12 and >> 13 will change. At some point device state is supposed to be written out and >> in case of the TPM we have to wait for the response to have come back into >> the backend. We won't start listening on the file descriptor for an >> outstanding response, so I guess we will still wait for a notification in >> that case as well. So I am not sure which parts are going to be simpler... > Why would qemu need to wait for completion of emulator? Couldn't qemu > & emulator save the current state, including in-flights commands? > That's apparently what usbredir does. How could we make sure whether the PCRExtend has internally completed the extensions of the PCR but we haven't received the response yet and would just send the same command again? In general, the TPM cannot be sent the same commands twice due to (sevaral) commands (cryptographically) altering the internal state of the TPM. Stefan > >> Stefan >> >> >>>> backends/tpm.c | 29 +++++ >>>> hw/tpm/tpm_emulator.c | 303 >>>> +++++++++++++++++++++++++++++++++++++++++-- >>>> hw/tpm/tpm_tis.c | 216 +++++++++++++++++------------- >>>> hw/tpm/tpm_util.c | 7 + >>>> hw/tpm/tpm_util.h | 7 + >>>> include/sysemu/tpm_backend.h | 22 ++++ >>>> 6 files changed, 483 insertions(+), 101 deletions(-) >>>> >>>> -- >>>> 2.5.5 >>>> >>>> >>> > >
Hi On Fri, Dec 22, 2017 at 6:47 PM, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote: > On 12/22/2017 11:13 AM, Marc-André Lureau wrote: >> >> Hi >> >> On Fri, Dec 22, 2017 at 4:59 PM, Stefan Berger >> <stefanb@linux.vnet.ibm.com> wrote: >>> >>> On 12/22/2017 07:49 AM, Marc-André Lureau wrote: >>>> >>>> Hi >>>> >>>> On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger >>>> <stefanb@linux.vnet.ibm.com> wrote: >>>>> >>>>> This set of patches implements support for migrating the state of the >>>>> external 'swtpm' TPM emulator as well as that of the emulated device >>>>> interfaces. I have primarily tested this with the TIS and TPM 1.2 so >>>>> far, but it also seems to work with TPM 2. >>>>> >>>>> The TIS is simplified first by reducing the number of buffers and read >>>>> and write offsets into these buffers. Following the state machine of >>>>> the >>>>> TIS, a single buffer and r/w offset is enough for all localities since >>>>> only one locality can ever be active. >>>>> >>>>> This series applies on top of my tpm-next branch. >>>>> >>>>> One of the challenges that is addressed by this set of patches is the >>>>> fact >>>>> that the TPM emulator may be processing a command while the state >>>>> serialization of the devices is supposed to happen. A necessary first >>>>> step >>>>> has been implemented here that ensures that a response has been >>>>> received >>>>> from the exernal emulator and the bottom half function, which delivers >>>>> the >>>>> response and adjusts device registers (TIS or CRB), has been executed, >>>>> before the device's state is serialized. >>>>> >>>>> A subsequent extension may need to address the live migration loop and >>>>> delay >>>>> the serialization of devices until the response from the external TPM >>>>> has >>>>> been received. Though the likelihood that someone executes a >>>>> long-lasting >>>>> TPM command while this is occurring is certainly rare. >>>>> >>>>> Stefan >>>>> >>>>> Stefan Berger (13): >>>>> tpm_tis: convert uint32_t to size_t >>>>> tpm_tis: limit size of buffer from backend >>>>> tpm_tis: remove TPMSizeBuffer usage >>>>> tpm_tis: move buffers from localities into common location >>>>> tpm_tis: merge read and write buffer into single buffer >>>>> tpm_tis: move r/w_offsets to TPMState >>>>> tpm_tis: merge r/w_offset into rw_offset >>>>> tpm: Implement tpm_sized_buffer_reset >>>> >>>> ok for the above (you could queue/pull-req this now) >>>> >>>>> tpm: Introduce condition to notify waiters of completed command >>>>> tpm: Introduce condition in TPM backend for notification >>>>> tpm: implement tpm_backend_wait_cmd_completed >>>>> tpm: extend TPM emulator with state migration support >>>>> tpm_tis: extend TPM TIS with state migration support >>>> >>>> Much of the complexity from this migration series comes with the >>>> handling & synchronization of the IO thread. >>>> >>>> I think having a seperate thread doesn't bring much today TPM thread. >>>> it is a workaround for the chardev API being mostly synchronous. Am I >>>> wrong? (yes, passthrough doesn't use chardev, but it should probably >>>> use qio or chardev internally) >>>> >>>> Other kind of devices using a seperate process suffer the same >>>> problem. Just grep for qemu_chr_fe_write and look for the preceding >>>> comment, it's a common flaw in qemu code base. Code use the >>>> synchronous API, and sometime use non-blocking write >>>> (hw/usb/redirect.c seems quite better!) >>>> >>>> I would like to get rid of the seperate thread in TPM before we add >>>> migration support. We should try to improve the chardev API to make it >>>> easier to do non-blocking IO. This should considerably simplify the >>>> above patches and benefit the rest of qemu (instead of having everyone >>>> doing its own thing with seperate thread or other kind of continuation >>>> state). >>>> >>>> What do you think? >>> >>> >>> I am wondering whether it will help us to get rid of the >>> conditions/notifications, like patches 9-11 of this series. I doubt 12 >>> and >>> 13 will change. At some point device state is supposed to be written out >>> and >>> in case of the TPM we have to wait for the response to have come back >>> into >>> the backend. We won't start listening on the file descriptor for an >>> outstanding response, so I guess we will still wait for a notification in >>> that case as well. So I am not sure which parts are going to be >>> simpler... >> >> Why would qemu need to wait for completion of emulator? Couldn't qemu >> & emulator save the current state, including in-flights commands? >> That's apparently what usbredir does. > > > How could we make sure whether the PCRExtend has internally completed the > extensions of the PCR but we haven't received the response yet and would > just send the same command again? In general, the TPM cannot be sent the > same commands twice due to (sevaral) commands (cryptographically) altering > the internal state of the TPM. > Why would it send the same command again? If the commands is still in-flight on emulator side, ex: - send PCRExtend cmd - send SaveState cmd - receive SaveState reply Then the received state should be able to restore the emulator so that PCRExtend reply is received after migration. Wouldn't that work? -- Marc-André Lureau
On 12/22/2017 12:52 PM, Marc-André Lureau wrote: > Hi > > On Fri, Dec 22, 2017 at 6:47 PM, Stefan Berger > <stefanb@linux.vnet.ibm.com> wrote: >> On 12/22/2017 11:13 AM, Marc-André Lureau wrote: >>> Hi >>> >>> On Fri, Dec 22, 2017 at 4:59 PM, Stefan Berger >>> <stefanb@linux.vnet.ibm.com> wrote: >>>> On 12/22/2017 07:49 AM, Marc-André Lureau wrote: >>>>> Hi >>>>> >>>>> On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger >>>>> <stefanb@linux.vnet.ibm.com> wrote: >>>>>> This set of patches implements support for migrating the state of the >>>>>> external 'swtpm' TPM emulator as well as that of the emulated device >>>>>> interfaces. I have primarily tested this with the TIS and TPM 1.2 so >>>>>> far, but it also seems to work with TPM 2. >>>>>> >>>>>> The TIS is simplified first by reducing the number of buffers and read >>>>>> and write offsets into these buffers. Following the state machine of >>>>>> the >>>>>> TIS, a single buffer and r/w offset is enough for all localities since >>>>>> only one locality can ever be active. >>>>>> >>>>>> This series applies on top of my tpm-next branch. >>>>>> >>>>>> One of the challenges that is addressed by this set of patches is the >>>>>> fact >>>>>> that the TPM emulator may be processing a command while the state >>>>>> serialization of the devices is supposed to happen. A necessary first >>>>>> step >>>>>> has been implemented here that ensures that a response has been >>>>>> received >>>>>> from the exernal emulator and the bottom half function, which delivers >>>>>> the >>>>>> response and adjusts device registers (TIS or CRB), has been executed, >>>>>> before the device's state is serialized. >>>>>> >>>>>> A subsequent extension may need to address the live migration loop and >>>>>> delay >>>>>> the serialization of devices until the response from the external TPM >>>>>> has >>>>>> been received. Though the likelihood that someone executes a >>>>>> long-lasting >>>>>> TPM command while this is occurring is certainly rare. >>>>>> >>>>>> Stefan >>>>>> >>>>>> Stefan Berger (13): >>>>>> tpm_tis: convert uint32_t to size_t >>>>>> tpm_tis: limit size of buffer from backend >>>>>> tpm_tis: remove TPMSizeBuffer usage >>>>>> tpm_tis: move buffers from localities into common location >>>>>> tpm_tis: merge read and write buffer into single buffer >>>>>> tpm_tis: move r/w_offsets to TPMState >>>>>> tpm_tis: merge r/w_offset into rw_offset >>>>>> tpm: Implement tpm_sized_buffer_reset >>>>> ok for the above (you could queue/pull-req this now) >>>>> >>>>>> tpm: Introduce condition to notify waiters of completed command >>>>>> tpm: Introduce condition in TPM backend for notification >>>>>> tpm: implement tpm_backend_wait_cmd_completed >>>>>> tpm: extend TPM emulator with state migration support >>>>>> tpm_tis: extend TPM TIS with state migration support >>>>> Much of the complexity from this migration series comes with the >>>>> handling & synchronization of the IO thread. >>>>> >>>>> I think having a seperate thread doesn't bring much today TPM thread. >>>>> it is a workaround for the chardev API being mostly synchronous. Am I >>>>> wrong? (yes, passthrough doesn't use chardev, but it should probably >>>>> use qio or chardev internally) >>>>> >>>>> Other kind of devices using a seperate process suffer the same >>>>> problem. Just grep for qemu_chr_fe_write and look for the preceding >>>>> comment, it's a common flaw in qemu code base. Code use the >>>>> synchronous API, and sometime use non-blocking write >>>>> (hw/usb/redirect.c seems quite better!) >>>>> >>>>> I would like to get rid of the seperate thread in TPM before we add >>>>> migration support. We should try to improve the chardev API to make it >>>>> easier to do non-blocking IO. This should considerably simplify the >>>>> above patches and benefit the rest of qemu (instead of having everyone >>>>> doing its own thing with seperate thread or other kind of continuation >>>>> state). >>>>> >>>>> What do you think? >>>> >>>> I am wondering whether it will help us to get rid of the >>>> conditions/notifications, like patches 9-11 of this series. I doubt 12 >>>> and >>>> 13 will change. At some point device state is supposed to be written out >>>> and >>>> in case of the TPM we have to wait for the response to have come back >>>> into >>>> the backend. We won't start listening on the file descriptor for an >>>> outstanding response, so I guess we will still wait for a notification in >>>> that case as well. So I am not sure which parts are going to be >>>> simpler... >>> Why would qemu need to wait for completion of emulator? Couldn't qemu >>> & emulator save the current state, including in-flights commands? >>> That's apparently what usbredir does. >> >> How could we make sure whether the PCRExtend has internally completed the >> extensions of the PCR but we haven't received the response yet and would >> just send the same command again? In general, the TPM cannot be sent the >> same commands twice due to (sevaral) commands (cryptographically) altering >> the internal state of the TPM. >> > Why would it send the same command again? If the commands is still > in-flight on emulator side, ex: > > - send PCRExtend cmd > - send SaveState cmd > - receive SaveState reply > > Then the received state should be able to restore the emulator so that > PCRExtend reply is received after migration. > > Wouldn't that work? If the command has completed entirely, we can also just wait for the response to come back. Otherwise commands could be in the middle of processing and what would we do then? Freeze the CPU state of the process that's executing the swtpm? No, I think we need to wait for the response to come back and then retrieve the state blobs of the TPM.
On 12/22/2017 11:13 AM, Marc-André Lureau wrote: > Hi > > On Fri, Dec 22, 2017 at 4:59 PM, Stefan Berger > <stefanb@linux.vnet.ibm.com> wrote: >> On 12/22/2017 07:49 AM, Marc-André Lureau wrote: >>> Hi >>> >>> On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger >>> <stefanb@linux.vnet.ibm.com> wrote: >>>> This set of patches implements support for migrating the state of the >>>> external 'swtpm' TPM emulator as well as that of the emulated device >>>> interfaces. I have primarily tested this with the TIS and TPM 1.2 so >>>> far, but it also seems to work with TPM 2. >>>> >>>> The TIS is simplified first by reducing the number of buffers and read >>>> and write offsets into these buffers. Following the state machine of the >>>> TIS, a single buffer and r/w offset is enough for all localities since >>>> only one locality can ever be active. >>>> >>>> This series applies on top of my tpm-next branch. >>>> >>>> One of the challenges that is addressed by this set of patches is the >>>> fact >>>> that the TPM emulator may be processing a command while the state >>>> serialization of the devices is supposed to happen. A necessary first >>>> step >>>> has been implemented here that ensures that a response has been received >>>> from the exernal emulator and the bottom half function, which delivers >>>> the >>>> response and adjusts device registers (TIS or CRB), has been executed, >>>> before the device's state is serialized. >>>> >>>> A subsequent extension may need to address the live migration loop and >>>> delay >>>> the serialization of devices until the response from the external TPM has >>>> been received. Though the likelihood that someone executes a long-lasting >>>> TPM command while this is occurring is certainly rare. >>>> >>>> Stefan >>>> >>>> Stefan Berger (13): >>>> tpm_tis: convert uint32_t to size_t >>>> tpm_tis: limit size of buffer from backend >>>> tpm_tis: remove TPMSizeBuffer usage >>>> tpm_tis: move buffers from localities into common location >>>> tpm_tis: merge read and write buffer into single buffer >>>> tpm_tis: move r/w_offsets to TPMState >>>> tpm_tis: merge r/w_offset into rw_offset >>>> tpm: Implement tpm_sized_buffer_reset >>> ok for the above (you could queue/pull-req this now) >>> >>>> tpm: Introduce condition to notify waiters of completed command >>>> tpm: Introduce condition in TPM backend for notification >>>> tpm: implement tpm_backend_wait_cmd_completed >>>> tpm: extend TPM emulator with state migration support >>>> tpm_tis: extend TPM TIS with state migration support >>> Much of the complexity from this migration series comes with the >>> handling & synchronization of the IO thread. >>> >>> I think having a seperate thread doesn't bring much today TPM thread. >>> it is a workaround for the chardev API being mostly synchronous. Am I >>> wrong? (yes, passthrough doesn't use chardev, but it should probably >>> use qio or chardev internally) >>> >>> Other kind of devices using a seperate process suffer the same >>> problem. Just grep for qemu_chr_fe_write and look for the preceding >>> comment, it's a common flaw in qemu code base. Code use the >>> synchronous API, and sometime use non-blocking write >>> (hw/usb/redirect.c seems quite better!) >>> >>> I would like to get rid of the seperate thread in TPM before we add >>> migration support. We should try to improve the chardev API to make it >>> easier to do non-blocking IO. This should considerably simplify the >>> above patches and benefit the rest of qemu (instead of having everyone >>> doing its own thing with seperate thread or other kind of continuation >>> state). >>> >>> What do you think? >> >> I am wondering whether it will help us to get rid of the >> conditions/notifications, like patches 9-11 of this series. I doubt 12 and >> 13 will change. At some point device state is supposed to be written out and >> in case of the TPM we have to wait for the response to have come back into >> the backend. We won't start listening on the file descriptor for an >> outstanding response, so I guess we will still wait for a notification in >> that case as well. So I am not sure which parts are going to be simpler... > Why would qemu need to wait for completion of emulator? Couldn't qemu > & emulator save the current state, including in-flights commands? > That's apparently what usbredir does. The thread is sending the commands to the external emulator and waits for the reception of the response. Once the response is received, the delivery of the response is handed off to the bottom half. Using the bottom half avoids synchronization primitives since we know that no other thread is in the device emulator when it runs (except for our thread sending to and receiving from the emulator). This series of patches introduces a tpm_busy flag that the device emulation thread sets once it hands off a command to the thread for processing. It does this in tpm_backend_deliver_request(). It resets that flag once it has scheduled the bottom half to run for delivering the response to the device emulator (TIS, CRB etc.). This in turn is happening in tpm_backend_worker_thread(). For the migration we have the following cases once the device (frontend or backend) is supposed to write out its state: [Once the state of the device is supposed to be written out we know what the OS is paused and no further TIS registers will be written to after that and no further thread is busy emulating writes to TIS registers. TIS being an example here for 'device emulation']. - no TPM command is currently processed: the tpm_busy flag is false and we can write out the state immediately. The .pre_save function does nothing in this case. - a TPM command is currently processed: we have to wait for the response to come back and the thread to set the tpm_busy flag to false. This is what tpm_backend_wait_cmd_completed() is good for. Once that has happened, the bottom half will have been scheduled by the thread but the bottom half will not run at this point, so we have to do what the bottom half is supposed to do. All this is happening in the .pre_save function. Here we call tpm_backend_wait_cmd_completed() which returns once the response has been received back. We then check a state variable that indicates whether the front-end is executing a command and run the bottom half's function. (Improvement: we could also return true in case we had to wait for the response to come back and avoid the check on the state variable and just run the bottom half's function due to the preceding wait) Your refactoring has actually helped a lot in simplifying this and pushed a lot of the code into the common tpm_backend_* functions. The code to suspend/resume looks rather simple at this point. It mostly comes down to refactoring the function the bottom half is supposed to run so it can be run from the .pre_save function as well. TIS: https://github.com/stefanberger/qemu-tpm/commit/3b0689619be9a1f3fbbea2aa2f98802512f96099 CRB: https://github.com/stefanberger/qemu-tpm/commit/62db4ace06e77c8e5f8274e8cb9240e5f8d663ab SPAPR: https://github.com/stefanberger/qemu-tpm/commit/1f31cd8b7f690f750405d911923f624b5f856a04 I may now modify tpm_backend_wait_cmd_completed() to 'bool run_bh_function = tpm_backend_wait_cmd_completed(s)'. This should help the cause and make the 3 devices' suspend code even look more similar. Regards, Stefan
© 2016 - 2024 Red Hat, Inc.