[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/5] x86/hvm: Rework HVM_HCALL_invalidate handling



>>> On 13.02.17 at 14:03, <andrew.cooper3@xxxxxxxxxx> wrote:
> Sending an invalidation to the device model is an internal detail of
> completing the hypercall; callers should not need to be responsible for it.
> Drop HVM_HCALL_invalidate entirely and call send_invalidate_req() when
> appropriate.
> 
> This makes the function boolean in nature, although the existing
> HVM_HCALL_{completed,preempted} to aid code clarity.

"are being retained" missing somewhere here?

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3874,7 +3874,7 @@ static const hypercall_table_t hvm_hypercall_table[] = 
> {
>  #undef HYPERCALL
>  #undef COMPAT_CALL
>  
> -int hvm_do_hypercall(struct cpu_user_regs *regs)
> +bool hvm_hypercall(struct cpu_user_regs *regs)

I don't think bool is a particularly good choice when the callers can't
sensibly use the result without comparing with HVM_HCALL_*

> @@ -4011,9 +4011,8 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
>          return HVM_HCALL_preempted;
>  
>      if ( unlikely(currd->arch.hvm_domain.qemu_mapcache_invalidate) &&
> -         test_and_clear_bool(currd->arch.hvm_domain.
> -                             qemu_mapcache_invalidate) )
> -        return HVM_HCALL_invalidate;
> +         
> test_and_clear_bool(currd->arch.hvm_domain.qemu_mapcache_invalidate) )
> +        send_invalidate_req();

I wonder why things were done the old way in the first place. Did
something else change, making that old model no longer required?
I'm somewhat afraid we overlook something here, and hence an
attempt to understand why this more immediate model wasn't
used (and perhaps usable) back then might help... That aside, the
patch looks fine.

Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.