Re: [Qemu-devel] [PATCH] target-i386: Sanity check host processor physical address width
On 07/09/15 21:11, Bandan Das wrote: > Laszlo Ersek writes: > ... 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__, 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! > > Thanks Igor and Laszlo, makes sense. I am wondering if this 1GB PCI > hole is always fixed so that I can simply include that in calculating the > maximum > guest ram address ? Or do we have to figure that out every time ? Please grep the tree for "above_4g_mem_size". The size of the 32-bit PCI hole is not constant, but all the necessary computation goes into "above_4g_mem_size" already. So I think you should derive the max possible gpa from "above_4g_mem_size" and the top of the hotpluggable memory area, and compare that against the PCPU address width, *if* EPT is enabled. (BTW "pcms->hotplug_memory.base" depends on "above_4g_mem_size" too.) Thanks Laszlo > >> (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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] target-i386: Sanity check host processor physical address width
Igor Mammedov writes:
> On Wed, 08 Jul 2015 18:42:01 -0400
> 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);
> max_phys_bits seems generic enough and could be applied to other targets
> as well.
I am a little clueless about other targets but is figuring this out from
userspace
as simple as it's on x86 (cpuid)? If not, then I agree, this could be made a
generic
value.
Bandan
> making it a property of machine, would make accessing/manipulating it easier.
> define default value for machine/TCG mode and when KVM is enabled
> it would override/set its own limit.
>
> then any board could easily access machine->max_gpa to make board specific
> checks.
>
>> +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);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] target-i386: Sanity check host processor physical address width
Laszlo Ersek writes: ... >>> >>> 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__, 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! Thanks Igor and Laszlo, makes sense. I am wondering if this 1GB PCI hole is always fixed so that I can simply include that in calculating the maximum guest ram address ? Or do we have to figure that out every time ? > (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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] target-i386: Sanity check host processor physical address width
On Wed, 08 Jul 2015 18:42:01 -0400
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);
max_phys_bits seems generic enough and could be applied to other targets
as well.
making it a property of machine, would make accessing/manipulating it easier.
define default value for machine/TCG mode and when KVM is enabled
it would override/set its own limit.
then any board could easily access machine->max_gpa to make board specific
checks.
> +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);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] target-i386: Sanity check host processor physical address width
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__, 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] target-i386: Sanity check host processor physical address width
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__, 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
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
