| Message ID | 20210422161130.652779-14-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i386: KVM: expand Hyper-V features early | expand |
On Thu, Apr 22, 2021 at 06:11:24PM +0200, Vitaly Kuznetsov wrote: > KVM_GET_SUPPORTED_HV_CPUID was made a system wide ioctl which can be called > prior to creating vCPUs and we are going to use that to expand Hyper-V cpu > features early. Use it when it is supported by KVM. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > target/i386/kvm/kvm.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index feec9f25ba12..5d08f3a39ef7 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -928,7 +928,8 @@ static struct { > }, > }; > > -static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max) > +static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max, > + bool do_sys_ioctl) > { > struct kvm_cpuid2 *cpuid; > int r, size; > @@ -937,7 +938,11 @@ static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max) > cpuid = g_malloc0(size); > cpuid->nent = max; > > - r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid); > + if (do_sys_ioctl) { > + r = kvm_ioctl(kvm_state, KVM_GET_SUPPORTED_HV_CPUID, cpuid); > + } else { > + r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid); > + } > if (r == 0 && cpuid->nent >= max) { > r = -E2BIG; > } > @@ -964,13 +969,17 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs) > /* 0x40000000..0x40000005, 0x4000000A, 0x40000080..0x40000080 leaves */ > int max = 10; > int i; > + bool do_sys_ioctl; > + > + do_sys_ioctl = > + kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID) > 0; > > /* > * When the buffer is too small, KVM_GET_SUPPORTED_HV_CPUID fails with > * -E2BIG, however, it doesn't report back the right size. Keep increasing > * it and re-trying until we succeed. > */ > - while ((cpuid = try_get_hv_cpuid(cs, max)) == NULL) { > + while ((cpuid = try_get_hv_cpuid(cs, max, do_sys_ioctl)) == NULL) { > max++; > } > > @@ -980,7 +989,7 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs) > * information early, just check for the capability and set the bit > * manually. > */ > - if (kvm_check_extension(cs->kvm_state, > + if (!do_sys_ioctl && kvm_check_extension(cs->kvm_state, > KVM_CAP_HYPERV_ENLIGHTENED_VMCS) > 0) { Oh, this conditional replaces the comment I suggested in patch 10/19. It makes it obvious that the hack can be deleted if we remove support for the VCPU ioctl. So, when exactly will we be able to delete the VCPU ioctl code and support only the system ioctl? Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > for (i = 0; i < cpuid->nent; i++) { > if (cpuid->entries[i].function == HV_CPUID_ENLIGHTMENT_INFO) { > -- > 2.30.2 >
Eduardo Habkost <ehabkost@redhat.com> writes: > On Thu, Apr 22, 2021 at 06:11:24PM +0200, Vitaly Kuznetsov wrote: >> KVM_GET_SUPPORTED_HV_CPUID was made a system wide ioctl which can be called >> prior to creating vCPUs and we are going to use that to expand Hyper-V cpu >> features early. Use it when it is supported by KVM. >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> --- >> target/i386/kvm/kvm.c | 17 +++++++++++++---- >> 1 file changed, 13 insertions(+), 4 deletions(-) >> >> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c >> index feec9f25ba12..5d08f3a39ef7 100644 >> --- a/target/i386/kvm/kvm.c >> +++ b/target/i386/kvm/kvm.c >> @@ -928,7 +928,8 @@ static struct { >> }, >> }; >> >> -static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max) >> +static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max, >> + bool do_sys_ioctl) >> { >> struct kvm_cpuid2 *cpuid; >> int r, size; >> @@ -937,7 +938,11 @@ static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max) >> cpuid = g_malloc0(size); >> cpuid->nent = max; >> >> - r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid); >> + if (do_sys_ioctl) { >> + r = kvm_ioctl(kvm_state, KVM_GET_SUPPORTED_HV_CPUID, cpuid); >> + } else { >> + r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid); >> + } >> if (r == 0 && cpuid->nent >= max) { >> r = -E2BIG; >> } >> @@ -964,13 +969,17 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs) >> /* 0x40000000..0x40000005, 0x4000000A, 0x40000080..0x40000080 leaves */ >> int max = 10; >> int i; >> + bool do_sys_ioctl; >> + >> + do_sys_ioctl = >> + kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID) > 0; >> >> /* >> * When the buffer is too small, KVM_GET_SUPPORTED_HV_CPUID fails with >> * -E2BIG, however, it doesn't report back the right size. Keep increasing >> * it and re-trying until we succeed. >> */ >> - while ((cpuid = try_get_hv_cpuid(cs, max)) == NULL) { >> + while ((cpuid = try_get_hv_cpuid(cs, max, do_sys_ioctl)) == NULL) { >> max++; >> } >> >> @@ -980,7 +989,7 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs) >> * information early, just check for the capability and set the bit >> * manually. >> */ >> - if (kvm_check_extension(cs->kvm_state, >> + if (!do_sys_ioctl && kvm_check_extension(cs->kvm_state, >> KVM_CAP_HYPERV_ENLIGHTENED_VMCS) > 0) { > > Oh, this conditional replaces the comment I suggested in patch > 10/19. It makes it obvious that the hack can be deleted if we > remove support for the VCPU ioctl. > > So, when exactly will we be able to delete the VCPU ioctl code > and support only the system ioctl? When QEMU drops support for kernels < 5.11? Note, current RHEL8 already supports system version so we're talking about upstream kernels/Ubuntu LTS/... I remember there was a list of supported kernels for QEMU somewhere but don't seem to be able to find it quickly, could you maybe point me in the right direction? > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > Thanks! >> for (i = 0; i < cpuid->nent; i++) { >> if (cpuid->entries[i].function == HV_CPUID_ENLIGHTMENT_INFO) { >> -- >> 2.30.2 >>
On Mon, May 24, 2021 at 02:08:26PM +0200, Vitaly Kuznetsov wrote: [...] > >> @@ -980,7 +989,7 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs) > >> * information early, just check for the capability and set the bit > >> * manually. > >> */ > >> - if (kvm_check_extension(cs->kvm_state, > >> + if (!do_sys_ioctl && kvm_check_extension(cs->kvm_state, > >> KVM_CAP_HYPERV_ENLIGHTENED_VMCS) > 0) { > > > > Oh, this conditional replaces the comment I suggested in patch > > 10/19. It makes it obvious that the hack can be deleted if we > > remove support for the VCPU ioctl. > > > > So, when exactly will we be able to delete the VCPU ioctl code > > and support only the system ioctl? > > When QEMU drops support for kernels < 5.11? Note, current RHEL8 already > supports system version so we're talking about upstream kernels/Ubuntu > LTS/... > > I remember there was a list of supported kernels for QEMU somewhere but > don't seem to be able to find it quickly, could you maybe point me in > the right direction? The KVM-specific kernel requirement is documented here: https://qemu-project.gitlab.io/qemu/system/target-i386.html?highlight=kvm#os-requirements I took a while to find it. Maybe we should have a more visible "runtime requirements" section in the docs, or it should be moved to the supported build platforms section. We have a clear policy on supported build platforms [https://qemu-project.gitlab.io/qemu/system/build-platforms.html], but not a clear policy for KVM kernel dependencies. There's a table with Python and GCC versions at [https://wiki.qemu.org/Supported_Build_Platforms]. Maybe it could include kernel version information as well.
On Wed, May 26, 2021 at 12:46:25PM -0400, Eduardo Habkost wrote: > On Mon, May 24, 2021 at 02:08:26PM +0200, Vitaly Kuznetsov wrote: > [...] > > >> @@ -980,7 +989,7 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs) > > >> * information early, just check for the capability and set the bit > > >> * manually. > > >> */ > > >> - if (kvm_check_extension(cs->kvm_state, > > >> + if (!do_sys_ioctl && kvm_check_extension(cs->kvm_state, > > >> KVM_CAP_HYPERV_ENLIGHTENED_VMCS) > 0) { > > > > > > Oh, this conditional replaces the comment I suggested in patch > > > 10/19. It makes it obvious that the hack can be deleted if we > > > remove support for the VCPU ioctl. > > > > > > So, when exactly will we be able to delete the VCPU ioctl code > > > and support only the system ioctl? > > > > When QEMU drops support for kernels < 5.11? Note, current RHEL8 already > > supports system version so we're talking about upstream kernels/Ubuntu > > LTS/... > > > > I remember there was a list of supported kernels for QEMU somewhere but > > don't seem to be able to find it quickly, could you maybe point me in > > the right direction? > > The KVM-specific kernel requirement is documented here: > https://qemu-project.gitlab.io/qemu/system/target-i386.html?highlight=kvm#os-requirements > > I took a while to find it. Maybe we should have a more visible > "runtime requirements" section in the docs, or it should be > moved to the supported build platforms section. > > We have a clear policy on supported build platforms > [https://qemu-project.gitlab.io/qemu/system/build-platforms.html], > but not a clear policy for KVM kernel dependencies. While it says "supported build platforms", that was implicitly assumed to also refer to "runtime platforms". We should just rename it to "supported-platforms.html" to make it more obvious. Thus, the minimum KVM kernel version we need follows the same rules. Look at whatever is the oldest kernel across the distros we target. Regards, Daniel
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index feec9f25ba12..5d08f3a39ef7 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -928,7 +928,8 @@ static struct { }, }; -static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max) +static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max, + bool do_sys_ioctl) { struct kvm_cpuid2 *cpuid; int r, size; @@ -937,7 +938,11 @@ static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max) cpuid = g_malloc0(size); cpuid->nent = max; - r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid); + if (do_sys_ioctl) { + r = kvm_ioctl(kvm_state, KVM_GET_SUPPORTED_HV_CPUID, cpuid); + } else { + r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid); + } if (r == 0 && cpuid->nent >= max) { r = -E2BIG; } @@ -964,13 +969,17 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs) /* 0x40000000..0x40000005, 0x4000000A, 0x40000080..0x40000080 leaves */ int max = 10; int i; + bool do_sys_ioctl; + + do_sys_ioctl = + kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID) > 0; /* * When the buffer is too small, KVM_GET_SUPPORTED_HV_CPUID fails with * -E2BIG, however, it doesn't report back the right size. Keep increasing * it and re-trying until we succeed. */ - while ((cpuid = try_get_hv_cpuid(cs, max)) == NULL) { + while ((cpuid = try_get_hv_cpuid(cs, max, do_sys_ioctl)) == NULL) { max++; } @@ -980,7 +989,7 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs) * information early, just check for the capability and set the bit * manually. */ - if (kvm_check_extension(cs->kvm_state, + if (!do_sys_ioctl && kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_ENLIGHTENED_VMCS) > 0) { for (i = 0; i < cpuid->nent; i++) { if (cpuid->entries[i].function == HV_CPUID_ENLIGHTMENT_INFO) {
KVM_GET_SUPPORTED_HV_CPUID was made a system wide ioctl which can be called prior to creating vCPUs and we are going to use that to expand Hyper-V cpu features early. Use it when it is supported by KVM. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- target/i386/kvm/kvm.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)