mbox series

[v3,00/28] Support AST2700 A1

Message ID 20250213033531.3367697-1-jamin_lin@aspeedtech.com (mailing list archive)
Headers show
Series Support AST2700 A1 | expand

Message

Jamin Lin Feb. 13, 2025, 3:35 a.m. UTC
v1:
 1. Refactor INTC model to support both INTC0 and INTC1.
 2. Support AST2700 A1.
 3. Create ast2700a0-evb machine.
 
v2:
  To streamline the review process, split the following patch series into
  three parts.
  https://patchwork.kernel.org/project/qemu-devel/cover/20250121070424.2465942-1-jamin_lin@aspeedtech.com/
  This patch series focuses on cleaning up the INTC model to
  facilitate future support for the INTC_IO model.

v3:
 1. Update and add functional test for AST2700
 2. Add AST2700 INTC design guidance and its block diagram.
 3. Retaining the INTC naming and introducing a new INTCIO model to support the AST2700 A1.
 4. Create ast2700a1-evb machine and rename ast2700a0-evb machine
 5. Fix silicon revision issue and support AST2700 A1.

With the patch applied, QEMU now supports two machines for running AST2700 SoCs:
ast2700a0-evb: Designed for AST2700 A0
ast2700a1-evb: Designed for AST2700 A1

Test information
1. QEMU version: https://github.com/qemu/qemu/commit/ffaf7f0376f8040ce9068d71ae9ae8722505c42e
2. ASPEED SDK v09.05 pre-built image
   https://github.com/AspeedTech-BMC/openbmc/releases/tag/v09.05
   ast2700-default-obmc.tar.gz (AST2700 A1)
   https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-default-obmc.tar.gz
   ast2700-a0-default-obmc.tar.gz (AST2700 A0)
   https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-a0-default-obmc.tar.gz

Known Issue:
The HACE crypto and hash engine is enable by default since AST2700 A1.
However, aspeed_hace.c(HACE model) currently does not support the CRYPTO command.
To boot AST2700 A1, I have created a Patch 21 which temporarily resolves the
issue by sending an interrupt to notify the firmware that the cryptographic
command has completed. It is a temporary workaround to resolve the boot issue
in the Crypto Manager SelfTest.

As a result, you will encounter the following kernel warning due to the
Crypto Manager test failure. If you don't want to see these kernel warning,
please add the following settings in your kernel config.

```
CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=y
```

```
alg: skcipher: aspeed-ctr-tdes encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
[    5.035966] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
[    5.036139] ------------[ cut here ]------------
[    5.037188] alg: self-tests for ctr(des3_ede) using aspeed-ctr-tdes failed (rc=-22)
[    5.037312] WARNING: CPU: 2 PID: 109 at /crypto/testmgr.c:5936 alg_test+0x42c/0x548
[    5.038049] Modules linked in:
[    5.038302] CPU: 2 PID: 109 Comm: cryptomgr_test Tainted: G        W          6.6.52-v00.06.04-gf52a0cf7c475 #1
[    5.038787] Hardware name: AST2700-EVB (DT)
[    5.038988] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    5.039315] pc : alg_test+0x42c/0x548
[    5.039505] lr : alg_test+0x42c/0x548
[    5.039697] sp : ffffffc0825e3d50
[    5.039862] x29: ffffffc0825e3df0 x28: 0000000000000004 x27: 0000000000000000
[    5.040226] x26: ffffffc080bcada0 x25: ffffffc081dac9d0 x24: 0000000000000004
[    5.040700] x23: 0000000000001285 x22: ffffff8003ded280 x21: ffffff8003ded200
[    5.041458] x20: 00000000ffffffff x19: 00000000ffffffea x18: ffffffffffffffff
[    5.041915] x17: 282064656c696166 x16: 20736564742d7274 x15: 00000000fffffffe
[    5.042287] x14: 0000000000000000 x13: ffffffc081ba555c x12: 65742d666c657320
[    5.042684] x11: 00000000fffeffff x10: ffffffc0818ff048 x9 : ffffffc0800a36e4
[    5.043077] x8 : 000000000017ffe8 x7 : c0000000fffeffff x6 : 000000000057ffa8
[    5.043461] x5 : 000000000000ffff x4 : 0000000000000000 x3 : 0000000000000000
[    5.043751] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffff800415e740
[    5.045544] Call trace:
[    5.045693]  alg_test+0x42c/0x548
[    5.045878]  cryptomgr_test+0x28/0x48
[    5.046052]  kthread+0x114/0x120
[    5.046226]  ret_from_fork+0x10/0x20
[    5.046413] ---[ end trace 0000000000000000 ]---
[    5.071510] alg: skcipher: aspeed-ctr-des encryption test failed (wrong result) on test vector 0, cfg="in-place (one sglist)"
[    5.072145] alg: self-tests for ctr(des) using aspeed-ctr-des failed (rc=-22)
```

