On Tue, May 20, 2025 at 01:30:01PM +0200, Magnus Kulke wrote:
> Create the MSHV virtual machine by opening a partition and issuing
> the necessary ioctl to initialize it. This sets up the basic VM
> structure and initial configuration used by MSHV to manage guest state.
>
> Signed-off-by: Magnus Kulke <magnusku...@linux.microsoft.com>
> ---
[...]
> diff --git a/accel/mshv/mshv-all.c b/accel/mshv/mshv-all.c
> index 63b0eca1fc..95f1008a48 100644
> --- a/accel/mshv/mshv-all.c
> +++ b/accel/mshv/mshv-all.c
> @@ -48,6 +48,178 @@ bool mshv_allowed;
>
> MshvState *mshv_state;
>
> +static int init_mshv(int *mshv_fd)
> +{
> + int fd = open("/dev/mshv", O_RDWR | O_CLOEXEC);
> + if (fd < 0) {
> + error_report("Failed to open /dev/mshv: %s", strerror(errno));
> + return -1;
> + }
> + *mshv_fd = fd;
> + return 0;
> +}
> +
> +/* freeze 1 to pause, 0 to resume */
> +static int set_time_freeze(int vm_fd, int freeze)
> +{
> + int ret;
> +
> + if (freeze != 0 && freeze != 1) {
> + error_report("Invalid time freeze value");
> + return -1;
> + }
> +
> + struct hv_input_set_partition_property in = {0};
> + in.property_code = HV_PARTITION_PROPERTY_TIME_FREEZE;
> + in.property_value = freeze;
> +
> + struct mshv_root_hvcall args = {0};
> + args.code = HVCALL_SET_PARTITION_PROPERTY;
> + args.in_sz = sizeof(in);
> + args.in_ptr = (uint64_t)∈
> +
> + ret = mshv_hvcall(vm_fd, &args);
> + if (ret < 0) {
> + error_report("Failed to set time freeze");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int pause_vm(int vm_fd)
I feel like the name of this function and its counter part is too broad.
Pausing the VM to me means not only freezing the time, but also making
sure VCPUs are no longer scheduled, all I/O are quiescent.
This is not blocking. Luckily we can always change the name later.
> +{
> + int ret;
> +
> + ret = set_time_freeze(vm_fd, 1);
> + if (ret < 0) {
> + error_report("Failed to pause partition: %s", strerror(errno));
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int resume_vm(int vm_fd)
> +{
[...]
> +
> +static int create_vm(int mshv_fd)
> +{
> + int vm_fd;
> +
> + int ret = create_partition(mshv_fd, &vm_fd);
> + if (ret < 0) {
> + close(mshv_fd);
No please don't close it here. This fd is not created by this function.
> + return -errno;
> + }
> +
> + ret = set_synthetic_proc_features(vm_fd);
> + if (ret < 0) {
> + return -errno;
> + }
> +
> + ret = initialize_vm(vm_fd);
> + if (ret < 0) {
> + return -1;
> + }
> +
> + ret = mshv_arch_post_init_vm(vm_fd);
> + if (ret < 0) {
> + return -1;
> + }
> +
> + /* Always create a frozen partition */
> + pause_vm(vm_fd);
> +
> + return vm_fd;
> +}
>
> static void mem_region_add(MemoryListener *listener,
> MemoryRegionSection *section)
> @@ -96,22 +268,54 @@ static void register_mshv_memory_listener(MshvState *s,
> MshvMemoryListener *mml,
> }
> }
> }
> +static void mshv_reset(void *param)
> +{
> + warn_report("mshv reset");
What's missing for this hook?
> +}
> +
> +int mshv_hvcall(int mshv_fd, const struct mshv_root_hvcall *args)
Please be consistent and change mshv_fd to vm_fd.
> +{
> + int ret = 0;
> +
> + ret = ioctl(mshv_fd, MSHV_ROOT_HVCALL, args);
> + if (ret < 0) {
> + error_report("Failed to perform hvcall: %s", strerror(errno));
> + return -1;
> + }
> + return ret;
> +}
>
>
> static int mshv_init(MachineState *ms)
> {
> MshvState *s;
> + int mshv_fd, ret;
> +
> s = MSHV_STATE(ms->accelerator);
>
> accel_blocker_init();
>
> s->vm = 0;
>
> + ret = init_mshv(&mshv_fd);
> + if (ret < 0) {
> + return -1;
> + }
> +
> + do {
> + int vm_fd = create_vm(mshv_fd);
> + s->vm = vm_fd;
> + } while (!s->vm);
> +
This loop doesn't make sense to me. The create_vm function doesn't
return 0 as "try again".
> + resume_vm(s->vm);
> +
mshv_fd is neither stashed into a state structure nor freed after this
point. Is it leaked?
Thanks,
Wei.
> s->nr_as = 1;
> s->as = g_new0(MshvAddressSpace, s->nr_as);
>
> mshv_state = s;
>
> + qemu_register_reset(mshv_reset, NULL);
> +
> register_mshv_memory_listener(s, &s->memory_listener,
> &address_space_memory,
> 0, "mshv-memory");
> memory_listener_register(&mshv_io_listener, &address_space_io);
