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

riscv: fix read/write virtual memory across page boundaries #1149

Merged
merged 3 commits into from
Oct 28, 2024

Conversation

zqb-all
Copy link
Contributor

@zqb-all zqb-all commented Oct 12, 2024

When read/write virtual addresses cross page boundaries, the physical addresses are not necessarily contiguous and need to call virt2phys again.

@zqb-all zqb-all force-pushed the read-write-cross-page branch from 179aab2 to d9c2ddb Compare October 12, 2024 13:28
Copy link
Collaborator

@TommyMurphyTM1234 TommyMurphyTM1234 left a comment

Choose a reason for hiding this comment

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

Small typos

src/target/riscv/riscv.c Outdated Show resolved Hide resolved
src/target/riscv/riscv.c Outdated Show resolved Hide resolved
@zqb-all zqb-all force-pushed the read-write-cross-page branch 2 times, most recently from 866a4eb to 788e4f9 Compare October 12, 2024 13:39
Copy link
Collaborator

@en-sc en-sc left a comment

Choose a reason for hiding this comment

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

Thanks for the patch!

I have only one significant comment.

src/target/riscv/riscv.c Outdated Show resolved Hide resolved
src/target/riscv/riscv.c Outdated Show resolved Hide resolved
@zqb-all zqb-all force-pushed the read-write-cross-page branch from 788e4f9 to be0554a Compare October 12, 2024 23:14
@zqb-all zqb-all requested a review from en-sc October 12, 2024 23:17
@zqb-all zqb-all force-pushed the read-write-cross-page branch 3 times, most recently from 2a4e2fe to 292873e Compare October 13, 2024 00:26
src/target/riscv/riscv.c Outdated Show resolved Hide resolved
@zqb-all zqb-all force-pushed the read-write-cross-page branch 2 times, most recently from 519ea7a to a3c3b09 Compare October 13, 2024 14:52
src/target/riscv/riscv.c Outdated Show resolved Hide resolved
en-sc
en-sc previously approved these changes Oct 15, 2024
Copy link
Collaborator

@en-sc en-sc left a comment

Choose a reason for hiding this comment

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

LGTM

@en-sc
Copy link
Collaborator

en-sc commented Oct 15, 2024

@JanMatCodasip, @MarekVCodasip, please, take a look.

Copy link
Collaborator

@JanMatCodasip JanMatCodasip left a comment

Choose a reason for hiding this comment

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

@zqb-all Thank you for the contribution. In principle, the change looks fine to me.

I have just several code-related suggestions (code style, log prints, TODO comments and similar). Please if you can look at them.

src/target/riscv/riscv.c Outdated Show resolved Hide resolved
src/target/riscv/riscv.c Show resolved Hide resolved
src/target/riscv/riscv.c Outdated Show resolved Hide resolved
src/target/riscv/riscv.c Outdated Show resolved Hide resolved
src/target/riscv/riscv.c Outdated Show resolved Hide resolved
src/target/riscv/riscv.c Outdated Show resolved Hide resolved
@zqb-all zqb-all force-pushed the read-write-cross-page branch 2 times, most recently from e62f4f4 to ac5f95d Compare October 24, 2024 12:33
Copy link
Collaborator

@JanMatCodasip JanMatCodasip left a comment

Choose a reason for hiding this comment

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

Thank you for addressing the comments so far.

I have two last small suggestions, otherwise it looks good.

src/target/riscv/riscv.c Outdated Show resolved Hide resolved
src/target/riscv/riscv.c Outdated Show resolved Hide resolved
When read/write virtual addresses cross page boundaries,
the physical addresses are not necessarily contiguous and
need to call virt2phys again.
When mmu is disabled, simply call the physical read/write function
@zqb-all zqb-all force-pushed the read-write-cross-page branch from ac5f95d to 92d7a57 Compare October 24, 2024 15:49
Copy link
Collaborator

@JanMatCodasip JanMatCodasip left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

@en-sc en-sc merged commit 9ff272e into riscv-collab:riscv Oct 28, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants