Re: [Xen-devel] [PATCH v2] x86/hvm: re-work viridian APIC assist code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.01.18 at 16:10, <paul.durrant@xxxxxxxxxx> wrote:
> -int viridian_complete_apic_assist(struct vcpu *v)
> +bool viridian_apic_assist_completed(struct vcpu *v)
>  {
>      uint32_t *va = v->arch.hvm_vcpu.viridian.vp_assist.va;
> -    int vector;
>  
>      if ( !va )
> -        return 0;
> +        return false;
>  
> -    if ( *va & 1u )
> -        return 0; /* Interrupt not yet processed by the guest. */
> -
> -    vector = v->arch.hvm_vcpu.viridian.vp_assist.vector;
> -    v->arch.hvm_vcpu.viridian.vp_assist.vector = 0;
> +    if ( v->arch.hvm_vcpu.viridian.vp_assist.pending &&
> +         !(*va & 1u) )
> +    {
> +        /* An EOI has been avoided */
> +        v->arch.hvm_vcpu.viridian.vp_assist.pending = false;
> +        return true;
> +    }

Especially with such a non-atomic update, please remind me: Despite
them having struct vcpu * parameters, these functions are all only
ever called with v == current? If the answer is yes, then
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with one more cosmetic remark:

> @@ -120,9 +120,9 @@ void viridian_time_ref_count_thaw(struct domain *d);
>  void viridian_vcpu_deinit(struct vcpu *v);
>  void viridian_domain_deinit(struct domain *d);
>  
> -void viridian_start_apic_assist(struct vcpu *v, int vector);
> -int viridian_complete_apic_assist(struct vcpu *v);
> -void viridian_abort_apic_assist(struct vcpu *v);
> +void viridian_set_apic_assist(struct vcpu *v);
> +bool viridian_apic_assist_completed(struct vcpu *v);
> +void viridian_clear_apic_assist(struct vcpu *v);

Could I talk you into naming them all viridian_apic_assist_...()?

Jan


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