Jamin Lin (28):
  hw/intc/aspeed: Support setting different memory and register size
  hw/intc/aspeed: Introduce helper functions for enable and status
    registers
  hw/intc/aspeed: Add object type name to trace events for better
    debugging
  hw/arm/aspeed: Rename IRQ table and machine name for AST2700 A0
  hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number
  hw/intc/aspeed: Support different memory region ops
  hw/intc/aspeed: Rename num_ints to num_inpins for clarity
  hw/intc/aspeed: Add support for multiple output pins in INTC
  hw/intc/aspeed: Refactor INTC to support separate input and output pin
    indices
  hw/intc/aspeed: Introduce AspeedINTCIRQ structure to save the irq
    index and register address
  hw/intc/aspeed: Introduce IRQ handler function to reduce code
    duplication
  hw/intc/aspeed: Add Support for Multi-Output IRQ Handling
  hw/intc/aspeed: Add Support for AST2700 INTCIO Controller
  hw/misc/aspeed_scu: Add Support for AST2700/AST2750 A1 Silicon
    Revisions
  hw/misc/aspeed_scu: Fix the revision ID cannot be set in the SOC layer
    for AST2700
  hw/arm/aspeed_ast27x0.c Support AST2700 A1 GIC Interrupt Mapping
  hw/arm/aspeed_ast27x0: Support two levels of INTC controllers for
    AST2700 A1
  hw/arm/aspeed: Add SoC and Machine Support for AST2700 A1
  hw/misc/aspeed_hace: Fix coding style
  hw/misc/aspeed_hace: Add AST2700 support
  hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
  hw/arm/aspeed_ast27x0: Add HACE support for AST2700
  test/functional/aspeed: Introduce new function to fetch assets
  tests/functional/aspeed: Introduce start_ast2700_test API and update
    hwmon path
  tests/functional/aspeed: Update test ASPEED SDK v09.05
  tests/functional/aspeed: Renamed test case and machine for AST2700 A0
  tests/functional/aspeed: Add test case for AST2700 A1
  docs/specs: add aspeed-intc

 docs/specs/aspeed-intc.rst              | 136 ++++++
 docs/specs/index.rst                    |   1 +
 hw/arm/aspeed.c                         |  21 +-
 hw/arm/aspeed_ast27x0.c                 | 291 +++++++++---
 hw/intc/aspeed_intc.c                   | 593 +++++++++++++++++++-----
 hw/intc/trace-events                    |  25 +-
 hw/misc/aspeed_hace.c                   |  44 +-
 hw/misc/aspeed_scu.c                    |   5 +-
 include/hw/arm/aspeed_soc.h             |   3 +-
 include/hw/intc/aspeed_intc.h           |  32 +-
 include/hw/misc/aspeed_hace.h           |   1 +
 include/hw/misc/aspeed_scu.h            |   2 +
 tests/functional/test_aarch64_aspeed.py |  47 +-
 13 files changed, 963 insertions(+), 238 deletions(-)
 create mode 100644 docs/specs/aspeed-intc.rst

Comments

Cédric Le Goater Feb. 18, 2025, 9:25 a.m. UTC | #1
Hello Jamin,


On 2/13/25 04:35, Jamin Lin wrote:
> v1:
>   1. Refactor INTC model to support both INTC0 and INTC1.
>   2. Support AST2700 A1.
>   3. Create ast2700a0-evb machine.
>   
> v2:
>    To streamline the review process, split the following patch series into
>    three parts.
>    https://patchwork.kernel.org/project/qemu-devel/cover/20250121070424.2465942-1-jamin_lin@aspeedtech.com/
>    This patch series focuses on cleaning up the INTC model to
>    facilitate future support for the INTC_IO model.
> 
> v3:
>   1. Update and add functional test for AST2700
>   2. Add AST2700 INTC design guidance and its block diagram.
>   3. Retaining the INTC naming and introducing a new INTCIO model to support the AST2700 A1.
>   4. Create ast2700a1-evb machine and rename ast2700a0-evb machine
>   5. Fix silicon revision issue and support AST2700 A1.
> 
> With the patch applied, QEMU now supports two machines for running AST2700 SoCs:
> ast2700a0-evb: Designed for AST2700 A0
> ast2700a1-evb: Designed for AST2700 A1
> 
> Test information
> 1. QEMU version: https://github.com/qemu/qemu/commit/ffaf7f0376f8040ce9068d71ae9ae8722505c42e
> 2. ASPEED SDK v09.05 pre-built image
>     https://github.com/AspeedTech-BMC/openbmc/releases/tag/v09.05
>     ast2700-default-obmc.tar.gz (AST2700 A1)
>     https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-default-obmc.tar.gz
>     ast2700-a0-default-obmc.tar.gz (AST2700 A0)
>     https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast2700-a0-default-obmc.tar.gz

