Message ID | 1446627828-3347-2-git-send-email-thuth@redhat.com |
---|---|
State | Superseded |
Headers | show |
On 04/11/15 10:03, Thomas Huth wrote: > Only using 32 memslots for KVM on powerpc is way too low, you can > nowadays hit this limit quite fast by adding a couple of PCI devices > and/or pluggable memory DIMMs to the guest. > x86 already increased the limit to 512 in total, to satisfy 256 > pluggable DIMM slots, 3 private slots and 253 slots for other things > like PCI devices. On powerpc, we only have 32 pluggable DIMMs in > QEMU, not 256, so we likely do not as much slots as on x86. Thus > setting the slot limit to 320 sounds like a good value for the > time being (until we have some code in the future to resize the > memslot array dynamically). > And while we're at it, also remove the KVM_MEM_SLOTS_NUM definition > from the powerpc-specific header since this gets defined in the > generic kvm_host.h header anyway. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > arch/powerpc/include/asm/kvm_host.h | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 887c259..89d387a 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -38,8 +38,7 @@ > > #define KVM_MAX_VCPUS NR_CPUS > #define KVM_MAX_VCORES NR_CPUS > -#define KVM_USER_MEM_SLOTS 32 > -#define KVM_MEM_SLOTS_NUM KVM_USER_MEM_SLOTS > +#define KVM_USER_MEM_SLOTS 320 > > #ifdef CONFIG_KVM_MMIO > #define KVM_COALESCED_MMIO_PAGE_OFFSET 1 > Ping! ... any comments on this one? Thomas -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 04, 2015 at 10:03:48AM +0100, Thomas Huth wrote: > Only using 32 memslots for KVM on powerpc is way too low, you can > nowadays hit this limit quite fast by adding a couple of PCI devices > and/or pluggable memory DIMMs to the guest. > x86 already increased the limit to 512 in total, to satisfy 256 > pluggable DIMM slots, 3 private slots and 253 slots for other things > like PCI devices. On powerpc, we only have 32 pluggable DIMMs in I agree with increasing the limit. Is there a reason we have only 32 pluggable DIMMs in QEMU on powerpc, not more? Should we be increasing that limit too? If so, maybe we should increase the number of memory slots to 512? > QEMU, not 256, so we likely do not as much slots as on x86. Thus "so we likely do not need as many slots as on x86" would be better English. > setting the slot limit to 320 sounds like a good value for the > time being (until we have some code in the future to resize the > memslot array dynamically). > And while we're at it, also remove the KVM_MEM_SLOTS_NUM definition > from the powerpc-specific header since this gets defined in the > generic kvm_host.h header anyway. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/12/15 04:28, Paul Mackerras wrote: > On Wed, Nov 04, 2015 at 10:03:48AM +0100, Thomas Huth wrote: >> Only using 32 memslots for KVM on powerpc is way too low, you can >> nowadays hit this limit quite fast by adding a couple of PCI devices >> and/or pluggable memory DIMMs to the guest. >> x86 already increased the limit to 512 in total, to satisfy 256 >> pluggable DIMM slots, 3 private slots and 253 slots for other things >> like PCI devices. On powerpc, we only have 32 pluggable DIMMs in > > I agree with increasing the limit. Is there a reason we have only 32 > pluggable DIMMs in QEMU on powerpc, not more? Should we be increasing > that limit too? If so, maybe we should increase the number of memory > slots to 512? Hmmmm ... the comment before the #define in QEMU reads: /* * This defines the maximum number of DIMM slots we can have for sPAPR * guest. This is not defined by sPAPR but we are defining it to 32 slots * based on default number of slots provided by PowerPC kernel. */ #define SPAPR_MAX_RAM_SLOTS 32 So as far as I can see, there's indeed a possibility that we'll increase this value once the kernel can handle more slots! So I guess you're right and we should play save and use more slots right from the start. I'll send a new patch with 512 instead. >> QEMU, not 256, so we likely do not as much slots as on x86. Thus > > "so we likely do not need as many slots as on x86" would be better > English. Oops, I'm sure that was my original intention ... anyway, thanks for pointing it out, it's always good to get some feedback as a non-native English speaker. Thomas -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 887c259..89d387a 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -38,8 +38,7 @@ #define KVM_MAX_VCPUS NR_CPUS #define KVM_MAX_VCORES NR_CPUS -#define KVM_USER_MEM_SLOTS 32 -#define KVM_MEM_SLOTS_NUM KVM_USER_MEM_SLOTS +#define KVM_USER_MEM_SLOTS 320 #ifdef CONFIG_KVM_MMIO #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
Only using 32 memslots for KVM on powerpc is way too low, you can nowadays hit this limit quite fast by adding a couple of PCI devices and/or pluggable memory DIMMs to the guest. x86 already increased the limit to 512 in total, to satisfy 256 pluggable DIMM slots, 3 private slots and 253 slots for other things like PCI devices. On powerpc, we only have 32 pluggable DIMMs in QEMU, not 256, so we likely do not as much slots as on x86. Thus setting the slot limit to 320 sounds like a good value for the time being (until we have some code in the future to resize the memslot array dynamically). And while we're at it, also remove the KVM_MEM_SLOTS_NUM definition from the powerpc-specific header since this gets defined in the generic kvm_host.h header anyway. Signed-off-by: Thomas Huth <thuth@redhat.com> --- arch/powerpc/include/asm/kvm_host.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)