Subject: Re: [PATCH v2 01/29] LoongArch: KVM: Add kvm related header files
To:     Paolo Bonzini <pbonzini@redhat.com>
References: <20230220065735.1282809-1-zhaotianrui@loongson.cn>
 <20230220065735.1282809-2-zhaotianrui@loongson.cn>
 <2b047b75-7397-0cce-e7af-ebba67ae2561@redhat.com>
Cc:     Huacai Chen <chenhuacai@kernel.org>,
        WANG Xuerui <kernel@xen0n.name>,
        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>, maobibo@loongson.cn
From:   Tianrui Zhao <zhaotianrui@loongson.cn>
Message-ID: <c9dada46-829c-d3cd-c3b6-68a570552e84@loongson.cn>
Date:   Tue, 21 Feb 2023 10:56:25 +0800
User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <2b047b75-7397-0cce-e7af-ebba67ae2561@redhat.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


在 2023年02月21日 02:22, Paolo Bonzini 写道:
> On 2/20/23 07:57, Tianrui Zhao wrote:
>> +
>> +/* Resume Flags */
>> +#define RESUME_FLAG_DR        (1<<0)    /* Reload guest nonvolatile 
>> state? */
>> +#define RESUME_FLAG_HOST    (1<<1)    /* Resume host? */
>> +
>> +#define RESUME_GUEST        0
>> +#define RESUME_GUEST_DR        RESUME_FLAG_DR
>> +#define RESUME_HOST        RESUME_FLAG_HOST
>> +
>
> Most of this code is dead, I'll give more instructions in a reply to 
> patch 8.
>
>> +    unsigned long guest_eentry;
>> +    unsigned long host_eentry;
>> +    int (*vcpu_run)(struct kvm_run *run, struct kvm_vcpu *vcpu);
>> +    int (*handle_exit)(struct kvm_run *run, struct kvm_vcpu *vcpu);
>> +
>> +    /* Host registers preserved across guest mode execution */
>> +    unsigned long host_stack;
>> +    unsigned long host_gp;
>> +    unsigned long host_pgd;
>> +    unsigned long host_pgdhi;
>> +    unsigned long host_entryhi;
>> +
>> +    /* Host CSR registers used when handling exits from guest */
>> +    unsigned long badv;
>> +    unsigned long host_estat;
>> +    unsigned long badi;
>> +    unsigned long host_ecfg;
>> +    unsigned long host_percpu;
>> +
>> +    /* GPRS */
>> +    unsigned long gprs[32];
>> +    unsigned long pc;
>> +
>> +    /* FPU State */
>> +    struct loongarch_fpu fpu FPU_ALIGN;
>> +    /* Which auxiliary state is loaded (KVM_LOONGARCH_AUX_*) */
>> +    unsigned int aux_inuse;
>> +
>> +    /* CSR State */
>> +    struct loongarch_csrs *csr;
>> +
>> +    /* GPR used as IO source/target */
>> +    u32 io_gpr;
>> +
>> +    struct hrtimer swtimer;
>> +    /* Count timer control KVM register */
>> +    u32 count_ctl;
>> +
>> +    /* Bitmask of exceptions that are pending */
>> +    unsigned long irq_pending;
>> +    /* Bitmask of pending exceptions to be cleared */
>> +    unsigned long irq_clear;
>> +
>> +    /* Cache some mmu pages needed inside spinlock regions */
>> +    struct kvm_mmu_memory_cache mmu_page_cache;
>> +
>> +    /* vcpu's vpid is different on each host cpu in an smp system */
>> +    u64 vpid[NR_CPUS];
>
> In _kvm_check_vmid(), you already have
>
> +    if (migrated || (ver != old)) {
> +        _kvm_update_vpid(vcpu, cpu);
> +        trace_kvm_vpid_change(vcpu, vcpu->arch.vpid[cpu]);
> +    }
>
> so a vpid will never be recycled if a vCPU migrates from physical CPU 
> A to B and back to A.
>
> So please keep the current VPID in the per-cpu struct vmcs, and you 
> can just copy it from there in _kvm_check_vmid().

Thanks,  that is to say we should remove the vpid[NR_CPUS] array and it 
is enough to use only one vpid variable?

Thanks
Tianrui Zhao

>
>> +    /* Period of stable timer tick in ns */
>> +    u64 timer_period;
>> +    /* Frequency of stable timer in Hz */
>> +    u64 timer_mhz;
>> +    /* Stable bias from the raw time */
>> +    u64 timer_bias;
>> +    /* Dynamic nanosecond bias (multiple of timer_period) to avoid 
>> overflow */
>> +    s64 timer_dyn_bias;
>> +    /* Save ktime */
>> +    ktime_t stable_ktime_saved;
>> +
>> +    u64 core_ext_ioisr[4];
>> +
>> +    /* Last CPU the VCPU state was loaded on */
>> +    int last_sched_cpu;
>> +    /* Last CPU the VCPU actually executed guest code on */
>> +    int last_exec_cpu;
>> +
>> +    u8 fpu_enabled;
>
> This field is always true, please remove it.

Thanks, i will remove this variable.

Thanks
Tianrui Zhao

>
>> +    struct kvm_guest_debug_arch guest_debug;
>
> This struct is empty, please remove it.

Ok, I will remove it.

Thanks
Tianrui Zhao

>
> Paolo