ARM-software / bsa-acs

Arm SystemReady : BSA Architecture Compliance Suite
Apache License 2.0
16 stars 42 forks source link

SMMU tests are misleading #123

Closed hrw closed 1 year ago

hrw commented 1 year ago

Test 352 says:

 352 : SMMU Revision,S-EL2 support Hyp                START

       SMMUv3.1 not providing Stage2 functionality
       Failed on PE -    0
       B_SMMU_18
       Checkpoint --  2                           : Result:  FAIL 
         END

So what exactly it tests (by looking at title)?

SMMU revision and S-EL2 support are checked in test 306:

 306 : SMMU revision and S-EL2 support                START

       B_SMMU_08
                                       : Result:  PASS 
         END

B_SMMU_18 says:

If Secure-EL2 is not implemented, stage 2 System MMU functionality must be provided by a System MMU compatible with the Arm SMMUv2 specification or Arm SMMUv3 specification.

I wonder should there be separate test for "is there Secure-EL2 implemented" as now both B_SMMU_08 and B_SMMU_18 tests can pass if one or another is provided.

For sbsa-ref we pass B_SMMU_08 as we have SMMU v3.0 implemented. Then we fail B_SMMU_18 because we lack Stage2 support.

hrw commented 1 year ago

And SMMU Stage2 functionality is tested by test 351 explicitly.

chetan-rathore commented 1 year ago

Hi @hrw,

Thanks for raising the issue, we will check the issue and keep you updated.

Thanks, ACS team

chetan-rathore commented 1 year ago

Hi @hrw,

The test 306 and 352 will need update in test description, they are checking smmu stage 1 and stage 2 functionality respectively when S-EL2 is not implemented. Code changes are expected to be upstream by this week.

Rules B_SMMU_09 and B_SMMU_20 cover the cases when S-EL2 is implemented, but they are part of SBSA checklist only and the test for these rules are present in sbsa.

Thanks, ACS team

chetan-rathore commented 1 year ago

Hi @hrw,

The changes are merged.

Thanks, ACS team

hrw commented 1 year ago
      *** Starting System MMU tests ***
Operating System View:
 301 : All SMMUs have same Arch Revision              START

       B_SMMU_01
                                       : Result:  PASS
         END

 302 : Check SMMU Granule Support                     START

       B_SMMU_02
                                       : Result:  PASS
         END

 303 : Check Large Physical Addr Support              START

       Large PA Not Supported in SMMU 0
       Failed on PE -    0
       B_SMMU_06
       Checkpoint --  2                           : Result:  FAIL
         END

 304 : S-EL2 not implemented & SMMU stage1 support         START

       S-EL2 implemented...Skipping
       B_SMMU_08
       Checkpoint --  2                           : Result:  SKIPPED
         END

Hypervisor View:
 352 : S-EL2 not implemented & SMMU stage2 support         START

       S-EL2 implemented...Skipping
       B_SMMU_16, B_SMMU_17, B_SMMU_18
       Checkpoint --  2                           : Result:  SKIPPED
         END

 354 : SMMUv3 Integration compliance                  START

       B_SMMU_21, SMMU_01
                                       : Result:  PASS
         END

      One or more SMMU tests failed or were skipped.

Can I suggest test descriptions?

Looks like test 304 should be followed with test for B_SMMU_09 which would check is Stage1 works properly if Secure-EL2 is implemented. And then I wonder should tests 304/352 PASS if Secure-EL2 is implemented.

Or maybe test 352 should be split to check for Stage2 (B_SMMU_16, B_SMMU_17) and to check for Secure-EL2 and Stage2 (B_SMMU_18). So systems without Stage2 support but with Secure-EL2 will pass/failure on proper tests.

chetan-rathore commented 1 year ago

Hi @hrw,

"Looks like test 304 should be followed with test for B_SMMU_09 which would check is Stage1 works properly if Secure-EL2 is implemented. And then I wonder should tests 304/352 PASS if Secure-EL2 is implemented.

Or maybe test 352 should be split to check for Stage2 (B_SMMU_16, B_SMMU_17) and to check for Secure-EL2 and Stage2 (B_SMMU_18). So systems without Stage2 support but with Secure-EL2 will pass/failure on proper tests."

Stage 1 and Stage 2 checks, when S-EL2 is implemented are part of SBSA Level 4. S-EL2 was introduced in arm v8.4 and v8.4 related requirements maps to SBSA L4. SBSA specification "1.2.1 SBSA levels and minimum component versions" summarize the minimum architecture version and SBSA rules mapping.

304/352 checks stage 1/2 functionality when S-EL2 is not implemented, so for systems implementing S-EL2 skip of these tests is valid.

But I understand your point..that if a System implements S-El2 and runs only BSA ...it might appear the stage1/2 functionality is not checked. Ideally in this case running the max SBSA level tests based on PE profile will give better compliance view of the system.

Thanks, ACS team

hrw commented 1 year ago

Moments like this one makes my recommendation to merge SBSA ACS into BSA ACS even more sensible.

hrw commented 1 year ago

@chetan-rathore the things gets complicated mostly due to my current target: SBSA Reference Platform in QEMU. Where some things are present, some are not. Some miss definitions in ACPI tables, some miss implementation.

Some cpu cores are v8.0 (a57/a72), some are v8.2 (n1), some are full of features but not map to any ISA level (max).

I have one physical Arm system with ACPI tables at home. But it's EDK2 is based on some old fork and running ACS there takes insane amount of time. And it is "SystemReady ES wannabe" only, not SR.

chetan-rathore commented 1 year ago

@chetan-rathore the things gets complicated mostly due to my current target: SBSA Reference Platform in QEMU. Where some things are present, some are not. Some miss definitions in ACPI tables, some miss implementation.

Some cpu cores are v8.0 (a57/a72), some are v8.2 (n1), some are full of features but not map to any ISA level (max).

I have one physical Arm system with ACPI tables at home. But it's EDK2 is based on some old fork and running ACS there takes insane amount of time. And it is "SystemReady ES wannabe" only, not SR.

May I suggest to setup RDN2 FVP... (https://github.com/ARM-software/arm-systemready/tree/main/SR#verification-of-the-sr-image-on-the-arm-neoverse-n2-reference-design-rd-n2)

jumping from qemu to rdn2 will 🏎 :)

chetan-rathore commented 1 year ago

Moments like this one makes my recommendation to merge SBSA ACS into BSA ACS even more sensible.

We are tracking the feedback as part of this https://github.com/ARM-software/bsa-acs/issues/101

chetan-rathore commented 1 year ago

Please do let us know in case you face any issues/errors setting up rdn2

hrw commented 1 year ago

Booted rdn2. Fun, no crypto extensions in BSA ACS.

chetan-rathore commented 1 year ago

Crypto extensions are optional in RDN2. More details: https://developer.arm.com/documentation/102337/0000/Functional-description/Security/Application-processor-security

chetan-rathore commented 1 year ago

Hi @hrw,

The changes related to this issue are merged and "feedback of merging bsa and sbsa acs" is tracked under #101.

If no further comments on this issue, can we close this issue.

Thanks, ACS team