linux-kernel - Re: [PATCH v15 27/30] LoongArch: KVM: Implement vcpu world switch

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]

Date:   Wed, 28 Jun 2023 18:11:35 +0800
From:   bibo mao <maobibo@...ngson.cn>
To:     WANG Xuerui <kernel@...0n.name>,
        zhaotianrui <zhaotianrui@...ngson.cn>,
        Jinyang He <hejinyang@...ngson.cn>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Huacai Chen <chenhuacai@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        loongarch@...ts.linux.dev, Jens Axboe <axboe@...nel.dk>,
        Mark Brown <broonie@...nel.org>,
        Alex Deucher <alexander.deucher@....com>,
        Oliver Upton <oliver.upton@...ux.dev>,
        Xi Ruoyao <xry111@...111.site>, tangyouling@...ngson.cn
Subject: Re: [PATCH v15 27/30] LoongArch: KVM: Implement vcpu world switch



在 2023/6/28 17:51, WANG Xuerui 写道:
> Hi,
> 
> On 2023/6/28 16:34, zhaotianrui wrote:
>>
>> 在 2023/6/28 上午11:42, Jinyang He 写道:
>>> On 2023-06-26 16:47, Tianrui Zhao wrote:
>>>
>>>> [snip]
>>>
>>>> +    ldx.d   t0, t1, t0
>>>> +    csrwr    t0, LOONGARCH_CSR_PGDL
>>>> +
>>>> +    /* Mix GID and RID */
>>>> +    csrrd        t1, LOONGARCH_CSR_GSTAT
>>>> +    bstrpick.w    t1, t1, CSR_GSTAT_GID_SHIFT_END, CSR_GSTAT_GID_SHIFT
>>>> +    csrrd        t0, LOONGARCH_CSR_GTLBC
>>>> +    bstrins.w    t0, t1, CSR_GTLBC_TGID_SHIFT_END, CSR_GTLBC_TGID_SHIFT
>>>> +    csrwr        t0, LOONGARCH_CSR_GTLBC
>>>> +
>>>> +    /*
>>>> +     * Switch to guest:
>>>> +     *  GSTAT.PGM = 1, ERRCTL.ISERR = 0, TLBRPRMD.ISTLBR = 0
>>>> +     *  ertn
>>>> +     */
>>>> +
>>>> +    /*
>>>> +     * Enable intr in root mode with future ertn so that host interrupt
>>>> +     * can be responsed during VM runs
>>>> +     * guest crmd comes from separate gcsr_CRMD register
>>>> +     */
>>>> +    ori    t0, zero, CSR_PRMD_PIE
>>> li.w t0, CSR_PRMD_PIE
>> Thanks for your advice, and I think it need not to replace it with "li.w" there, as it has the same meaning with "ori" instruction, and "ori" instruction is simple and clear enough. The same as the following "move" instructions. What do you think of it.
> 
> Just my 2c: I'd agree that pseudo-instructions should be used wherever possible and helping readability.
"lu12i.w+srli.w" can be replaced by "li.w t0, KVM_GPGD" 
we accept the suggestion two instructions should be replaced with pseudo-instruction.

For the instruction "ori    t0, zero, CSR_PRMD_PIE"
what is advantage of this pseudo-instruction
    li.w t0, CSR_PRMD_PIE

is "ori t0, zero, CSR_PRMD_PIE" hard to understand? It is basic arithmetic instr and easy to understand also. To be frank I do not see the advantage of using li.w, also there is no document that pseudo-instruction should be used with high priority.

Regards
Bibo Mao
> 
> FYI there were similar usages way before, but all were cleaned up with my previous commit 57ce5d3eefac ("LoongArch: Use the "move" pseudo-instruction where applicable").
> 
> Such usages apparently came from an era when the LoongArch toolchains didn't support any pseudo-instruction, and are less intuitive especially for someone not familiar with LoongArch assembly. Given that familiarity with LoongArch won't be widespread (unlike with e.g. RISC-V that are adopted more widely), we should optimize for readability when writing code; it's also a best practice in general because code is read way more often than written, and people care about semantics not unnecessary details like "how are moves like this or that materialized".
>