[PATCH] [v7] nouveau: add command-line GSP-RM registry support

[Danilo Krummrich]

On 4/17/24 23:53, Timur Tabi wrote:

<snip>

> +
> +/**
> + * r535_gsp_rpc_set_registry - build registry RPC and call GSP-RM
> + * @gsp: gsp pointer
> + *
> + * The GSP-RM registry is a set of key/value pairs that configure some aspects
> + * of GSP-RM. The keys are strings, and the values are 32-bit integers.
> + *
> + * The registry is built from a combination of a static hard-coded list (see
> + * above) and entries passed on the driver's command line.
> + */
>   static int
>   r535_gsp_rpc_set_registry(struct nvkm_gsp *gsp)
>   {
>   	PACKED_REGISTRY_TABLE *rpc;
> -	char *strings;
> -	int str_offset;
> -	int i;
> -	size_t rpc_size = struct_size(rpc, entries, NV_GSP_REG_NUM_ENTRIES);
> +	unsigned int i;
> +	int ret;
>   
> -	/* add strings + null terminator */
> -	for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++)
> -		rpc_size += strlen(r535_registry_entries[i].name) + 1;
> +	INIT_LIST_HEAD(&gsp->registry_list);
> +	gsp->registry_rpc_size = sizeof(PACKED_REGISTRY_TABLE);
>   
> -	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, rpc_size);
> -	if (IS_ERR(rpc))
> -		return PTR_ERR(rpc);
> +	for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) {
> +		ret = add_registry_num(gsp, r535_registry_entries[i].name,
> +				       r535_registry_entries[i].value);
> +		if (ret) {
> +			clean_registry(gsp);
> +			return ret;
> +		}
> +	}
>   
> -	rpc->numEntries = NV_GSP_REG_NUM_ENTRIES;
> +	/*
> +	 * The NVreg_RegistryDwords parameter is a string of key=value
> +	 * pairs separated by semicolons. We need to extract and trim each
> +	 * substring, and then parse the substring to extract the key and
> +	 * value.
> +	 */
> +	if (NVreg_RegistryDwords) {
> +		char *p = kstrdup(NVreg_RegistryDwords, GFP_KERNEL);
> +		char *start, *next = p, *equal;
> +		size_t length;
> +
> +		/* Remove any whitespace from the parameter string */
> +		length = strip(p, " \t\n");

With that, I see the following warning compiling this patch.

warning: variable ‘length’ set but not used [-Wunused-but-set-variable]

Did you intend to use the length for anything?

Also, looking at the warning made me aware of 'p' potentially being NULL.

If you agree, I can fix the warning and add the corresponding NULL check when
applying the patch.

- Danilo

> +
> +		while ((start = strsep(&next, ";"))) {
> +			long value;
> +
> +			equal = strchr(start, '=');
> +			if (!equal || equal == start || equal[1] == 0) {
> +				nvkm_error(&gsp->subdev,
> +					   "ignoring invalid registry string '%s'\n",
> +					   start);
> +				continue;
> +			}
>   
> -	str_offset = offsetof(typeof(*rpc), entries[NV_GSP_REG_NUM_ENTRIES]);
> -	strings = (char *)rpc + str_offset;
> -	for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) {
> -		int name_len = strlen(r535_registry_entries[i].name) + 1;
> -
> -		rpc->entries[i].nameOffset = str_offset;
> -		rpc->entries[i].type = 1;
> -		rpc->entries[i].data = r535_registry_entries[i].value;
> -		rpc->entries[i].length = 4;
> -		memcpy(strings, r535_registry_entries[i].name, name_len);
> -		strings += name_len;
> -		str_offset += name_len;
> +			/* Truncate the key=value string to just key */
> +			*equal = 0;
> +
> +			ret = kstrtol(equal + 1, 0, &value);
> +			if (!ret) {
> +				ret = add_registry_num(gsp, start, value);
> +			} else {
> +				/* Not a number, so treat it as a string */
> +				ret = add_registry_string(gsp, start, equal + 1);
> +			}
> +
> +			if (ret) {
> +				nvkm_error(&gsp->subdev,
> +					   "ignoring invalid registry key/value '%s=%s'\n",
> +					   start, equal + 1);
> +				continue;
> +			}
> +		}
> +
> +		kfree(p);
>   	}
> -	rpc->size = str_offset;
> +
> +	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, gsp->registry_rpc_size);
> +	if (IS_ERR(rpc)) {
> +		clean_registry(gsp);
> +		return PTR_ERR(rpc);
> +	}
> +
> +	build_registry(gsp, rpc);
>   
>   	return nvkm_gsp_rpc_wr(gsp, rpc, false);
>   }
> 
> base-commit: f7ad2ce5fd89ab5d146da8f486a310746df5dc9e
> prerequisite-patch-id: 9bb653b6a53dcba4171d653e24a242461374f9fe
> prerequisite-patch-id: 7093a9db84053e43f6f278bf1d159a25d14ceebf