Re: [PATCH] ARM/ARM64: KVM: remove 'config KVM_ARM_MAX_VCPUS'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 2, 2015 at 7:42 PM, Ming Lei <ming.lei@xxxxxxxxxxxxx> wrote:
> On Wed, Sep 2, 2015 at 6:25 PM, Christoffer Dall
> <christoffer.dall@xxxxxxxxxx> wrote:
>> On Wed, Sep 02, 2015 at 02:31:21PM +0800, Ming Lei wrote:
>>> This patch removes config option of KVM_ARM_MAX_VCPUS,
>>> and like other ARCHs, just choose the maximum allowed
>>> value from hardware, and follows the reasons:
>>>
>>> 1) from distribution view, the option has to be
>>> defined as the max allowed value because it need to
>>> meet all kinds of virtulization applications and
>>> need to support most of SoCs;
>>>
>>> 2) using a bigger value doesn't introduce extra memory
>>> consumption, and the help text in Kconfig isn't accurate
>>> because kvm_vpu structure isn't allocated until request
>>> of creating VCPU is sent from QEMU;
>>
>> This used to be true because of the vgic bitmaps, but that is now
>> dynamically allocated, so I believe you're correct in saying that the
>> text is no longer accurate.
>>
>>>
>>> 3) the main effect is that the field of vcpus[] in 'struct kvm'
>>> becomes a bit bigger(sizeof(void *) per vcpu) and need more cache
>>> lines to hold the structure, but 'struct kvm' is one generic struct,
>>> and it has worked well on other ARCHs already in this way. Also,
>>> the world switch frequecy is often low, for example, it is ~2000
>>> when running kernel building load in VM from APM xgene KVM host,
>>> so the effect is very small, and the difference can't be observed
>>> in my test at all.
>>
>> While I'm not prinicipally opposed to removing this option, I have to
>> point out that this analysis is far far over-simplified.  You have
>> chosen a workload which excercised only CPU and memory virtualization,
>> mostly solved by the hardware virtualization support, and therefore you
>> don't see many exits.
>>
>> Try running an I/O bound workload, or something which involves a lot of
>> virtual IPIs, and you'll see a higher number of exits.
>
> Yeah, the frequency of exits becomes higher(6600/sec) when I run a
> totally I/O benchmark(fio: 4 jobs, bs 4k, libaio over virtio-blk) in a
> quad-core VM, but it is still not high enough to cause any difference
> on the test result.
>
>>
>> However, I still doubt that the effects will be noticable in the grand
>> scheme of things.
>>>
>>> Cc: Dann Frazier <dann.frazier@xxxxxxxxxxxxx>
>>> Cc: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
>>> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
>>> Cc: kvmarm@xxxxxxxxxxxxxxxxxxxxx
>>> Cc: kvm@xxxxxxxxxxxxxxx
>>> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxxxxx>
>>> ---
>>>  arch/arm/include/asm/kvm_host.h   |  8 ++------
>>>  arch/arm/kvm/Kconfig              | 11 -----------
>>>  arch/arm64/include/asm/kvm_host.h |  8 ++------
>>>  arch/arm64/kvm/Kconfig            | 11 -----------
>>>  include/kvm/arm_vgic.h            |  6 +-----
>>>  virt/kvm/arm/vgic-v3.c            |  2 +-
>>>  6 files changed, 6 insertions(+), 40 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>>> index dcba0fa..c8c226a 100644
>>> --- a/arch/arm/include/asm/kvm_host.h
>>> +++ b/arch/arm/include/asm/kvm_host.h
>>> @@ -29,12 +29,6 @@
>>>
>>>  #define __KVM_HAVE_ARCH_INTC_INITIALIZED
>>>
>>> -#if defined(CONFIG_KVM_ARM_MAX_VCPUS)
>>> -#define KVM_MAX_VCPUS CONFIG_KVM_ARM_MAX_VCPUS
>>> -#else
>>> -#define KVM_MAX_VCPUS 0
>>> -#endif
>>> -
>>>  #define KVM_USER_MEM_SLOTS 32
>>>  #define KVM_PRIVATE_MEM_SLOTS 4
>>>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>>> @@ -44,6 +38,8 @@
>>>
>>>  #include <kvm/arm_vgic.h>
>>>
>>> +#define KVM_MAX_VCPUS VGIC_V2_MAX_CPUS
>>> +
>>>  u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
>>>  int __attribute_const__ kvm_target_cpu(void);
>>>  int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
>>> diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
>>> index bfb915d..210ecca 100644
>>> --- a/arch/arm/kvm/Kconfig
>>> +++ b/arch/arm/kvm/Kconfig
>>> @@ -45,15 +45,4 @@ config KVM_ARM_HOST
>>>       ---help---
>>>         Provides host support for ARM processors.
>>>
>>> -config KVM_ARM_MAX_VCPUS
>>> -     int "Number maximum supported virtual CPUs per VM"
>>> -     depends on KVM_ARM_HOST
>>> -     default 4
>>> -     help
>>> -       Static number of max supported virtual CPUs per VM.
>>> -
>>> -       If you choose a high number, the vcpu structures will be quite
>>> -       large, so only choose a reasonable number that you expect to
>>> -       actually use.
>>> -
>>>  endif # VIRTUALIZATION
>>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>>> index 415938d..3fb58ea 100644
>>> --- a/arch/arm64/include/asm/kvm_host.h
>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>> @@ -30,12 +30,6 @@
>>>
>>>  #define __KVM_HAVE_ARCH_INTC_INITIALIZED
>>>
>>> -#if defined(CONFIG_KVM_ARM_MAX_VCPUS)
>>> -#define KVM_MAX_VCPUS CONFIG_KVM_ARM_MAX_VCPUS
>>> -#else
>>> -#define KVM_MAX_VCPUS 0
>>> -#endif
>>> -
>>>  #define KVM_USER_MEM_SLOTS 32
>>>  #define KVM_PRIVATE_MEM_SLOTS 4
>>>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>>> @@ -43,6 +37,8 @@
>>>  #include <kvm/arm_vgic.h>
>>>  #include <kvm/arm_arch_timer.h>
>>>
>>> +#define KVM_MAX_VCPUS VGIC_V3_MAX_CPUS
>>> +
>>>  #define KVM_VCPU_MAX_FEATURES 3
>>>
>>>  int __attribute_const__ kvm_target_cpu(void);
>>> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
>>> index bfffe8f..5c7e920 100644
>>> --- a/arch/arm64/kvm/Kconfig
>>> +++ b/arch/arm64/kvm/Kconfig
>>> @@ -41,15 +41,4 @@ config KVM_ARM_HOST
>>>       ---help---
>>>         Provides host support for ARM processors.
>>>
>>> -config KVM_ARM_MAX_VCPUS
>>> -     int "Number maximum supported virtual CPUs per VM"
>>> -     depends on KVM_ARM_HOST
>>> -     default 4
>>> -     help
>>> -       Static number of max supported virtual CPUs per VM.
>>> -
>>> -       If you choose a high number, the vcpu structures will be quite
>>> -       large, so only choose a reasonable number that you expect to
>>> -       actually use.
>>> -
>>>  endif # VIRTUALIZATION
>>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>>> index d901f1a..4e14dac 100644
>>> --- a/include/kvm/arm_vgic.h
>>> +++ b/include/kvm/arm_vgic.h
>>> @@ -35,11 +35,7 @@
>>>  #define VGIC_V3_MAX_LRS              16
>>>  #define VGIC_MAX_IRQS                1024
>>>  #define VGIC_V2_MAX_CPUS     8
>>> -
>>> -/* Sanity checks... */
>>> -#if (KVM_MAX_VCPUS > 255)
>>> -#error Too many KVM VCPUs, the VGIC only supports up to 255 VCPUs for now
>>> -#endif
>>> +#define VGIC_V3_MAX_CPUS     255
>>>
>>>  #if (VGIC_NR_IRQS_LEGACY & 31)
>>>  #error "VGIC_NR_IRQS must be a multiple of 32"
>>> diff --git a/virt/kvm/arm/vgic-v3.c b/virt/kvm/arm/vgic-v3.c
>>> index afbf925..7dd5d62 100644
>>> --- a/virt/kvm/arm/vgic-v3.c
>>> +++ b/virt/kvm/arm/vgic-v3.c
>>> @@ -288,7 +288,7 @@ int vgic_v3_probe(struct device_node *vgic_node,
>>>
>>>       vgic->vctrl_base = NULL;
>>>       vgic->type = VGIC_V3;
>>> -     vgic->max_gic_vcpus = KVM_MAX_VCPUS;
>>> +     vgic->max_gic_vcpus = VGIC_V3_MAX_CPUS;
>>>
>>>       kvm_info("%s@%llx IRQ%d\n", vgic_node->name,
>>>                vcpu_res.start, vgic->maint_irq);
>>> --
>>> 1.9.1
>>
>>
>> Reviewed-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>

Hi Guys,

Is there any chance to merge this patch?

Thanks,
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux