| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <25ff4e20-158d-d09b-14a4-7cbbabadf7c9@loongson.cn>
Date: Mon, 15 Jul 2024 14:21:39 +0800
From: maobibo <maobibo@...ngson.cn>
To: Xianglai Li <lixianglai@...ngson.cn>, linux-kernel@...r.kernel.org
Cc: Tianrui Zhao <zhaotianrui@...ngson.cn>,
Huacai Chen <chenhuacai@...nel.org>, kvm@...r.kernel.org,
loongarch@...ts.linux.dev, Min Zhou <zhoumin@...ngson.cn>,
Paolo Bonzini <pbonzini@...hat.com>, WANG Xuerui <kernel@...0n.name>
Subject: Re: [PATCH 10/11] LoongArch: KVM: Add PCHPIC user mode read and write
functions
On 2024/7/5 上午10:38, Xianglai Li wrote:
> Implements the communication interface between the user mode
> program and the kernel in PCHPIC interrupt control simulation,
> which is used to obtain or send the simulation data of the
> interrupt controller in the user mode process, and is used
> in VM migration or VM saving and restoration.
>
> Signed-off-by: Tianrui Zhao <zhaotianrui@...ngson.cn>
> Signed-off-by: Xianglai Li <lixianglai@...ngson.cn>
> ---
> Cc: Bibo Mao <maobibo@...ngson.cn>
> Cc: Huacai Chen <chenhuacai@...nel.org>
> Cc: kvm@...r.kernel.org
> Cc: loongarch@...ts.linux.dev
> Cc: Min Zhou <zhoumin@...ngson.cn>
> Cc: Paolo Bonzini <pbonzini@...hat.com>
> Cc: Tianrui Zhao <zhaotianrui@...ngson.cn>
> Cc: WANG Xuerui <kernel@...0n.name>
> Cc: Xianglai li <lixianglai@...ngson.cn>
>
> arch/loongarch/include/uapi/asm/kvm.h | 4 +
> arch/loongarch/kvm/intc/pch_pic.c | 128 +++++++++++++++++++++++++-
> 2 files changed, 130 insertions(+), 2 deletions(-)
>
> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> index 6d5ad95fcb75..ba7f473bc8b6 100644
> --- a/arch/loongarch/include/uapi/asm/kvm.h
> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> @@ -113,4 +113,8 @@ struct kvm_iocsr_entry {
>
> #define KVM_DEV_LOONGARCH_EXTIOI_GRP_REGS 1
>
> +#define KVM_DEV_LOONGARCH_PCH_PIC_GRP_CTRL 0
> +#define KVM_DEV_LOONGARCH_PCH_PIC_CTRL_INIT 0
> +#define KVM_DEV_LOONGARCH_PCH_PIC_GRP_REGS 1
> +
> #endif /* __UAPI_ASM_LOONGARCH_KVM_H */
> diff --git a/arch/loongarch/kvm/intc/pch_pic.c b/arch/loongarch/kvm/intc/pch_pic.c
> index 4ad85277fced..abb7bab84f2d 100644
> --- a/arch/loongarch/kvm/intc/pch_pic.c
> +++ b/arch/loongarch/kvm/intc/pch_pic.c
> @@ -313,16 +313,140 @@ static const struct kvm_io_device_ops kvm_loongarch_pch_pic_ops = {
> .write = kvm_loongarch_pch_pic_write,
> };
>
> +static int kvm_loongarch_pch_pic_init(struct kvm_device *dev, u64 addr)
> +{
> + int ret;
> + struct loongarch_pch_pic *s = dev->kvm->arch.pch_pic;
> + struct kvm_io_device *device;
> + struct kvm *kvm = dev->kvm;
> +
> + s->pch_pic_base = addr;
> + device = &s->device;
> + /* init device by pch pic writing and reading ops */
> + kvm_iodevice_init(device, &kvm_loongarch_pch_pic_ops);
> + mutex_lock(&kvm->slots_lock);
> + /* register pch pic device */
> + ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS, addr, PCH_PIC_SIZE, device);
> + mutex_unlock(&kvm->slots_lock);
> + if (ret < 0)
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +/* used by user space to get or set pch pic registers */
> +static int kvm_loongarch_pch_pic_regs_access(struct kvm_device *dev,
> + struct kvm_device_attr *attr,
> + bool is_write)
> +{
> + int addr, len = 8, ret = 0;
> + void __user *data;
> + void *p = NULL;
> + struct loongarch_pch_pic *s;
> +
> + s = dev->kvm->arch.pch_pic;
> + addr = attr->attr;
> + data = (void __user *)attr->addr;
> +
> + spin_lock(&s->lock);
> + /* get pointer to pch pic register by addr */
> + switch (addr) {
> + case PCH_PIC_MASK_START:
> + p = &s->mask;
> + break;
> + case PCH_PIC_HTMSI_EN_START:
> + p = &s->htmsi_en;
> + break;
> + case PCH_PIC_EDGE_START:
> + p = &s->edge;
> + break;
> + case PCH_PIC_AUTO_CTRL0_START:
> + p = &s->auto_ctrl0;
> + break;
> + case PCH_PIC_AUTO_CTRL1_START:
> + p = &s->auto_ctrl1;
> + break;
> + case PCH_PIC_ROUTE_ENTRY_START:
> + p = s->route_entry;
> + len = 64;
Can we use macro rather than hard-coded 64 here?
> + break;
> + case PCH_PIC_HTMSI_VEC_START:
> + p = s->htmsi_vector;
> + len = 64;
Ditto
> + break;
> + case PCH_PIC_INT_IRR_START:
> + p = &s->irr;
> + break;
> + case PCH_PIC_INT_ISR_START:
> + p = &s->isr;
> + break;
> + case PCH_PIC_POLARITY_START:
> + p = &s->polarity;
> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
Do we need check default path and return directly here?
> + /* write or read value according to is_write */
> + if (is_write) {
> + if (copy_from_user(p, data, len))
> + ret = -EFAULT;
> + } else {
> + if (copy_to_user(data, p, len))
> + ret = -EFAULT;
> + }
> +
> + spin_unlock(&s->lock);
Please put spin_unlock() ahead of copy_from_user/copy_to_user
Regards
Bibo Mao
> + return ret;
> +}
> +
> static int kvm_loongarch_pch_pic_get_attr(struct kvm_device *dev,
> struct kvm_device_attr *attr)
> {
> - return 0;
> + /* only support pch pic group registers */
> + if (attr->group == KVM_DEV_LOONGARCH_PCH_PIC_GRP_REGS)
> + return kvm_loongarch_pch_pic_regs_access(dev, attr, false);
> +
> + return -EINVAL;
> }
>
> static int kvm_loongarch_pch_pic_set_attr(struct kvm_device *dev,
> struct kvm_device_attr *attr)
> {
> - return 0;
> + int ret = -EINVAL;
> + u64 addr;
> + void __user *uaddr = (void __user *)(long)attr->addr;
> +
> + switch (attr->group) {
> + case KVM_DEV_LOONGARCH_PCH_PIC_GRP_CTRL:
> + switch (attr->attr) {
> + case KVM_DEV_LOONGARCH_PCH_PIC_CTRL_INIT:
> + if (copy_from_user(&addr, uaddr, sizeof(addr)))
> + return -EFAULT;
> +
> + if (!dev->kvm->arch.pch_pic) {
> + kvm_err("%s: please create pch_pic irqchip first!\n", __func__);
> + ret = -EFAULT;
> + break;
> + }
> +
> + ret = kvm_loongarch_pch_pic_init(dev, addr);
> + break;
> + default:
> + kvm_err("%s: unknown group (%d) attr (%lld)\n", __func__, attr->group,
> + attr->attr);
> + ret = -EINVAL;
> + break;
> + }
> + break;
> + case KVM_DEV_LOONGARCH_PCH_PIC_GRP_REGS:
> + ret = kvm_loongarch_pch_pic_regs_access(dev, attr, true);
> + break;
> + default:
> + break;
> + }
> +
> + return ret;
> }
>
> static void kvm_loongarch_pch_pic_destroy(struct kvm_device *dev)
>
Powered by blists - more mailing lists