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

[Tianrui Zhao <zhaotianrui@xxxxxxxxxxx>]

在 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