Fix NetBSD build and add support for HAX_VCPU_IOCTL_SET_CPUID #281
Conversation
| @@ -58,6 +58,40 @@ struct cdevsw hax_vcpu_cdevsw = { | |||
| .d_flag = D_OTHER | D_MPSAFE | |||
| }; | |||
|
|
|||
| #define load_user_data(dest, src, body_len, body_max, arg_t, body_t) \ | |||
There was a problem hiding this comment.
Why not create a function? My ioctl will use it too.
There was a problem hiding this comment.
A function is better.. but I want to avoid divergence with other kernels.
There was a problem hiding this comment.
How does a function lead to divergence?
There was a problem hiding this comment.
Linux + Darwin use macro here.
There was a problem hiding this comment.
Why not create a function?
This pair of function-like macros are designed for all interfaces with variable-length arguments. As there are types, struct members in the macro parameters, it is hard to define a real function to handle these cases. But I agree to bring the definition to a common place, such as hax.h, instead of duplicating the codes here.
Linux + Darwin use macro here.
As for the minor difference between NetBSD/Darwin and Linux, should we merge them into one macro by platform switch?
There was a problem hiding this comment.
It's better to keep the macro in private files of kernels as there are OS-specific differences.
| cpuid_t hax_supported; // Hypervisor supported features | ||
| bool initialized; | ||
| uint32_t data[CPUID_CACHE_SIZE]; // Host cached features | ||
| hax_cpuid_t host_supported; // Physical CPU supported features |
There was a problem hiding this comment.
Is cpuid_t present in NetBSD headers and renaming is to resolve intersection issues?
There was a problem hiding this comment.
According to HAXM current naming rules, the types leading by hax_ are used for external IOCTLs, while types ending with _t are used for internal storage. Most recent codes follow this rule. Could you consider to rename the type present in NetBSD rather than replacing in the common places?
There was a problem hiding this comment.
There is no option to rename this. _t is a reserved namespace for POSIX/implementation and third party code must not reuse it for internal purposes.
There was a problem hiding this comment.
There were also several other namespace conflicts in the past, such as vaddr_t. They were patched accordingly.
There was a problem hiding this comment.
| cpuid_t hax_supported; // Hypervisor supported features | ||
| bool initialized; | ||
| uint32_t data[CPUID_CACHE_SIZE]; // Host cached features | ||
| hax_cpuid_t host_supported; // Physical CPU supported features |
There was a problem hiding this comment.
According to HAXM current naming rules, the types leading by hax_ are used for external IOCTLs, while types ending with _t are used for internal storage. Most recent codes follow this rule. Could you consider to rename the type present in NetBSD rather than replacing in the common places?
| @@ -58,6 +58,40 @@ struct cdevsw hax_vcpu_cdevsw = { | |||
| .d_flag = D_OTHER | D_MPSAFE | |||
| }; | |||
|
|
|||
| #define load_user_data(dest, src, body_len, body_max, arg_t, body_t) \ | |||
There was a problem hiding this comment.
Why not create a function?
This pair of function-like macros are designed for all interfaces with variable-length arguments. As there are types, struct members in the macro parameters, it is hard to define a real function to handle these cases. But I agree to bring the definition to a common place, such as hax.h, instead of duplicating the codes here.
Linux + Darwin use macro here.
As for the minor difference between NetBSD/Darwin and Linux, should we merge them into one macro by platform switch?
Signed-off-by: Kamil Rytarowski <n54@gmx.com>
Signed-off-by: Kamil Rytarowski <n54@gmx.com>
|
Done. |
|
Thanks for your update. Your pull request is ready to merge. |
|
@krytarowski, HAXM has introduced the new API HAX_VCPU_IOCTL_GET_CPUID, would you implement it for NetBSD before the next release? Thanks. |
No description provided.