> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 29 March 2018 10:10 > 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 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. Paul > Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
