On (21/07/12 16:42), Marc Zyngier wrote: > > > > PV-vcpu-state is a per-CPU struct, which, for the time being, > > holds boolean `preempted' vCPU state. During the startup, > > given that host supports PV-state, each guest vCPU sends > > a pointer to its per-CPU variable to the host as a payload > > What is the expected memory type for this memory region? What is its > life cycle? Where is it allocated from? Guest per-CPU area, which physical addresses is shared with the host. > > with the SMCCC HV call, so that host can update vCPU state > > when it puts or loads vCPU. > > > > This has impact on the guest's scheduler: > > > > [..] > > wake_up_process() > > try_to_wake_up() > > select_task_rq_fair() > > available_idle_cpu() > > vcpu_is_preempted() > > > > Some sched benchmarks data is available on the github page [0]. > > > > [0] https://github.com/sergey-senozhatsky/arm64-vcpu_is_preempted > > Please include these results in the cover letter. I tend to reply to > email while offline, and I can't comment on GH. ACK. > > +struct vcpu_state { > > If this is KVM specific (which it most likely is), please name-space > it correctly, and move it to a KVM-specific location. ACK. > > + bool preempted; > > + u8 reserved[63]; > > Why 63? Do you attach any particular meaning to a 64byte structure > (and before you say "cache line size", please look at some of the > cache line sizes we have to deal with...). We do have some future plans to share some bits of the guest's context with the host. > This should also be versioned from day-1, one way or another. Makes sense. > > +}; > > + > > #ifdef CONFIG_PARAVIRT > > #include <linux/static_call_types.h> > > > > @@ -20,8 +25,22 @@ static inline u64 paravirt_steal_clock(int cpu) > > > > int __init pv_time_init(void); > > > > +bool dummy_vcpu_is_preempted(unsigned int cpu); > > + > > +extern struct static_key pv_vcpu_is_preempted_enabled; > > +DECLARE_STATIC_CALL(pv_vcpu_is_preempted, dummy_vcpu_is_preempted); > > + > > +static inline bool paravirt_vcpu_is_preempted(unsigned int cpu) > > +{ > > + return static_call(pv_vcpu_is_preempted)(cpu); > > +} > > + > > +int __init pv_vcpu_state_init(void); > > + > > #else > > > > +#define pv_vcpu_state_init() do {} while (0) > > + > > #define pv_time_init() do {} while (0) > > > > #endif // CONFIG_PARAVIRT > > diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c > > index 75fed4460407..d8fc46795d94 100644 > > --- a/arch/arm64/kernel/paravirt.c > > +++ b/arch/arm64/kernel/paravirt.c > > @@ -40,6 +40,11 @@ struct pv_time_stolen_time_region { > > > > static DEFINE_PER_CPU(struct pv_time_stolen_time_region, stolen_time_region); > > > > +static DEFINE_PER_CPU(struct vcpu_state, vcpus_states); > > nit: there is only one 'state' structure per CPU, so I'd prefer the > singular form. ACK. > > +struct static_key pv_vcpu_is_preempted_enabled; > > + > > +DEFINE_STATIC_CALL(pv_vcpu_is_preempted, dummy_vcpu_is_preempted); > > + > > static bool steal_acc = true; > > static int __init parse_no_stealacc(char *arg) > > { > > @@ -165,3 +170,92 @@ int __init pv_time_init(void) > > > > return 0; > > } > > + > > +bool dummy_vcpu_is_preempted(unsigned int cpu) > > Why does this have to be global? I think this can be moved away from the header, so then we don't need to DECLARE_STATIC_CALL() with a dummy function. > > +static bool has_pv_vcpu_state(void) > > +{ > > + struct arm_smccc_res res; > > + > > + /* To detect the presence of PV time support we require SMCCC 1.1+ */ > > + if (arm_smccc_1_1_get_conduit() == SMCCC_CONDUIT_NONE) > > + return false; > > + > > + arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID, > > + ARM_SMCCC_HV_PV_VCPU_STATE_FEATURES, > > + &res); > > + > > + if (res.a0 != SMCCC_RET_SUCCESS) > > + return false; > > + return true; > > Please move all this over the the KVM-specific discovery mechanism. Will take a look. > > +static int __pv_vcpu_state_hook(unsigned int cpu, int event) > > +{ > > + struct arm_smccc_res res; > > + struct vcpu_state *st; > > + > > + st = &per_cpu(vcpus_states, cpu); > > + arm_smccc_1_1_invoke(event, virt_to_phys(st), &res); > > + if (res.a0 != SMCCC_RET_SUCCESS) > > + return -EINVAL; > > + return 0; > > +} > > + > > +static int vcpu_state_init(unsigned int cpu) > > +{ > > + int ret = __pv_vcpu_state_hook(cpu, ARM_SMCCC_HV_PV_VCPU_STATE_INIT); > > + > > + if (ret) > > + pr_warn("Unable to ARM_SMCCC_HV_PV_STATE_INIT\n"); > > pr_warn_once(), please. ACK. _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