The part adding new functional tests needs a rework. See comment.

> Known Issue:
> The HACE crypto and hash engine is enable by default since AST2700 A1.
> However, aspeed_hace.c(HACE model) currently does not support the CRYPTO command.
> To boot AST2700 A1, I have created a Patch 21 which temporarily resolves the
> issue by sending an interrupt to notify the firmware that the cryptographic
> command has completed. It is a temporary workaround to resolve the boot issue
> in the Crypto Manager SelfTest.
> 
> As a result, you will encounter the following kernel warning due to the
> Crypto Manager test failure. If you don't want to see these kernel warning,
> please add the following settings in your kernel config.
> 
> ```
> CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=y
> ```

Would it be possible to send the hace changes in its own series ?


> 
> Jamin Lin (28):
>    hw/intc/aspeed: Support setting different memory and register size
>    hw/intc/aspeed: Introduce helper functions for enable and status
>      registers
>    hw/intc/aspeed: Add object type name to trace events for better
>      debugging
>    hw/arm/aspeed: Rename IRQ table and machine name for AST2700 A0
>    hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number
>    hw/intc/aspeed: Support different memory region ops
>    hw/intc/aspeed: Rename num_ints to num_inpins for clarity
>    hw/intc/aspeed: Add support for multiple output pins in INTC
>    hw/intc/aspeed: Refactor INTC to support separate input and output pin
>      indices
>    hw/intc/aspeed: Introduce AspeedINTCIRQ structure to save the irq
>      index and register address
>    hw/intc/aspeed: Introduce IRQ handler function to reduce code
>      duplication
>    hw/intc/aspeed: Add Support for Multi-Output IRQ Handling
>    hw/intc/aspeed: Add Support for AST2700 INTCIO Controller
>    hw/misc/aspeed_scu: Add Support for AST2700/AST2750 A1 Silicon
>      Revisions
>    hw/misc/aspeed_scu: Fix the revision ID cannot be set in the SOC layer
>      for AST2700
>    hw/arm/aspeed_ast27x0.c Support AST2700 A1 GIC Interrupt Mapping
>    hw/arm/aspeed_ast27x0: Support two levels of INTC controllers for
>      AST2700 A1
>    hw/arm/aspeed: Add SoC and Machine Support for AST2700 A1
>    hw/misc/aspeed_hace: Fix coding style
>    hw/misc/aspeed_hace: Add AST2700 support
>    hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
>    hw/arm/aspeed_ast27x0: Add HACE support for AST2700
>    test/functional/aspeed: Introduce new function to fetch assets
>    tests/functional/aspeed: Introduce start_ast2700_test API and update
>      hwmon path
>    tests/functional/aspeed: Update test ASPEED SDK v09.05
>    tests/functional/aspeed: Renamed test case and machine for AST2700 A0
>    tests/functional/aspeed: Add test case for AST2700 A1
>    docs/specs: add aspeed-intc
> 
>   docs/specs/aspeed-intc.rst              | 136 ++++++
>   docs/specs/index.rst                    |   1 +
>   hw/arm/aspeed.c                         |  21 +-
>   hw/arm/aspeed_ast27x0.c                 | 291 +++++++++---
>   hw/intc/aspeed_intc.c                   | 593 +++++++++++++++++++-----
>   hw/intc/trace-events                    |  25 +-
>   hw/misc/aspeed_hace.c                   |  44 +-
>   hw/misc/aspeed_scu.c                    |   5 +-
>   include/hw/arm/aspeed_soc.h             |   3 +-
>   include/hw/intc/aspeed_intc.h           |  32 +-
>   include/hw/misc/aspeed_hace.h           |   1 +
>   include/hw/misc/aspeed_scu.h            |   2 +
>   tests/functional/test_aarch64_aspeed.py |  47 +-
>   13 files changed, 963 insertions(+), 238 deletions(-)
>   create mode 100644 docs/specs/aspeed-intc.rst
> 

Patch 1-9 and the hace changes could be merged quickly.


I need  some help on patch 10,12,16,17.

Andrew,

Would you have time please ?

Thanks,

C.
Jamin Lin Feb. 20, 2025, 5:11 a.m. UTC | #2
Hi Cedric,

> Subject: Re: [PATCH v3 00/28] Support AST2700 A1
> 
> Hello Jamin,
> 
> 
> On 2/13/25 04:35, Jamin Lin wrote:
> > v1:
> >   1. Refactor INTC model to support both INTC0 and INTC1.
> >   2. Support AST2700 A1.
> >   3. Create ast2700a0-evb machine.
> >
> > v2:
> >    To streamline the review process, split the following patch series into
> >    three parts.
> >
> https://patchwork.kernel.org/project/qemu-devel/cover/20250121070424.246
> 5942-1-jamin_lin@aspeedtech.com/
> >    This patch series focuses on cleaning up the INTC model to
> >    facilitate future support for the INTC_IO model.
> >
> > v3:
> >   1. Update and add functional test for AST2700
> >   2. Add AST2700 INTC design guidance and its block diagram.
> >   3. Retaining the INTC naming and introducing a new INTCIO model to
> support the AST2700 A1.
> >   4. Create ast2700a1-evb machine and rename ast2700a0-evb machine
> >   5. Fix silicon revision issue and support AST2700 A1.
> >
> > With the patch applied, QEMU now supports two machines for running
> AST2700 SoCs:
> > ast2700a0-evb: Designed for AST2700 A0
> > ast2700a1-evb: Designed for AST2700 A1
> >
> > Test information
> > 1. QEMU version:
> >
> https://github.com/qemu/qemu/commit/ffaf7f0376f8040ce9068d71ae9ae872
> 25
> > 05c42e
> > 2. ASPEED SDK v09.05 pre-built image
> >     https://github.com/AspeedTech-BMC/openbmc/releases/tag/v09.05
> >     ast2700-default-obmc.tar.gz (AST2700 A1)
> >
> https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast
> 2700-default-obmc.tar.gz
> >     ast2700-a0-default-obmc.tar.gz (AST2700 A0)
> >
> >
> https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast
> > 2700-a0-default-obmc.tar.gz
> 
> The part adding new functional tests needs a rework. See comment.
> 
> > Known Issue:
> > The HACE crypto and hash engine is enable by default since AST2700 A1.
> > However, aspeed_hace.c(HACE model) currently does not support the
> CRYPTO command.
> > To boot AST2700 A1, I have created a Patch 21 which temporarily
> > resolves the issue by sending an interrupt to notify the firmware that
> > the cryptographic command has completed. It is a temporary workaround
> > to resolve the boot issue in the Crypto Manager SelfTest.
> >
> > As a result, you will encounter the following kernel warning due to
> > the Crypto Manager test failure. If you don't want to see these kernel
> > warning, please add the following settings in your kernel config.
> >
> > ```
> > CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=y
> > ```
> 
> Would it be possible to send the hace changes in its own series ?
> 
> 
Currently, the HACE HW engine and crypto self-tests are enabled by default in
SDK v09.05 FW. To boot QEMU for AST2700 A1 with the SDK v09.05 pre-built image,
a CRYPTO workaround patch is required.

The AST2700 A1 patch series includes functional tests. To make the functional tests
pass for AST2700 A1, I have included the HACE patch in the same patch series.

There are two ways to split this patch series:

Solution A:

1. Create series 1 to support AST2700 A1.
2. Create series 2 to support HACE.
3. Create series 3 to support AST2700 A1 functional tests.

Series 3 should depend on series 1 and 2.

Solution B:

1. Place a pre-built image called "ast2700-a1-qemu-disable-self-test" in the SDK v09.05 Github repository.
https://github.com/AspeedTech-BMC/openbmc/releases/tag/v09.05
2. Create one patch series to support AST2700 A1 with its functional tests.
3. Create series 2 to support HACE.

Could you tell me which solution you prefer or could you please give me any suggestion?

Thanks-Jamin

