nuttxpr commented on PR #15071:
URL: https://github.com/apache/nuttx/pull/15071#issuecomment-2522019855

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   This PR does *not* fully meet the NuttX requirements as presented.  While it 
provides a summary of the change itself, it's missing crucial information 
required by the template.
   
   Here's a breakdown of what's missing:
   
   * **Summary:**  While the one-line summary is okay for a quick overview, it 
lacks detail.  It needs to explicitly state:
       * **Why:** *Why* is this change necessary? What bug does it fix? What 
improvement does it provide?  "pci dev alloc bridge mem error" is a symptom, 
not a reason.  Explain the underlying problem.
       * **What:** What specific part of the PCI code is being changed?  
Mention the affected files/functions.
       * **How:** Explain the mechanism of the fix.  What code was modified and 
what is the new behavior?
       * **Related Issues:**  Are there any related NuttX or NuttX Apps issues 
or PRs?  Provide links.
   
   * **Impact:**  The impact section is entirely placeholder text.  It needs to 
address *all* the points raised in the template (user impact, build impact, 
hardware impact, documentation impact, security impact, compatibility impact, 
and any other considerations).  Even if the answer is "NO", explicitly state it 
for each item.  Don't leave any ambiguity.
   
   * **Testing:**  The testing section is also placeholder text.  It needs to 
specify:
       * **Build Host Details:**  Provide the specifics of your build 
environment (OS, CPU architecture, compiler version, etc.).
       * **Target Details:**  Specify the target architecture, board, and 
configuration you used for testing.
       * **Logs:**  Critically, you *must* provide "before" and "after" logs 
demonstrating the issue and the fix.  This is essential for reviewers to 
understand the problem and verify the solution's effectiveness.
   
   
   **Example of how to improve the PR description:**
   
   ```markdown
   ## Summary
   
   This change fixes a memory allocation error that occurs when allocating 
memory for PCI devices on systems with PCI bridges.  The issue arises due to 
incorrect handling of the bridge's address space during memory allocation in 
the `drivers/pci/pci_alloc.c` file.  Specifically, the `pci_alloc_resource` 
function was not correctly calculating the available memory range when a bridge 
was present, leading to potential out-of-bounds memory access.  This change 
modifies the address range calculation in `pci_alloc_resource` to account for 
the bridge's address space, ensuring correct memory allocation.
   
   Related [NuttX Issue](https://github.com/apache/nuttx/issues/XXXX) -  (If 
applicable, link to the relevant issue)
   
   
   ## Impact
   
   * Is new feature added? NO
   * Is existing feature changed? YES. This fixes a bug in PCI memory 
allocation.
   * Impact on user: NO. Users should not experience any noticeable changes 
except that the system will no longer crash due to the memory allocation error.
   * Impact on build: NO.
   * Impact on hardware: YES.  This fix affects systems with PCI bridges. 
Systems without PCI bridges are not affected.
   * Impact on documentation: NO.
   * Impact on security: YES. The incorrect memory allocation could potentially 
lead to a security vulnerability. This fix mitigates that risk.
   * Impact on compatibility: NO. This fix should maintain backward 
compatibility.
   * Anything else to consider? NO.
   
   
   ## Testing
   
   I confirm that changes are verified on local setup and works as intended:
   * Build Host(s): Linux (Ubuntu 20.04), x86_64, GCC 9.4.0
   * Target(s): qemu-system-x86_64, pc-i440fx-2.11
   
   
   Testing logs before change:
   
   ```
   kernel: pci_alloc_resource: Failed to allocate memory for device 
0x1234:0x5678
   kernel: PANIC!!!
   ```
   
   Testing logs after change:
   ```
   kernel: PCI device 0x1234:0x5678 initialized successfully
   ```
   ```
   
   
   By providing this level of detail, reviewers can effectively assess the 
changes and ensure the PR meets the NuttX project's standards.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to