RE: Testing the geode-aes driver with the tcrypt module completely freezes the machine

RE: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Martin Schiller]

On Thursday, May 03, 2007 00:58:45 -0700 , Evgeniy Polyakov wrote:

> Hi Martin.

Hi Evgeniy

>> Sorry for my late answer, but I didn't get your message because I haven't
>> subscribed to the mailing-list.
>> I've "found" it yesterday on the mailing-list archive. So please, could
you
>> reply directly to me and to the mailing-list on any further messages?
>
> I did, but your mail server bounced message.
> This message is also sent to you directly, hope it will pass through.
> Btw, AMD list bounces too.

It didn't, but I talked to our mail server admin and now your mails should
reach me. :-)

After adding the debug outputs from your last patch, none of them was
displayed.
So I started adding some more outputs and doing a "line-by-line" debugging
and found out this (so far):

The freeze is the result of an invinite while-loop in the function
geode_ecb_encrypt()
(and maybe also in the geode_ecb_decrypt()).

The cause for this seems to be, that the op->src == op->dst whereby the
geode_aes_crypt() function does nothing at all,
and so the value of nbytes will never be changed.

Regards,
Martin

-------------------------------------------
 
Martin Schiller
Dipl.-Inf. FH
Entwicklung / Development


T.D.T. Transfer Data Test GmbH
Siemensstrasse 18
Gewerbegebiet Altheim
84051 Essenbach
Germany 

mschiller@tdt.de
www.tdt.de 

tel: +49 8703 929 00
fax: +49 8703 929 201

Geschäftsführer: Elisabeth Pickhardt &  Michael Pickhardt
Registergericht Landshut, HRB: 1193 

Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Evgeniy Polyakov]

On Thu, May 03, 2007 at 03:12:43PM +0200, Martin Schiller (mschiller@tdt.de) wrote:
> The freeze is the result of an invinite while-loop in the function
> geode_ecb_encrypt()
> (and maybe also in the geode_ecb_decrypt()).
> 
> The cause for this seems to be, that the op->src == op->dst whereby the
> geode_aes_crypt() function does nothing at all,
> and so the value of nbytes will never be changed.

Hm, driver does not perform encryption in-place at all.
Since we did not hear from AMD quite for a while, could you please
remove src==dst check in geode_aes_crypt() and run tests again.
If it is software protection against hardware bug, I doubt such hardware
should be used at all...

> Regards,
> Martin

-- 
	Evgeniy Polyakov

Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Herbert Xu]

Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote:
> 
> Hm, driver does not perform encryption in-place at all.
> Since we did not hear from AMD quite for a while, could you please
> remove src==dst check in geode_aes_crypt() and run tests again.
> If it is software protection against hardware bug, I doubt such hardware
> should be used at all...

I agree.  Jordan, could you please see if this can be fixed up?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Jordan Crouse]

On 03/05/07 23:49 +1000, Herbert Xu wrote:
> Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote:
> > 
> > Hm, driver does not perform encryption in-place at all.
> > Since we did not hear from AMD quite for a while, could you please
> > remove src==dst check in geode_aes_crypt() and run tests again.
> > If it is software protection against hardware bug, I doubt such hardware
> > should be used at all...
>
> I agree.  Jordan, could you please see if this can be fixed up?

On older versions of the chip, in-place encryption was not possible, even
though there was no hardware protection against it.  I can't remember
if the newer chip version can handle in place encryption or not.

I missed out on the context of this thread - does the tcrypt demand 
in-place encryption?

Jordan

-- 
Jordan Crouse
Senior Linux Engineer
Advanced Micro Devices, Inc.
<www.amd.com/embeddedprocessors>

Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Evgeniy Polyakov]

