[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 3/4] xen/arm: set vpidr on the pcpu where the vcpu will run
Hi Stefano, On 16/02/2018 20:59, Stefano Stabellini wrote: On Fri, 16 Feb 2018, Julien Grall wrote:On 16/02/2018 20:31, Stefano Stabellini wrote:On Fri, 16 Feb 2018, Julien Grall wrote:Hi Stefano, On 15/02/18 23:16, Stefano Stabellini wrote:On big.LITTLE systems not all cores have the same midr. Instead of initializing the vpidr to the boot cpu midr, set it to the value of the midr of the pcpu where the vcpu will run. This way, assuming that the vcpu has been created with the right pcpu affinity, the guest will be able to read the right vpidr value, matching the one of the physical cpu. Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx> --- xen/arch/arm/domain.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 532e824..280125f 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -315,6 +315,22 @@ static void schedule_tail(struct vcpu *prev) static void continue_new_vcpu(struct vcpu *prev) { current->arch.actlr = READ_SYSREG32(ACTLR_EL1); + /* + * Default the virtual ID to match the physical. + * + * In case the big.LITTLE systems, a guest should be created with + * cpu affinity set so that all vcpus run on the same kind of pcpus. + * Warn if it is not the case.continue_new_vcpu is only called once at domain creation. So this looks pointless to check that here and probably in ctxt_switch_to. But I don't want to see such check at every context switch. This is expensive and we should not impact all the platforms for the benefits of an unsafe configuration. If you really want to do that, then it should only be done when the vCPU is migrating. That will reduce a lot the performance impact.I don't want a check for every context switch either. I added it here because continue_new_vcpu is only called once per vcpu at domain creation -- it is a one time check. vcpus are supposed to be pinned (or cpu affinity specified) anyway, so I thought I wouldn't add the check in vcpu_migrate too. In any case, I am also happy to remove the check completely, as we have already warned the user enough.If you agree that continue_new_vcpu is only called once per vCPU. Then I am not sure to understand the purpose of the check. What are you trying to warn the user with that?The intention was to warn the user if she made a mistake with vcpu pinning and/or cpu affinity. Oh that is current->domain->arch.vpidr and not vCPU. Sorry for that.vpidr should be per vCPU. It is very dangerous to recommend the user to pin there all vCPUs of a domain to either only big or LITTLE. This is even worst than what we have today. Even with this series and vcpu pinning, I assumed that only scenarios with vcpus assigned to pcpus of the same kind are allowed (see my other reply). Thus, I added this check to test once at boot that all vcpus in a domain have the same actlr. That's plain wrong. You really can't assume that same actlr means same type of CPUs. Imagine they are RES0 on both. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |