On Mon, 16 May 2022 at 21:52, Michael S. Tsirkin <[email protected]> wrote:
>
> From: Jonathan Cameron <[email protected]>
>
> The concept of these is introduced in [1] in terms of the
> description the CEDT ACPI table. The principal is more general.
> Unlike once traffic hits the CXL root bridges, the host system
> memory address routing is implementation defined and effectively
> static once observable by standard / generic system software.
> Each CXL Fixed Memory Windows (CFMW) is a region of PA space
> which has fixed system dependent routing configured so that
> accesses can be routed to the CXL devices below a set of target
> root bridges. The accesses may be interleaved across multiple
> root bridges.
Hi; Coverity points out a memory leak in this function
(CID 1488872):
> +void cxl_fixed_memory_window_config(MachineState *ms,
> + CXLFixedMemoryWindowOptions *object,
> + Error **errp)
> +{
> + CXLFixedWindow *fw = g_malloc0(sizeof(*fw));
Here we allocate memory...
> + strList *target;
> + int i;
> +
> + for (target = object->targets; target; target = target->next) {
> + fw->num_targets++;
> + }
> +
> + fw->enc_int_ways = cxl_interleave_ways_enc(fw->num_targets, errp);
> + if (*errp) {
> + return;
...but in the error returns here..
> + }
> +
> + fw->targets = g_malloc0_n(fw->num_targets, sizeof(*fw->targets));
> + for (i = 0, target = object->targets; target; i++, target =
> target->next) {
> + /* This link cannot be resolved yet, so stash the name for now */
> + fw->targets[i] = g_strdup(target->value);
> + }
> +
> + if (object->size % (256 * MiB)) {
> + error_setg(errp,
> + "Size of a CXL fixed memory window must my a multiple of
> 256MiB");
> + return;
...here..
> + }
> + fw->size = object->size;
> +
> + if (object->has_interleave_granularity) {
> + fw->enc_int_gran =
> + cxl_interleave_granularity_enc(object->interleave_granularity,
> + errp);
> + if (*errp) {
> + return;
...and here we fail to free the memory we allocated.
> + }
> + } else {
> + /* Default to 256 byte interleave */
> + fw->enc_int_gran = 0;
> + }
> +
> + ms->cxl_devices_state->fixed_windows =
> + g_list_append(ms->cxl_devices_state->fixed_windows, fw);
> +
> + return;
> +}
Would you mind sending a patch to fix this bug ?
The neatest approach probably uses g_autofree and g_steal_pointer().
thanks
-- PMM
