Re: [PATCH v13 04/25] virt: geniezone: Add GenieZone hypervisor driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 14/11/2024 11:07, Liju-clr Chen wrote:
> +
> +static int gzvm_dev_open(struct inode *inode, struct file *file)
> +{
> +	/*
> +	 * Reference count to prevent this module is unload without destroying
> +	 * VM

So you re-implemented suppress-bind attrs... no, drop.

> +	 */
> +	try_module_get(THIS_MODULE);
> +	return 0;
> +}
> +
> +static int gzvm_dev_release(struct inode *inode, struct file *file)
> +{
> +	module_put(THIS_MODULE);
> +	return 0;
> +}
> +
> +static const struct file_operations gzvm_chardev_ops = {
> +	.llseek		= noop_llseek,
> +	.open		= gzvm_dev_open,
> +	.release	= gzvm_dev_release,
> +};
> +
> +static struct miscdevice gzvm_dev = {
> +	.minor = MISC_DYNAMIC_MINOR,
> +	.name = KBUILD_MODNAME,
> +	.fops = &gzvm_chardev_ops,
> +};
> +
> +static int gzvm_drv_probe(struct platform_device *pdev)
> +{
> +	if (gzvm_arch_probe(gzvm_drv.drv_version, &gzvm_drv.hyp_version) != 0) {
> +		dev_err(&pdev->dev, "Not found available conduit\n");

So you can autodetect your hypervisor? Why your soc info drivers cannot
instantiate this device thus removing any need for fake DT node (fake
because no resources and used only to satisfy Linux driver instantiation)?


> +		return -ENODEV;
> +	}
> +
> +	pr_debug("Found GenieZone hypervisor version %u.%u.%llu\n",
> +		 gzvm_drv.hyp_version.major, gzvm_drv.hyp_version.minor,
> +		 gzvm_drv.hyp_version.sub);
> +
> +	return misc_register(&gzvm_dev);
> +}
> +
> +static void gzvm_drv_remove(struct platform_device *pdev)
> +{
> +	misc_deregister(&gzvm_dev);
> +}
> +
> +static const struct of_device_id gzvm_of_match[] = {
> +	{ .compatible = "mediatek,geniezone" },
> +	{/* sentinel */},
> +};
> +
> +static struct platform_driver gzvm_driver = {
> +	.probe = gzvm_drv_probe,
> +	.remove = gzvm_drv_remove,
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.of_match_table = gzvm_of_match,
> +	},
> +};
> +
> +module_platform_driver(gzvm_driver);
> +
> +MODULE_DEVICE_TABLE(of, gzvm_of_match);

This is immediately after next to ID table. Never in different place, so
I wonder from which obscure code did you copy it and what other issues
like that we can find...

> +MODULE_AUTHOR("MediaTek");
> +MODULE_DESCRIPTION("GenieZone interface for VMM");
> +MODULE_LICENSE("GPL");
Best regards,
Krzysztof