linux-kernel - Re: [PATCH v1 01/24] LoongArch: KVM: Implement kvm module related interface

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y+6FTC1vBeZoZx8V@linux.dev>
Date:   Thu, 16 Feb 2023 19:34:36 +0000
From:   Oliver Upton <oliver.upton@...ux.dev>
To:     Tianrui Zhao <zhaotianrui@...ngson.cn>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Huacai Chen <chenhuacai@...nel.org>,
        WANG Xuerui <kernel@...0n.name>, loongarch@...ts.linux.dev,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        Jens Axboe <axboe@...nel.dk>, Mark Brown <broonie@...nel.org>,
        Alex Deucher <alexander.deucher@....com>
Subject: Re: [PATCH v1 01/24] LoongArch: KVM: Implement kvm module related
 interface

On Tue, Feb 14, 2023 at 09:00:50PM +0800, Tianrui Zhao wrote:
> 在 2023年02月14日 17:58, Greg Kroah-Hartman 写道:
> > On Tue, Feb 14, 2023 at 05:00:56PM +0800, Tianrui Zhao wrote:
> > > > > +#define KVM_GET_CSRS		_IOWR(KVMIO, 0xc5, struct kvm_csrs)
> > > > > +#define KVM_SET_CSRS		_IOW(KVMIO,  0xc6, struct kvm_csrs)
> > > > Why does this arch need new ioctls?
> > > We want to use this ioctl to access multiple csrs at one time. If without
> > > this, we only access one csr.
> > What is wrong with accessing only one csr at a time?  Isn't this what
> > other architectures do?
> 
> Generally, using KVM_GET_ONE_GET ioctl to get one reg, but we want a
> more convenient interface to get serial regs at one time, so we add this
> ioctl.

Have you found register accesses through the KVM_{GET,SET}_ONE_REG
ioctls to actually be a bounding issue? I'd be surprised if that were
actually the case.

An architecture-neutral implementation was entertained a few years ago
[*], but even then it saved an inconsequential amount of time relative
to the rest of VM serialization (at least for arm64). The one thing that
series got right was to share the plumbing across all architectures that
use the ONE_REG interface (i.e. everyone but x86).

If you have data that supports the thesis that a batched ioctl is
useful then please do share. But in any case this should not use an ioctl
tied down to a single architecture.

[*] https://lore.kernel.org/kvm/20201120125616.14436-1-darkhan@amazon.com/

-- 
Thanks,
Oliver