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

ace: mm: tlb: Ignore unmapping error in driver initalization #78612

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

softwarecki
Copy link
Collaborator

The sys_mm_drv_unmap_region_initial function is responsible for unmapping all unused virtual memory during tlb driver initialization. Most addresses will not have a mapped page. Ignore the error code indicating unmapped memory that will occur when trying to unmap.

Fix for issue detected in: #78479

gbernatxintel
gbernatxintel previously approved these changes Sep 18, 2024
tmleman
tmleman previously approved these changes Sep 18, 2024
dcpleung
dcpleung previously approved these changes Sep 18, 2024
@lyakh
Copy link
Collaborator

lyakh commented Sep 19, 2024

@softwarecki you need to re-add your now reverted 311ddf9 too, right? Without it this PR doesn't really work?

@softwarecki softwarecki changed the title ace: mm: tlb: Ignore unmappig error in driver initalization ace: mm: tlb: Ignore unmapping error in driver initalization Sep 20, 2024
@softwarecki
Copy link
Collaborator Author

Added previously reverted commit from #78227
Created PR in SOF to test this PR: thesofproject/sof#9504

@lyakh
Copy link
Collaborator

lyakh commented Sep 24, 2024

Added previously reverted commit from #78227 Created PR in SOF to test this PR: thesofproject/sof#9504

Thanks for adding a SOF PR. Since it has failed some tests, let me mark this PR "DNM" for now, feel free to remove the label once those failures are fixed

@lyakh lyakh added the DNM This PR should not be merged (Do Not Merge) label Sep 24, 2024
The sys_mm_drv_unmap_region_initial function is responsible for unmapping
all unused virtual memory during tlb driver initialization. Most addresses
will not have a mapped page. Ignore the error code indicating unmapped
memory that will occur when trying to unmap.

Signed-off-by: Adrian Warecki <[email protected]>
Before unmapping a memory page, the cache is flushed. If the given memory
page is not mapped, this operation ends with a cpu exception on the
ptl platform. Add check if tlb translation is active before flushing.

Signed-off-by: Adrian Warecki <[email protected]>
@softwarecki
Copy link
Collaborator Author

@lyakh Can we merge it?

@lyakh lyakh removed the DNM This PR should not be merged (Do Not Merge) label Oct 15, 2024
@lyakh
Copy link
Collaborator

lyakh commented Oct 15, 2024

@lyakh Can we merge it?

@softwarecki sure, sorry, I was on holiday, which was why I suggested to remove the label without me. I was a bit concerned about using a specific error code to identify a "soft failure" in sys_mm_drv_unmap_page_wflush(), but it's a static ACE-specific function, so shouldn't be a problem. BTW, we could use "ace" somewhere in the names of such static functions to make it more obvious. E.g. ace_mm_drv_unmap_page_wflush()

@nashif nashif merged commit 256ab0c into zephyrproject-rtos:main Oct 15, 2024
23 of 24 checks passed
@softwarecki
Copy link
Collaborator Author

softwarecki commented Oct 16, 2024

@lyakh This error code is well described in the sys_mm_drv_unmap_page function documentation.

/**
 * @brief Remove mapping for one page of the provided virtual address
 *
 * This unmaps one page from the virtual address space.
 *
 * When this completes, the relevant translation table entries will be
 * updated as if no mapping was ever made for that memory page. No previous
 * context needs to be preserved. This function must update mapping in
 * all active translation tables.
 *
 * Behavior when providing unaligned address is undefined, this
 * is assumed to be page aligned.
 *
 * Implementations must invalidate translation caching as necessary.
 *
 * @param virt Page-aligned virtual address to un-map
 *
 * @retval 0 if successful
 * @retval -EINVAL if invalid arguments are provided
 * @retval -EFAULT if virtual address is not mapped
 */
int sys_mm_drv_unmap_page(void *virt);

In this driver the sys_mm_drv_unmap_page function just call the sys_mm_drv_unmap_page_wflush so it return a appropriate error code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants