| Message ID | 20250228170720.144739-4-sgarzare@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enlightened vTPM support for SVSM on SEV-SNP | expand |
On Fri, Feb 28, 2025 at 06:07:17PM +0100, Stefano Garzarella wrote: > + int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len, > + size_t to_send); Please describe the meaning and purpose of to_send. BR, Jarkko
On 2/28/25 11:07, Stefano Garzarella wrote: > Some devices do not support interrupts and provide a single operation > to send the command and receive the response on the same buffer. > > To support this scenario, a driver could set TPM_CHIP_FLAG_IRQ in the > chip's flags to get recv() to be called immediately after send() in > tpm_try_transmit(). > > Instead of abusing TPM_CHIP_FLAG_IRQ, introduce a new callback > send_recv(). If that callback is defined, it is called in > tpm_try_transmit() to send the command and receive the response on > the same buffer in a single call. > > Suggested-by: Jason Gunthorpe <jgg@ziepe.ca> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > include/linux/tpm.h | 2 ++ > drivers/char/tpm/tpm-interface.c | 8 +++++++- > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index 20a40ade8030..2ede8e0592d3 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -88,6 +88,8 @@ struct tpm_class_ops { > bool (*req_canceled)(struct tpm_chip *chip, u8 status); > int (*recv) (struct tpm_chip *chip, u8 *buf, size_t len); > int (*send) (struct tpm_chip *chip, u8 *buf, size_t len); > + int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len, > + size_t to_send); > void (*cancel) (struct tpm_chip *chip); > u8 (*status) (struct tpm_chip *chip); > void (*update_timeouts)(struct tpm_chip *chip, > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > index b1daa0d7b341..4f92b0477696 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -82,6 +82,9 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz) > return -E2BIG; > } > > + if (chip->ops->send_recv) > + goto out_recv; It might look a bit cleaner if you issue the send_recv() call here and then jump to a new label after the recv() call just before 'len' is checked. Thanks, Tom > + > rc = chip->ops->send(chip, buf, count); > if (rc < 0) { > if (rc != -EPIPE) > @@ -123,7 +126,10 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz) > return -ETIME; > > out_recv: > - len = chip->ops->recv(chip, buf, bufsiz); > + if (chip->ops->send_recv) > + len = chip->ops->send_recv(chip, buf, bufsiz, count); > + else > + len = chip->ops->recv(chip, buf, bufsiz); > if (len < 0) { > rc = len; > dev_err(&chip->dev, "tpm_transmit: tpm_recv: error %d\n", rc);
On Sat, Mar 01, 2025 at 03:45:10AM +0200, Jarkko Sakkinen wrote: >On Fri, Feb 28, 2025 at 06:07:17PM +0100, Stefano Garzarella wrote: >> + int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len, >> + size_t to_send); > >Please describe the meaning and purpose of to_send. Sure, I'll add in the commit description. Should I add documentation in the code as well? The other callbacks don't have that, but if you think it's useful we can start with that, I mean something like this: /** * send_recv() - send the command and receive the response on the same * buffer in a single call. * * @chip: The TPM chip * @buf: A buffer used to both send the command and receive the response * @buf_len: The size of the buffer * @to_send: Number of bytes in the buffer to send * * Return: number of received bytes on success, negative error code on * failure. */ int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len, size_t to_send); Thanks, Stefano
On Mon, Mar 03, 2025 at 08:06:43AM -0600, Tom Lendacky wrote: >On 2/28/25 11:07, Stefano Garzarella wrote: >> Some devices do not support interrupts and provide a single operation >> to send the command and receive the response on the same buffer. >> >> To support this scenario, a driver could set TPM_CHIP_FLAG_IRQ in the >> chip's flags to get recv() to be called immediately after send() in >> tpm_try_transmit(). >> >> Instead of abusing TPM_CHIP_FLAG_IRQ, introduce a new callback >> send_recv(). If that callback is defined, it is called in >> tpm_try_transmit() to send the command and receive the response on >> the same buffer in a single call. >> >> Suggested-by: Jason Gunthorpe <jgg@ziepe.ca> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> --- >> include/linux/tpm.h | 2 ++ >> drivers/char/tpm/tpm-interface.c | 8 +++++++- >> 2 files changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/tpm.h b/include/linux/tpm.h >> index 20a40ade8030..2ede8e0592d3 100644 >> --- a/include/linux/tpm.h >> +++ b/include/linux/tpm.h >> @@ -88,6 +88,8 @@ struct tpm_class_ops { >> bool (*req_canceled)(struct tpm_chip *chip, u8 status); >> int (*recv) (struct tpm_chip *chip, u8 *buf, size_t len); >> int (*send) (struct tpm_chip *chip, u8 *buf, size_t len); >> + int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len, >> + size_t to_send); >> void (*cancel) (struct tpm_chip *chip); >> u8 (*status) (struct tpm_chip *chip); >> void (*update_timeouts)(struct tpm_chip *chip, >> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c >> index b1daa0d7b341..4f92b0477696 100644 >> --- a/drivers/char/tpm/tpm-interface.c >> +++ b/drivers/char/tpm/tpm-interface.c >> @@ -82,6 +82,9 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz) >> return -E2BIG; >> } >> >> + if (chip->ops->send_recv) >> + goto out_recv; > >It might look a bit cleaner if you issue the send_recv() call here and >then jump to a new label after the recv() call just before 'len' is checked. Yep, I see, I was undecided to avoid adding a new label and just have out_recv which in future cases always handles the send_recv() case. But maybe I overthought, I will do as you suggest. Thanks, Stefano > >Thanks, >Tom > >> + >> rc = chip->ops->send(chip, buf, count); >> if (rc < 0) { >> if (rc != -EPIPE) >> @@ -123,7 +126,10 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz) >> return -ETIME; >> >> out_recv: >> - len = chip->ops->recv(chip, buf, bufsiz); >> + if (chip->ops->send_recv) >> + len = chip->ops->send_recv(chip, buf, bufsiz, count); >> + else >> + len = chip->ops->recv(chip, buf, bufsiz); >> if (len < 0) { >> rc = len; >> dev_err(&chip->dev, "tpm_transmit: tpm_recv: error %d\n", rc); >
On Mon, 2025-03-03 at 17:21 +0100, Stefano Garzarella wrote: > On Sat, Mar 01, 2025 at 03:45:10AM +0200, Jarkko Sakkinen wrote: > > On Fri, Feb 28, 2025 at 06:07:17PM +0100, Stefano Garzarella wrote: > > > + int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t > > > buf_len, > > > + size_t to_send); > > > > Please describe the meaning and purpose of to_send. > > Sure, I'll add in the commit description. It's always a command, right? So better be more concerete than "to_send", e.g. "cmd_len". I'd do instead: if (!chip->send) goto out_recv; And change recv into: int (*recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len, cmd_len); Those who don't need the last parameter, can ignore it. This also reduces meaningless possible states for the ops structure such as "send_recv and send or recv defined", i.e. makes it overall more mutually exclusive. > > Should I add documentation in the code as well? > > The other callbacks don't have that, but if you think it's useful we > can > start with that, I mean something like this: > > /** > * send_recv() - send the command and receive the response > on the same > * buffer in a single call. > * > * @chip: The TPM chip > * @buf: A buffer used to both send the command and receive > the response > * @buf_len: The size of the buffer > * @to_send: Number of bytes in the buffer to send > * > * Return: number of received bytes on success, negative > error code on > * failure. > */ > int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t > buf_len, > size_t to_send); I would not document in callback level as their implementation is not global. This is probably stance also taken by file_operations, vm_ops and many other places with "ops" structure. > > Thanks, > Stefano > > BR, Jarkko
On Tue, Mar 04, 2025 at 06:56:02PM +0200, Jarkko Sakkinen wrote: > On Mon, 2025-03-03 at 17:21 +0100, Stefano Garzarella wrote: > > On Sat, Mar 01, 2025 at 03:45:10AM +0200, Jarkko Sakkinen wrote: > > > On Fri, Feb 28, 2025 at 06:07:17PM +0100, Stefano Garzarella wrote: > > > > + int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t > > > > buf_len, > > > > + size_t to_send); > > > > > > Please describe the meaning and purpose of to_send. > > > > Sure, I'll add in the commit description. > > It's always a command, right? So better be more concerete than > "to_send", e.g. "cmd_len". > > I'd do instead: > > if (!chip->send) > goto out_recv; > > And change recv into: > > int (*recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len, > cmd_len); I think I went here over the top, and *if* we need a new callback putting send_recv would be fine. Only thing I'd take from this is to rename to_len as cmd_len. However, I don't think there are strong enough reasons to add complexity to the callback interface with the basis of this single driver. You should deal with this internally inside the driver instead. So do something along the lines of, e.g.: 1. Create dummy send() copying the command to internal buffer. 2. Create ->status() returning zero, and set req_complete_mask and req_complete_val to zero. 3. Performan transaction in recv(). How you split send_recv() between send() and recv() is up to you. This was merely showing that we don't need send_recv() desperately. BR, Jarkko
On Tue, Mar 04, 2025 at 10:21:55PM +0200, Jarkko Sakkinen wrote: >On Tue, Mar 04, 2025 at 06:56:02PM +0200, Jarkko Sakkinen wrote: >> On Mon, 2025-03-03 at 17:21 +0100, Stefano Garzarella wrote: >> > On Sat, Mar 01, 2025 at 03:45:10AM +0200, Jarkko Sakkinen wrote: >> > > On Fri, Feb 28, 2025 at 06:07:17PM +0100, Stefano Garzarella wrote: >> > > > + int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t >> > > > buf_len, >> > > > + size_t to_send); >> > > >> > > Please describe the meaning and purpose of to_send. >> > >> > Sure, I'll add in the commit description. >> >> It's always a command, right? So better be more concerete than >> "to_send", e.g. "cmd_len". Right! >> >> I'd do instead: >> >> if (!chip->send) >> goto out_recv; >> >> And change recv into: >> >> int (*recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len, >> cmd_len); > >I think I went here over the top, and *if* we need a new callback >putting send_recv would be fine. Only thing I'd take from this is to >rename to_len as cmd_len. Got it. > >However, I don't think there are strong enough reasons to add complexity >to the callback interface with the basis of this single driver. You >should deal with this internally inside the driver instead. > >So do something along the lines of, e.g.: > >1. Create dummy send() copying the command to internal > buffer. >2. Create ->status() returning zero, and set req_complete_mask and > req_complete_val to zero. >3. Performan transaction in recv(). > >How you split send_recv() between send() and recv() is up to you. This >was merely showing that we don't need send_recv() desperately. We did something similar in v1 [1], but instead of your point 2, we just set `chip->flags |= TPM_CHIP_FLAG_IRQ;` in the probe() after we allocated the chip. Jason suggested the send_recv() ops [2], which I liked, but if you prefer to avoid that, I can restore what we did in v1 and replace the TPM_CHIP_FLAG_IRQ hack with your point 2 (or use TPM_CHIP_FLAG_IRQ if you think it is fine). @Jarkko, @Jason, I don't have a strong preference about it, so your choice :-) Thanks, Stefano [1] https://lore.kernel.org/linux-integrity/20241210143423.101774-2-sgarzare@redhat.com/ [2] https://lore.kernel.org/linux-integrity/CAGxU2F51EoqDqi6By6eBa7qT+VT006DJ9+V-PANQ6GQrwVWt_Q@mail.gmail.com/
On Wed, Mar 05, 2025 at 10:04:25AM +0100, Stefano Garzarella wrote: > Jason suggested the send_recv() ops [2], which I liked, but if you prefer to > avoid that, I can restore what we did in v1 and replace the > TPM_CHIP_FLAG_IRQ hack with your point 2 (or use TPM_CHIP_FLAG_IRQ if you > think it is fine). I think it is a pretty notable simplification for the driver as it does not need to implement send, status, req_canceled and more ops. Given the small LOC on the core side I'd call that simplification a win.. Jason
On Wed, Mar 05, 2025 at 03:02:29PM -0400, Jason Gunthorpe wrote: > On Wed, Mar 05, 2025 at 10:04:25AM +0100, Stefano Garzarella wrote: > > Jason suggested the send_recv() ops [2], which I liked, but if you prefer to > > avoid that, I can restore what we did in v1 and replace the > > TPM_CHIP_FLAG_IRQ hack with your point 2 (or use TPM_CHIP_FLAG_IRQ if you > > think it is fine). > > I think it is a pretty notable simplification for the driver as it > does not need to implement send, status, req_canceled and more ops. > > Given the small LOC on the core side I'd call that simplification a > win.. I'm sorry to disagree with you on this but adding a callback for one leaf driver is not what I would call "a win" :-) I mean, it's either a minor twist in 1. "the framework code" which affects in a way all other leaf drivers. At bare minimum it adds a tiny bit of complexity to the callback interface and a tiny bit of accumulated maintenance cost. 2. in the leaf driver So I'd really would want to keep that tiny bit of extra complexity localized. > > Jason BR, Jarkko
On Wed, Mar 05, 2025 at 10:04:25AM +0100, Stefano Garzarella wrote: > On Tue, Mar 04, 2025 at 10:21:55PM +0200, Jarkko Sakkinen wrote: > > On Tue, Mar 04, 2025 at 06:56:02PM +0200, Jarkko Sakkinen wrote: > > > On Mon, 2025-03-03 at 17:21 +0100, Stefano Garzarella wrote: > > > > On Sat, Mar 01, 2025 at 03:45:10AM +0200, Jarkko Sakkinen wrote: > > > > > On Fri, Feb 28, 2025 at 06:07:17PM +0100, Stefano Garzarella wrote: > > > > > > + int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t > > > > > > buf_len, > > > > > > + size_t to_send); > > > > > > > > > > Please describe the meaning and purpose of to_send. > > > > > > > > Sure, I'll add in the commit description. > > > > > > It's always a command, right? So better be more concerete than > > > "to_send", e.g. "cmd_len". > > Right! > > > > > > > I'd do instead: > > > > > > if (!chip->send) > > > goto out_recv; > > > > > > And change recv into: > > > > > > int (*recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len, > > > cmd_len); > > > > I think I went here over the top, and *if* we need a new callback > > putting send_recv would be fine. Only thing I'd take from this is to > > rename to_len as cmd_len. > > Got it. > > > > > However, I don't think there are strong enough reasons to add complexity > > to the callback interface with the basis of this single driver. You > > should deal with this internally inside the driver instead. > > > > So do something along the lines of, e.g.: > > > > 1. Create dummy send() copying the command to internal > > buffer. > > 2. Create ->status() returning zero, and set req_complete_mask and > > req_complete_val to zero. > > 3. Performan transaction in recv(). > > > > How you split send_recv() between send() and recv() is up to you. This > > was merely showing that we don't need send_recv() desperately. > > We did something similar in v1 [1], but instead of your point 2, we just set > `chip->flags |= TPM_CHIP_FLAG_IRQ;` in the probe() after we allocated the > chip. > > Jason suggested the send_recv() ops [2], which I liked, but if you prefer to > avoid that, I can restore what we did in v1 and replace the > TPM_CHIP_FLAG_IRQ hack with your point 2 (or use TPM_CHIP_FLAG_IRQ if you > think it is fine). > > @Jarkko, @Jason, I don't have a strong preference about it, so your choice > :-) I'd say, unless you have actual identified blocker, please go with a driver where the complexity is managed within the driver. > > Thanks, > Stefano > > [1] https://lore.kernel.org/linux-integrity/20241210143423.101774-2-sgarzare@redhat.com/ > [2] https://lore.kernel.org/linux-integrity/CAGxU2F51EoqDqi6By6eBa7qT+VT006DJ9+V-PANQ6GQrwVWt_Q@mail.gmail.com/ BR, Jarkko
On Thu, Mar 06, 2025 at 11:52:46PM +0200, Jarkko Sakkinen wrote: >On Wed, Mar 05, 2025 at 03:02:29PM -0400, Jason Gunthorpe wrote: >> On Wed, Mar 05, 2025 at 10:04:25AM +0100, Stefano Garzarella wrote: >> > Jason suggested the send_recv() ops [2], which I liked, but if you prefer to >> > avoid that, I can restore what we did in v1 and replace the >> > TPM_CHIP_FLAG_IRQ hack with your point 2 (or use TPM_CHIP_FLAG_IRQ if you >> > think it is fine). >> >> I think it is a pretty notable simplification for the driver as it >> does not need to implement send, status, req_canceled and more ops. >> >> Given the small LOC on the core side I'd call that simplification a >> win.. > >I'm sorry to disagree with you on this but adding a callback for >one leaf driver is not what I would call "a win" :-) IIUC in the ftpm driver (tpm_ftpm_tee.c) we could also use send_recv() and save a memcpy() to a temporally buffer (pvt_data->resp_buf) and also that 4k buffer allocated with the private data of the driver. BTW if you agree, for now I'll do something similar of what we do in the ftpm driver (which would be what Jarkko recommended - status() returns 0, .req_complete_mask = 0, .req_complete_val = 0) and we can discuss send_recv() in a new series where I can include changes for the ftpm driver too, to see whether it makes sense or not. WDYT? Thanks, Stefano > >I mean, it's either a minor twist in > >1. "the framework code" which affects in a way all other leaf drivers. > At bare minimum it adds a tiny bit of complexity to the callback > interface and a tiny bit of accumulated maintenance cost. >2. in the leaf driver > >So I'd really would want to keep that tiny bit of extra complexity >localized. > >> >> Jason > >BR, Jarkko >
On Fri, Mar 07, 2025 at 12:15:34AM +0200, Jarkko Sakkinen wrote: >On Wed, Mar 05, 2025 at 10:04:25AM +0100, Stefano Garzarella wrote: >> On Tue, Mar 04, 2025 at 10:21:55PM +0200, Jarkko Sakkinen wrote: >> > On Tue, Mar 04, 2025 at 06:56:02PM +0200, Jarkko Sakkinen wrote: >> > > On Mon, 2025-03-03 at 17:21 +0100, Stefano Garzarella wrote: >> > > > On Sat, Mar 01, 2025 at 03:45:10AM +0200, Jarkko Sakkinen wrote: >> > > > > On Fri, Feb 28, 2025 at 06:07:17PM +0100, Stefano Garzarella wrote: >> > > > > > + int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t >> > > > > > buf_len, >> > > > > > + size_t to_send); >> > > > > >> > > > > Please describe the meaning and purpose of to_send. >> > > > >> > > > Sure, I'll add in the commit description. >> > > >> > > It's always a command, right? So better be more concerete than >> > > "to_send", e.g. "cmd_len". >> >> Right! >> >> > > >> > > I'd do instead: >> > > >> > > if (!chip->send) >> > > goto out_recv; >> > > >> > > And change recv into: >> > > >> > > int (*recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len, >> > > cmd_len); >> > >> > I think I went here over the top, and *if* we need a new callback >> > putting send_recv would be fine. Only thing I'd take from this is to >> > rename to_len as cmd_len. >> >> Got it. >> >> > >> > However, I don't think there are strong enough reasons to add complexity >> > to the callback interface with the basis of this single driver. You >> > should deal with this internally inside the driver instead. >> > >> > So do something along the lines of, e.g.: >> > >> > 1. Create dummy send() copying the command to internal >> > buffer. >> > 2. Create ->status() returning zero, and set req_complete_mask and >> > req_complete_val to zero. >> > 3. Performan transaction in recv(). >> > >> > How you split send_recv() between send() and recv() is up to you. This >> > was merely showing that we don't need send_recv() desperately. >> >> We did something similar in v1 [1], but instead of your point 2, we just set >> `chip->flags |= TPM_CHIP_FLAG_IRQ;` in the probe() after we allocated the >> chip. >> >> Jason suggested the send_recv() ops [2], which I liked, but if you prefer to >> avoid that, I can restore what we did in v1 and replace the >> TPM_CHIP_FLAG_IRQ hack with your point 2 (or use TPM_CHIP_FLAG_IRQ if you >> think it is fine). >> >> @Jarkko, @Jason, I don't have a strong preference about it, so your choice >> :-) > >I'd say, unless you have actual identified blocker, please go with >a driver where the complexity is managed within the driver. Yep, got it ;-) Thanks, Stefano
On Fri, Mar 07, 2025 at 04:37:12PM +0100, Stefano Garzarella wrote: > On Thu, Mar 06, 2025 at 11:52:46PM +0200, Jarkko Sakkinen wrote: > > On Wed, Mar 05, 2025 at 03:02:29PM -0400, Jason Gunthorpe wrote: > > > On Wed, Mar 05, 2025 at 10:04:25AM +0100, Stefano Garzarella wrote: > > > > Jason suggested the send_recv() ops [2], which I liked, but if you prefer to > > > > avoid that, I can restore what we did in v1 and replace the > > > > TPM_CHIP_FLAG_IRQ hack with your point 2 (or use TPM_CHIP_FLAG_IRQ if you > > > > think it is fine). > > > > > > I think it is a pretty notable simplification for the driver as it > > > does not need to implement send, status, req_canceled and more ops. > > > > > > Given the small LOC on the core side I'd call that simplification a > > > win.. > > > > I'm sorry to disagree with you on this but adding a callback for > > one leaf driver is not what I would call "a win" :-) > > IIUC in the ftpm driver (tpm_ftpm_tee.c) we could also use send_recv() and > save a memcpy() to a temporally buffer (pvt_data->resp_buf) and also that 4k > buffer allocated with the private data of the driver. > > BTW if you agree, for now I'll do something similar of what we do in the > ftpm driver (which would be what Jarkko recommended - status() returns 0, > .req_complete_mask = 0, .req_complete_val = 0) and we can discuss > send_recv() in a new series where I can include changes for the ftpm driver > too, to see whether it makes sense or not. > > WDYT? Yeah, that would work. Althought not related to this callback interface per se, also tpm-dev-common.c is one example (in a way). > > Thanks, > Stefano BR, Jarkko
diff --git a/include/linux/tpm.h b/include/linux/tpm.h index 20a40ade8030..2ede8e0592d3 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -88,6 +88,8 @@ struct tpm_class_ops { bool (*req_canceled)(struct tpm_chip *chip, u8 status); int (*recv) (struct tpm_chip *chip, u8 *buf, size_t len); int (*send) (struct tpm_chip *chip, u8 *buf, size_t len); + int (*send_recv)(struct tpm_chip *chip, u8 *buf, size_t buf_len, + size_t to_send); void (*cancel) (struct tpm_chip *chip); u8 (*status) (struct tpm_chip *chip); void (*update_timeouts)(struct tpm_chip *chip, diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index b1daa0d7b341..4f92b0477696 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -82,6 +82,9 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz) return -E2BIG; } + if (chip->ops->send_recv) + goto out_recv; + rc = chip->ops->send(chip, buf, count); if (rc < 0) { if (rc != -EPIPE) @@ -123,7 +126,10 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz) return -ETIME; out_recv: - len = chip->ops->recv(chip, buf, bufsiz); + if (chip->ops->send_recv) + len = chip->ops->send_recv(chip, buf, bufsiz, count); + else + len = chip->ops->recv(chip, buf, bufsiz); if (len < 0) { rc = len; dev_err(&chip->dev, "tpm_transmit: tpm_recv: error %d\n", rc);
Some devices do not support interrupts and provide a single operation to send the command and receive the response on the same buffer. To support this scenario, a driver could set TPM_CHIP_FLAG_IRQ in the chip's flags to get recv() to be called immediately after send() in tpm_try_transmit(). Instead of abusing TPM_CHIP_FLAG_IRQ, introduce a new callback send_recv(). If that callback is defined, it is called in tpm_try_transmit() to send the command and receive the response on the same buffer in a single call. Suggested-by: Jason Gunthorpe <jgg@ziepe.ca> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- include/linux/tpm.h | 2 ++ drivers/char/tpm/tpm-interface.c | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-)