On 03/06/06 14:53 +0400, Evgeniy Polyakov wrote:
> > +static inline void AWRITE(unsigned long val, unsigned short reg) {
> > +#ifdef DEBUG
> > + printk("[AES] W [%x]=%x\n", reg, val);
> > +#endif
> > + iowrite32(val, _iobase + reg);
> > +}
> > +
> > +static inline unsigned int AREAD(unsigned short reg) {
> > + unsigned int val = ioread32(_iobase + reg);
> > +#ifdef DEBUG
> > + printk("[AES] R [%x]=%x\n", reg, val);
> > +#endif
> > + return val;
> > +}
>
> Remove above functions and use ioread/iowrite directly.
I'll turn AWRITE and AREAD into macros - I would rather abstract the
ioread/iowrite functionality away so we can easily maintain it.
> Do not use __pa, use virt_to_phys() although it is the same for i386.
Sometimes its easy to get lazy when you're only playing with x86.. :)
> > + AWRITE(len, AES_LENA_REG);
> > +
> > + /* Start the operation */
> > + AWRITE(AES_CTRL_START | flags, AES_CTRLA_REG);
> > +
> > + /* According to the silicon developers, the status will only
> > + * fail to clear on an catastrophic failure, so an infinite
> > + * loop is valid here
> > + */
> > +
> > + do
> > + status = AREAD(AES_INTR_REG);
> > + while(!(status & AES_INTRA_PENDING));
> > +
> > + /* Clear the event */
> > + AWRITE((status & 0xFF) | AES_INTRA_PENDING, AES_INTR_REG);
> > +}
>
> And how will system behave if that catastrophic failure happend?
> Maybe it would be better to have counter and failure device shutdown in
> case of error?
I struggle with this - on one hand, it is more correct to fail, but on
the other hand, catastrophic failure means that something horrible happened
to the silicon block (like overheating). Since the AES block is in the
northbridge, and the northbridge is integrated with the CPU, well - lets put
it this way - if the AES block stops working, then you probably have
more concerns then decrypting a block of data.
Regardless, I'll stick a stupidly huge number counter here, and fail if it
hits zero.
> > +unsigned int
> > +geode_aes_crypt(struct geode_aes_op *op)
> > +{
> > + u32 flags = 0;
> > +
> > + if (op->len == 0 || op->src == op->dst)
> > + return 0;
> > +
> > + if (mutex_lock_interruptible(&emutex))
> > + return 0;
>
> I.e. it can not be used in atomic context?
> That will break a lot of setups.
Is there any way to differentiate if a crypto API call was made in an
atomic context or not? The problem here, is that I only have one engine,
so I need to protect it from multiple processes accessing it at the
same time. I can get rid of the sleep in an atomic context, but we expect
to have both atomic and process contexts heavily accessing the engine at
the same time, we really run the risk of having the atomic contexts fail
more often then they should due to a process holding the mutex.
> ...
>
> > + .cipher = {
> > + .cia_min_keysize = AES_KEY_LENGTH,
> > + .cia_max_keysize = AES_KEY_LENGTH,
>
> I.e. it does not support different keys?
> If so, it should be called geode-aes-128, imho.
No - there is only one key size (128) - so I agree - its more correct to call
it geode-aes-128.
Jordan
--
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>
-
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
