Skip to content
This repository has been archived by the owner on Jan 28, 2023. It is now read-only.

Add IOCTL to support haxm max vcpu value query #287

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bowen218
Copy link

@bowen218 bowen218 commented Apr 8, 2020

This commit tried to add ioctl HAX_IOCTL_CAP_MAX_VCPU that will return
the HAXM max vcpu support. Also, the switch case for hax device and vm
device add return -ENOSYS in the default case. Moreover, the max HAXM
supported vcpu value is updated from 16 to 64. Previously, HAXM doesn't
support IOCTL that would return the max vcpu value.

This issue is resolved by adding a new IOCTL HAX_IOCTL_CAP_MAX_VCPU that
will simply return HAX_MAX_VCPUS. (IOCTL naming credit to KVM)

This commit results in if QEMU calls this IOCTL, it will get the max
vcpu value HAXM driver supported and can then compare it with the QEMU
max value and smp value to determine whether the smp value is valid.

Signed-off-by: WangBowen bowen.wang@intel.com

@bowen218 bowen218 force-pushed the vcpu-ioctl branch 2 times, most recently from 47e6f4d to a92e878 Compare April 8, 2020 01:16
@krytarowski
Copy link
Contributor

The NetBSD part looks good, however I would keep HAX_MAX_VCPUS at level 16 for this OS. It's not urgent to switch it to a higher number and I would like to write a proper support for emulated devfs (haxmvfs) that will create dynamically the /dev nodes.

@bowen218 bowen218 force-pushed the vcpu-ioctl branch 2 times, most recently from ddbf30a to 6b4cfb0 Compare April 8, 2020 01:40
@bowen218
Copy link
Author

bowen218 commented Apr 8, 2020

The NetBSD part looks good, however I would keep HAX_MAX_VCPUS at level 16 for this OS. It's not urgent to switch it to a higher number and I would like to write a proper support for emulated devfs (haxmvfs) that will create dynamically the /dev nodes.

Thanks for you review! I have updated the code, please take a look~

@krytarowski
Copy link
Contributor

Looks good to me, maybe we could update the comment // TODO: Handle 64 VMs to reflect the new reality.

@bowen218
Copy link
Author

bowen218 commented Apr 8, 2020

Looks good to me, maybe we could update the comment // TODO: Handle 64 VMs to reflect the new reality.

Do you mean to add a comment // TODO: Handle 64 VCPUS ?

@krytarowski
Copy link
Contributor

Yes. I would rephrase the existing comment to: // TODO: Handle 64 VMs and 64 VCPUs.

@bowen218
Copy link
Author

bowen218 commented Apr 8, 2020

Yes. I would rephrase the existing comment to: // TODO: Handle 64 VMs and 64 VCPUs.

Thanks for your suggestion! I have updated the code~

@@ -714,6 +715,16 @@ NTSTATUS HaxDeviceControl(PDEVICE_OBJECT DeviceObject,
infret = sizeof(uint32_t);
ret = STATUS_SUCCESS;
break;
case HAX_IOCTL_CAP_MAX_VCPU:
if (outBufLength < sizeof(uint32_t)) {
Irp->IoStatus.Information = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Irp->IoStatus.Information is changed after done:
done:
Irp->IoStatus.Status = ret;
Irp->IoStatus.Information = infret;
infret is zero after init, so this line (Irp->IoStatus.Information = 0;) is not needed.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your suggestion! I have updated the code~

This commit tried to add ioctl HAX_IOCTL_CAP_MAX_VCPU that will return
the HAXM max vcpu support. Also, the switch case for Linux hax and vm
device add return -ENOSYS in the default case. Moreover, the max HAXM
vcpu value is updated from 16 to 64 (netbsd still 16). Previously, HAXM
doesn't support IOCTL that would return the max vcpu value.

This issue is resolved by adding a new IOCTL HAX_IOCTL_CAP_MAX_VCPU that
will simply return HAX_MAX_VCPUS. (IOCTL naming credit to KVM)

This commit results in if QEMU calls this IOCTL, it will get the max
vcpu value HAXM driver supported and can then compare it with the QEMU
max value and smp value to determine whether the smp value is valid.

Signed-off-by: WangBowen <bowen.wang@intel.com>
Copy link
Contributor

@coxuintel coxuintel left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@nevilad
Copy link
Contributor

nevilad commented Apr 10, 2020

How this feature will be used, is there any patch for qemu?

@coxuintel coxuintel linked an issue Apr 13, 2020 that may be closed by this pull request
@bowen218
Copy link
Author

How this feature will be used, is there any patch for qemu?

Basically, this feature will be used for QEMU to obtain the maximum vcpu HAXM supported when creating the vm to check whether the requested vcpu number is valid. Also, as PR#255 supports more host cpus, we proceed to support more vcpus. (deals with Issue#195. And there will be a qemu patch (already in review process).

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

Successfully merging this pull request may close these issues.

Support more than 16 vCPUs
4 participants