Re: [Xen-devel] [PATCH v3 4/6] xen/arm: set VPIDR based on the MIDR value of the underlying pCPU

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

Something is wrong with the threading of this series. Most of the 
patches are threaded below #1 rather than #0.
On 01/03/18 23:26, Stefano Stabellini wrote:
> On big.LITTLE systems not all cores have the same MIDR. Instead of
> storing only one VPIDR per domain, initialize 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>
Reviewed-by: Julien Grall <julien.grall@xxxxxxx>

Cheers,

> ---
>
> Changes in v3:
> - improve commit message
> - do not store vpidr in struct vcpu
>
> Changes in v2:
> - remove warning message
> - make vpidr per vcpu
> ---
>   xen/arch/arm/domain.c        | 8 ++++----
>   xen/arch/arm/vcpreg.c        | 4 ++--
>   xen/include/asm-arm/domain.h | 3 ---
>   3 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 5e76809..545bbf6 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -172,6 +172,8 @@ static void ctxt_switch_from(struct vcpu *p)
>   
>   static void ctxt_switch_to(struct vcpu *n)
>   {
> +    uint32_t vpidr;
> +
>       /* When the idle VCPU is running, Xen will always stay in hypervisor
>        * mode. Therefore we don't need to restore the context of an idle VCPU.
>        */
> @@ -180,7 +182,8 @@ static void ctxt_switch_to(struct vcpu *n)
>   
>       p2m_restore_state(n);
>   
> -    WRITE_SYSREG32(n->domain->arch.vpidr, VPIDR_EL2);
> +    vpidr = READ_SYSREG32(MIDR_EL1);
> +    WRITE_SYSREG32(vpidr, VPIDR_EL2);
>       WRITE_SYSREG(n->arch.vmpidr, VMPIDR_EL2);
>   
>       /* VGIC */
> @@ -595,9 +598,6 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags,
>       if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>           goto fail;
>   
> -    /* Default the virtual ID to match the physical */
> -    d->arch.vpidr = boot_cpu_data.midr.bits;
> -
>       clear_page(d->shared_info);
>       share_xen_page_with_guest(
>           virt_to_page(d->shared_info), d, XENSHARE_writable);
> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
> index e363183..b04d996 100644
> --- a/xen/arch/arm/vcpreg.c
> +++ b/xen/arch/arm/vcpreg.c
> @@ -230,7 +230,6 @@ void do_cp14_32(struct cpu_user_regs *regs, const union hsr 
> hsr)
>   {
>       const struct hsr_cp32 cp32 = hsr.cp32;
>       int regidx = cp32.reg;
> -    struct domain *d = current->domain;
>   
>       if ( !check_conditional_instr(regs, hsr) )
>       {
> @@ -295,7 +294,8 @@ void do_cp14_32(struct cpu_user_regs *regs, const union hsr 
> hsr)
>            *  - Variant and Revision bits match MDIR
>            */
>           val = (1 << 24) | (5 << 16);
> -        val |= ((d->arch.vpidr >> 20) & 0xf) | (d->arch.vpidr & 0xf);
> +        val |= ((current_cpu_data.midr.bits >> 20) & 0xf) |
> +                (current_cpu_data.midr.bits & 0xf);
>           set_user_reg(regs, regidx, val);
>   
>           break;
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 4fe189b..0dd8c95 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -63,9 +63,6 @@ struct arch_domain
>           RELMEM_done,
>       } relmem;
>   
> -    /* Virtual CPUID */
> -    uint32_t vpidr;
> -
>       struct {
>           uint64_t offset;
>       } phys_timer_base;
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel