On 6/22/2016 10:41 PM, Julien Grall wrote:


On 22/06/2016 20:37, Corneliu ZUZU wrote:
I've also realized that it's a bit complicated to avoid writing HCR from
2 places.
That's because:
- p2m_restore_state is part of the process of switching to another vCPU
and the HCR write _must be committed_ here because other components
depend on that, like address-translation functions
- I want vm_event_vcpu_enter to be called _after_ the switch to the vCPU
is completed
- I want HCR_TVM to be set in vm_event_vcpu_enter because setting
necessary traps _for cr vm-events_ to work should be done there (setting
HCR_TVM bit makes sense to be there and the purpose is to centralize
operations such as this for code comprehensibility; also, on the X86
counterpart a similar operation is done for trapping CR3, so it would be
nice to keep the symmetry)

Would it be such a stretch to have HCR written in 2 places? (the second
time happens rarely anyway: it's unlikely(..) to have to do the write in
vm_event_vcpu_enter)

Not really. It was mostly to avoid setting/clearing HCR bits in different place in the code. It makes more difficult to know what is the final result of the register.

Anyway, let's skip it for now, if it is too difficult.

Regards,


Then perhaps something like the following would be suitable:

1. store hcr in arch_domain (register_t hcr)

2. add a function in asm-arm/processor.h (or where else?) which only does:
    static inline void update_hcr(struct domain *d)
    {
        WRITE_SYSREG(d->arch.hcr, HCR_EL2);
        isb();
    }

3.  modify p2m_restore_state to do:
    n->domain->arch.hcr &= ~HCR_VM;
    update_hcr(n->domain);
    p2m_load_VTTBR(n->domain);

    n->domain->arch.hcr |= HCR_VM;

    if ( is_32bit_domain(n->domain) )
        n->domain->arch.hcr &= ~HCR_RW;
    else
        n->domain->arch.hcr |= HCR_RW;

    update_hcr(n->domain);

    WRITE_SYSREG(n->arch.sctlr, SCTLR_EL1);
    isb();

4. and vcpu_enter_adjust_traps to

    if ( unlikely(0 != v->domain->arch.monitor.write_ctrlreg_enabled) )
    {
         if ( likely(v->domain->arch.hcr & HCR_TVM) )
             return;
         v->domain->arch.hcr |= HCR_TVM;
    }
    else
    {
         if ( likely(!(v->domain->arch.hcr & HCR_TVM)) )
             return;
         v->domain->arch.hcr &= ~HCR_TVM;
    }

    update_hcr(v->domain);

That way at least it's easier to follow where update_hcr is called.

Corneliu.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Reply via email to