> >
> > Jamin Lin (28):
> >    hw/intc/aspeed: Support setting different memory and register size
> >    hw/intc/aspeed: Introduce helper functions for enable and status
> >      registers
> >    hw/intc/aspeed: Add object type name to trace events for better
> >      debugging
> >    hw/arm/aspeed: Rename IRQ table and machine name for AST2700 A0
> >    hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number
> >    hw/intc/aspeed: Support different memory region ops
> >    hw/intc/aspeed: Rename num_ints to num_inpins for clarity
> >    hw/intc/aspeed: Add support for multiple output pins in INTC
> >    hw/intc/aspeed: Refactor INTC to support separate input and output pin
> >      indices
> >    hw/intc/aspeed: Introduce AspeedINTCIRQ structure to save the irq
> >      index and register address
> >    hw/intc/aspeed: Introduce IRQ handler function to reduce code
> >      duplication
> >    hw/intc/aspeed: Add Support for Multi-Output IRQ Handling
> >    hw/intc/aspeed: Add Support for AST2700 INTCIO Controller
> >    hw/misc/aspeed_scu: Add Support for AST2700/AST2750 A1 Silicon
> >      Revisions
> >    hw/misc/aspeed_scu: Fix the revision ID cannot be set in the SOC layer
> >      for AST2700
> >    hw/arm/aspeed_ast27x0.c Support AST2700 A1 GIC Interrupt Mapping
> >    hw/arm/aspeed_ast27x0: Support two levels of INTC controllers for
> >      AST2700 A1
> >    hw/arm/aspeed: Add SoC and Machine Support for AST2700 A1
> >    hw/misc/aspeed_hace: Fix coding style
> >    hw/misc/aspeed_hace: Add AST2700 support
> >    hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
> >    hw/arm/aspeed_ast27x0: Add HACE support for AST2700
> >    test/functional/aspeed: Introduce new function to fetch assets
> >    tests/functional/aspeed: Introduce start_ast2700_test API and update
> >      hwmon path
> >    tests/functional/aspeed: Update test ASPEED SDK v09.05
> >    tests/functional/aspeed: Renamed test case and machine for AST2700
> A0
> >    tests/functional/aspeed: Add test case for AST2700 A1
> >    docs/specs: add aspeed-intc
> >
> >   docs/specs/aspeed-intc.rst              | 136 ++++++
> >   docs/specs/index.rst                    |   1 +
> >   hw/arm/aspeed.c                         |  21 +-
> >   hw/arm/aspeed_ast27x0.c                 | 291 +++++++++---
> >   hw/intc/aspeed_intc.c                   | 593
> +++++++++++++++++++-----
> >   hw/intc/trace-events                    |  25 +-
> >   hw/misc/aspeed_hace.c                   |  44 +-
> >   hw/misc/aspeed_scu.c                    |   5 +-
> >   include/hw/arm/aspeed_soc.h             |   3 +-
> >   include/hw/intc/aspeed_intc.h           |  32 +-
> >   include/hw/misc/aspeed_hace.h           |   1 +
> >   include/hw/misc/aspeed_scu.h            |   2 +
> >   tests/functional/test_aarch64_aspeed.py |  47 +-
> >   13 files changed, 963 insertions(+), 238 deletions(-)
> >   create mode 100644 docs/specs/aspeed-intc.rst
> >
> 
> Patch 1-9 and the hace changes could be merged quickly.
> 
> 
> I need  some help on patch 10,12,16,17.
> 
> Andrew,
> 
> Would you have time please ?
> 
> Thanks,
> 
> C.
Cédric Le Goater Feb. 21, 2025, 5:24 p.m. UTC | #3
On 2/20/25 06:11, Jamin Lin wrote:
> Hi Cedric,
> 
>> Subject: Re: [PATCH v3 00/28] Support AST2700 A1
>>
>> Hello Jamin,
>>
>>
>> On 2/13/25 04:35, Jamin Lin wrote:
>>> v1:
>>>    1. Refactor INTC model to support both INTC0 and INTC1.
>>>    2. Support AST2700 A1.
>>>    3. Create ast2700a0-evb machine.
>>>
>>> v2:
>>>     To streamline the review process, split the following patch series into
>>>     three parts.
>>>
>> https://patchwork.kernel.org/project/qemu-devel/cover/20250121070424.246
>> 5942-1-jamin_lin@aspeedtech.com/
>>>     This patch series focuses on cleaning up the INTC model to
>>>     facilitate future support for the INTC_IO model.
>>>
>>> v3:
>>>    1. Update and add functional test for AST2700
>>>    2. Add AST2700 INTC design guidance and its block diagram.
>>>    3. Retaining the INTC naming and introducing a new INTCIO model to
>> support the AST2700 A1.
>>>    4. Create ast2700a1-evb machine and rename ast2700a0-evb machine
>>>    5. Fix silicon revision issue and support AST2700 A1.
>>>
>>> With the patch applied, QEMU now supports two machines for running
>> AST2700 SoCs:
>>> ast2700a0-evb: Designed for AST2700 A0
>>> ast2700a1-evb: Designed for AST2700 A1
>>>
>>> Test information
>>> 1. QEMU version:
>>>
>> https://github.com/qemu/qemu/commit/ffaf7f0376f8040ce9068d71ae9ae872
>> 25
>>> 05c42e
>>> 2. ASPEED SDK v09.05 pre-built image
>>>      https://github.com/AspeedTech-BMC/openbmc/releases/tag/v09.05
>>>      ast2700-default-obmc.tar.gz (AST2700 A1)
>>>
>> https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast
>> 2700-default-obmc.tar.gz
>>>      ast2700-a0-default-obmc.tar.gz (AST2700 A0)
>>>
>>>
>> https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/ast
>>> 2700-a0-default-obmc.tar.gz
>>
>> The part adding new functional tests needs a rework. See comment.
>>
>>> Known Issue:
>>> The HACE crypto and hash engine is enable by default since AST2700 A1.
>>> However, aspeed_hace.c(HACE model) currently does not support the
>> CRYPTO command.
>>> To boot AST2700 A1, I have created a Patch 21 which temporarily
>>> resolves the issue by sending an interrupt to notify the firmware that
>>> the cryptographic command has completed. It is a temporary workaround
>>> to resolve the boot issue in the Crypto Manager SelfTest.
>>>
>>> As a result, you will encounter the following kernel warning due to
>>> the Crypto Manager test failure. If you don't want to see these kernel
>>> warning, please add the following settings in your kernel config.
>>>
>>> ```
>>> CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=y
>>> ```
>>
>> Would it be possible to send the hace changes in its own series ?
>>
>>
> Currently, the HACE HW engine and crypto self-tests are enabled by default in
> SDK v09.05 FW. To boot QEMU for AST2700 A1 with the SDK v09.05 pre-built image,
> a CRYPTO workaround patch is required.

