From a96d2a8f2dd3eb7e32b383821fe30cfd7cdb2248 Mon Sep 17 00:00:00 2001 From: Taylor Beebe <31827475+TaylorBeebe@users.noreply.github.com> Date: Tue, 8 Nov 2022 11:56:07 -0800 Subject: [PATCH 1/5] PrmPkg: Don't Set Access Attributes of Runtime MMIO Ranges Passing in access attributes to SetMemorySpaceAttributes() will cause the existing attributes to be overwritten. The MMIO region should have the appropriate attributes applied during memory protection initialization and the attributes of the memory space descriptor are inaccurate. Don't pass in any CPU arch attributes so SetMemorySpaceAttributes() doesn't subsequently call gCpu->SetMemoryAttributes(). Signed-off-by: Oliver Smith-Denny --- PrmPkg/PrmConfigDxe/PrmConfigDxe.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/PrmPkg/PrmConfigDxe/PrmConfigDxe.c b/PrmPkg/PrmConfigDxe/PrmConfigDxe.c index 550ee64b4c53e..7a3913ee090c9 100644 --- a/PrmPkg/PrmConfigDxe/PrmConfigDxe.c +++ b/PrmPkg/PrmConfigDxe/PrmConfigDxe.c @@ -152,10 +152,15 @@ SetRuntimeMemoryRangeAttributes ( continue; } + // The memory space descriptor access attributes are not accurate. Don't pass + // in access attributes so SetMemorySpaceAttributes() doesn't update them. + // EFI_MEMORY_RUNTIME is not a CPU arch attribute, so calling + // SetMemorySpaceAttributes() with only it set will not clear existing page table + // attributes for this region, such as EFI_MEMORY_XP Status = gDS->SetMemorySpaceAttributes ( RuntimeMmioRanges->Range[Index].PhysicalBaseAddress, (UINT64)RuntimeMmioRanges->Range[Index].Length, - Descriptor.Attributes | EFI_MEMORY_RUNTIME + EFI_MEMORY_RUNTIME ); ASSERT_EFI_ERROR (Status); if (EFI_ERROR (Status)) { From 3f0c4cee940d550cf7543eef188b3b068df8b818 Mon Sep 17 00:00:00 2001 From: Michael Kubacki Date: Mon, 22 Jul 2024 17:14:55 -0400 Subject: [PATCH 2/5] BaseTools/GetMaintainer.py: Add GitHub username argument Adds a new `-g` parameter so that output will also include the GitHub username. This change uses a simple regular expression as opposed to directly returning the original line from the file to make the extraction of GitHub usernames more robust to other changes on the line in the maintainers text file. Signed-off-by: Michael Kubacki --- BaseTools/Scripts/GetMaintainer.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/BaseTools/Scripts/GetMaintainer.py b/BaseTools/Scripts/GetMaintainer.py index 8097ba4e7bd62..986550c4a9d13 100644 --- a/BaseTools/Scripts/GetMaintainer.py +++ b/BaseTools/Scripts/GetMaintainer.py @@ -179,6 +179,10 @@ def get_modified_files(repo, args): PARSER.add_argument('-l', '--lookup', help='Find section matches for path LOOKUP', required=False) + PARSER.add_argument('-g', '--github', + action='store_true', + help='Include GitHub usernames in output', + required=False) ARGS = PARSER.parse_args() REPO = SetupGit.locate_repo() @@ -203,5 +207,8 @@ def get_modified_files(repo, args): for address in ADDRESSES: if '<' in address and '>' in address: - address = address.split('>', 1)[0] + '>' - print(' %s' % address) + address, github_id = address.split('>', 1) + address = address + '>' + github_id = github_id.strip() if ARGS.github else '' + + print(' %s %s' % (address, github_id)) From 89a06a245bd27c6a83fa2b2ed80cfe186dab0029 Mon Sep 17 00:00:00 2001 From: Michael Kubacki Date: Mon, 22 Jul 2024 17:15:52 -0400 Subject: [PATCH 3/5] .github: Add GitHub helper python script Adds a script that provides GitHub API helpers for workflows and other GitHub automation in the repository. Signed-off-by: Michael Kubacki --- .github/scripts/GitHub.py | 187 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 .github/scripts/GitHub.py diff --git a/.github/scripts/GitHub.py b/.github/scripts/GitHub.py new file mode 100644 index 0000000000000..cd6bea5205f7f --- /dev/null +++ b/.github/scripts/GitHub.py @@ -0,0 +1,187 @@ +## @file +# GitHub API helper functions. +# +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: BSD-2-Clause-Patent +# + +import logging +import re +import requests + +from collections import OrderedDict +from edk2toollib.utility_functions import RunCmd, RunPythonScript +from io import StringIO +from typing import List + +"""GitHub API helper functions.""" + + +def leave_pr_comment( + token: str, owner: str, repo: str, pr_number: str, comment_body: str +): + """Leaves a comment on a PR. + + Args: + token (str): The GitHub token to use for authentication. + owner (str): The GitHub owner (organization) name. + repo (str): The GitHub repository name (e.g. 'edk2'). + pr_number (str): The pull request number. + comment_body (str): The comment text. Markdown is supported. + """ + url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments" + headers = { + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github.v3+json", + } + data = {"body": comment_body} + response = requests.post(url, json=data, headers=headers) + response.raise_for_status() + + +def get_reviewers_for_current_branch( + workspace_path: str, maintainer_file_path: str, target_branch: str = "master" +) -> List[str]: + """Get the reviewers for the current branch. + + Args: + workspace_path (str): The workspace path. + maintainer_file_path (str): The maintainer file path. + target_branch (str, optional): The name of the target branch that the + current HEAD will merge to. Defaults to "master". + + Returns: + List[str]: A list of GitHub usernames. + """ + + commit_stream_buffer = StringIO() + cmd_ret = RunCmd( + "git", + f"log --format=format:%H {target_branch}..HEAD", + workingdir=workspace_path, + outstream=commit_stream_buffer, + logging_level=logging.INFO, + ) + if cmd_ret != 0: + print( + f"::error title=Commit Lookup Error!::Error getting branch commits: [{cmd_ret}]: {commit_stream_buffer.getvalue()}" + ) + return [] + + raw_reviewers = [] + for commit_sha in commit_stream_buffer.getvalue().splitlines(): + reviewer_stream_buffer = StringIO() + cmd_ret = RunPythonScript( + maintainer_file_path, + f"-g {commit_sha}", + workingdir=workspace_path, + outstream=reviewer_stream_buffer, + logging_level=logging.INFO, + ) + if cmd_ret != 0: + print( + f"::error title=Reviewer Lookup Error!::Error calling GetMaintainer.py: [{cmd_ret}]: {reviewer_stream_buffer.getvalue()}" + ) + return [] + + commit_reviewers = reviewer_stream_buffer.getvalue() + + pattern = r"\[(.*?)\]" + matches = re.findall(pattern, commit_reviewers) + if not matches: + return [] + + print( + f"::debug title=Commit {commit_sha[:7]} Reviewer(s)::{', '.join(matches)}" + ) + + raw_reviewers.extend(matches) + + reviewers = list(OrderedDict.fromkeys([r.strip() for r in raw_reviewers])) + + print(f"::debug title=Total Reviewer Set::{', '.join(reviewers)}") + + return reviewers + + +def download_gh_file(github_url: str, local_path: str, token=None): + """Downloads a file from GitHub. + + Args: + github_url (str): The GitHub raw file URL. + local_path (str): A local path to write the file contents to. + token (_type_, optional): A GitHub authentication token. + Only needed for a private repo. Defaults to None. + """ + headers = {} + if token: + headers["Authorization"] = f"Bearer {token}" + + try: + response = requests.get(github_url, headers=headers) + response.raise_for_status() + except requests.exceptions.HTTPError: + print( + f"::error title=HTTP Error!::Error downloading {github_url}: {response.reason}" + ) + return + + with open(local_path, "w", encoding="utf-8") as file: + file.write(response.text) + + +def add_reviewers_to_pr( + token: str, owner: str, repo: str, pr_number: str, user_names: List[str] +): + """Adds the set of GitHub usernames as reviewers to the PR. + + Args: + token (str): The GitHub token to use for authentication. + owner (str): The GitHub owner (organization) name. + repo (str): The GitHub repository name (e.g. 'edk2'). + pr_number (str): The pull request number. + user_names (List[str]): List of GitHub usernames to add as reviewers. + """ + headers = { + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github.v3+json", + } + pr_author_url = f"https://api.github.com/repos/{owner}/{repo}/pulls/{pr_number}" + url = f"https://api.github.com/repos/{owner}/{repo}/pulls/{pr_number}/requested_reviewers" + + response = requests.get(pr_author_url, headers=headers) + if response.status_code != 200: + print(f"::error title=HTTP Error!::Error getting PR author: {response.reason}") + return + pr_author = response.json().get("user").get("login").strip() + while pr_author in user_names: + user_names.remove(pr_author) + data = {"reviewers": user_names} + response = requests.post(url, json=data, headers=headers) + try: + response.raise_for_status() + except requests.exceptions.HTTPError: + if ( + response.status_code == 422 + and "Reviews may only be requested from collaborators" + in response.json().get("message") + ): + print( + f"::error title=User is not a Collaborator!::{response.json().get('message')}" + ) + leave_pr_comment( + token, + owner, + repo, + pr_number, + f"⚠ **WARNING: Cannot add reviewers**: A user specified as a " + f"reviewer for this PR is not a collaborator " + f"of the edk2 repository. Please add them as a collaborator to the " + f"repository and re-request the review.\n\n" + f"Users requested:\n{', '.join(user_names)}", + ) + elif response.status_code == 422: + print( + "::error title=Invalid Request!::The request is invalid. " + "Verify the API request string." + ) From 6271b617b4e653029246152871cde93f3926e144 Mon Sep 17 00:00:00 2001 From: Michael Kubacki Date: Mon, 22 Jul 2024 17:42:43 -0400 Subject: [PATCH 4/5] .github/workflows/request-reviews.yml: Add workflow Adds a new GitHub workflow to automatically add reviewers to pull requests when they are opened, reopened, synchronized, and if a draft pull request is marked as ready for review. The workflow will not run on draft pull requests. The workflow is meant to be simple to understand and modify, relying on existing logic in GetMaintainer.py to determine the relevant reviewers and using simple Python GitHub REST API wrappers with the default GitHub token for authentication. Future changes may optimize the workflow. Signed-off-by: Michael Kubacki --- .github/workflows/request-reviews.yml | 73 +++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 .github/workflows/request-reviews.yml diff --git a/.github/workflows/request-reviews.yml b/.github/workflows/request-reviews.yml new file mode 100644 index 0000000000000..4f43d56122219 --- /dev/null +++ b/.github/workflows/request-reviews.yml @@ -0,0 +1,73 @@ + +# This workflow automatically adds the appropriate reviewers to a pull request. +# +# The workflow directly reuses logic in the BaseTools/Scripts/GetMaintainer.py script +# to determine the appropriate reviewers, so it matches what a user would see running +# the script locally. +# +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: BSD-2-Clause-Patent + +name: Add Pull Request Reviewers + +on: + pull_request: + branches: + - master + types: [opened, ready_for_review, reopened, synchronize] + +env: + GET_MAINTAINER_REL_PATH: "BaseTools/Scripts/GetMaintainer.py" + +jobs: + auto-request-review: + name: Add Pull Request Reviewers + # Do not run on draft PRs and only run on PRs in the tianocore organization + if: ${{ github.event.pull_request.draft == false && github.repository_owner == 'tianocore' }} + runs-on: ubuntu-latest + + permissions: + contents: read + issues: write + pull-requests: write + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.x' + + - name: Install PIP Modules + run: pip install edk2-pytool-library edk2-pytool-extensions requests + + - name: Add Reviewers to Pull Request + shell: python + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + ORG_NAME: ${{ github.repository_owner }} + PR_NUMBER: ${{ github.event.number}} + REPO_NAME: ${{ github.event.pull_request.base.repo.name }} + TARGET_BRANCH: ${{ github.event.pull_request.base.ref }} + WORKSPACE_PATH: ${{ github.workspace }} + run: | + import os + import sys + sys.path.append(os.path.join(os.environ['WORKSPACE_PATH'], ".github")) + from scripts import GitHub + + WORKSPACE_PATH = os.environ['WORKSPACE_PATH'] + GET_MAINTAINER_LOCAL_PATH = os.path.join(WORKSPACE_PATH, os.environ['GET_MAINTAINER_REL_PATH']) + + reviewers = GitHub.get_reviewers_for_current_branch(WORKSPACE_PATH, GET_MAINTAINER_LOCAL_PATH, f"origin/{os.environ['TARGET_BRANCH']}") + if not reviewers: + print("::notice title=No Reviewers Found!::No reviewers found for this PR.") + sys.exit(1) + + print(f"::notice title=Reviewer List::Reviewers found for PR {os.environ['PR_NUMBER']}:\n{', '.join(reviewers)}") + + GitHub.add_reviewers_to_pr(os.environ['GH_TOKEN'], os.environ['ORG_NAME'], os.environ['REPO_NAME'], os.environ['PR_NUMBER'], reviewers) From 556640bceac32c86a843d09a1dd0688e44883781 Mon Sep 17 00:00:00 2001 From: Dongyan Qian Date: Tue, 25 Jun 2024 13:51:44 +0800 Subject: [PATCH 5/5] UefiCpuPkg/MpInitLib: Reduce compiler dependencies for LoongArch Structure assignment may depend on the compiler to expand to memcpy. For this, we may need to add -mno-memcpy to the compilation flag. Here, we reduce dependencies and use CopyMem for data conversion without memcpy. Cc: Ray Ni Cc: Rahul Kumar Cc: Gerd Hoffmann Cc: Jiaxin Wu Cc: Chao Li Signed-off-by: Dongyan Qian Co-authored-by: Chao Li --- UefiCpuPkg/Library/MpInitLib/LoongArch64/MpLib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/LoongArch64/MpLib.c b/UefiCpuPkg/Library/MpInitLib/LoongArch64/MpLib.c index c18671e95f9be..54c23ecf10919 100644 --- a/UefiCpuPkg/Library/MpInitLib/LoongArch64/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/LoongArch64/MpLib.c @@ -130,8 +130,8 @@ SortApicId ( } else { for ( ; Index2 <= ApCount; Index2++) { if (CpuInfoInHob[Index2].ApicId == INVALID_APIC_ID) { - CopyMem (&CpuInfoInHob[Index2], &CpuInfoInHob[Index1], sizeof (CPU_INFO_IN_HOB)); - CpuMpData->CpuData[Index2] = CpuMpData->CpuData[Index1]; + CopyMem (CpuInfoInHob + Index2, CpuInfoInHob + Index1, sizeof (CPU_INFO_IN_HOB)); + CopyMem (CpuMpData->CpuData + Index2, CpuMpData->CpuData + Index1, sizeof (CPU_AP_DATA)); CpuInfoInHob[Index1].ApicId = INVALID_APIC_ID; break; }