> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 29 March 2018 10:41 > To: Paul Durrant <paul.durr...@citrix.com> > Cc: Andrew Cooper <andrew.coop...@citrix.com>; xen-devel <xen- > de...@lists.xenproject.org> > Subject: RE: [PATCH RFC] x86/HVM: suppress I/O completion for port output > > >>> 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 > >> > >> >>> On 29.03.18 at 10:54, <paul.durr...@citrix.com> wrote: > >> >> --- a/xen/arch/x86/hvm/emulate.c > >> >> +++ b/xen/arch/x86/hvm/emulate.c > >> >> @@ -282,7 +282,7 @@ static int hvmemul_do_io( > >> >> rc = hvm_send_ioreq(s, &p, 0); > >> >> if ( rc != X86EMUL_RETRY || currd->is_shutting_down ) > >> >> vio->io_req.state = STATE_IOREQ_NONE; > >> >> - else if ( data_is_addr ) > >> >> + else if ( data_is_addr || (!is_mmio && dir == IOREQ_WRITE) > >> >> ) > >> > > >> > I'm not entirely sure, but it seems like this test might actually be > >> > !hvm_vcpu_io_need_completion()... > >> > > >> >> rc = X86EMUL_OKAY; > >> >> } > >> >> break; > >> >> --- 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. > > Hmm, yes, but even then I'm not sure the compiler would realize > it can omit the .state check. I may try out that transformation once > I know whether this helps in the first place. >
Ok. Fair enough. Paul > Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
