[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: Ping: [PATCH v3] x86/HVM: more consistently set I/O completion
> -----Original Message----- > From: Jan Beulich <jbeulich@xxxxxxxx> > Sent: 04 September 2020 09:16 > To: Paul Durrant <paul@xxxxxxx> > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper > <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; > Roger Pau Monné <roger.pau@xxxxxxxxxx>; Jun Nakajima > <jun.nakajima@xxxxxxxxx>; Kevin Tian > <kevin.tian@xxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxxxxx> > Subject: Ping: [PATCH v3] x86/HVM: more consistently set I/O completion > > On 27.08.2020 09:09, Jan Beulich wrote: > > Doing this just in hvm_emulate_one_insn() is not enough. > > hvm_ud_intercept() and hvm_emulate_one_vm_event() can get invoked for > > insns requiring one or more continuations, and at least in principle > > hvm_emulate_one_mmio() could, too. Without proper setting of the field, > > handle_hvm_io_completion() will do nothing completion-wise, and in > > particular the missing re-invocation of the insn emulation paths will > > lead to emulation caching not getting disabled in due course, causing > > the ASSERT() in {svm,vmx}_vmenter_helper() to trigger. > > > > Reported-by: Don Slutz <don.slutz@xxxxxxxxx> > > > > Similar considerations go for the clearing of vio->mmio_access, which > > gets moved as well. > > > > Additionally all updating of vio->mmio_* now gets done dependent upon > > the new completion value, rather than hvm_ioreq_needs_completion()'s > > return value. This is because it is the completion chosen which controls > > what path will be taken when handling the completion, not the simple > > boolean return value. In particular, PIO completion doesn't involve > > going through the insn emulator, and hence emulator state ought to get > > cleared early (or it won't get cleared at all). > > > > The new logic, besides allowing for a caller override for the > > continuation type to be set (for VMX real mode emulation), will also > > avoid setting an MMIO completion when a simpler PIO one will do. This > > is a minor optimization only as a side effect - the behavior is strictly > > needed at least for hvm_ud_intercept(), as only memory accesses can > > successfully complete through handle_mmio(). Care of course needs to be > > taken to correctly deal with "mixed" insns (doing both MMIO and PIO at > > the same time, i.e. INS/OUTS). For this, hvmemul_validate() now latches > > whether the insn being emulated is a memory access, as this information > > is no longer easily available at the point where we want to consume it. > > > > Note that the presence of non-NULL .validate fields in the two ops > > structures in hvm_emulate_one_mmio() was really necessary even before > > the changes here: Without this, passing non-NULL as middle argument to > > hvm_emulate_init_once() is meaningless. > > > > The restrictions on when the #UD intercept gets actually enabled are why > > it was decided that this is not a security issue: > > - the "hvm_fep" option to enable its use is a debugging option only, > > - for the cross-vendor case is considered experimental, even if > > unfortunately SUPPORT.md doesn't have an explicit statement about > > this. > > The other two affected functions are > > - hvm_emulate_one_vm_event(), used for introspection, > > - hvm_emulate_one_mmio(), used for Dom0 only, > > which aren't qualifying this as needing an XSA either. > > > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > > Tested-by: Don Slutz <don.slutz@xxxxxxxxx> > > Paul (in particular)? > Sorry... been on my TODO list since you posted it. Will try and take a look today. Paul
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |