Re: [Qemu-devel] [PATCH] target-i386: Sanity check host processor physical address width

[Igor Mammedov]

On Thu, 09 Jul 2015 09:02:38 +0200
Laszlo Ersek <address@hidden> 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 <address@hidden>
> > Signed-off-by: Bandan Das <address@hidden>
> > ---
> >  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

> 
> Thanks
> Laszlo
>