Re: [PATCH v2 01/29] LoongArch: KVM: Add kvm related header files

[Tianrui Zhao <zhaotianrui@loongson.cn>]

在 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