Fix vhost xen mmio queue reset cleanup #16
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Move macros derived from the VIRTIO specification to a shared include. This change allows the VIRTIO standard definitions to be referenced from outside the driver implementation. The following definitions have also been added: - VIRTIO_F_ANY_LAYOUT - VIRTIO_F_VERSION_1 - VIRTIO_F_ACCESS_PLATFORM - VIRTIO_RING_F_INDIRECT_DESC - VIRTIO_RING_F_EVENT_IDX - VIRTQ_AVAIL_F_NO_INTERRUPT - VIRTQ_USED_F_NO_NOTIFY Signed-off-by: TOKITA Hiroshi <[email protected]>
This hypercall used to obtain version of xen hypervisor Signed-off-by: Dmytro Semenets <[email protected]> Acked-by: Volodymyr Babchuk <[email protected]> Acked-by: Mykola Kvach <[email protected]>
This hypercall can be used get some information about physical machine and running guests: - sysctl hypercall "xen_sysctl_getphysinfo" allows read information about physical machine: number CPUs, memory sizes, hardware capabilities, etc. - sysctl hypercall "xen_sysctl_getdomaininfolist" returns array of domain info structures that provide information about particular domain(s). - sysctl hypercall "get_cpu_info" provides information about physical CPU(s). Signed-off-by: Dmytro Semenets <[email protected]> Acked-by: Volodymyr Babchuk <[email protected]> Acked-by: Mykola Kvach <[email protected]>
domctl hypercall "getvcpu" provides information about domain's virtual CPU(s). Signed-off-by: Dmytro Semenets <[email protected]> Acked-by: Volodymyr Babchuk <[email protected]> Acked-by: Mykola Kvach <[email protected]>
The size is passed in bytes, not in megabytes. So rename the parameter to avoid confusion. Signed-off-by: Mykyta Poturai <[email protected]> Acked-by: Dmytro Firsov <[email protected]>
Document all of the public functions in the domctl API with doxygen Signed-off-by: Mykyta Poturai <[email protected]> Acked-by: Dmytro Firsov <[email protected]>
Document all of the public functions in the sysctl API with doxygen Signed-off-by: Mykyta Poturai <[email protected]> Acked-by: Dmytro Firsov <[email protected]>
If 0 is passed as domain id to the Xen createdomain hypercall, it will allocate a new domain id and return it via the domctl structure. Allow callers to access this new domain id via a pointer arg. This will allow to create domains without explicitly specifying the domain id for them. Signed-off-by: Mykyta Poturai <[email protected]> Acked-by: Mykola Kvach <[email protected]> Acked-by: Dmytro Firsov <[email protected]> Reviewed-by: Grygorii Strashko <[email protected]> (cherry picked from commit eb0ed21 zephyr-v3.6.0-xt)
Xen event channel notification may fail during hypervisor handling, so it will be good for user to have a possibility to check results. Previous implementation ignored hypervisor return code, now it will be passed to caller. Signed-off-by: Dmytro Firsov <[email protected]>
Import `zephyr-xenlib` module. That contains xen public headers. Signed-off-by: TOKITA Hiroshi <[email protected]>
Currently, the MIT-licensed Xen public headers have been incorporated under the `include/zephyr/xen/public` directory. We will migrate these files to zephyr-xenlib and use them. The recent policy is not to include sources other than Apache-2 in the main tree, so it is the preferred method. Signed-off-by: TOKITA Hiroshi <[email protected]>
Replace `gnttab_get_page()`/`gnttab_put_pages(addr)` with `gnttab_get_pages(npages)`/`gnttab_put_pages(addr, npages)` for supporting multi-page operation. Note: This is a breaking change, update callers accordingly. Signed-off-by: TOKITA Hiroshi <[email protected]>
Add XENMEM_acquire_resource wrapper to map device model related buffers.
This is required for communication with the ioreq server.
- memory
- xendom_acquire_resource:
HYPERVISOR_memory_op(XENMEM_acquire_resource)
Signed-off-by: TOKITA Hiroshi <[email protected]>
Add SCHEDOP_poll wrapper used by Xen backends to wait on event
channels efficiently.
- sched
- sched_poll:
HYPERVISOR_sched_op(SCHEDOP_poll)
Signed-off-by: TOKITA Hiroshi <[email protected]>
Import `zephyr-xenlib` module. That contains xen public headers. Signed-off-by: TOKITA Hiroshi <[email protected]>
Add wrappers for following XEN_DMOP_* hypercalls.
These enables Xen device model control path:
dm_op provides operations to create/manage the ioreq server
so guest MMIO accesses are trapped and handled by the hypervisor.
These are guarded by CONFIG_XEN_DMOP.
- dmop
- dmop_create_ioreq_server
XEN_DMOP_create_ioreq_server
- dmop_map_io_range_to_ioreq_server
XEN_DMOP_map_io_range_to_ioreq_server
- dmop_set_ioreq_server_state
XEN_DMOP_set_ioreq_server_state
- dmop_nr_vcpus
XEN_DMOP_nr_vcpus
- dmop_set_irq_level:
XEN_DMOP_set_irq_level
Signed-off-by: TOKITA Hiroshi <[email protected]>
Add HYPERVISOR_dm_op hypercall implementation for ARM64 architecture. This hypercall is required for device model operations in Xen virtualization environment. Signed-off-by: TOKITA Hiroshi <[email protected]>
This introduces the vhost driver framework, providing standard APIs for VIRTIO backend implementations in Zephyr. Includes vringh utility for host-side VIRTIO ring processing based on Linux kernel implementation. Signed-off-by: TOKITA Hiroshi <[email protected]>
Implements VirtIO backend over Xen MMIO interface Signed-off-by: TOKITA Hiroshi <[email protected]>
Add sample application demonstrating vhost driver usage for VIRTIO backend implementations. Includes basic setup and configuration examples for Xen MMIO VirtIO backend. Signed-off-by: TOKITA Hiroshi <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
drivers/vhost/vhost_xen_mmio.c
Outdated
Comment on lines
361
to
373
| if (chunk->map && chunk->count > 0) { | ||
| struct mapped_pages *map = chunk->map; | ||
| const size_t count = chunk->count; | ||
|
|
||
| chunk->releasing = true; | ||
| k_spin_unlock(&vq_ctx->lock, key); | ||
|
|
||
| free_pages_array(map, count); | ||
| k_free(map); | ||
|
|
||
| key = k_spin_lock(&vq_ctx->lock); | ||
| chunk->map = NULL; | ||
| chunk->count = 0; |
There was a problem hiding this comment.
[P1] Clear chunk pointer before dropping queue lock
The new reset logic frees chunk->map after releasing vq_ctx->lock but only nulls chunk->map and chunk->count after reacquiring it. During that unlocked window the struct still exposes the old pointer, yet the memory is being freed. Callers such as init_pages_chunks() use chunk->map without holding vq_ctx->lock or checking chunk->releasing and may run concurrently with a reset (e.g. when the device is reset while the frontend is still preparing I/O). In that case they can see a non‑NULL chunk->map and attempt to free or reuse it while reset_queue() is freeing the same memory, leading to double free/use‑after‑free. Saving the pointer locally but clearing the struct fields before dropping the lock would avoid the race while still allowing the cleanup to run outside the lock.
Useful? React with 👍 / 👎.
soburi
force-pushed
the
codex/modify-reset_queue-to-improve-cleanup-logic
branch
from
254156d to
9a9525f
Compare
soburi
force-pushed
the
xen-virtio-backend
branch
from
de7c012 to
3641e2d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68d1416d790483228a0160b9876a1159