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;
Subject: Re: [PATCH v1 1/4] selftests: kvm: Add kvm selftests header files for
 LoongArch
To:     Sean Christopherson <seanjc@google.com>
Cc:     Shuah Khan <shuah@kernel.org>, Paolo Bonzini <pbonzini@redhat.com>,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        Vishal Annapurve <vannapurve@google.com>,
        Huacai Chen <chenhuacai@kernel.org>,
        WANG Xuerui <kernel@xen0n.name>, loongarch@lists.linux.dev,
        Peter Xu <peterx@redhat.com>,
        Vipin Sharma <vipinsh@google.com>, maobibo@loongson.cn
References: <20230801020206.1957986-1-zhaotianrui@loongson.cn>
 <20230801020206.1957986-2-zhaotianrui@loongson.cn>
 <ZMqL7qPyngxOH4Y0@google.com>
From:   zhaotianrui <zhaotianrui@loongson.cn>
Message-ID: <06a4e9f0-483c-e79c-fc64-7e9e0ce7348d@loongson.cn>
Date:   Thu, 3 Aug 2023 11:30:34 +0800
User-Agent: Mozilla/5.0 (X11; Linux loongarch64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <ZMqL7qPyngxOH4Y0@google.com>
Content-Type: text/plain; charset=gbk; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk


?? 2023/8/3 ????1:01, Sean Christopherson д??:
> On Tue, Aug 01, 2023, Tianrui Zhao wrote:
>> Add kvm selftests header files for LoongArch, including processor.h,
>> sysreg.h, and kvm_util_base.h. Those mainly contain LoongArch CSR
>> register defines and page table information.
>>
>> Based-on: <20230720062813.4126751-1-zhaotianrui@loongson.cn>
>> Signed-off-by: Tianrui Zhao <zhaotianrui@loongson.cn>
>> ---
>>   .../selftests/kvm/include/kvm_util_base.h     |  5 ++
>>   .../kvm/include/loongarch/processor.h         | 28 ++++++
>>   .../selftests/kvm/include/loongarch/sysreg.h  | 89 +++++++++++++++++++
>>   3 files changed, 122 insertions(+)
>>   create mode 100644 tools/testing/selftests/kvm/include/loongarch/processor.h
>>   create mode 100644 tools/testing/selftests/kvm/include/loongarch/sysreg.h
>>
>> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
>> index 07732a157ccd..8747127e0bab 100644
>> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
>> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
>> @@ -197,6 +197,11 @@ extern enum vm_guest_mode vm_mode_default;
>>   #define MIN_PAGE_SHIFT			12U
>>   #define ptes_per_page(page_size)	((page_size) / 8)
>>   
>> +#elif defined(__loongarch__)
>> +#define VM_MODE_DEFAULT			VM_MODE_P36V47_16K
>> +#define MIN_PAGE_SHIFT			14U
>> +#define ptes_per_page(page_size)	((page_size) / 8)
>> +
>>   #endif
>>   
>>   #define MIN_PAGE_SIZE		(1U << MIN_PAGE_SHIFT)
>> diff --git a/tools/testing/selftests/kvm/include/loongarch/processor.h b/tools/testing/selftests/kvm/include/loongarch/processor.h
>> new file mode 100644
>> index 000000000000..d67796af51a0
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/include/loongarch/processor.h
>> @@ -0,0 +1,28 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * LoongArch processor specific defines
> Nit, my preference is to not bother with these types of comments, it should be
> quite obvious from the file name that that everything in here is LoongArch
> specific.
Thanks, I will simplify this comment.
>
>> + */
>> +#ifndef SELFTEST_KVM_PROCESSOR_H
>> +#define SELFTEST_KVM_PROCESSOR_H
>> +
>> +#include <linux/compiler.h>
>> +#define _PAGE_VALID_SHIFT	0
>> +#define _PAGE_DIRTY_SHIFT	1
>> +#define _PAGE_PLV_SHIFT		2  /* 2~3, two bits */
>> +#define _CACHE_SHIFT		4  /* 4~5, two bits */
>> +#define _PAGE_PRESENT_SHIFT	7
>> +#define _PAGE_WRITE_SHIFT	8
>> +
>> +#define PLV_KERN		0
>> +#define PLV_USER		3
>> +#define PLV_MASK		0x3
>> +
>> +#define _PAGE_VALID		(0x1UL << _PAGE_VALID_SHIFT)
>> +#define _PAGE_PRESENT		(0x1UL << _PAGE_PRESENT_SHIFT)
>> +#define _PAGE_WRITE		(0x1UL << _PAGE_WRITE_SHIFT)
>> +#define _PAGE_DIRTY		(0x1UL << _PAGE_DIRTY_SHIFT)
>> +#define _PAGE_USER		(PLV_USER << _PAGE_PLV_SHIFT)
>> +#define __READABLE		(_PAGE_VALID)
>> +#define __WRITEABLE		(_PAGE_DIRTY | _PAGE_WRITE)
>> +#define _CACHE_CC		(0x1UL << _CACHE_SHIFT) /* Coherent Cached */
>> +#endif
>> diff --git a/tools/testing/selftests/kvm/include/loongarch/sysreg.h b/tools/testing/selftests/kvm/include/loongarch/sysreg.h
>> new file mode 100644
>> index 000000000000..04f53674c9d8
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/include/loongarch/sysreg.h
> Any reason these can't simply go in processor.h?  Neither file is particular large,
> especially for CPU definition files.
Thanks, I will move the contents of sysreg.h into processor.h to make 
the file easier.
>
>> @@ -0,0 +1,89 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +#ifndef SELFTEST_KVM_SYSREG_H
>> +#define SELFTEST_KVM_SYSREG_H
>> +
>> +/*
>> + * note that this declaration raises a checkpatch warning, but
>> + * no good way to avoid it.
>> + */
> Definitely drop this comment, once the patch is applied the fact that checkpatch
> complains is irrelevant.
Ok, I will drop this comment.

Thanks
Tianrui Zhao
>
>> +#define zero	$r0