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

add uc_mem_read_virtual #2121

Draft
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

PhilippTakacs
Copy link
Contributor

New api to read from a vaddr. When using the MMU it's useful to direct read from virtual addresses.

Needs some tests.

I'm not sure about a write_virtual() function. Could also be useful, but is a lot more complex to implement, because of snapshots.

A virtual to physical translate function would be also nice, but currently the tlb_fill() function has no probe argument.

@wtdcode
Copy link
Member

wtdcode commented Feb 26, 2025

I have dreamed of this API for quite a while because uc_mem_read/write only deals with physical addresses and super inconvenient with MMU. Thanks for prototyping!

It would be cool to even have translation API as suggested, but many QEMU MMU fill implementation has serious side effects.

Why does "write_vritual" not work with snapshots? What's the current uc_mem_write behavior? Maybe just keep the same behavior.

Lastly please target dev branch instead (for 2.2.0), the master branch is preparing for 2.1.3 and this change can't go into 2.1.3 obviously =).

@PhilippTakacs PhilippTakacs changed the base branch from master to dev February 26, 2025 09:02
@PhilippTakacs
Copy link
Contributor Author

Lastly please target dev

Sorry my bad.

Why does "write_vritual" not work with snapshots?

It would work, it's just more code and maybe duplicated code, because of the snapshot handling. With a simple virtual to physical API it would be more simple.

It would be cool to even have translation API as suggested, but many QEMU MMU fill implementation has serious side effects.

In the current QEMU version is tlb_fill() replaced by tlb_fill_align(). This function has a probe parameter, which would be the key for a virtual to physical translation API.

New api to read from a vaddr. When using the MMU it's useful to direct
read from virtual addresses.
@wtdcode
Copy link
Member

wtdcode commented Feb 26, 2025

CI is failing at this moment and I lack time to fix it. Will do later this week.

@PhilippTakacs
Copy link
Contributor Author

CI is failing at this moment

There were also some bugs in my changes which causes the CI to fail.

but many QEMU MMU fill implementation has serious side effects

I have added a testcase (on x86) and read the MMU implementations to verify this would work. As far as I see it works on every MMU expect for SPARC. Would it be OK to just document this issue?

About the write: Would it be OK to just have a read implementation at the moment?

@PhilippTakacs
Copy link
Contributor Author

What's the current uc_mem_write behavior? Maybe just keep the same behavior.

Currently uc_mem_write does COW when memory snapshots are enabled. I would also expect this from a uc_mem_write_virtual function.

The problem is: This is quite complicated code and with the given tlb_vaddr_to_host it would be more complicated to implement for a uc_mem_write_virtual function. Given the fact that I have two times missed some cases in the uc_mem_write function, I would try to avoid implement this without a simple way to translate virtual to physical addresses.

@wtdcode
Copy link
Member

wtdcode commented Feb 27, 2025

I just revisited the tlb_vaddr_to_host and noticed the probe parameter, which should avoid the side effects. Sorry I forget the functionality here yesterday without further checking the code. I also saw documents in SPARC and that's okay we can document it and return something like UC_ERR_ARG.

Btw the tlb_fill function indeed has the probe argument, I assume we can expose the translation API directly? Maybe just expose tlb_vaddr_to_host? In this case, we can avoid code duplication by using tlb_vaddr_to_host to convert vaddr to phyaddr and use uc_mem_read/write to do memory operations.

typedef struct CPUClass {
    // ...
    bool (*tlb_fill)(CPUState *cpu, vaddr address, int size,
                     MMUAccessType access_type, int mmu_idx,
                     bool probe, uintptr_t retaddr);
    // ...
}

The sparc mmu currently doesn't support probe mode which is used by
uc_mem_read_virtual. So for now return UC_ERR_ARG when the sparc mmu
is used.
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.

2 participants