1. Patchew
  2. QEMU
  3. [PATCH v3 00/28] Support AST2700 A1
  4. View patch

[PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test

Jamin Lin via posted 28 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
Posted by Jamin Lin via 1 month, 2 weeks ago 
Currently, it does not support the CRYPT command. Instead, it only sends an
interrupt to notify the firmware that the crypt command has completed.
It is a temporary workaround to resolve the boot issue in the Crypto Manager
Self Test.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 86422cb3be..4d0999e7e9 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -59,6 +59,7 @@
 /* Other cmd bits */
 #define  HASH_IRQ_EN                    BIT(9)
 #define  HASH_SG_EN                     BIT(18)
+#define  CRYPT_IRQ_EN                   BIT(12)
 /* Scatter-gather data list */
 #define SG_LIST_LEN_SIZE                4
 #define SG_LIST_LEN_MASK                0x0FFFFFFF
@@ -343,6 +344,13 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
                 qemu_irq_lower(s->irq);
             }
         }
+        if (data & CRYPT_IRQ) {
+            data &= ~CRYPT_IRQ;
+
+            if (s->regs[addr] & CRYPT_IRQ) {
+                qemu_irq_lower(s->irq);
+            }
+        }
         break;
     case R_HASH_SRC:
         data &= ahc->src_mask;
@@ -388,6 +396,10 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
     case R_CRYPT_CMD:
         qemu_log_mask(LOG_UNIMP, "%s: Crypt commands not implemented\n",
                        __func__);
+        s->regs[R_STATUS] |= CRYPT_IRQ;
+        if (data & CRYPT_IRQ_EN) {
+            qemu_irq_raise(s->irq);
+        }
         break;
     default:
         break;
