>>> On 29.03.18 at 11:13, <paul.durr...@citrix.com> wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 29 March 2018 10:10
>> >> --- a/xen/include/asm-x86/hvm/vcpu.h
>> >> +++ b/xen/include/asm-x86/hvm/vcpu.h
>> >> @@ -91,10 +91,12 @@ struct hvm_vcpu_io {
>> >>      const struct g2m_ioport *g2m_ioport;
>> >>  };
>> >>
>> >> -static inline bool_t hvm_vcpu_io_need_completion(const struct
>> >> hvm_vcpu_io *vio)
>> >> +static inline bool hvm_vcpu_io_need_completion(const struct
>> >> hvm_vcpu_io *vio)
>> >>  {
>> >>      return (vio->io_req.state == STATE_IOREQ_READY) &&
>> >> -           !vio->io_req.data_is_ptr;
>> >> +           !vio->io_req.data_is_ptr &&
>> >> +           (vio->io_req.type != IOREQ_TYPE_PIO ||
>> >> +            vio->io_req.dir != IOREQ_WRITE);
>> >
>> > ... now that you've updated it here.
>> 
>> It could have been before, and it wasn't, so I didn't want to change
>> that. My assumption is that the function wasn't used to leverage
>> local variables (and avoid the .state comparison altogether).
> 
> Yes, that's why it was like it is.
> 
>> Technically it could be switched, I agree. I guess I should at least
>> attach a comment, clarifying that this is an open-coded, slightly
>> optimized variant of the function.
>> 
> 
> Alternatively if the macro is modified to take an ioreq_t pointer directly 
> rather than a struct hvm_vcpu_io pointer, then I think you could just pass 
> the on-stack ioreq_t to it in hvmemul_do_io() and avoid any real need for the 
> open-coded test.

We can't use the on-stack ioreq_t, as hvm_send_ioreq() alters its
state field. I'm now going to see whether &vio->io_req could be
used here, but the first try already produced worse code, and I
don't expect this extra change to improve things (but rather make
them slightly worse).

Jan


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

Reply via email to