From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37289) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZD8gY-00058Y-DP for qemu-devel@nongnu.org; Thu, 09 Jul 2015 06:04:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZD8gV-0003mk-51 for qemu-devel@nongnu.org; Thu, 09 Jul 2015 06:04:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51532) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZD8gU-0003md-U2 for qemu-devel@nongnu.org; Thu, 09 Jul 2015 06:04:03 -0400 Message-ID: <559E470C.9000301@redhat.com> Date: Thu, 09 Jul 2015 12:03:56 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <559E1C8E.6080508@redhat.com> <20150709112728.38a953b6@nial.brq.redhat.com> In-Reply-To: <20150709112728.38a953b6@nial.brq.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] target-i386: Sanity check host processor physical address width List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: Paolo Bonzini , Bandan Das , qemu-devel@nongnu.org, kvm@vger.kernel.org, Eduardo Habkost On 07/09/15 11:27, Igor Mammedov wrote: > On Thu, 09 Jul 2015 09:02:38 +0200 > Laszlo Ersek wrote: > >> On 07/09/15 00:42, Bandan Das wrote: >>> >>> If a Linux guest is assigned more memory than is supported >>> by the host processor, the guest is unable to boot. That >>> is expected, however, there's no message indicating the user >>> what went wrong. This change prints a message to stderr if >>> KVM has the corresponding capability. >>> >>> Reported-by: Laszlo Ersek >>> Signed-off-by: Bandan Das >>> --- >>> linux-headers/linux/kvm.h | 1 + >>> target-i386/kvm.c | 6 ++++++ >>> 2 files changed, 7 insertions(+) >>> >>> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h >>> index 3bac873..6afad49 100644 >>> --- a/linux-headers/linux/kvm.h >>> +++ b/linux-headers/linux/kvm.h >>> @@ -817,6 +817,7 @@ struct kvm_ppc_smmu_info { >>> #define KVM_CAP_DISABLE_QUIRKS 116 >>> #define KVM_CAP_X86_SMM 117 >>> #define KVM_CAP_MULTI_ADDRESS_SPACE 118 >>> +#define KVM_CAP_PHY_ADDR_WIDTH 119 >>> >>> #ifdef KVM_CAP_IRQ_ROUTING >>> >>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c >>> index 066d03d..66e3448 100644 >>> --- a/target-i386/kvm.c >>> +++ b/target-i386/kvm.c >>> @@ -892,6 +892,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) >>> uint64_t shadow_mem; >>> int ret; >>> struct utsname utsname; >>> + int max_phys_bits; >>> >>> ret = kvm_get_supported_msrs(s); >>> if (ret < 0) { >>> @@ -945,6 +946,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s) >>> } >>> } >>> >>> + max_phys_bits = kvm_check_extension(s, KVM_CAP_PHY_ADDR_WIDTH); >>> + if (max_phys_bits && (1ULL << max_phys_bits) <= ram_size) >>> + fprintf(stderr, "Warning: The amount of memory assigned to the guest " >>> + "is more than that supported by the host CPU(s). Guest may be unstable.\n"); >>> + >>> if (kvm_check_extension(s, KVM_CAP_X86_SMM)) { >>> smram_machine_done.notify = register_smram_listener; >>> qemu_add_machine_init_done_notifier(&smram_machine_done); >>> >> >> First, see my comments on the KVM patch. >> >> Second, ram_size is not the right thing to compare. What should be >> checked is whether the highest guest-physical address that maps to RAM >> can be represented in the address width of the host processor (and only >> if EPT is enabled, but that sub-condition belongs to the KVM patch). >> >> Note that this is not the same as the check written in the patch. For >> example, if you assume a 32-bit PCI hole with size 1 GB, then a total >> guest RAM of size 63 GB will result in the highest guest-phys memory >> address being 0xF_FFFF_FFFF, which just fits into 36 bits. >> >> Correspondingly, the above code would not print the warning for >> >> -m $((63 * 1024 + 1)) >> >> on my laptop (which has "address sizes : 36 bits physical, ..."), even >> though such a guest would not boot for me (with EPT enabled). >> >> Please see >> >> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/15418/focus=15447 >> >> So, "ram_size" in the controlling expression should be replaced with >> "maximum_guest_ram_address" (which should be inclusive, and the <= relop >> should be preserved). > also with memory hotplug tuned on we should check if the end of > hotplug memory area is less then limit, i.e.: > > pcms->hotplug_memory.base + hotplug_mem_size < 1ULL << max_phys_bits Seems reasonable, thanks for the hint! (The LHS in this instance is exclusive though, so equality should *not* trigger the warning. "maximum_guest_ram_address" is inclusive, and equality should trigger the warning. (Although equality seems quite impossible in practice.)) Thanks! Laszlo