Message-ID: <57e66ff5-1cb6-06bd-ee6f-a3c3dadd6aef@loongson.cn>
Date: Tue, 2 Apr 2024 09:43:59 +0800
From: maobibo <maobibo@...ngson.cn>
To: WANG Xuerui <kernel@...0n.name>, Huacai Chen <chenhuacai@...nel.org>,
 Tianrui Zhao <zhaotianrui@...ngson.cn>, Juergen Gross <jgross@...e.com>,
 Paolo Bonzini <pbonzini@...hat.com>, Jonathan Corbet <corbet@....net>
Cc: loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org,
 virtualization@...ts.linux.dev, kvm@...r.kernel.org
Subject: Re: [PATCH v7 3/7] LoongArch: KVM: Add cpucfg area for kvm hypervisor



On 2024/3/24 上午3:02, WANG Xuerui wrote:
> On 3/15/24 16:07, Bibo Mao wrote:
>> Instruction cpucfg can be used to get processor features. And there
>> is trap exception when it is executed in VM mode, and also it is
>> to provide cpu features to VM. On real hardware cpucfg area 0 - 20
>> is used.  Here one specified area 0x40000000 -- 0x400000ff is used
>> for KVM hypervisor to privide PV features, and the area can be extended
>> for other hypervisors in future. This area will never be used for
>> real HW, it is only used by software.
>>
>> Signed-off-by: Bibo Mao <maobibo@...ngson.cn>
>> ---
>>   arch/loongarch/include/asm/inst.h      |  1 +
>>   arch/loongarch/include/asm/loongarch.h | 10 +++++
>>   arch/loongarch/kvm/exit.c              | 59 +++++++++++++++++++-------
>>   3 files changed, 54 insertions(+), 16 deletions(-)
>>
> 
> Sorry for the late reply, but I think it may be a bit non-constructive 
> to repeatedly submit the same code without due explanation in our 
> previous review threads. Let me try to recollect some of the details 
> though...
Because your review comments about hypercall method is wrong, I need not 
adopt it.
> 
> If I remember correctly, during the previous reviews, it was mentioned 
> that the only upsides of using CPUCFG were:
> 
> - it was exactly identical to the x86 approach,
> - it would not require access to the LoongArch Reference Manual Volume 3 
> to use, and
> - it was plain old data.
> 
> But, for the first point, we don't have to follow x86 convention after 
X86 virtualization is successfully and widely applied in our life and 
products. It it normal to follow it if there is not obvious issues.

> all. The second reason might be compelling, but on the one hand that's 
> another problem orthogonal to the current one, and on the other hand 
> HVCL is:
> 
> - already effectively public because of the fact that this very patchset 
> is public,
> - its semantics is trivial to implement even without access to the LVZ 
> manual, because of its striking similarity with SYSCALL, and
> - by being a function call, we reserve the possibility for hypervisors 
> to invoke logic for self-identification purposes, even if this is likely 
> overkill from today's perspective.
> 
> And, even if we decide that using HVCL for self-identification is 
> overkill after all, we still have another choice that's IOCSR. We 
> already read LOONGARCH_IOCSR_FEATURES (0x8) for its bit 11 (IOCSRF_VM) 
> to populate the CPU_FEATURE_HYPERVISOR bit, and it's only natural that 
> we put the identification word in the IOCSR space. As far as I can see, 
> the IOCSR space is plenty and equally available for making reservations; 
> it can only be even easier when it's done by a Loongson team.
IOCSR method is possible also, about chip design CPUCFG is used for cpu 
features and IOCSR is for device featurs. Here CPUCFG method is 
selected, I am KVM LoongArch maintainer and I can decide to select 
methods if the method works well. Is that right?

If you are interested in KVM LoongArch, you can submit more patches and 
become maintainer or write new hypervisor support such xen/xvisor etc, 
and use your method.

Also you are interested in Linux kernel, there are some issues. Can you 
help to improve it?

1. T0-T7 are scratch registers during SYSCALL ABI, this is what you 
suggest, does there exist information leaking to user space from T0-T7 
registers?

2. LoongArch KVM depends on AS_HAS_LVZ_EXTENSION, which requires the 
latest binutils. It is also what you suggest. Some kernel developers 
does not have the latest binutils and common kvm code is modified and 
LoongArch KVM fails to compile. But they can not find it since their 
LoongArch cross-compile is old and LoongArch KVM is disabled. This issue 
can be found at https://lkml.org/lkml/2023/11/15/828.

Regards
Bibo Mao
> 
> Finally, I've mentioned multiple times, that varying CPUCFG behavior 
> based on PLV is not something well documented on the manuals, hence not 
> friendly to low-level developers. Devs of third-party firmware and/or 
> kernels do exist, I've personally spoken to some of them on the 
> 2023-11-18 3A6000 release event; in order for the varying CPUCFG 
> behavior approach to pass for me, at the very least, the LoongArch 
> reference manual must be amended to explicitly include an explanation of 
> it, and a reference to potential use cases.
> 

Powered by blists - more mailing lists