can we upstream the HACE changes before having full AST2700 A1 support ?

Thanks,

C.




> 
> The AST2700 A1 patch series includes functional tests. To make the functional tests
> pass for AST2700 A1, I have included the HACE patch in the same patch series.
> 
> There are two ways to split this patch series:
> 
> Solution A:
> 
> 1. Create series 1 to support AST2700 A1.
> 2. Create series 2 to support HACE.
> 3. Create series 3 to support AST2700 A1 functional tests.
> 
> Series 3 should depend on series 1 and 2.
> 
> Solution B:
> 
> 1. Place a pre-built image called "ast2700-a1-qemu-disable-self-test" in the SDK v09.05 Github repository.
> https://github.com/AspeedTech-BMC/openbmc/releases/tag/v09.05
> 2. Create one patch series to support AST2700 A1 with its functional tests.
> 3. Create series 2 to support HACE.
> 
> Could you tell me which solution you prefer or could you please give me any suggestion?
> 
> Thanks-Jamin
> 
>>>
>>> Jamin Lin (28):
>>>     hw/intc/aspeed: Support setting different memory and register size
>>>     hw/intc/aspeed: Introduce helper functions for enable and status
>>>       registers
>>>     hw/intc/aspeed: Add object type name to trace events for better
>>>       debugging
>>>     hw/arm/aspeed: Rename IRQ table and machine name for AST2700 A0
>>>     hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number
>>>     hw/intc/aspeed: Support different memory region ops
>>>     hw/intc/aspeed: Rename num_ints to num_inpins for clarity
>>>     hw/intc/aspeed: Add support for multiple output pins in INTC
>>>     hw/intc/aspeed: Refactor INTC to support separate input and output pin
>>>       indices
>>>     hw/intc/aspeed: Introduce AspeedINTCIRQ structure to save the irq
>>>       index and register address
>>>     hw/intc/aspeed: Introduce IRQ handler function to reduce code
>>>       duplication
>>>     hw/intc/aspeed: Add Support for Multi-Output IRQ Handling
>>>     hw/intc/aspeed: Add Support for AST2700 INTCIO Controller
>>>     hw/misc/aspeed_scu: Add Support for AST2700/AST2750 A1 Silicon
>>>       Revisions
>>>     hw/misc/aspeed_scu: Fix the revision ID cannot be set in the SOC layer
>>>       for AST2700
>>>     hw/arm/aspeed_ast27x0.c Support AST2700 A1 GIC Interrupt Mapping
>>>     hw/arm/aspeed_ast27x0: Support two levels of INTC controllers for
>>>       AST2700 A1
>>>     hw/arm/aspeed: Add SoC and Machine Support for AST2700 A1
>>>     hw/misc/aspeed_hace: Fix coding style
>>>     hw/misc/aspeed_hace: Add AST2700 support
>>>     hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
>>>     hw/arm/aspeed_ast27x0: Add HACE support for AST2700
>>>     test/functional/aspeed: Introduce new function to fetch assets
>>>     tests/functional/aspeed: Introduce start_ast2700_test API and update
>>>       hwmon path
>>>     tests/functional/aspeed: Update test ASPEED SDK v09.05
>>>     tests/functional/aspeed: Renamed test case and machine for AST2700
>> A0
>>>     tests/functional/aspeed: Add test case for AST2700 A1
>>>     docs/specs: add aspeed-intc
>>>
>>>    docs/specs/aspeed-intc.rst              | 136 ++++++
>>>    docs/specs/index.rst                    |   1 +
>>>    hw/arm/aspeed.c                         |  21 +-
>>>    hw/arm/aspeed_ast27x0.c                 | 291 +++++++++---
>>>    hw/intc/aspeed_intc.c                   | 593
>> +++++++++++++++++++-----
>>>    hw/intc/trace-events                    |  25 +-
>>>    hw/misc/aspeed_hace.c                   |  44 +-
>>>    hw/misc/aspeed_scu.c                    |   5 +-
>>>    include/hw/arm/aspeed_soc.h             |   3 +-
>>>    include/hw/intc/aspeed_intc.h           |  32 +-
>>>    include/hw/misc/aspeed_hace.h           |   1 +
>>>    include/hw/misc/aspeed_scu.h            |   2 +
>>>    tests/functional/test_aarch64_aspeed.py |  47 +-
>>>    13 files changed, 963 insertions(+), 238 deletions(-)
>>>    create mode 100644 docs/specs/aspeed-intc.rst
>>>
>>
>> Patch 1-9 and the hace changes could be merged quickly.
>>
>>
>> I need  some help on patch 10,12,16,17.
>>
>> Andrew,
>>
>> Would you have time please ?
>>
>> Thanks,
>>
>> C.
>
Jamin Lin Feb. 25, 2025, 8:02 a.m. UTC | #4
Hi Cedric, 

