Upstream - #94
Conversation
Qualcomm AI ReviewClick to expand Code ReviewReviewed Commits: a51fbb3, 39d3bf7
Reduce GSL_CSHM_MAGIC_WORD from 24-bit (0x47534D) to 16-bit (0x534D), Also rename GSL_MAGIC_WORD_MASK to GSL_CSHM_MAGIC_WORD_MASK for Signed-off-by: ffrancis ffrancis@qti.qualcomm.com
Update spf headers to support force recognition change Signed-off-by: ffrancis ffrancis@qti.qualcomm.com Pull Request OverviewThis PR modifies shared memory management constants and adds support for batch drain completion notifications in the detection engine API. Files Changed Summary
Key Changes
Critical Issues Identified
[FUNCTIONALITY] High Severity - Reduced Memory ID Address Space May Cause CollisionsThe changes to Impact Analysis:
Potential Issues:
Recommendation: Verify that this change is intentional and that all dependent code has been updated. Consider whether the reduced address space is sufficient for the system's requirements. If this is a deliberate optimization, ensure comprehensive testing across all platforms. Fixed Code Snippet: // If the reduction is unintentional, revert to original values:
#define GSL_CSHM_MAGIC_WORD 0x47534D
#define GSL_CSHM_MAGIC_WORD_MASK 0xFFFFFF
#define GSL_CSHM_IDX_SHIFT 24
// OR if intentional, add validation to ensure no overflow:
static inline gsl_mem_id_t to_gsl_mem_id(uint8_t index)
{
// Ensure we're not losing bits in the encoding
uint32_t encoded = (GSL_CSHM_MAGIC_WORD | (index << GSL_CSHM_IDX_SHIFT));
return (gsl_mem_id_t)((uintptr_t)encoded);
}[FUNCTIONALITY] High Severity - Backward Compatibility Risk with Memory ID Encoding ChangesThe modifications to the memory ID encoding scheme pose a significant backward compatibility risk. Any existing memory allocations or persistent memory IDs stored using the old encoding (24-bit magic word, 24-bit shift) will not be correctly validated or decoded with the new scheme (16-bit magic word, 16-bit shift). Specific Concerns:
Impact: This could cause runtime failures, memory access violations, or data corruption if the system attempts to use pre-existing memory IDs. Recommendation:
Fixed Code Snippet: // Add backward compatibility support
#define GSL_CSHM_MAGIC_WORD_V1 0x47534D // Legacy magic word
#define GSL_CSHM_MAGIC_WORD_V2 0x534D // New magic word
#define GSL_CSHM_IDX_SHIFT_V1 24 // Legacy shift
#define GSL_CSHM_IDX_SHIFT_V2 16 // New shift
static inline bool_t is_valid_gsl_mem_id(gsl_mem_id_t mem_id)
{
uint32_t magic = (uintptr_t)mem_id & 0xFFFFFF;
// Check both old and new magic words during transition
if (magic == GSL_CSHM_MAGIC_WORD_V1 ||
(magic & 0xFFFF) == GSL_CSHM_MAGIC_WORD_V2) {
return true;
}
return false;
}
static inline uint8_t to_gsl_cshm_indx(gsl_mem_id_t mem_id)
{
uint32_t magic = (uintptr_t)mem_id & 0xFFFFFF;
// Detect version and use appropriate shift
if (magic == GSL_CSHM_MAGIC_WORD_V1) {
return ((uintptr_t)mem_id >> GSL_CSHM_IDX_SHIFT_V1) & 0xff;
}
return ((uintptr_t)mem_id >> GSL_CSHM_IDX_SHIFT_V2) & 0xff;
} |
![qualcomm-ai-code-review-assistant[bot]](https://avatars.githubusercontent.com/in/4128059?s=60&v=4){kind=small}
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Qualcomm AI ReviewClick to expand Code ReviewReviewed commit: 4de3d56 "args: update header for force recognition in batching mode Update spf headers to support force recognition change Signed-off-by: ffrancis ffrancis@qti.qualcomm.com" Pull Request OverviewThis PR introduces support for batch drain completion notifications in the detection engine API, specifically for Force Recognition in batching mode. The changes add a new event type ( Files Changed Summary
Key Changes
Critical IssuesNo critical issues identified. The changes are primarily additive and follow existing patterns in the codebase. |
Reduce GSL_CSHM_MAGIC_WORD from 24-bit (0x47534D) to 16-bit (0x534D), update the mask macro to GSL_CSHM_MAGIC_WORD_MASK (0xFFFF), and adjust GSL_CSHM_IDX_SHIFT from 24 to 16 to match the new encoding width. Also rename GSL_MAGIC_WORD_MASK to GSL_CSHM_MAGIC_WORD_MASK for consistency with the GSL_CSHM_ naming convention, and fix missing newline at end of file. Signed-off-by: ffrancis <ffrancis@qti.qualcomm.com>
Update spf headers to support force recognition change Signed-off-by: ffrancis <ffrancis@qti.qualcomm.com>
Qualcomm AI ReviewClick to expand Code ReviewReviewed commit: 0307aee "args: update header for force recognition in batching mode Update spf headers to support force recognition change Signed-off-by: ffrancis ffrancis@qti.qualcomm.com" Pull Request OverviewThis PR introduces support for batch drain completion notifications in the detection engine API, specifically for Force Recognition in batching mode. The changes add a new event type ( Files Changed Summary
Key Changes
Critical IssuesNo critical issues identified. The changes are primarily additive and follow existing patterns in the codebase. One medium-severity issue related to validation logic has been identified. [FUNCTIONALITY] Missing validation for drain_complete field - Medium SeverityThe
Recommendation: Either:
Fixed Code Snippet: /* Option 1: Remove redundant field */
struct batch_drain_complete_info_t
{
uint32_t status;
/**< @h2xmle_description {Status of batch drain operation. 0=error, 1=success}
@h2xmle_range {0..1}
@h2xmle_default {1} */
}; |
5 participants
commits included:
gsl: cshm: Update magic word and mask to 16-bit encoding
args: update header for force recognition in batching mode