Re: [PATCH v12 1/2] ACPI:RAS2: Add ACPI RAS2 driver

[Yazen Ghannam <yazen.ghannam@amd.com>]

On Tue, Sep 02, 2025 at 06:30:39PM +0100, shiju.jose@huawei.com wrote:
> From: Shiju Jose <shiju.jose@huawei.com>

[...]

> +static int ras2_add_aux_device(char *name, int channel, u32 pxm_inst)
> +{
> +	unsigned long start_pfn, size_pfn;
> +	struct ras2_mem_ctx *ras2_ctx;
> +	int id, rc;

'rc' is uninitialized, and LLVM gives a warning.

The issue is the "goto ctx_free" paths return 'rc' before it is set.

> +
> +	ras2_ctx = kzalloc(sizeof(*ras2_ctx), GFP_KERNEL);
> +	if (!ras2_ctx)
> +		return -ENOMEM;
> +
> +	ras2_ctx->sys_comp_nid = pxm_to_node(pxm_inst);
> +	/*
> +	 * Retrieve the lowest contiguous physical memory address range within
> +	 * the NUMA node.
> +	 */
> +	start_pfn = node_start_pfn(ras2_ctx->sys_comp_nid);
> +	size_pfn = node_spanned_pages(ras2_ctx->sys_comp_nid);
> +	if (!size_pfn) {
> +		pr_debug("Failed to find phy addr range for NUMA node(%u)\n",
> +			 pxm_inst);
> +		goto ctx_free;
> +	}
> +	ras2_ctx->mem_base_addr = __pfn_to_phys(start_pfn);
> +	ras2_ctx->mem_size = __pfn_to_phys(size_pfn);
> +
> +	rc = ras2_register_pcc_channel(ras2_ctx, channel);
> +	if (rc < 0) {
> +		pr_debug("Failed to register pcc channel rc=%d\n", rc);
> +		goto ctx_free;
> +	}
> +
> +	id = ida_alloc(&ras2_ida, GFP_KERNEL);
> +	if (id < 0) {
> +		rc = id;
> +		goto ctx_free;
> +	}
> +
> +	ras2_ctx->adev.id		= id;
> +	ras2_ctx->adev.name		= RAS2_MEM_DEV_ID_NAME;
> +	ras2_ctx->adev.dev.release	= ras2_release;
> +	ras2_ctx->adev.dev.parent	= ras2_ctx->dev;
> +
> +	rc = auxiliary_device_init(&ras2_ctx->adev);
> +	if (rc)
> +		goto ida_free;
> +
> +	rc = auxiliary_device_add(&ras2_ctx->adev);
> +	if (rc) {
> +		auxiliary_device_uninit(&ras2_ctx->adev);
> +		return rc;
> +	}
> +
> +	return 0;
> +
> +ida_free:
> +	ida_free(&ras2_ida, id);
> +ctx_free:
> +	kfree(ras2_ctx);
> +
> +	return rc;
> +}
> +
> +static int acpi_ras2_parse(struct acpi_table_ras2 *ras2_tab)
> +{
> +	struct acpi_ras2_pcc_desc *pcc_desc_list;
> +	int rc;
> +	u16 i;
> +
> +	if (ras2_tab->header.length < sizeof(*ras2_tab)) {
> +		pr_warn(FW_WARN "ACPI RAS2 table present but broken (too short #1)\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!ras2_tab->num_pcc_descs) {
> +		pr_warn(FW_WARN "No PCC descs in ACPI RAS2 table\n");
> +		return -EINVAL;
> +	}
> +
> +	pcc_desc_list = (struct acpi_ras2_pcc_desc *)(ras2_tab + 1);
> +	for (i = 0; i < ras2_tab->num_pcc_descs; i++, pcc_desc_list++) {
> +		if (pcc_desc_list->feature_type != RAS2_FEAT_TYPE_MEMORY)
> +			continue;
> +
> +		rc = ras2_add_aux_device(RAS2_MEM_DEV_ID_NAME,
> +					 pcc_desc_list->channel_id,
> +					 pcc_desc_list->instance);
> +		if (rc)
> +			pr_warn("Failed to add RAS2 auxiliary device rc=%d\n", rc);
> +	}
> +
> +	return 0;

Should this return 'rc' from above? Or is the 'warning' case not a total
failure?

Thanks,
Yazen