Message-ID: <6ea77f032fc0f2df5ca76d89e88acec3cc8d37c1.camel@mediatek.com>
Date: Mon, 29 Jul 2024 03:35:57 +0000
From: Liju-clr Chen (陳麗如)
	<Liju-clr.Chen@...iatek.com>
To: "horms@...nel.org" <horms@...nel.org>
CC: "corbet@....net" <corbet@....net>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-trace-kernel@...r.kernel.org"
	<linux-trace-kernel@...r.kernel.org>,
	"angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>, "rostedt@...dmis.org"
	<rostedt@...dmis.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Ze-yu Wang (王澤宇) <Ze-yu.Wang@...iatek.com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Chi-shen Yeh (葉奇軒) <Chi-shen.Yeh@...iatek.com>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"quic_tsoni@...cinc.com" <quic_tsoni@...cinc.com>, "dbrazdil@...gle.com"
	<dbrazdil@...gle.com>, PeiLun Suei (隋培倫)
	<PeiLun.Suei@...iatek.com>, Kevenny Hsieh (謝宜芸)
	<Kevenny.Hsieh@...iatek.com>, "catalin.marinas@....com"
	<catalin.marinas@....com>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"robh@...nel.org" <robh@...nel.org>, "richardcochran@...il.com"
	<richardcochran@...il.com>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "mhiramat@...nel.org"
	<mhiramat@...nel.org>, Yingshiuan Pan (潘穎軒)
	<Yingshiuan.Pan@...iatek.com>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>, Shawn Hsiao (蕭志祥)
	<shawn.hsiao@...iatek.com>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"mathieu.desnoyers@...icios.com" <mathieu.desnoyers@...icios.com>,
	"will@...nel.org" <will@...nel.org>
Subject: Re: [PATCH v11 08/21] virt: geniezone: Add vcpu support

On Fri, 2024-05-31 at 21:36 +0100, Simon Horman wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On Wed, May 29, 2024 at 04:42:26PM +0800, Liju-clr Chen wrote:
> > From: Yi-De Wu <yi-de.wu@...iatek.com>
> > 
> > From: "Yingshiuan Pan" <yingshiuan.pan@...iatek.com>
> 
> nit: I think there should be at most one From line,
>      denoting the author of the patch. Based on the Signed-off-by
>      lines I assume that is Yingshiuan Pan.
> 
>      If there are multiple authors perhaps
>      the Co-developed-by tag should be used below.
> 
>      And on that note, it's not clear to me what the significance
>      of the Signed-off-by lines, other than that of
>      Yingshiuan Pan (presumed author) and Liju Chen (sender) are.
>      I'd suggest deleting them unless they are
>      accompanied by Co-developed-by tags.
> 
>      And, lastly, the sender's signed-off-by line should come last.
> 
>      See: 
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

Hi Simon,
Thank you for your review. I will update it in next version.

> 
> > 
> > VMM use this interface to create vcpu instance which is a fd, and
> this
> > fd will be for any vcpu operations, such as setting vcpu registers
> and
> > accepts the most important ioctl GZVM_VCPU_RUN which requests
> GenieZone
> > hypervisor to do context switch to execute VM's vcpu context.
> > 
> > Signed-off-by: Yingshiuan Pan <yingshiuan.pan@...iatek.com>
> > Signed-off-by: Jerry Wang <ze-yu.wang@...iatek.com>
> > Signed-off-by: kevenny hsieh <kevenny.hsieh@...iatek.com>
> > Signed-off-by: Liju Chen <liju-clr.chen@...iatek.com>
> > Signed-off-by: Yi-De Wu <yi-de.wu@...iatek.com>
> 
> ...
> 
> > diff --git a/drivers/virt/geniezone/Makefile
> b/drivers/virt/geniezone/Makefile
> > index 25614ea3dea2..9cc453c0819b 100644
> > --- a/drivers/virt/geniezone/Makefile
> > +++ b/drivers/virt/geniezone/Makefile
> > @@ -6,4 +6,5 @@
> >  
> >  GZVM_DIR ?= ../../../drivers/virt/geniezone
> >  
> > -gzvm-y := $(GZVM_DIR)/gzvm_main.o $(GZVM_DIR)/gzvm_vm.o
> > +gzvm-y := $(GZVM_DIR)/gzvm_main.o $(GZVM_DIR)/gzvm_vm.o \
> > +  $(GZVM_DIR)/gzvm_vcpu.o
> > diff --git a/drivers/virt/geniezone/gzvm_vcpu.c
> b/drivers/virt/geniezone/gzvm_vcpu.c
> > new file mode 100644
> > index 000000000000..1aca13fef422
> > --- /dev/null
> > +++ b/drivers/virt/geniezone/gzvm_vcpu.c
> > @@ -0,0 +1,249 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2023 MediaTek Inc.
> > + */
> > +
> > +#include <asm/sysreg.h>
> 
> nit: It's not clear to me that sysreg.h needs to be included in this
> file.

Ok. I will update it in next version.

> 
> > +#include <linux/anon_inodes.h>
> > +#include <linux/device.h>
> > +#include <linux/file.h>
> > +#include <linux/mm.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +#include <linux/soc/mediatek/gzvm_drv.h>
> > +
> > +/* maximum size needed for holding an integer */
> > +#define ITOA_MAX_LEN 12
> > +
> > +static long gzvm_vcpu_update_one_reg(struct gzvm_vcpu *vcpu,
> > +     void __user *argp,
> > +     bool is_write)
> > +{
> > +struct gzvm_one_reg reg;
> > +void __user *reg_addr;
> > +u64 data = 0;
> > +u64 reg_size;
> > +long ret;
> > +
> > +if (copy_from_user(&reg, argp, sizeof(reg)))
> > +return -EFAULT;
> > +
> > +reg_addr = (void __user *)reg.addr;
> 
> nit: Perhaps u64_to_user_ptr() is appropriate here.
> 
>      Also in gzvm_vm_ioctl_create_device() in patch 09/21.

Ok. I will update it in next version.

Thanks,
Liju

> 
> > +reg_size = (reg.id & GZVM_REG_SIZE_MASK) >> GZVM_REG_SIZE_SHIFT;
> > +reg_size = BIT(reg_size);
> > +
> > +if (reg_size != 1 && reg_size != 2 && reg_size != 4 && reg_size !=
> 8)
> > +return -EINVAL;
> > +
> > +if (is_write) {
> > +/* GZ hypervisor would filter out invalid vcpu register access */
> > +if (copy_from_user(&data, reg_addr, reg_size))
> > +return -EFAULT;
> > +} else {
> > +return -EOPNOTSUPP;
> > +}
> > +
> > +ret = gzvm_arch_vcpu_update_one_reg(vcpu, reg.id, is_write,
> &data);
> > +
> > +if (ret)
> > +return ret;
> > +
> > +return 0;
> > +}
> 
> ...

Powered by blists - more mailing lists