-- 
2.34.1
Re: [PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
Posted by Cédric Le Goater 1 month, 2 weeks ago 
On 2/13/25 04:35, Jamin Lin wrote:
> Currently, it does not support the CRYPT command. Instead, it only sends an
> interrupt to notify the firmware that the crypt command has completed.
> It is a temporary workaround to resolve the boot issue in the Crypto Manager
> Self Test.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>

Please add an AspeedHACEClass class attribute (bool) to handle
this workaround and a comment in the code mentioning the issue.


Thanks,

C.



> ---
>   hw/misc/aspeed_hace.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> index 86422cb3be..4d0999e7e9 100644
> --- a/hw/misc/aspeed_hace.c
> +++ b/hw/misc/aspeed_hace.c
> @@ -59,6 +59,7 @@
>   /* Other cmd bits */
>   #define  HASH_IRQ_EN                    BIT(9)
>   #define  HASH_SG_EN                     BIT(18)
> +#define  CRYPT_IRQ_EN                   BIT(12)
>   /* Scatter-gather data list */
>   #define SG_LIST_LEN_SIZE                4
>   #define SG_LIST_LEN_MASK                0x0FFFFFFF
> @@ -343,6 +344,13 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
>                   qemu_irq_lower(s->irq);
>               }
>           }
> +        if (data & CRYPT_IRQ) {
> +            data &= ~CRYPT_IRQ;
> +
> +            if (s->regs[addr] & CRYPT_IRQ) {
> +                qemu_irq_lower(s->irq);
> +            }
> +        }
>           break;
>       case R_HASH_SRC:
>           data &= ahc->src_mask;
> @@ -388,6 +396,10 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
>       case R_CRYPT_CMD:
>           qemu_log_mask(LOG_UNIMP, "%s: Crypt commands not implemented\n",
>                          __func__);
> +        s->regs[R_STATUS] |= CRYPT_IRQ;
> +        if (data & CRYPT_IRQ_EN) {
> +            qemu_irq_raise(s->irq);
> +        }
>           break;
>       default:
>           break;
RE: [PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
Posted by Jamin Lin 1 month, 1 week ago 
Hi Cedric,

> Subject: Re: [PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in the
> Crypto Manager Self Test
> 
> On 2/13/25 04:35, Jamin Lin wrote:
> > Currently, it does not support the CRYPT command. Instead, it only
> > sends an interrupt to notify the firmware that the crypt command has
> completed.
> > It is a temporary workaround to resolve the boot issue in the Crypto
> > Manager Self Test.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> 
> Please add an AspeedHACEClass class attribute (bool) to handle this
> workaround and a comment in the code mentioning the issue.
> 
Thanks for review and suggestion.
I will add the use_crypt_workaround attribute to the AspeedHACEClass and introduce the use-crypt-workaround property.
Do you have any concerns, or do you have a preferred naming convention?
Thanks-Jamin

> 
> Thanks,
> 
> C.
> 
> 
> 
> > ---
> >   hw/misc/aspeed_hace.c | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> >
> > diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index
> > 86422cb3be..4d0999e7e9 100644
> > --- a/hw/misc/aspeed_hace.c
> > +++ b/hw/misc/aspeed_hace.c
> > @@ -59,6 +59,7 @@
> >   /* Other cmd bits */
> >   #define  HASH_IRQ_EN                    BIT(9)
> >   #define  HASH_SG_EN                     BIT(18)
> > +#define  CRYPT_IRQ_EN                   BIT(12)
> >   /* Scatter-gather data list */
> >   #define SG_LIST_LEN_SIZE                4
> >   #define SG_LIST_LEN_MASK                0x0FFFFFFF
> > @@ -343,6 +344,13 @@ static void aspeed_hace_write(void *opaque,
> hwaddr addr, uint64_t data,
> >                   qemu_irq_lower(s->irq);
> >               }
> >           }
> > +        if (data & CRYPT_IRQ) {
> > +            data &= ~CRYPT_IRQ;
> > +
> > +            if (s->regs[addr] & CRYPT_IRQ) {
> > +                qemu_irq_lower(s->irq);
> > +            }
> > +        }
> >           break;
> >       case R_HASH_SRC:
> >           data &= ahc->src_mask;
> > @@ -388,6 +396,10 @@ static void aspeed_hace_write(void *opaque,
> hwaddr addr, uint64_t data,
> >       case R_CRYPT_CMD:
> >           qemu_log_mask(LOG_UNIMP, "%s: Crypt commands not
> implemented\n",
> >                          __func__);
> > +        s->regs[R_STATUS] |= CRYPT_IRQ;
> > +        if (data & CRYPT_IRQ_EN) {
> > +            qemu_irq_raise(s->irq);
> > +        }
> >           break;
> >       default:
> >           break;
Re: [PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
Posted by Cédric Le Goater 1 month, 1 week ago 
On 2/21/25 06:43, Jamin Lin wrote:
> Hi Cedric,
> 
>> Subject: Re: [PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in the
>> Crypto Manager Self Test
>>
>> On 2/13/25 04:35, Jamin Lin wrote:
>>> Currently, it does not support the CRYPT command. Instead, it only
>>> sends an interrupt to notify the firmware that the crypt command has
>> completed.
>>> It is a temporary workaround to resolve the boot issue in the Crypto
>>> Manager Self Test.
>>>
>>> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>>
>> Please add an AspeedHACEClass class attribute (bool) to handle this
>> workaround and a comment in the code mentioning the issue.
>>
> Thanks for review and suggestion.
> I will add the use_crypt_workaround attribute to the AspeedHACEClass and introduce the use-crypt-workaround property.
> Do you have any concerns, or do you have a preferred naming convention?

May be 'raise_crypt_interrupt_workaround' to reflect what is being done ?

Thanks,

C.



> Thanks-Jamin
> 
>>
>> Thanks,
>>
>> C.
>>
>>
>>
>>> ---
>>>    hw/misc/aspeed_hace.c | 12 ++++++++++++
>>>    1 file changed, 12 insertions(+)
>>>
>>> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index
>>> 86422cb3be..4d0999e7e9 100644
>>> --- a/hw/misc/aspeed_hace.c
>>> +++ b/hw/misc/aspeed_hace.c
>>> @@ -59,6 +59,7 @@
>>>    /* Other cmd bits */
>>>    #define  HASH_IRQ_EN                    BIT(9)
>>>    #define  HASH_SG_EN                     BIT(18)
>>> +#define  CRYPT_IRQ_EN                   BIT(12)
>>>    /* Scatter-gather data list */
>>>    #define SG_LIST_LEN_SIZE                4
>>>    #define SG_LIST_LEN_MASK                0x0FFFFFFF
>>> @@ -343,6 +344,13 @@ static void aspeed_hace_write(void *opaque,
>> hwaddr addr, uint64_t data,
>>>                    qemu_irq_lower(s->irq);
>>>                }
>>>            }
>>> +        if (data & CRYPT_IRQ) {
>>> +            data &= ~CRYPT_IRQ;
>>> +
>>> +            if (s->regs[addr] & CRYPT_IRQ) {
>>> +                qemu_irq_lower(s->irq);
>>> +            }
>>> +        }
>>>            break;
>>>        case R_HASH_SRC:
>>>            data &= ahc->src_mask;
>>> @@ -388,6 +396,10 @@ static void aspeed_hace_write(void *opaque,
>> hwaddr addr, uint64_t data,
>>>        case R_CRYPT_CMD:
>>>            qemu_log_mask(LOG_UNIMP, "%s: Crypt commands not
>> implemented\n",
>>>                           __func__);
>>> +        s->regs[R_STATUS] |= CRYPT_IRQ;
>>> +        if (data & CRYPT_IRQ_EN) {
>>> +            qemu_irq_raise(s->irq);
>>> +        }
>>>            break;
>>>        default:
>>>            break;
>
RE: [PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
Posted by Jamin Lin 1 month, 1 week ago 
Hi Cedric,

> Subject: Re: [PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in the
> Crypto Manager Self Test
> 
> On 2/21/25 06:43, Jamin Lin wrote:
> > Hi Cedric,
> >
> >> Subject: Re: [PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in
> >> the Crypto Manager Self Test
> >>
> >> On 2/13/25 04:35, Jamin Lin wrote:
> >>> Currently, it does not support the CRYPT command. Instead, it only
> >>> sends an interrupt to notify the firmware that the crypt command has
> >> completed.
> >>> It is a temporary workaround to resolve the boot issue in the Crypto
> >>> Manager Self Test.
> >>>
> >>> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >>
> >> Please add an AspeedHACEClass class attribute (bool) to handle this
> >> workaround and a comment in the code mentioning the issue.
> >>
> > Thanks for review and suggestion.
> > I will add the use_crypt_workaround attribute to the AspeedHACEClass and
> introduce the use-crypt-workaround property.
> > Do you have any concerns, or do you have a preferred naming convention?
> 
> May be 'raise_crypt_interrupt_workaround' to reflect what is being done ?
> 
Thanks for your suggestion.
Will add it.

Jamin

> Thanks,
> 
> C.
> 
> 
> 
> > Thanks-Jamin
> >
> >>
> >> Thanks,
> >>
> >> C.
> >>
> >>
> >>
> >>> ---
> >>>    hw/misc/aspeed_hace.c | 12 ++++++++++++
> >>>    1 file changed, 12 insertions(+)
> >>>
> >>> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index
> >>> 86422cb3be..4d0999e7e9 100644
> >>> --- a/hw/misc/aspeed_hace.c
> >>> +++ b/hw/misc/aspeed_hace.c
> >>> @@ -59,6 +59,7 @@
> >>>    /* Other cmd bits */
> >>>    #define  HASH_IRQ_EN                    BIT(9)
> >>>    #define  HASH_SG_EN                     BIT(18)
> >>> +#define  CRYPT_IRQ_EN                   BIT(12)
> >>>    /* Scatter-gather data list */
> >>>    #define SG_LIST_LEN_SIZE                4
> >>>    #define SG_LIST_LEN_MASK                0x0FFFFFFF
> >>> @@ -343,6 +344,13 @@ static void aspeed_hace_write(void *opaque,
> >> hwaddr addr, uint64_t data,
> >>>                    qemu_irq_lower(s->irq);
> >>>                }
> >>>            }
> >>> +        if (data & CRYPT_IRQ) {
> >>> +            data &= ~CRYPT_IRQ;
> >>> +
> >>> +            if (s->regs[addr] & CRYPT_IRQ) {
> >>> +                qemu_irq_lower(s->irq);
> >>> +            }
> >>> +        }
> >>>            break;
> >>>        case R_HASH_SRC:
> >>>            data &= ahc->src_mask;
> >>> @@ -388,6 +396,10 @@ static void aspeed_hace_write(void *opaque,
> >> hwaddr addr, uint64_t data,
> >>>        case R_CRYPT_CMD:
> >>>            qemu_log_mask(LOG_UNIMP, "%s: Crypt commands not
> >> implemented\n",
> >>>                           __func__);
> >>> +        s->regs[R_STATUS] |= CRYPT_IRQ;
> >>> +        if (data & CRYPT_IRQ_EN) {
> >>> +            qemu_irq_raise(s->irq);
> >>> +        }
> >>>            break;
> >>>        default:
> >>>            break;
> >
RE: [PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
Posted by Jamin Lin 1 month, 1 week ago 
Hi Cedric, 

> 
> Hi Cedric,
> 
> > Subject: Re: [PATCH v3 21/28] hw/misc/aspeed_hace: Fix boot issue in
> > the Crypto Manager Self Test
> >
> > On 2/13/25 04:35, Jamin Lin wrote:
> > > Currently, it does not support the CRYPT command. Instead, it only
> > > sends an interrupt to notify the firmware that the crypt command has
> > completed.
> > > It is a temporary workaround to resolve the boot issue in the Crypto
> > > Manager Self Test.
> > >
> > > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >
> > Please add an AspeedHACEClass class attribute (bool) to handle this
> > workaround and a comment in the code mentioning the issue.
> >
> Thanks for review and suggestion.
> I will add the use_crypt_workaround attribute to the AspeedHACEClass and
> introduce the use-crypt-workaround property.
> Do you have any concerns, or do you have a preferred naming convention?


Update my comments.
I do not need to create a property. I can set use_crypt_workaround to true in aspeed_ast2700_hace_class_init
Thanks-Jamin

> Thanks-Jamin
> 
> >
> > Thanks,
> >
> > C.
> >
> >
> >
> > > ---
> > >   hw/misc/aspeed_hace.c | 12 ++++++++++++
> > >   1 file changed, 12 insertions(+)
> > >
> > > diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index
> > > 86422cb3be..4d0999e7e9 100644
> > > --- a/hw/misc/aspeed_hace.c
> > > +++ b/hw/misc/aspeed_hace.c
> > > @@ -59,6 +59,7 @@
> > >   /* Other cmd bits */
> > >   #define  HASH_IRQ_EN                    BIT(9)
> > >   #define  HASH_SG_EN                     BIT(18)
> > > +#define  CRYPT_IRQ_EN                   BIT(12)
> > >   /* Scatter-gather data list */
> > >   #define SG_LIST_LEN_SIZE                4
> > >   #define SG_LIST_LEN_MASK                0x0FFFFFFF
> > > @@ -343,6 +344,13 @@ static void aspeed_hace_write(void *opaque,
> > hwaddr addr, uint64_t data,
> > >                   qemu_irq_lower(s->irq);
> > >               }
> > >           }
> > > +        if (data & CRYPT_IRQ) {
> > > +            data &= ~CRYPT_IRQ;
> > > +
> > > +            if (s->regs[addr] & CRYPT_IRQ) {
> > > +                qemu_irq_lower(s->irq);
> > > +            }
> > > +        }
> > >           break;
> > >       case R_HASH_SRC:
> > >           data &= ahc->src_mask;
> > > @@ -388,6 +396,10 @@ static void aspeed_hace_write(void *opaque,
> > hwaddr addr, uint64_t data,
> > >       case R_CRYPT_CMD:
> > >           qemu_log_mask(LOG_UNIMP, "%s: Crypt commands not
> > implemented\n",
> > >                          __func__);
> > > +        s->regs[R_STATUS] |= CRYPT_IRQ;
> > > +        if (data & CRYPT_IRQ_EN) {
> > > +            qemu_irq_raise(s->irq);
> > > +        }
> > >           break;
> > >       default:
> > >           break;

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.