| 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
| ||
|
Date: Mon, 26 Feb 2024 16:04:50 +0800
From: maobibo <maobibo@...ngson.cn>
To: Huacai Chen <chenhuacai@...nel.org>, Jiaxun Yang <jiaxun.yang@...goat.com>
Cc: Tianrui Zhao <zhaotianrui@...ngson.cn>, Juergen Gross <jgross@...e.com>,
Paolo Bonzini <pbonzini@...hat.com>, loongarch@...ts.linux.dev,
linux-kernel@...r.kernel.org, virtualization@...ts.linux.dev,
kvm@...r.kernel.org
Subject: Re: [PATCH v5 3/6] LoongArch: KVM: Add cpucfg area for kvm hypervisor
On 2024/2/26 下午2:12, Huacai Chen wrote:
> On Mon, Feb 26, 2024 at 10:04 AM maobibo <maobibo@...ngson.cn> wrote:
>>
>>
>>
>> On 2024/2/24 下午5:13, Huacai Chen wrote:
>>> Hi, Bibo,
>>>
>>> On Thu, Feb 22, 2024 at 11:28 AM Bibo Mao <maobibo@...ngson.cn> 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.
>>> After reading and thinking, I find that the hypercall method which is
>>> used in our productive kernel is better than this cpucfg method.
>>> Because hypercall is more simple and straightforward, plus we don't
>>> worry about conflicting with the real hardware.
>> No, I do not think so. cpucfg is simper than hypercall, hypercall can
>> be in effect when system runs in guest mode. In some scenario like TCG
>> mode, hypercall is illegal intruction, however cpucfg can work.
> Nearly all architectures use hypercall except x86 for its historical
Only x86 support multiple hypervisors and there is multiple hypervisor
in x86 only. It is an advantage, not historical reason.
> reasons. If we use CPUCFG, then the hypervisor information is
> unnecessarily leaked to userspace, and this may be a security issue.
> Meanwhile, I don't think TCG mode needs PV features.
Besides PV features, there is other features different with real hw such
as virtio device, virtual interrupt controller.
Regards
Bibo Mao
>
> I consulted with Jiaxun before, and maybe he can give some more comments.
>
>>
>> Extioi virtualization extension will be added later, cpucfg can be used
>> to get extioi features. It is unlikely that extioi driver depends on
>> PARA_VIRT macro if hypercall is used to get features.
> CPUCFG is per-core information, if we really need something about
> extioi, it should be in iocsr (LOONGARCH_IOCSR_FEATURES).
>
>
> Huacai
>
>>
>> Regards
>> Bibo Mao
>>
>>>
>>> Huacai
>>>
>>>>
>>>> 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 | 46 +++++++++++++++++---------
>>>> 3 files changed, 41 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/arch/loongarch/include/asm/inst.h b/arch/loongarch/include/asm/inst.h
>>>> index d8f637f9e400..ad120f924905 100644
>>>> --- a/arch/loongarch/include/asm/inst.h
>>>> +++ b/arch/loongarch/include/asm/inst.h
>>>> @@ -67,6 +67,7 @@ enum reg2_op {
>>>> revhd_op = 0x11,
>>>> extwh_op = 0x16,
>>>> extwb_op = 0x17,
>>>> + cpucfg_op = 0x1b,
>>>> iocsrrdb_op = 0x19200,
>>>> iocsrrdh_op = 0x19201,
>>>> iocsrrdw_op = 0x19202,
>>>> diff --git a/arch/loongarch/include/asm/loongarch.h b/arch/loongarch/include/asm/loongarch.h
>>>> index 46366e783c84..a1d22e8b6f94 100644
>>>> --- a/arch/loongarch/include/asm/loongarch.h
>>>> +++ b/arch/loongarch/include/asm/loongarch.h
>>>> @@ -158,6 +158,16 @@
>>>> #define CPUCFG48_VFPU_CG BIT(2)
>>>> #define CPUCFG48_RAM_CG BIT(3)
>>>>
>>>> +/*
>>>> + * cpucfg index area: 0x40000000 -- 0x400000ff
>>>> + * SW emulation for KVM hypervirsor
>>>> + */
>>>> +#define CPUCFG_KVM_BASE 0x40000000UL
>>>> +#define CPUCFG_KVM_SIZE 0x100
>>>> +#define CPUCFG_KVM_SIG CPUCFG_KVM_BASE
>>>> +#define KVM_SIGNATURE "KVM\0"
>>>> +#define CPUCFG_KVM_FEATURE (CPUCFG_KVM_BASE + 4)
>>>> +
>>>> #ifndef __ASSEMBLY__
>>>>
>>>> /* CSR */
>>>> diff --git a/arch/loongarch/kvm/exit.c b/arch/loongarch/kvm/exit.c
>>>> index 923bbca9bd22..6a38fd59d86d 100644
>>>> --- a/arch/loongarch/kvm/exit.c
>>>> +++ b/arch/loongarch/kvm/exit.c
>>>> @@ -206,10 +206,37 @@ int kvm_emu_idle(struct kvm_vcpu *vcpu)
>>>> return EMULATE_DONE;
>>>> }
>>>>
>>>> -static int kvm_trap_handle_gspr(struct kvm_vcpu *vcpu)
>>>> +static int kvm_emu_cpucfg(struct kvm_vcpu *vcpu, larch_inst inst)
>>>> {
>>>> int rd, rj;
>>>> unsigned int index;
>>>> +
>>>> + rd = inst.reg2_format.rd;
>>>> + rj = inst.reg2_format.rj;
>>>> + ++vcpu->stat.cpucfg_exits;
>>>> + index = vcpu->arch.gprs[rj];
>>>> +
>>>> + /*
>>>> + * By LoongArch Reference Manual 2.2.10.5
>>>> + * Return value is 0 for undefined cpucfg index
>>>> + */
>>>> + switch (index) {
>>>> + case 0 ... (KVM_MAX_CPUCFG_REGS - 1):
>>>> + vcpu->arch.gprs[rd] = vcpu->arch.cpucfg[index];
>>>> + break;
>>>> + case CPUCFG_KVM_SIG:
>>>> + vcpu->arch.gprs[rd] = *(unsigned int *)KVM_SIGNATURE;
>>>> + break;
>>>> + default:
>>>> + vcpu->arch.gprs[rd] = 0;
>>>> + break;
>>>> + }
>>>> +
>>>> + return EMULATE_DONE;
>>>> +}
>>>> +
>>>> +static int kvm_trap_handle_gspr(struct kvm_vcpu *vcpu)
>>>> +{
>>>> unsigned long curr_pc;
>>>> larch_inst inst;
>>>> enum emulation_result er = EMULATE_DONE;
>>>> @@ -224,21 +251,8 @@ static int kvm_trap_handle_gspr(struct kvm_vcpu *vcpu)
>>>> er = EMULATE_FAIL;
>>>> switch (((inst.word >> 24) & 0xff)) {
>>>> case 0x0: /* CPUCFG GSPR */
>>>> - if (inst.reg2_format.opcode == 0x1B) {
>>>> - rd = inst.reg2_format.rd;
>>>> - rj = inst.reg2_format.rj;
>>>> - ++vcpu->stat.cpucfg_exits;
>>>> - index = vcpu->arch.gprs[rj];
>>>> - er = EMULATE_DONE;
>>>> - /*
>>>> - * By LoongArch Reference Manual 2.2.10.5
>>>> - * return value is 0 for undefined cpucfg index
>>>> - */
>>>> - if (index < KVM_MAX_CPUCFG_REGS)
>>>> - vcpu->arch.gprs[rd] = vcpu->arch.cpucfg[index];
>>>> - else
>>>> - vcpu->arch.gprs[rd] = 0;
>>>> - }
>>>> + if (inst.reg2_format.opcode == cpucfg_op)
>>>> + er = kvm_emu_cpucfg(vcpu, inst);
>>>> break;
>>>> case 0x4: /* CSR{RD,WR,XCHG} GSPR */
>>>> er = kvm_handle_csr(vcpu, inst);
>>>> --
>>>> 2.39.3
>>>>
>>
>>
Powered by blists - more mailing lists