Re: [PATCH 1/3] kvm tools: Add memory gap for larger RAM sizes

[Ingo Molnar <mingo@elte.hu>]

* Sasha Levin <levinsasha928@gmail.com> wrote:

> Add a memory gap between 0xe0000000 and 0x100000000
> when using more than 0xe0000000 bytes for guest RAM.
> 
> This space is used by several things, PCI configuration
> space for example.
> 
> This patch updates the e820 table, slot allocations
> used for KVM_SET_USER_MEMORY_REGION, and the address
> translation.

Btw., in changelogs you really want to mention the *motivation* and effect of 
patches - not just the workings of the patch. Everyone can see what a patch 
does, so the motivation and effects are a lot more important and should be 
mentioned first in the patch - explaining how the patch does it is a distant 
third factor in terms of changelog-content importance ...

If someone reads these without context he has no idea that these patches are 
fixing crashes and memory corruption with guest RAM sizes that cross the 4GB 
boundary.

> +	if (kvm->ram_size < 0xe0000000) {
> +			.size		= 0xe0000000 - BZ_KERNEL_START,
> +			.size		= kvm->ram_size - 0xe0000000 - BZ_KERNEL_START,
> +	 * We have a gap between 0xe0000000 and 0x100000000.
> +	if (offset < 0xe0000000)
> +		return self->ram_start + 0xe0000000 + (offset - 0x100000000);
> +	if (self->ram_size < 0xe0000000) {
> +		kvm_register_mem_slot(self, 0, 0, 0xe0000000, (u64)self->ram_start);
> +		kvm_register_mem_slot(self, 1, 0x100000000ULL, self->ram_size - 0xe0000000, (u64)self->ram_start + 0xe0000000);

0xe0000000 is repeated 9 times in the code!

We tend to add symbols when values are repeated only twice. Often we add a 
symbol and an explanation when a magic value is used only *once*.

It is absolutely vital to define a symbol for this and document that it means 
and why we need it.

Thanks,

	Ingo