> Cc: Troy Lee <troy_lee@aspeedtech.com>
> Subject: Re: [PATCH v3 00/28] Support AST2700 A1
> 
> On 2/20/25 06:11, Jamin Lin wrote:
> > Hi Cedric,
> >
> >> Subject: Re: [PATCH v3 00/28] Support AST2700 A1
> >>
> >> Hello Jamin,
> >>
> >>
> >> On 2/13/25 04:35, Jamin Lin wrote:
> >>> v1:
> >>>    1. Refactor INTC model to support both INTC0 and INTC1.
> >>>    2. Support AST2700 A1.
> >>>    3. Create ast2700a0-evb machine.
> >>>
> >>> v2:
> >>>     To streamline the review process, split the following patch series into
> >>>     three parts.
> >>>
> >> https://patchwork.kernel.org/project/qemu-devel/cover/20250121070424.
> >> 246
> >> 5942-1-jamin_lin@aspeedtech.com/
> >>>     This patch series focuses on cleaning up the INTC model to
> >>>     facilitate future support for the INTC_IO model.
> >>>
> >>> v3:
> >>>    1. Update and add functional test for AST2700
> >>>    2. Add AST2700 INTC design guidance and its block diagram.
> >>>    3. Retaining the INTC naming and introducing a new INTCIO model
> >>> to
> >> support the AST2700 A1.
> >>>    4. Create ast2700a1-evb machine and rename ast2700a0-evb
> machine
> >>>    5. Fix silicon revision issue and support AST2700 A1.
> >>>
> >>> With the patch applied, QEMU now supports two machines for running
> >> AST2700 SoCs:
> >>> ast2700a0-evb: Designed for AST2700 A0
> >>> ast2700a1-evb: Designed for AST2700 A1
> >>>
> >>> Test information
> >>> 1. QEMU version:
> >>>
> >>
> https://github.com/qemu/qemu/commit/ffaf7f0376f8040ce9068d71ae9ae872
> >> 25
> >>> 05c42e
> >>> 2. ASPEED SDK v09.05 pre-built image
> >>>      https://github.com/AspeedTech-BMC/openbmc/releases/tag/v09.05
> >>>      ast2700-default-obmc.tar.gz (AST2700 A1)
> >>>
> >>
> https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/as
> >> t
> >> 2700-default-obmc.tar.gz
> >>>      ast2700-a0-default-obmc.tar.gz (AST2700 A0)
> >>>
> >>>
> >>
> https://github.com/AspeedTech-BMC/openbmc/releases/download/v09.05/as
> >> t
> >>> 2700-a0-default-obmc.tar.gz
> >>
> >> The part adding new functional tests needs a rework. See comment.
> >>
> >>> Known Issue:
> >>> The HACE crypto and hash engine is enable by default since AST2700 A1.
> >>> However, aspeed_hace.c(HACE model) currently does not support the
> >> CRYPTO command.
> >>> To boot AST2700 A1, I have created a Patch 21 which temporarily
> >>> resolves the issue by sending an interrupt to notify the firmware
> >>> that the cryptographic command has completed. It is a temporary
> >>> workaround to resolve the boot issue in the Crypto Manager SelfTest.
> >>>
> >>> As a result, you will encounter the following kernel warning due to
> >>> the Crypto Manager test failure. If you don't want to see these
> >>> kernel warning, please add the following settings in your kernel config.
> >>>
> >>> ```
> >>> CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=y
> >>> ```
> >>
> >> Would it be possible to send the hace changes in its own series ?
> >>
> >>
> > Currently, the HACE HW engine and crypto self-tests are enabled by
> > default in SDK v09.05 FW. To boot QEMU for AST2700 A1 with the SDK
> > v09.05 pre-built image, a CRYPTO workaround patch is required.
> 
> can we upstream the HACE changes before having full AST2700 A1 support ?
> 

I send HACE patch here, https://patchwork.kernel.org/project/qemu-devel/cover/20250225075622.305515-1-jamin_lin@aspeedtech.com/
Thanks-Jamin

