Xen devel - Jan 2008 - Re: [PATCH] [Linux] Transfer TPM locality info in the ringstructure

>--- a/drivers/xen/tpmback/tpmback.c	Thu Dec 20 16:58:14 2007 +0000
>+++ b/drivers/xen/tpmback/tpmback.c	Wed Jan 02 14:19:04 2008 -0500
>@@ -298,6 +298,18 @@ int _packet_write(struct packet *pak,
> 	return rc;
> }
> 
>+
>+static u8 get_locty_ring(tpmif_t *tpmif)
>+{
>+	tpmif_tx_request_t *tx = &tpmif->tx->ring[0].req;
>+
>+	if (tx->version == 1)
Shouldn''t this be >= 1?
>+		return tx->locality;
>+
>+	return 0;
>+}
>+
>+
> /*
>  * Read data from the shared memory and copy it directly into the
>  * provided buffer. Advance the read_last indicator which tells
Also, while your patch at the first glance appears to take care of backward
compatibility, I''m not sure it really does: In the old code, I
can''t see where
the ''unused'' member of ''struct
tpmif_tx_request'' gets zero-initialized.
Wouldn''t it be possible to clear the shared page in the backend rather
than
the frontend?

Jan

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

"Jan Beulich" <jbeulich@novell.com> wrote on 01/04/2008 03:56:24
AM:
> >--- a/drivers/xen/tpmback/tpmback.c   Thu Dec 20 16:58:14 2007 +0000
> >+++ b/drivers/xen/tpmback/tpmback.c   Wed Jan 02 14:19:04 2008 -0500
> >@@ -298,6 +298,18 @@ int _packet_write(struct packet *pak,
> >    return rc;
> > }
> > 
> >+
> >+static u8 get_locty_ring(tpmif_t *tpmif)
> >+{
> >+   tpmif_tx_request_t *tx = &tpmif->tx->ring[0].req;
> >+
> >+   if (tx->version == 1)
> 
> Shouldn''t this be >= 1?
> 
> >+      return tx->locality;
> >+
> >+   return 0;
> >+}
> >+
> >+
> > /*
> >  * Read data from the shared memory and copy it directly into the
> >  * provided buffer. Advance the read_last indicator which tells
> 
> Also, while your patch at the first glance appears to take care of 
backward
> compatibility, I''m not sure it really does: In the old code, I
can''tsee
where
> the ''unused'' member of ''struct
tpmif_tx_request'' gets zero-initialized.
Yes, that''s a problem. What I''ll do is set the
''unused'' member to zero and
leave it at that.

Thanks.

   Stefan

> Wouldn''t it be possible to clear the shared page in the backend
rather
than
> the frontend?
> 
> Jan
> 
> Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

How does that fix the problem if an old tpm frontend runs against a new tpm
backend? The unused field will not be initialised by the old frontend, hence
can contain garbage to confuse the new backend?

 -- Keir

On 4/1/08 14:12, "Stefan Berger" <stefanb@us.ibm.com> wrote:
>>> > >  * Read data from the shared memory and copy it directly
into the
>>> > >  * provided buffer. Advance the read_last indicator which
tells
>> > 
>> > Also, while your patch at the first glance appears to take care of
backward
>> > compatibility, I''m not sure it really does: In the old
code, I can''tsee
>> where
>> > the ''unused'' member of ''struct
tpmif_tx_request'' gets zero-initialized.
> 
> Yes, that''s a problem. What I''ll do is set the
''unused'' member to zero and
> leave it at that.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote on 01/06/2008 06:12:06 PM:
> How does that fix the problem if an old tpm frontend runs against a 
> new tpm backend? The unused field will not be initialised by the old
> frontend, hence can contain garbage to confuse the new backend?
> 
I don''t know of any better solution than initializing this
''now''.

   Stefan

>  -- Keir
> 
> On 4/1/08 14:12, "Stefan Berger" <stefanb@us.ibm.com>
wrote:
> > >  * Read data from the shared memory and copy it directly into the
> > >  * provided buffer. Advance the read_last indicator which tells
> > 
> > Also, while your patch at the first glance appears to take care of 
backward
> > compatibility, I''m not sure it really does: In the old code,
I
> can''tsee where
> > the ''unused'' member of ''struct
tpmif_tx_request'' gets
zero-initialized. 
> 
> Yes, that''s a problem. What I''ll do is set the
''unused'' member to
> zero and leave it at that. 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Personally I do not much care about compatibility of new backend with old
frontend, or vice versa, since I doubt that the vtpm drivers have many
users. But others with an interest in the vtpm drivers might care, as the
errors that mismatched drivers will cause are likely to be moderately hard
to debug. If compatibility is required, you¹ll have to negotiate the new
request format via xenstore. It¹s not that hard: see the
request-foo/feature-foo handshake that goes on e.g., between netfront and
netback.

 -- Keir

On 7/1/08 11:53, "Stefan Berger" <stefanb@us.ibm.com> wrote:
> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote on 01/06/2008 06:12:06
PM:
> 
>> > How does that fix the problem if an old tpm frontend runs against
a
>> > new tpm backend? The unused field will not be initialised by the
old
>> > frontend, hence can contain garbage to confuse the new backend?
>> > 
> 
> I don''t know of any better solution than initializing this
''now''.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote on 01/07/2008 07:58:25 AM:
> Personally I do not much care about compatibility of new backend 
> with old frontend, or vice versa, since I doubt that the vtpm 
> drivers have many users. But others with an interest in the vtpm 
> drivers might care, as the errors that mismatched drivers will cause
> are likely to be moderately hard to debug. If compatibility is 
> required, you?ll have to negotiate the new request format via 
> xenstore. It?s not that hard: see the request-foo/feature-foo 
> handshake that goes on e.g., between netfront and netback.
Actually, intializing the ring to zeroes once the mapping is done does 
solve the problem for the frontend that currently does not touch the 
uninitialized variable. Though, to correct this, I am also initializing 
the ''unused'' member on the frontend in a patch that
I''l post soon.

   Stefan
> 
>  -- Keir
> 
> On 7/1/08 11:53, "Stefan Berger" <stefanb@us.ibm.com>
wrote:
> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote on 01/06/2008 06:12:06
PM:
> 
> > How does that fix the problem if an old tpm frontend runs against a 
> > new tpm backend? The unused field will not be initialised by the old
> > frontend, hence can contain garbage to confuse the new backend?
> > 
> 
> I don''t know of any better solution than initializing this
''now''.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel