* [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
@ 2020-04-27 14:17 ` Marc Zyngier
0 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2020-04-27 14:17 UTC (permalink / raw)
To: kvmarm, kvm
Cc: James Morse, Julien Thierry, Suzuki K Poulose, Will Deacon,
Andre Przywara, Ard Biesheuvel
On arm64, the maximum number of vcpus is constrained by the type
of interrupt controller that has been selected (GICv2 imposes a
limit of 8 vcpus, while GICv3 currently has a limit of 512).
It is thus important to request this limit on the VM file descriptor
rather than on the one that corresponds to /dev/kvm, as the latter
is likely to return something that doesn't take the constraints into
account.
Reported-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
kvm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kvm.c b/kvm.c
index e327541..3d5173d 100644
--- a/kvm.c
+++ b/kvm.c
@@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
{
int ret;
- ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
+ ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
if (ret <= 0)
/*
* api.txt states that if KVM_CAP_NR_VCPUS does not exist,
@@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
{
int ret;
- ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
+ ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
if (ret <= 0)
ret = kvm__recommended_cpus(kvm);
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
2020-04-27 14:17 ` Marc Zyngier
@ 2020-04-27 14:44 ` Alexandru Elisei
-1 siblings, 0 replies; 14+ messages in thread
From: Alexandru Elisei @ 2020-04-27 14:44 UTC (permalink / raw)
To: Marc Zyngier, kvmarm, kvm; +Cc: Andre Przywara, Will Deacon, Ard Biesheuvel
Hi,
On 4/27/20 3:17 PM, Marc Zyngier wrote:
> On arm64, the maximum number of vcpus is constrained by the type
> of interrupt controller that has been selected (GICv2 imposes a
> limit of 8 vcpus, while GICv3 currently has a limit of 512).
>
> It is thus important to request this limit on the VM file descriptor
> rather than on the one that corresponds to /dev/kvm, as the latter
> is likely to return something that doesn't take the constraints into
> account.
>
> Reported-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> kvm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kvm.c b/kvm.c
> index e327541..3d5173d 100644
> --- a/kvm.c
> +++ b/kvm.c
> @@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
> {
> int ret;
>
> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> if (ret <= 0)
> /*
> * api.txt states that if KVM_CAP_NR_VCPUS does not exist,
> @@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
> {
> int ret;
>
> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> if (ret <= 0)
> ret = kvm__recommended_cpus(kvm);
>
I've checked that gic__create comes before the call kvm__recommended_capus:
gic__create is in core_init (called via kvm__init->kvm_arch_init), and
kvm__recommended_cpus is in base_init (called via kvm__cpu_init ->
kvm__{recommended,max}_cpus).
The KVM api documentation states that KVM_CHECK_EXTENSION is available for the vm
fd only if the system capability KVM_CAP_CHECK_EXTENSION_VM is present. kvmtool
already has a function for checking extensions on the vm fd, it's called
kvm__supports_vm_extension. Can we use that instead of doing the ioctl directly on
the vm fd?
Thanks,
Alex
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
@ 2020-04-27 14:44 ` Alexandru Elisei
0 siblings, 0 replies; 14+ messages in thread
From: Alexandru Elisei @ 2020-04-27 14:44 UTC (permalink / raw)
To: Marc Zyngier, kvmarm, kvm
Cc: James Morse, Julien Thierry, Suzuki K Poulose, Will Deacon,
Andre Przywara, Ard Biesheuvel
Hi,
On 4/27/20 3:17 PM, Marc Zyngier wrote:
> On arm64, the maximum number of vcpus is constrained by the type
> of interrupt controller that has been selected (GICv2 imposes a
> limit of 8 vcpus, while GICv3 currently has a limit of 512).
>
> It is thus important to request this limit on the VM file descriptor
> rather than on the one that corresponds to /dev/kvm, as the latter
> is likely to return something that doesn't take the constraints into
> account.
>
> Reported-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> kvm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kvm.c b/kvm.c
> index e327541..3d5173d 100644
> --- a/kvm.c
> +++ b/kvm.c
> @@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
> {
> int ret;
>
> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> if (ret <= 0)
> /*
> * api.txt states that if KVM_CAP_NR_VCPUS does not exist,
> @@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
> {
> int ret;
>
> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> if (ret <= 0)
> ret = kvm__recommended_cpus(kvm);
>
I've checked that gic__create comes before the call kvm__recommended_capus:
gic__create is in core_init (called via kvm__init->kvm_arch_init), and
kvm__recommended_cpus is in base_init (called via kvm__cpu_init ->
kvm__{recommended,max}_cpus).
The KVM api documentation states that KVM_CHECK_EXTENSION is available for the vm
fd only if the system capability KVM_CAP_CHECK_EXTENSION_VM is present. kvmtool
already has a function for checking extensions on the vm fd, it's called
kvm__supports_vm_extension. Can we use that instead of doing the ioctl directly on
the vm fd?
Thanks,
Alex
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
2020-04-27 14:44 ` Alexandru Elisei
@ 2020-04-27 15:00 ` Alexandru Elisei
-1 siblings, 0 replies; 14+ messages in thread
From: Alexandru Elisei @ 2020-04-27 15:00 UTC (permalink / raw)
To: Marc Zyngier, kvmarm, kvm; +Cc: Andre Przywara, Will Deacon, Ard Biesheuvel
Hi,
On 4/27/20 3:44 PM, Alexandru Elisei wrote:
> Hi,
>
> On 4/27/20 3:17 PM, Marc Zyngier wrote:
>> On arm64, the maximum number of vcpus is constrained by the type
>> of interrupt controller that has been selected (GICv2 imposes a
>> limit of 8 vcpus, while GICv3 currently has a limit of 512).
>>
>> It is thus important to request this limit on the VM file descriptor
>> rather than on the one that corresponds to /dev/kvm, as the latter
>> is likely to return something that doesn't take the constraints into
>> account.
>>
>> Reported-by: Ard Biesheuvel <ardb@kernel.org>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>> kvm.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kvm.c b/kvm.c
>> index e327541..3d5173d 100644
>> --- a/kvm.c
>> +++ b/kvm.c
>> @@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
>> {
>> int ret;
>>
>> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
>> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
>> if (ret <= 0)
>> /*
>> * api.txt states that if KVM_CAP_NR_VCPUS does not exist,
>> @@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
>> {
>> int ret;
>>
>> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
>> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
>> if (ret <= 0)
>> ret = kvm__recommended_cpus(kvm);
>>
> I've checked that gic__create comes before the call kvm__recommended_capus:
> gic__create is in core_init (called via kvm__init->kvm_arch_init), and
> kvm__recommended_cpus is in base_init (called via kvm__cpu_init ->
> kvm__{recommended,max}_cpus).
>
> The KVM api documentation states that KVM_CHECK_EXTENSION is available for the vm
> fd only if the system capability KVM_CAP_CHECK_EXTENSION_VM is present. kvmtool
> already has a function for checking extensions on the vm fd, it's called
> kvm__supports_vm_extension. Can we use that instead of doing the ioctl directly on
> the vm fd?
Scratch that, kvm__supports_vm_extension returns a bool, not an int. How about we
write kvm__check_vm_extension that returns an int, and kvm__supports_vm_extension
calls it?
>
> Thanks,
> Alex
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
@ 2020-04-27 15:00 ` Alexandru Elisei
0 siblings, 0 replies; 14+ messages in thread
From: Alexandru Elisei @ 2020-04-27 15:00 UTC (permalink / raw)
To: Marc Zyngier, kvmarm, kvm
Cc: James Morse, Julien Thierry, Suzuki K Poulose, Will Deacon,
Andre Przywara, Ard Biesheuvel
Hi,
On 4/27/20 3:44 PM, Alexandru Elisei wrote:
> Hi,
>
> On 4/27/20 3:17 PM, Marc Zyngier wrote:
>> On arm64, the maximum number of vcpus is constrained by the type
>> of interrupt controller that has been selected (GICv2 imposes a
>> limit of 8 vcpus, while GICv3 currently has a limit of 512).
>>
>> It is thus important to request this limit on the VM file descriptor
>> rather than on the one that corresponds to /dev/kvm, as the latter
>> is likely to return something that doesn't take the constraints into
>> account.
>>
>> Reported-by: Ard Biesheuvel <ardb@kernel.org>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>> kvm.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kvm.c b/kvm.c
>> index e327541..3d5173d 100644
>> --- a/kvm.c
>> +++ b/kvm.c
>> @@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
>> {
>> int ret;
>>
>> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
>> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
>> if (ret <= 0)
>> /*
>> * api.txt states that if KVM_CAP_NR_VCPUS does not exist,
>> @@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
>> {
>> int ret;
>>
>> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
>> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
>> if (ret <= 0)
>> ret = kvm__recommended_cpus(kvm);
>>
> I've checked that gic__create comes before the call kvm__recommended_capus:
> gic__create is in core_init (called via kvm__init->kvm_arch_init), and
> kvm__recommended_cpus is in base_init (called via kvm__cpu_init ->
> kvm__{recommended,max}_cpus).
>
> The KVM api documentation states that KVM_CHECK_EXTENSION is available for the vm
> fd only if the system capability KVM_CAP_CHECK_EXTENSION_VM is present. kvmtool
> already has a function for checking extensions on the vm fd, it's called
> kvm__supports_vm_extension. Can we use that instead of doing the ioctl directly on
> the vm fd?
Scratch that, kvm__supports_vm_extension returns a bool, not an int. How about we
write kvm__check_vm_extension that returns an int, and kvm__supports_vm_extension
calls it?
>
> Thanks,
> Alex
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
2020-04-27 15:00 ` Alexandru Elisei
@ 2020-04-27 17:33 ` Marc Zyngier
-1 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2020-04-27 17:33 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: kvm, Andre Przywara, Ard Biesheuvel, Will Deacon, kvmarm
On Mon, 27 Apr 2020 16:00:58 +0100
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> Hi,
>
> On 4/27/20 3:44 PM, Alexandru Elisei wrote:
> > Hi,
> >
> > On 4/27/20 3:17 PM, Marc Zyngier wrote:
> >> On arm64, the maximum number of vcpus is constrained by the type
> >> of interrupt controller that has been selected (GICv2 imposes a
> >> limit of 8 vcpus, while GICv3 currently has a limit of 512).
> >>
> >> It is thus important to request this limit on the VM file descriptor
> >> rather than on the one that corresponds to /dev/kvm, as the latter
> >> is likely to return something that doesn't take the constraints into
> >> account.
> >>
> >> Reported-by: Ard Biesheuvel <ardb@kernel.org>
> >> Signed-off-by: Marc Zyngier <maz@kernel.org>
> >> ---
> >> kvm.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/kvm.c b/kvm.c
> >> index e327541..3d5173d 100644
> >> --- a/kvm.c
> >> +++ b/kvm.c
> >> @@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
> >> {
> >> int ret;
> >>
> >> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> >> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> >> if (ret <= 0)
> >> /*
> >> * api.txt states that if KVM_CAP_NR_VCPUS does not exist,
> >> @@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
> >> {
> >> int ret;
> >>
> >> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> >> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> >> if (ret <= 0)
> >> ret = kvm__recommended_cpus(kvm);
> >>
> > I've checked that gic__create comes before the call kvm__recommended_capus:
> > gic__create is in core_init (called via kvm__init->kvm_arch_init), and
> > kvm__recommended_cpus is in base_init (called via kvm__cpu_init ->
> > kvm__{recommended,max}_cpus).
> >
> > The KVM api documentation states that KVM_CHECK_EXTENSION is available for the vm
> > fd only if the system capability KVM_CAP_CHECK_EXTENSION_VM is present. kvmtool
> > already has a function for checking extensions on the vm fd, it's called
> > kvm__supports_vm_extension. Can we use that instead of doing the ioctl directly on
> > the vm fd?
>
> Scratch that, kvm__supports_vm_extension returns a bool, not an int.
> How about we write kvm__check_vm_extension that returns an int, and
> kvm__supports_vm_extension calls it?
That, or we just change the return type for kvm__supports_vm_extension,
and hack the only places that uses it so far (the GIC code) to detect
the error.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
@ 2020-04-27 17:33 ` Marc Zyngier
0 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2020-04-27 17:33 UTC (permalink / raw)
To: Alexandru Elisei
Cc: kvmarm, kvm, James Morse, Julien Thierry, Suzuki K Poulose,
Will Deacon, Andre Przywara, Ard Biesheuvel
On Mon, 27 Apr 2020 16:00:58 +0100
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> Hi,
>
> On 4/27/20 3:44 PM, Alexandru Elisei wrote:
> > Hi,
> >
> > On 4/27/20 3:17 PM, Marc Zyngier wrote:
> >> On arm64, the maximum number of vcpus is constrained by the type
> >> of interrupt controller that has been selected (GICv2 imposes a
> >> limit of 8 vcpus, while GICv3 currently has a limit of 512).
> >>
> >> It is thus important to request this limit on the VM file descriptor
> >> rather than on the one that corresponds to /dev/kvm, as the latter
> >> is likely to return something that doesn't take the constraints into
> >> account.
> >>
> >> Reported-by: Ard Biesheuvel <ardb@kernel.org>
> >> Signed-off-by: Marc Zyngier <maz@kernel.org>
> >> ---
> >> kvm.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/kvm.c b/kvm.c
> >> index e327541..3d5173d 100644
> >> --- a/kvm.c
> >> +++ b/kvm.c
> >> @@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
> >> {
> >> int ret;
> >>
> >> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> >> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> >> if (ret <= 0)
> >> /*
> >> * api.txt states that if KVM_CAP_NR_VCPUS does not exist,
> >> @@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
> >> {
> >> int ret;
> >>
> >> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> >> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> >> if (ret <= 0)
> >> ret = kvm__recommended_cpus(kvm);
> >>
> > I've checked that gic__create comes before the call kvm__recommended_capus:
> > gic__create is in core_init (called via kvm__init->kvm_arch_init), and
> > kvm__recommended_cpus is in base_init (called via kvm__cpu_init ->
> > kvm__{recommended,max}_cpus).
> >
> > The KVM api documentation states that KVM_CHECK_EXTENSION is available for the vm
> > fd only if the system capability KVM_CAP_CHECK_EXTENSION_VM is present. kvmtool
> > already has a function for checking extensions on the vm fd, it's called
> > kvm__supports_vm_extension. Can we use that instead of doing the ioctl directly on
> > the vm fd?
>
> Scratch that, kvm__supports_vm_extension returns a bool, not an int.
> How about we write kvm__check_vm_extension that returns an int, and
> kvm__supports_vm_extension calls it?
That, or we just change the return type for kvm__supports_vm_extension,
and hack the only places that uses it so far (the GIC code) to detect
the error.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
2020-04-27 17:33 ` Marc Zyngier
@ 2020-04-28 9:09 ` Alexandru Elisei
-1 siblings, 0 replies; 14+ messages in thread
From: Alexandru Elisei @ 2020-04-28 9:09 UTC (permalink / raw)
To: Marc Zyngier; +Cc: kvm, Andre Przywara, Ard Biesheuvel, Will Deacon, kvmarm
Hi,
On 4/27/20 6:33 PM, Marc Zyngier wrote:
> On Mon, 27 Apr 2020 16:00:58 +0100
> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>
>> Hi,
>>
>> On 4/27/20 3:44 PM, Alexandru Elisei wrote:
>>> Hi,
>>>
>>> On 4/27/20 3:17 PM, Marc Zyngier wrote:
>>>> On arm64, the maximum number of vcpus is constrained by the type
>>>> of interrupt controller that has been selected (GICv2 imposes a
>>>> limit of 8 vcpus, while GICv3 currently has a limit of 512).
>>>>
>>>> It is thus important to request this limit on the VM file descriptor
>>>> rather than on the one that corresponds to /dev/kvm, as the latter
>>>> is likely to return something that doesn't take the constraints into
>>>> account.
>>>>
>>>> Reported-by: Ard Biesheuvel <ardb@kernel.org>
>>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>>> ---
>>>> kvm.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/kvm.c b/kvm.c
>>>> index e327541..3d5173d 100644
>>>> --- a/kvm.c
>>>> +++ b/kvm.c
>>>> @@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
>>>> {
>>>> int ret;
>>>>
>>>> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
>>>> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
>>>> if (ret <= 0)
>>>> /*
>>>> * api.txt states that if KVM_CAP_NR_VCPUS does not exist,
>>>> @@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
>>>> {
>>>> int ret;
>>>>
>>>> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
>>>> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
>>>> if (ret <= 0)
>>>> ret = kvm__recommended_cpus(kvm);
>>>>
>>> I've checked that gic__create comes before the call kvm__recommended_capus:
>>> gic__create is in core_init (called via kvm__init->kvm_arch_init), and
>>> kvm__recommended_cpus is in base_init (called via kvm__cpu_init ->
>>> kvm__{recommended,max}_cpus).
>>>
>>> The KVM api documentation states that KVM_CHECK_EXTENSION is available for the vm
>>> fd only if the system capability KVM_CAP_CHECK_EXTENSION_VM is present. kvmtool
>>> already has a function for checking extensions on the vm fd, it's called
>>> kvm__supports_vm_extension. Can we use that instead of doing the ioctl directly on
>>> the vm fd?
>> Scratch that, kvm__supports_vm_extension returns a bool, not an int.
>> How about we write kvm__check_vm_extension that returns an int, and
>> kvm__supports_vm_extension calls it?
> That, or we just change the return type for kvm__supports_vm_extension,
> and hack the only places that uses it so far (the GIC code) to detect
> the error.
Yep, whatever you prefer.
Thanks,
Alex
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
@ 2020-04-28 9:09 ` Alexandru Elisei
0 siblings, 0 replies; 14+ messages in thread
From: Alexandru Elisei @ 2020-04-28 9:09 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, kvm, James Morse, Julien Thierry, Suzuki K Poulose,
Will Deacon, Andre Przywara, Ard Biesheuvel
Hi,
On 4/27/20 6:33 PM, Marc Zyngier wrote:
> On Mon, 27 Apr 2020 16:00:58 +0100
> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>
>> Hi,
>>
>> On 4/27/20 3:44 PM, Alexandru Elisei wrote:
>>> Hi,
>>>
>>> On 4/27/20 3:17 PM, Marc Zyngier wrote:
>>>> On arm64, the maximum number of vcpus is constrained by the type
>>>> of interrupt controller that has been selected (GICv2 imposes a
>>>> limit of 8 vcpus, while GICv3 currently has a limit of 512).
>>>>
>>>> It is thus important to request this limit on the VM file descriptor
>>>> rather than on the one that corresponds to /dev/kvm, as the latter
>>>> is likely to return something that doesn't take the constraints into
>>>> account.
>>>>
>>>> Reported-by: Ard Biesheuvel <ardb@kernel.org>
>>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>>> ---
>>>> kvm.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/kvm.c b/kvm.c
>>>> index e327541..3d5173d 100644
>>>> --- a/kvm.c
>>>> +++ b/kvm.c
>>>> @@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
>>>> {
>>>> int ret;
>>>>
>>>> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
>>>> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
>>>> if (ret <= 0)
>>>> /*
>>>> * api.txt states that if KVM_CAP_NR_VCPUS does not exist,
>>>> @@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
>>>> {
>>>> int ret;
>>>>
>>>> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
>>>> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
>>>> if (ret <= 0)
>>>> ret = kvm__recommended_cpus(kvm);
>>>>
>>> I've checked that gic__create comes before the call kvm__recommended_capus:
>>> gic__create is in core_init (called via kvm__init->kvm_arch_init), and
>>> kvm__recommended_cpus is in base_init (called via kvm__cpu_init ->
>>> kvm__{recommended,max}_cpus).
>>>
>>> The KVM api documentation states that KVM_CHECK_EXTENSION is available for the vm
>>> fd only if the system capability KVM_CAP_CHECK_EXTENSION_VM is present. kvmtool
>>> already has a function for checking extensions on the vm fd, it's called
>>> kvm__supports_vm_extension. Can we use that instead of doing the ioctl directly on
>>> the vm fd?
>> Scratch that, kvm__supports_vm_extension returns a bool, not an int.
>> How about we write kvm__check_vm_extension that returns an int, and
>> kvm__supports_vm_extension calls it?
> That, or we just change the return type for kvm__supports_vm_extension,
> and hack the only places that uses it so far (the GIC code) to detect
> the error.
Yep, whatever you prefer.
Thanks,
Alex
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
2020-04-27 14:17 ` Marc Zyngier
@ 2020-04-27 15:37 ` André Przywara
-1 siblings, 0 replies; 14+ messages in thread
From: André Przywara @ 2020-04-27 15:37 UTC (permalink / raw)
To: Marc Zyngier, kvmarm, kvm; +Cc: Will Deacon, Ard Biesheuvel
On 27/04/2020 15:17, Marc Zyngier wrote:
Hi,
> On arm64, the maximum number of vcpus is constrained by the type
> of interrupt controller that has been selected (GICv2 imposes a
> limit of 8 vcpus, while GICv3 currently has a limit of 512).
>
> It is thus important to request this limit on the VM file descriptor
> rather than on the one that corresponds to /dev/kvm, as the latter
> is likely to return something that doesn't take the constraints into
> account.
That sounds reasonable, but I fail to find any distinction in the kernel
code. We don't make any difference between the VM or the system FD in
the ioctl handler for those two extensions. For arm64 we always return
max. 512 (max VCPUs on GICv3), and number of online host cores for the
recommended value. For arm there was a distinction between GICv3 support
compiled in or not, but otherwise the same constant values returned.
Quickly tested on Juno and N1SDP, the ioctls return the same expected
values, regardless of sys_fd vs vm_fd.
So what am I missing here? Is this for some older or even newer kernels?
Cheers,
Andre.
>
> Reported-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> kvm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kvm.c b/kvm.c
> index e327541..3d5173d 100644
> --- a/kvm.c
> +++ b/kvm.c
> @@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
> {
> int ret;
>
> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> if (ret <= 0)
> /*
> * api.txt states that if KVM_CAP_NR_VCPUS does not exist,
> @@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
> {
> int ret;
>
> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> if (ret <= 0)
> ret = kvm__recommended_cpus(kvm);
>
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
@ 2020-04-27 15:37 ` André Przywara
0 siblings, 0 replies; 14+ messages in thread
From: André Przywara @ 2020-04-27 15:37 UTC (permalink / raw)
To: Marc Zyngier, kvmarm, kvm
Cc: James Morse, Julien Thierry, Suzuki K Poulose, Will Deacon,
Ard Biesheuvel, Alexandru Elisei
On 27/04/2020 15:17, Marc Zyngier wrote:
Hi,
> On arm64, the maximum number of vcpus is constrained by the type
> of interrupt controller that has been selected (GICv2 imposes a
> limit of 8 vcpus, while GICv3 currently has a limit of 512).
>
> It is thus important to request this limit on the VM file descriptor
> rather than on the one that corresponds to /dev/kvm, as the latter
> is likely to return something that doesn't take the constraints into
> account.
That sounds reasonable, but I fail to find any distinction in the kernel
code. We don't make any difference between the VM or the system FD in
the ioctl handler for those two extensions. For arm64 we always return
max. 512 (max VCPUs on GICv3), and number of online host cores for the
recommended value. For arm there was a distinction between GICv3 support
compiled in or not, but otherwise the same constant values returned.
Quickly tested on Juno and N1SDP, the ioctls return the same expected
values, regardless of sys_fd vs vm_fd.
So what am I missing here? Is this for some older or even newer kernels?
Cheers,
Andre.
>
> Reported-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> kvm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kvm.c b/kvm.c
> index e327541..3d5173d 100644
> --- a/kvm.c
> +++ b/kvm.c
> @@ -406,7 +406,7 @@ int kvm__recommended_cpus(struct kvm *kvm)
> {
> int ret;
>
> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
> if (ret <= 0)
> /*
> * api.txt states that if KVM_CAP_NR_VCPUS does not exist,
> @@ -421,7 +421,7 @@ int kvm__max_cpus(struct kvm *kvm)
> {
> int ret;
>
> - ret = ioctl(kvm->sys_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> + ret = ioctl(kvm->vm_fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS);
> if (ret <= 0)
> ret = kvm__recommended_cpus(kvm);
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
2020-04-27 15:37 ` André Przywara
@ 2020-04-27 16:49 ` Marc Zyngier
-1 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2020-04-27 16:49 UTC (permalink / raw)
To: André Przywara; +Cc: kvm, Ard Biesheuvel, Will Deacon, kvmarm
On 2020-04-27 16:37, André Przywara wrote:
> On 27/04/2020 15:17, Marc Zyngier wrote:
> Hi,
>
>> On arm64, the maximum number of vcpus is constrained by the type
>> of interrupt controller that has been selected (GICv2 imposes a
>> limit of 8 vcpus, while GICv3 currently has a limit of 512).
>>
>> It is thus important to request this limit on the VM file descriptor
>> rather than on the one that corresponds to /dev/kvm, as the latter
>> is likely to return something that doesn't take the constraints into
>> account.
>
> That sounds reasonable, but I fail to find any distinction in the
> kernel
> code. We don't make any difference between the VM or the system FD in
> the ioctl handler for those two extensions. For arm64 we always return
> max. 512 (max VCPUs on GICv3), and number of online host cores for the
> recommended value. For arm there was a distinction between GICv3
> support
> compiled in or not, but otherwise the same constant values returned.
> Quickly tested on Juno and N1SDP, the ioctls return the same expected
> values, regardless of sys_fd vs vm_fd.
>
> So what am I missing here? Is this for some older or even newer
> kernels?
You're missing this:
https://lore.kernel.org/kvm/20200427141507.284985-1-maz@kernel.org/
which adds the missing bits to the kernel.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH][kvmtool] kvm: Request VM specific limits instead of system-wide ones
@ 2020-04-27 16:49 ` Marc Zyngier
0 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2020-04-27 16:49 UTC (permalink / raw)
To: André Przywara
Cc: kvmarm, kvm, James Morse, Julien Thierry, Suzuki K Poulose,
Will Deacon, Ard Biesheuvel, Alexandru Elisei
On 2020-04-27 16:37, André Przywara wrote:
> On 27/04/2020 15:17, Marc Zyngier wrote:
> Hi,
>
>> On arm64, the maximum number of vcpus is constrained by the type
>> of interrupt controller that has been selected (GICv2 imposes a
>> limit of 8 vcpus, while GICv3 currently has a limit of 512).
>>
>> It is thus important to request this limit on the VM file descriptor
>> rather than on the one that corresponds to /dev/kvm, as the latter
>> is likely to return something that doesn't take the constraints into
>> account.
>
> That sounds reasonable, but I fail to find any distinction in the
> kernel
> code. We don't make any difference between the VM or the system FD in
> the ioctl handler for those two extensions. For arm64 we always return
> max. 512 (max VCPUs on GICv3), and number of online host cores for the
> recommended value. For arm there was a distinction between GICv3
> support
> compiled in or not, but otherwise the same constant values returned.
> Quickly tested on Juno and N1SDP, the ioctls return the same expected
> values, regardless of sys_fd vs vm_fd.
>
> So what am I missing here? Is this for some older or even newer
> kernels?
You're missing this:
https://lore.kernel.org/kvm/20200427141507.284985-1-maz@kernel.org/
which adds the missing bits to the kernel.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 14+ messages in thread