On Thu, May 03, 2007 at 08:47:44AM -0600, Jordan Crouse (jordan.crouse@amd.com) wrote:
> On 03/05/07 23:49 +1000, Herbert Xu wrote:
> > Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote:
> > > 
> > > Hm, driver does not perform encryption in-place at all.
> > > Since we did not hear from AMD quite for a while, could you please
> > > remove src==dst check in geode_aes_crypt() and run tests again.
> > > If it is software protection against hardware bug, I doubt such hardware
> > > should be used at all...
> >
> > I agree.  Jordan, could you please see if this can be fixed up?
> 
> On older versions of the chip, in-place encryption was not possible, even
> though there was no hardware protection against it.  I can't remember
> if the newer chip version can handle in place encryption or not.
> 
> I missed out on the context of this thread - does the tcrypt demand 
> in-place encryption?

Majority of the in-kernel crypto users require in-place crypto
processing. The only way to fix this I see is to allocate a buffer, copy
data and then perform crypto processing. But I seriously doubt it will
be faster then software encryption/decryption on that processor.

Test for possibility for in-place encryption can be done in module load
time and in case of failed crypto processing driver should fail into
alternative (with allocation) ecryption way (at least similar check I
perform in hifn module).

> Jordan

-- 
	Evgeniy Polyakov

RE: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Martin Schiller]

On Thursday, May 03, 2007 3:23 PM, Evgeniy Polyakov wrote:

> On Thu, May 03, 2007 at 03:12:43PM +0200, Martin Schiller
> (mschiller@tdt.de) wrote: 
>> The freeze is the result of an invinite while-loop in the function
>> geode_ecb_encrypt() (and maybe also in the geode_ecb_decrypt()).
>> 
>> The cause for this seems to be, that the op->src == op->dst whereby
>> the geode_aes_crypt() function does nothing at all, and so the value
>> of nbytes will never be changed.
> 
> Hm, driver does not perform encryption in-place at all.
> Since we did not hear from AMD quite for a while, could you please
> remove src==dst check in geode_aes_crypt() and run tests again. If it
> is software protection against hardware bug, I doubt such hardware
> should be used at all...  
> 

Removing the src==dst check solves the freeze problem, but the
encryption/decryption results are wrong:

Encryption:
Should be: 	69c4e0d86a7b0430d8cdb78070b4c55a
But is:	c8a331ff8edd3db175e1545dbefb760b

Decryption:
Should be:	00112233445566778899aabbccddeeff
But is:	69c4e0d86a7b0430d8cdb78070b4c55a (same as input)


Regards,
Martin

Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Evgeniy Polyakov]

On Thu, May 03, 2007 at 04:08:29PM +0200, Martin Schiller (mschiller@tdt.de) wrote:
> > Hm, driver does not perform encryption in-place at all.
> > Since we did not hear from AMD quite for a while, could you please
> > remove src==dst check in geode_aes_crypt() and run tests again. If it
> > is software protection against hardware bug, I doubt such hardware
> > should be used at all...  
> > 
> 
> Removing the src==dst check solves the freeze problem, but the
> encryption/decryption results are wrong:
> 
> Encryption:
> Should be: 	69c4e0d86a7b0430d8cdb78070b4c55a
> But is:	c8a331ff8edd3db175e1545dbefb760b
> 
> Decryption:
> Should be:	00112233445566778899aabbccddeeff
> But is:	69c4e0d86a7b0430d8cdb78070b4c55a (same as input)

That means that hardware does not work when source and destination
addresses are the same, roughly saying it will not work with majority
cases of ipsec and dm-crypt.

As a workaround one needs to allocate a buffer and copy there data and
then encrypt/decrypt from buffer into destination.
 
> Regards,
> Martin
> 

-- 
	Evgeniy Polyakov

Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Martin Schiller]

Hello,

here is what I do:

- I have an AEWIN CB-6971 with an AMD Geode LX800.

- Testing with kernel 2.6.21-rc7-git5.

- When the geode_aes module is loaded and I am loading the tcrypt module,
  the machine freezes completely.

- When the geode_aes module is NOT loaded and I am loading the tcrypt
module,
  the test works well.

Has anyone else tried to test the geode-aes driver with the tcrypt module?

