Re: [PATCH v10 15/21] virt: geniezone: Add demand paging support

[Simon Horman]

On Fri, Apr 12, 2024 at 02:57:12PM +0800, Yi-De Wu wrote:
> From: "Yingshiuan Pan" <yingshiuan.pan@xxxxxxxxxxxx>
> 
> This page fault handler helps GenieZone hypervisor to do demand paging.
> On a lower level translation fault, GenieZone hypervisor will first
> check the fault GPA (guest physical address or IPA in ARM) is valid
> e.g. within the registered memory region, then it will setup the
> vcpu_run->exit_reason with necessary information for returning to
> gzvm driver.
> 
> With the fault information, the gzvm driver looks up the physical
> address and call the MT_HVC_GZVM_MAP_GUEST to request the hypervisor
> maps the found PA to the fault GPA (IPA).
> 
> There is one exception, for protected vm, we will populate full VM's
> memory region in advance in order to improve performance.
> 
> Signed-off-by: Yingshiuan Pan <yingshiuan.pan@xxxxxxxxxxxx>
> Signed-off-by: Jerry Wang <ze-yu.wang@xxxxxxxxxxxx>
> Signed-off-by: kevenny hsieh <kevenny.hsieh@xxxxxxxxxxxx>
> Signed-off-by: Liju Chen <liju-clr.chen@xxxxxxxxxxxx>
> Signed-off-by: Yi-De Wu <yi-de.wu@xxxxxxxxxxxx>

..

> diff --git a/drivers/virt/geniezone/gzvm_exception.c b/drivers/virt/geniezone/gzvm_exception.c
> new file mode 100644
> index 000000000000..475bc15b0689
> --- /dev/null
> +++ b/drivers/virt/geniezone/gzvm_exception.c
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023 MediaTek Inc.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/soc/mediatek/gzvm_drv.h>
> +
> +/**
> + * gzvm_handle_guest_exception() - Handle guest exception
> + * @vcpu: Pointer to struct gzvm_vcpu_run in userspace
> + * Return:
> + * * true - This exception has been processed, no need to back to VMM.
> + * * false - This exception has not been processed, require userspace.
> + */
> +bool gzvm_handle_guest_exception(struct gzvm_vcpu *vcpu)

Hi Yi-De Wu,

The return type is bool, however the function actually
returns either a bool or signed int.

I think that either:

1. The return type should be changed to int,
   and returning true and false should be updated.

2. The function should always return true or false.

Flagged by Smatch.

> +{
> +	int ret;
> +
> +	for (int i = 0; i < ARRAY_SIZE(vcpu->run->exception.reserved); i++) {
> +		if (vcpu->run->exception.reserved[i])
> +			return -EINVAL;
> +	}
> +
> +	switch (vcpu->run->exception.exception) {
> +	case GZVM_EXCEPTION_PAGE_FAULT:
> +		ret = gzvm_handle_page_fault(vcpu);
> +		break;
> +	case GZVM_EXCEPTION_UNKNOWN:
> +		fallthrough;
> +	default:
> +		ret = -EFAULT;
> +	}
> +
> +	if (!ret)
> +		return true;
> +	else
> +		return false;
> +}

..