Re: [Xen-devel] [PATCH v5 12/13] x86/traps: move some PV specific functions to pv/traps.c

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/06/17 17:28, Wei Liu wrote:
> @@ -148,6 +150,108 @@ void init_int80_direct_trap(struct vcpu *v)
>          tb->flags = TBF_EXCEPTION | (TI_GET_IF(ti) ? TBF_INTERRUPT : 0);
>  }
>  
> +struct softirq_trap {
> +    struct domain *domain; /* domain to inject trap */
> +    struct vcpu *vcpu;     /* vcpu to inject trap */
> +    int processor;         /* physical cpu to inject trap */
> +};
> +
> +static DEFINE_PER_CPU(struct softirq_trap, softirq_trap);
> +
> +static void nmi_mce_softirq(void)
> +{
> +    int cpu = smp_processor_id();
> +    struct softirq_trap *st = &per_cpu(softirq_trap, cpu);
> +
> +    BUG_ON(st->vcpu == NULL);
> +
> +    /*
> +     * Set the tmp value unconditionally, so that the check in the iret
> +     * hypercall works.
> +     */
> +    cpumask_copy(st->vcpu->cpu_hard_affinity_tmp,
> +                 st->vcpu->cpu_hard_affinity);
> +
> +    if ( (cpu != st->processor) ||
> +         (st->processor != st->vcpu->processor) )
> +    {
> +
> +        /*
> +      * We are on a different physical cpu.  Make sure to wakeup the vcpu on
> +      * the specified processor.
> +         */

You have some stray tabs here.

> +        vcpu_set_hard_affinity(st->vcpu, cpumask_of(st->processor));
> +
> +        /* Affinity is restored in the iret hypercall. */
> +    }
> +
> +    /*
> +     * Only used to defer wakeup of domain/vcpu to a safe (non-NMI/MCE)
> +     * context.
> +     */
> +    vcpu_kick(st->vcpu);
> +    st->vcpu = NULL;
> +}
> +
> +void __init pv_trap_init(void)
> +{
> +    /* The 32-on-64 hypercall vector is only accessible from ring 1. */
> +    _set_gate(idt_table + HYPERCALL_VECTOR,
> +              SYS_DESC_trap_gate, 1, entry_int82);
> +
> +    /* Fast trap for int80 (faster than taking the #GP-fixup path). */
> +    _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,
> +              &int80_direct_trap);
> +
> +    open_softirq(NMI_MCE_SOFTIRQ, nmi_mce_softirq);
> +}
> +
> +int pv_raise_interrupt(struct vcpu *v, uint8_t trap_nr)

Please s/trap_nr/vector/ as you are moving it.  Unlike
guest_has_trap_callback(), the uint8_t here isn't important.

> +{
> +    struct softirq_trap *st = &per_cpu(softirq_trap, smp_processor_id());
> +
> +    switch ( trap_nr )
> +    {
> +    case TRAP_nmi:
> +        if ( cmpxchgptr(&st->vcpu, NULL, v) )
> +            return -EBUSY;
> +        if ( !test_and_set_bool(v->nmi_pending) )
> +        {
> +            st->domain = v->domain;
> +            st->processor = v->processor;
> +
> +            /* Not safe to wake up a vcpu here */
> +            raise_softirq(NMI_MCE_SOFTIRQ);
> +            return 0;
> +        }
> +        st->vcpu = NULL;
> +        break;
> +
> +    case TRAP_machine_check:
> +        if ( cmpxchgptr(&st->vcpu, NULL, v) )
> +            return -EBUSY;
> +
> +        /*
> +      * We are called by the machine check (exception or polling) handlers
> +      * on the physical CPU that reported a machine check error.
> +         */

And more tabs here.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

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