> Thanks,
> 
> C.
> 
> 
> 
> 
> >
> > The AST2700 A1 patch series includes functional tests. To make the
> > functional tests pass for AST2700 A1, I have included the HACE patch in the
> same patch series.
> >
> > There are two ways to split this patch series:
> >
> > Solution A:
> >
> > 1. Create series 1 to support AST2700 A1.
> > 2. Create series 2 to support HACE.
> > 3. Create series 3 to support AST2700 A1 functional tests.
> >
> > Series 3 should depend on series 1 and 2.
> >
> > Solution B:
> >
> > 1. Place a pre-built image called "ast2700-a1-qemu-disable-self-test" in the
> SDK v09.05 Github repository.
> > https://github.com/AspeedTech-BMC/openbmc/releases/tag/v09.05
> > 2. Create one patch series to support AST2700 A1 with its functional tests.
> > 3. Create series 2 to support HACE.
> >
> > Could you tell me which solution you prefer or could you please give me any
> suggestion?
> >
> > Thanks-Jamin
> >
> >>>
> >>> Jamin Lin (28):
> >>>     hw/intc/aspeed: Support setting different memory and register size
> >>>     hw/intc/aspeed: Introduce helper functions for enable and status
> >>>       registers
> >>>     hw/intc/aspeed: Add object type name to trace events for better
> >>>       debugging
> >>>     hw/arm/aspeed: Rename IRQ table and machine name for AST2700
> A0
> >>>     hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number
> >>>     hw/intc/aspeed: Support different memory region ops
> >>>     hw/intc/aspeed: Rename num_ints to num_inpins for clarity
> >>>     hw/intc/aspeed: Add support for multiple output pins in INTC
> >>>     hw/intc/aspeed: Refactor INTC to support separate input and output
> pin
> >>>       indices
> >>>     hw/intc/aspeed: Introduce AspeedINTCIRQ structure to save the irq
> >>>       index and register address
> >>>     hw/intc/aspeed: Introduce IRQ handler function to reduce code
> >>>       duplication
> >>>     hw/intc/aspeed: Add Support for Multi-Output IRQ Handling
> >>>     hw/intc/aspeed: Add Support for AST2700 INTCIO Controller
> >>>     hw/misc/aspeed_scu: Add Support for AST2700/AST2750 A1 Silicon
> >>>       Revisions
> >>>     hw/misc/aspeed_scu: Fix the revision ID cannot be set in the SOC
> layer
> >>>       for AST2700
> >>>     hw/arm/aspeed_ast27x0.c Support AST2700 A1 GIC Interrupt
> Mapping
> >>>     hw/arm/aspeed_ast27x0: Support two levels of INTC controllers for
> >>>       AST2700 A1
> >>>     hw/arm/aspeed: Add SoC and Machine Support for AST2700 A1
> >>>     hw/misc/aspeed_hace: Fix coding style
> >>>     hw/misc/aspeed_hace: Add AST2700 support
> >>>     hw/misc/aspeed_hace: Fix boot issue in the Crypto Manager Self Test
> >>>     hw/arm/aspeed_ast27x0: Add HACE support for AST2700
> >>>     test/functional/aspeed: Introduce new function to fetch assets
> >>>     tests/functional/aspeed: Introduce start_ast2700_test API and update
> >>>       hwmon path
> >>>     tests/functional/aspeed: Update test ASPEED SDK v09.05
> >>>     tests/functional/aspeed: Renamed test case and machine for
> >>> AST2700
> >> A0
> >>>     tests/functional/aspeed: Add test case for AST2700 A1
> >>>     docs/specs: add aspeed-intc
> >>>
> >>>    docs/specs/aspeed-intc.rst              | 136 ++++++
> >>>    docs/specs/index.rst                    |   1 +
> >>>    hw/arm/aspeed.c                         |  21 +-
> >>>    hw/arm/aspeed_ast27x0.c                 | 291 +++++++++---
> >>>    hw/intc/aspeed_intc.c                   | 593
> >> +++++++++++++++++++-----
> >>>    hw/intc/trace-events                    |  25 +-
> >>>    hw/misc/aspeed_hace.c                   |  44 +-
> >>>    hw/misc/aspeed_scu.c                    |   5 +-
> >>>    include/hw/arm/aspeed_soc.h             |   3 +-
> >>>    include/hw/intc/aspeed_intc.h           |  32 +-
> >>>    include/hw/misc/aspeed_hace.h           |   1 +
> >>>    include/hw/misc/aspeed_scu.h            |   2 +
> >>>    tests/functional/test_aarch64_aspeed.py |  47 +-
> >>>    13 files changed, 963 insertions(+), 238 deletions(-)
> >>>    create mode 100644 docs/specs/aspeed-intc.rst
> >>>
> >>
> >> Patch 1-9 and the hace changes could be merged quickly.
> >>
> >>
> >> I need  some help on patch 10,12,16,17.
> >>
> >> Andrew,
> >>
> >> Would you have time please ?
> >>
> >> Thanks,
> >>
> >> C.
> >