On Wed, 26 Sep 2018 11:41:59 +0200
David Hildenbrand <da...@redhat.com> wrote:
> Make address_space_end point at the real end, instead of end + 1, so we don't
> have to handle special cases like it being 0. This will allow us to
> place a memory device at the very end of the guest physical 64bit address
> space (if ever possible).
[...]
> @@ -115,7 +116,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> }
> address_space_start = ms->device_memory->base;
> address_space_end = address_space_start +
> - memory_region_size(&ms->device_memory->mr);
> + memory_region_size(&ms->device_memory->mr) - 1;
I'm terrible at math, lets assume size = 1 so after this
'real end' would end up at 'address_space_start', which makes
it rather confusing beside not matching variable name anymore.
I'd drop it and maybe extend the following assert to abort
on overflow condition.
> g_assert(address_space_end >= address_space_start);
>
> /* address_space_start indicates the maximum alignment we expect */
> @@ -149,7 +150,8 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> "] before 0x%" PRIx64, new_addr, size,
> address_space_start);
> return 0;
> - } else if ((new_addr + size) > address_space_end) {
> + } else if (new_addr + size - 1 < new_addr ||
> + new_addr + size - 1 > address_space_end) {
> error_setg(errp, "can't add memory [0x%" PRIx64 ":0x%" PRIx64
> "] beyond 0x%" PRIx64, new_addr, size,
> address_space_end);
> @@ -182,7 +184,8 @@ uint64_t memory_device_get_free_addr(MachineState *ms,
> const uint64_t *hint,
> }
> }
>
> - if (new_addr + size > address_space_end) {
> + if (new_addr + size - 1 < new_addr || !new_addr ||
> + new_addr + size - 1 > address_space_end) {
> error_setg(errp, "could not find position in guest address space for
> "
> "memory device - memory fragmented due to alignments");
> goto out;
I strongly suggest replace non obvious math with several
plain <>= conditions even if adds more conditions compared
to math variant.
At least reader doesn't have to do calculations manually
to figure out what's going on
