| Message ID | 20250213033531.3367697-22-jamin_lin@aspeedtech.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Support AST2700 A1 | expand |
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;
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;
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;
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; >
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; > >
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;
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(+)