I am also not able to use the geode-aes driver with openswan-2.4.7 on kernel
2.6.19 (with patched-in geode driver).

Regards,
Martin


--------------------------
 
Martin Schiller
Dipl.-Inf. FH
Development


T.D.T. Transfer Data Test GmbH
Siemensstrasse 18
Gewerbegebiet Altheim
84051 Essenbach
Germany 

mschiller@tdt.de
www.tdt.de 

tel: +49 8703 929 00
fax: +49 8703 929 201

Geschäftsführer: Elisabeth Pickhardt &  Michael Pickhardt
Registergericht Landshut, HRB: 1193 

Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Sebastian Siewior]

* Martin Schiller | 2007-04-26 11:04:21 [+0200]:

>- When the geode_aes module is loaded and I am loading the tcrypt module,
>  the machine freezes completely.

could you modify the tcrpto module to test only ecb(aes) enc/dec with
keysize = 16 (1 instead of AES_ENC_TEST_VECTORS) and see if it is still
freezing?

Sebastian

RE: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Martin Schiller]

On Thursday, April 26, 2007 1:05 PM, Sebastian Siewior wrote:

> * Martin Schiller | 2007-04-26 11:04:21 [+0200]:
> 
>> - When the geode_aes module is loaded and I am loading the tcrypt
>>  module, the machine freezes completely.
> 
> could you modify the tcrpto module to test only ecb(aes) enc/dec with
> keysize = 16 (1 instead of AES_ENC_TEST_VECTORS) and see if it is
> still freezing?  
> 

Tried it and it's still freezing.

Any other suggestions?

Regards,
Martin

--------------------------------
Martin Schiller
Dipl.-Inf. FH
Entwicklung / Development


T.D.T. Transfer Data Test GmbH
Siemensstrasse 18
Gewerbegebiet Altheim
84051 Essenbach
Germany

mschiller@tdt.de
www.tdt.de

tel: +49 8703 929 00
fax: +49 8703 929 201

Geschäftsführer: Elisabeth Pickhardt &  Michael Pickhardt
Registergericht Landshut, HRB: 1193

Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Herbert Xu]

On Thu, Apr 26, 2007 at 09:04:21AM +0000, Martin Schiller wrote:
> 
> here is what I do:
> 
> - I have an AEWIN CB-6971 with an AMD Geode LX800.
> 
> - Testing with kernel 2.6.21-rc7-git5.
> 
> - When the geode_aes module is loaded and I am loading the tcrypt module,
>   the machine freezes completely.
> 
> - When the geode_aes module is NOT loaded and I am loading the tcrypt
> module,
>   the test works well.
> 
> Has anyone else tried to test the geode-aes driver with the tcrypt module?
> 
> I am also not able to use the geode-aes driver with openswan-2.4.7 on kernel
> 2.6.19 (with patched-in geode driver).

Jordan, do you have any ideas why this is happening?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Evgeniy Polyakov]

On Fri, Apr 27, 2007 at 06:10:51PM +1000, Herbert Xu (herbert@gondor.apana.org.au) wrote:
> > Has anyone else tried to test the geode-aes driver with the tcrypt module?
> > 
> > I am also not able to use the geode-aes driver with openswan-2.4.7 on kernel
> > 2.6.19 (with patched-in geode driver).
> 
> Jordan, do you have any ideas why this is happening?

Could it be compiler problem and broken hardware?
Martin, can you test attached patch?

diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index 6d3840e..724169b 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -78,7 +78,7 @@ static int
 do_crypt(void *src, void *dst, int len, u32 flags)
 {
 	u32 status;
-	u32 counter = AES_OP_TIMEOUT;
+	int counter = AES_OP_TIMEOUT;
 
 	iowrite32(virt_to_phys(src), _iobase + AES_SOURCEA_REG);
 	iowrite32(virt_to_phys(dst), _iobase + AES_DSTA_REG);
@@ -89,7 +89,9 @@ do_crypt(void *src, void *dst, int len, u32 flags)
 
 	do
 		status = ioread32(_iobase + AES_INTR_REG);
-	while(!(status & AES_INTRA_PENDING) && --counter);
+	while(--counter > 0 && !(status & AES_INTRA_PENDING));
+
+	WARN_ON(!counter);
 
 	/* Clear the event */
 	iowrite32((status & 0xFF) | AES_INTRA_PENDING, _iobase + AES_INTR_REG);
-- 
	Evgeniy Polyakov

Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Evgeniy Polyakov]

On Fri, Apr 27, 2007 at 01:50:37PM +0400, Evgeniy Polyakov (johnpol@2ka.mipt.ru) wrote:
> On Fri, Apr 27, 2007 at 06:10:51PM +1000, Herbert Xu (herbert@gondor.apana.org.au) wrote:
> > > Has anyone else tried to test the geode-aes driver with the tcrypt module?
> > > 
> > > I am also not able to use the geode-aes driver with openswan-2.4.7 on kernel
> > > 2.6.19 (with patched-in geode driver).
> > 
> > Jordan, do you have any ideas why this is happening?
> 
> Could it be compiler problem and broken hardware?
> Martin, can you test attached patch?

Or better this one:

diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index 6d3840e..724169b 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -78,7 +78,7 @@ static int
 do_crypt(void *src, void *dst, int len, u32 flags)
 {
 	u32 status;
-	u32 counter = AES_OP_TIMEOUT;
+	int counter = 0x1000;
 
 	iowrite32(virt_to_phys(src), _iobase + AES_SOURCEA_REG);
 	iowrite32(virt_to_phys(dst), _iobase + AES_DSTA_REG);
@@ -89,7 +89,9 @@ do_crypt(void *src, void *dst, int len, u32 flags)
 
 	do
 		status = ioread32(_iobase + AES_INTR_REG);
-	while(!(status & AES_INTRA_PENDING) && --counter);
+	while(--counter > 0 && !(status & AES_INTRA_PENDING));
+
+	WARN_ON(!counter);
 
 	/* Clear the event */
 	iowrite32((status & 0xFF) | AES_INTRA_PENDING, _iobase + AES_INTR_REG);

-- 
	Evgeniy Polyakov

RE: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Martin Schiller]

On Friday, April 27, 2007 02:53:30 -0700 , Evgeniy Polyakov wrote:

>> Could it be compiler problem and broken hardware?
>> Martin, can you test attached patch?
>
>Or better this one:
>
>diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
>index 6d3840e..724169b 100644
>--- a/drivers/crypto/geode-aes.c
>+++ b/drivers/crypto/geode-aes.c
>@@ -78,7 +78,7 @@ static int
> do_crypt(void *src, void *dst, int len, u32 flags)
> {
>        u32 status;
>-       u32 counter = AES_OP_TIMEOUT;
>+       int counter = 0x1000;
> 
>        iowrite32(virt_to_phys(src), _iobase + AES_SOURCEA_REG);
>        iowrite32(virt_to_phys(dst), _iobase + AES_DSTA_REG);
>@@ -89,7 +89,9 @@ do_crypt(void *src, void *dst, int len, u32 flags)
> 
>        do
>                status = ioread32(_iobase + AES_INTR_REG);
>-       while(!(status & AES_INTRA_PENDING) && --counter);
>+       while(--counter > 0 && !(status & AES_INTRA_PENDING));
>+
>+       WARN_ON(!counter);
> 
>        /* Clear the event */
>        iowrite32((status & 0xFF) | AES_INTRA_PENDING, _iobase +
AES_INTR_REG);
>

Hi Evgeniy,

Sorry for my late answer, but I didn't get your message because I haven't
subscribed to the mailing-list.
I've "found" it yesterday on the mailing-list archive. So please, could you
reply directly to me and to the mailing-list on any further messages?

I've tested the patch now, but nothing changed. When doing any aes cipher
tests with the tcrypt test module, the machine freezes without any error.

Regards, 
Martin

Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine

[Evgeniy Polyakov]

On Thu, May 03, 2007 at 08:19:52AM +0200, Martin Schiller (mschiller@tdt.de) wrote:
> Hi Evgeniy,

Hi Martin.

> Sorry for my late answer, but I didn't get your message because I haven't
> subscribed to the mailing-list.
> I've "found" it yesterday on the mailing-list archive. So please, could you
> reply directly to me and to the mailing-list on any further messages?

I did, but your mail server bounced message.
This message is also sent to you directly, hope it will pass through.
Btw, AMD list bounces too.

> I've tested the patch now, but nothing changed. When doing any aes cipher
> tests with the tcrypt test module, the machine freezes without any error.

Hmm, that means that main waiting loop of the driver is not an issue.
Since it is PCI, likely bus is locked by incorrect written value into
the space. Can you run with this patch (it will freeze too, but at
least with some useful info), since AMD keeps silence, we will go hard
way:

diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index 6d3840e..6ddfab9 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -61,8 +61,10 @@ static inline void
 _writefield(u32 offset, void *value)
 {
 	int i;
+	printk("%s: start, offset: %x.\n", __func__, offset);
 	for(i = 0; i < 4; i++)
 		iowrite32(((u32 *) value)[i], _iobase + offset + (i * 4));
+	printk("%s: finish, offset: %x.\n", __func__, offset);
 }
 
 /* Read a 128 bit field (either a writable key or IV) */
@@ -70,8 +72,10 @@ static inline void
 _readfield(u32 offset, void *value)
 {
 	int i;
+	printk("%s: start, offset: %x.\n", __func__, offset);
 	for(i = 0; i < 4; i++)
 		((u32 *) value)[i] = ioread32(_iobase + offset + (i * 4));
+	printk("%s: finish, offset: %x.\n", __func__, offset);
 }
 
 static int
@@ -80,19 +84,24 @@ do_crypt(void *src, void *dst, int len, u32 flags)
 	u32 status;
 	u32 counter = AES_OP_TIMEOUT;
 
+	printk("%s: src: %p, dst: %p, len: %d, flags: %x\n", __func__, src, dst, len, flags);
 	iowrite32(virt_to_phys(src), _iobase + AES_SOURCEA_REG);
 	iowrite32(virt_to_phys(dst), _iobase + AES_DSTA_REG);
 	iowrite32(len,  _iobase + AES_LENA_REG);
 
+	printk("%s: start op, likely will freeze.\n", __func__);
 	/* Start the operation */
 	iowrite32(AES_CTRL_START | flags, _iobase + AES_CTRLA_REG);
+	printk("%s: started, but we are stil alive.\n", __func__);
 
-	do
+	do {
 		status = ioread32(_iobase + AES_INTR_REG);
-	while(!(status & AES_INTRA_PENDING) && --counter);
+		printk("%s: status: %x, counter: %u.\n", __func__, status, counter);
+	} while(!(status & AES_INTRA_PENDING) && --counter);
 
 	/* Clear the event */
 	iowrite32((status & 0xFF) | AES_INTRA_PENDING, _iobase + AES_INTR_REG);
+	printk("%s: completed.\n", __func__);
 	return counter ? 0 : 1;
 }
 
@@ -113,6 +122,7 @@ geode_aes_crypt(struct geode_aes_op *op)
 
 	/* Start the critical section */
 
+	printk("%s: start.\n", __func__);
 	spin_lock_irqsave(&lock, iflags);
 
 	if (op->mode == AES_MODE_CBC) {
@@ -131,6 +141,7 @@ geode_aes_crypt(struct geode_aes_op *op)
 		_readfield(AES_WRITEIV0_REG, op->iv);
 
 	spin_unlock_irqrestore(&lock, iflags);
+	printk("%s: completed.\n", __func__);
 
 	return op->len;
 }


-- 
	Evgeniy Polyakov