On Mon, 6 Nov 2023 at 14:23, Maciej S. Szmigiero
<m...@maciej.szmigiero.name> wrote:
>
> From: "Maciej S. Szmigiero" <maciej.szmigi...@oracle.com>
>
> One of advantages of using this protocol over ACPI-based PC DIMM hotplug is
> that it allows hot-adding memory in much smaller granularity because the
> ACPI DIMM slot limit does not apply.
>
> In order to enable this functionality a new memory backend needs to be
> created and provided to the driver via the "memdev" parameter.
>
> This can be achieved by, for example, adding
> "-object memory-backend-ram,id=mem1,size=32G" to the QEMU command line and
> then instantiating the driver with "memdev=mem1" parameter.
>
> The device will try to use multiple memslots to cover the memory backend in
> order to reduce the size of metadata for the not-yet-hot-added part of the
> memory backend.
>
> Co-developed-by: David Hildenbrand <da...@redhat.com>
> Acked-by: David Hildenbrand <da...@redhat.com>
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigi...@oracle.com>
Hi; I was looking at this code because Coverity reported an
issue in it. I think that's because Coverity has got confused
about the way you're doing memory allocation here. But
in looking at the code I see that you're using alloca() in
this function.
Please could you rewrite this not to do that -- we don't use
alloca() or variable-length-arrays in QEMU except in a few
cases which we're trying to get rid of, so we'd like not to
add new uses to the code base.
> +static void hv_balloon_hot_add_posting(HvBalloon *balloon, StateDesc *stdesc)
> +{
> + PageRange *hot_add_range = &balloon->hot_add_range;
> + uint64_t *current_count = &balloon->ha_current_count;
> + VMBusChannel *chan = hv_balloon_get_channel(balloon);
> + struct dm_hot_add *ha;
> + size_t ha_size = sizeof(*ha) + sizeof(ha->range);
> + union dm_mem_page_range *ha_region;
> + uint64_t align, chunk_max_size;
> + ssize_t ret;
> +
> + assert(balloon->state == S_HOT_ADD_POSTING);
> + assert(hot_add_range->count > 0);
> +
> + align = (1 << balloon->caps.cap_bits.hot_add_alignment) *
> + (MiB / HV_BALLOON_PAGE_SIZE);
> + if (align >= HV_BALLOON_HA_CHUNK_PAGES) {
> + /*
> + * If the required alignment is higher than the chunk size we let it
> + * override that size.
> + */
> + chunk_max_size = align;
> + } else {
> + chunk_max_size = QEMU_ALIGN_DOWN(HV_BALLOON_HA_CHUNK_PAGES, align);
> + }
> +
> + /*
> + * hot_add_range->count starts aligned in hv_balloon_hot_add_setup(),
> + * then it is either reduced by subtracting aligned current_count or
> + * further hot-adds are prevented by marking the whole remaining our
> range
> + * as unusable in hv_balloon_handle_hot_add_response().
> + */
> + *current_count = MIN(hot_add_range->count, chunk_max_size);
> +
> + ha = alloca(ha_size);
> + ha_region = &(&ha->range)[1];
> + memset(ha, 0, ha_size);
> + ha->hdr.type = DM_MEM_HOT_ADD_REQUEST;
> + ha->hdr.size = ha_size;
> + ha->hdr.trans_id = balloon->trans_id;
> +
> + ha->range.finfo.start_page = hot_add_range->start;
> + ha->range.finfo.page_cnt = *current_count;
> + ha_region->finfo.start_page = hot_add_range->start;
> + ha_region->finfo.page_cnt = ha->range.finfo.page_cnt;
> +
> + trace_hv_balloon_outgoing_hot_add(ha->hdr.trans_id,
> + *current_count, hot_add_range->start);
> +
> + ret = vmbus_channel_send(chan, VMBUS_PACKET_DATA_INBAND,
> + NULL, 0, ha, ha_size, false,
> + ha->hdr.trans_id);
> + if (ret <= 0) {
> + error_report("error %zd when posting hot add msg, expect problems",
> + ret);
> + }
> +
> + HV_BALLOON_STATE_DESC_SET(stdesc, S_HOT_ADD_REPLY_WAIT);
> +}
thanks
-- PMM
