Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <1218d3f9-4955-7176-afbd-a0dfa0bd7565@loongson.cn>
Date:   Tue, 23 May 2023 10:54:23 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH v10 00/30] Add KVM LoongArch support
Content-Language: en-US
To:     WANG Xuerui <kernel@xen0n.name>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Huacai Chen <chenhuacai@kernel.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        loongarch@lists.linux.dev, linux-kernel@vger.kernel.org,
        kvm@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
        Mark Brown <broonie@kernel.org>,
        Alex Deucher <alexander.deucher@amd.com>,
        Oliver Upton <oliver.upton@linux.dev>,
        Xi Ruoyao <xry111@xry111.site>,
        Tianrui Zhao <zhaotianrui@loongson.cn>
References: <20230515021522.2445551-1-zhaotianrui@loongson.cn>
 <02f07d8e-e1c2-2ec0-59c3-f5b4ef0463dc@loongson.cn>
 <4529ee5b-364a-7819-c727-71cf94057b8b@xen0n.name>
 <99371487-717a-64d6-1c3d-aaeaee6f20db@loongson.cn>
 <90b2fc60-af26-4ba6-f775-7db2514a62f4@xen0n.name>
From:   maobibo <maobibo@loongson.cn>
In-Reply-To: <90b2fc60-af26-4ba6-f775-7db2514a62f4@xen0n.name>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk


在 2023/5/22 10:37, WANG Xuerui 写道:
> On 2023/5/22 09:39, maobibo wrote:
>>
>>
>> 在 2023/5/21 18:22, WANG Xuerui 写道:
>>> On 2023/5/18 10:56, maobibo wrote:
>>>> <snip>
>>> (BTW, how do people usually deal with pre-release hardware wit documentation not out yet? I suppose similar situations like this should turn up fairly often.)
>> Manual is actually one issue, however it does not prevent the review
>> process. There are some drivers for *fruit* devices, I can not find
>> the hw manual also.  With the manual, it helps to review and points
>> out the further and detailed issues.
> 
> There's a *slight* difference: the certain vendor you've mentioned is historically uncooperative in providing the documentation, so outside contributors had to reverse-engineer and document the HW themselves; but in Loongson's case, you *are* the vendor, so you are probably in a position that can make everyone's life easier by at least pushing for the docs release...
> 
>>>
>>> Aside from this, there's another point: use of undocumented instructions in raw form with ".word". This currently doesn't work in LLVM/Clang <snip>
>> As for one new architecture, it is normal to use .word or .insn, instruction
>> will update for the first few years and also compiler may be not supported
>> timely. The other arch has the same phenomenon if you grep "\.insn", also
>> llvm on LoongArch supports ".word" directives.
>>
>> After three or five years, we will remove these ".insn" macro when hw and
>> compiler is matured.
> 
> Sorry for the confusion at my side; `.word` certainly works, what doesn't work currently seems to be the `parse_r` helper. I know because I've tried in the last week with latest LLVM/Clang snapshot. And you can't write ergonomic inline asm with proper register allocator awareness without the helper; the LoongArch assembler isn't capable of assembling in a certain encoding format. With RISC-V `.insn` you can do things like `.insn r 0xNN, 0, 0, a0, a1, a2`, but you cannot simply e.g. express gcsrxchg with `.insn DJK 0x05000000, a0, a1, a2` because no such instruction format convention has been standardized. (The notation demonstrated here is taken from [1].)

I hate parse_r helper also, it is hard to understand, the kernel about
LoongArch has the same issue. How about using a fixed register like this?

/* GCSR */
static __always_inline u64 gcsr_read(u32 reg)
{
	u64 val = 0;

	BUILD_BUG_ON(!__builtin_constant_p(reg));
	/* Instructions will be available in binutils later */
	asm volatile (
		"parse_r __reg, %[val]\n\t"
		/*
		 * read val from guest csr register %[reg]
		 * gcsrrd %[val], %[reg]
		 */
		".word 0x5 << 24 | %[reg] << 10 | 0 << 5 | __reg\n\t"
		: [val] "+r" (val)
		: [reg] "i" (reg)
		: "memory");

	return val;
}

/* GCSR */
static __always_inline u64 gcsr_read(u32 reg)
{
        register unsigned long val asm("t0");

        BUILD_BUG_ON(!__builtin_constant_p(reg));
        /* Instructions will be available in binutils later */
        asm volatile (
                "parse_r __reg, %[val]\n\t"
                /*
                 * read val from guest csr register %[reg]
                 * gcsrrd %[val], %[reg]
                 */
                ".word 0x5 << 24 | %[reg] << 10 | 0 << 5 | 12 \n\t"
                : : [reg] "i" (reg)
                : "memory", "t0");

        return val;
} 

Regards
Bibo, Mao
> 
> In any case, it seems best to at least wait for the documentation release a little bit, or you should state clearly that this is not going to happen soon, so people can properly manage their expectation and prioritize. (For example, if I know docs and/or assembler support for the virtualization extension won't come soon, then I'd work on supporting the .word idiom before other things. Otherwise there are more important things than that.)
> 
> [1]: https://github.com/loongson/LoongArch-Documentation/pull/56
>