Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QemuQ35Pkg: Define SMM_CRYPTO_ARCH #840

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

Javagedes
Copy link
Contributor

Description

defines SMM_CRYPTO_ARCH in QemuQ35Pkg.dsc to resolve parser warnings regarding suspicious expressions that occur because $(SMM_CRYPTO_ARCH) is not replaced with a value (because a value is not defined).

Prints multiple of the following parser warning:

INFO - Parser...
INFO - c:\src\mu_tiano_platforms\MU_BASECORE\CryptoPkg\Driver\Bin\Crypto.pcd.TINY_SHA.inc.dsc(112): warning: Suspicious expression: == Comparison between Operand of string type and Boolean/Number Type always return False.
INFO -     !if $(SMM_CRYPTO_ARCH) == X64
  • Impacts functionality?
    • Functionality - Does the change ultimately impact how firmware functions?
    • Examples: Add a new library, publish a new PPI, update an algorithm, ...
  • Impacts security?
    • Security - Does the change have a direct security impact on an application,
      flow, or firmware?
    • Examples: Crypto algorithm change, buffer overflow fix, parameter
      validation improvement, ...
  • Breaking change?
    • Breaking change - Will anyone consuming this change experience a break
      in build or boot behavior?
    • Examples: Add a new library class, move a module to a different repo, call
      a function in a new library class in a pre-existing module, ...
  • Includes tests?
    • Tests - Does the change include any explicit test code?
    • Examples: Unit tests, integration tests, robot tests, ...
  • Includes documentation?
    • Documentation - Does the change contain explicit documentation additions
      outside direct code modifications (and comments)?
    • Examples: Update readme file, add feature readme file, link to documentation
      on an a separate Web page, ...

How This Was Tested

Built with and without SMM_CRYPTO_ARCH being defined, and verified the parser warning was removed when it was defined.

Integration Instructions

N/A

@github-actions github-actions bot added the impact:non-functional Does not have a functional impact label Feb 5, 2024
@Javagedes
Copy link
Contributor Author

Pipelines are automatically downloading rust v1.74.0, but our rust-toolchain file has not been updated to 1.74.0.

Waiting for microsoft/mu_devops@af0c4bd to be sync'd to all repositories

Copy link
Member

@makubacki makubacki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kun12, I understand why the DSC parser complains about this. But we might want to update this statement from microsoft/mu_basecore#713 in the release notes since it seems a little misleading/open to confusion at the moment.

This change updated the autogeneration script to only require ARCH when CRYPTO_SERVICES is not NONE, and accordingly only include the needed modules and libraries.

@makubacki
Copy link
Member

Pipelines are automatically downloading rust v1.74.0, but our rust-toolchain file has not been updated to 1.74.0.

Waiting for microsoft/mu_devops@af0c4bd to be sync'd to all repositories

Just kicked a sync.

@kuqin12
Copy link
Contributor

kuqin12 commented Feb 6, 2024

@Kun12, I understand why the DSC parser complains about this. But we might want to update this statement from microsoft/mu_basecore#713 in the release notes since it seems a little misleading/open to confusion at the moment.

This change updated the autogeneration script to only require ARCH when CRYPTO_SERVICES is not NONE, and accordingly only include the needed modules and libraries.

@makubacki I think the issue might be that there are some other references in the inclusion file I did not clean up. I will take a look to see if any more pruning needed.

defines SMM_CRYPTO_ARCH in QemuQ35Pkg.dsc to resolve parser warnings
regarding suspicious expressions that occur because $(SMM_CRYPTO_ARCH)
is not replaced with a value.
@Javagedes Javagedes merged commit 960ebde into microsoft:main Feb 6, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:non-functional Does not have a functional impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants