RE: [PATCH v3] x86/HVM: more consistently set I/O completion
> -Original Message-
> From: Jan Beulich
> Sent: 15 September 2020 09:26
> To: Andrew Cooper ; Paul Durrant
> Cc: xen-devel@lists.xenproject.org; Wei Liu ; Roger Pau Monné
> ; Jun
> Nakajima ; Kevin Tian ; George
> Dunlap
>
> Subject: Re: [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
> >
> > 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
> > Tested-by: Don Slutz
> > ---
> > v3: Add comment ahead of _hvm_emulate_one(). Add parentheses in a
> > conditional expr. Justify why this does not need an XSA.
> > v2: Make updating of vio->mmio_* fields fully driven by the new
> > completion value.
> > ---
> > I further think that the entire tail of _hvm_emulate_one() (everything
> > past the code changed/added there by this patch) wants skipping in case
> > a completion is needed, at the very least for the mmio and realmode
> > cases, where we know we'll come back here.
>
> Does one of the two of you have an opinion on this aspect?
>
It seems reasonable that we only want to execute the tail once but I'm unsure
of the consequences of deferring it until I/O emulation is complete.
Paul
> Jan
Re: [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
>
> 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
> Tested-by: Don Slutz
> ---
> v3: Add comment ahead of _hvm_emulate_one(). Add parentheses in a
> conditional expr. Justify why this does not need an XSA.
> v2: Make updating of vio->mmio_* fields fully driven by the new
> completion value.
> ---
> I further think that the entire tail of _hvm_emulate_one() (everything
> past the code changed/added there by this patch) wants skipping in case
> a completion is needed, at the very least for the mmio and realmode
> cases, where we know we'll come back here.
Does one of the two of you have an opinion on this aspect?
Jan
RE: [PATCH v3] x86/HVM: more consistently set I/O completion
> From: Jan Beulich
> Sent: Thursday, August 27, 2020 3:09 PM
>
> 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
>
> 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
> Tested-by: Don Slutz
Reviewed-by: Kevin Tian
> ---
> v3: Add comment ahead of _hvm_emulate_one(). Add parentheses in a
> conditional expr. Justify why this does not need an XSA.
> v2: Make updating of vio->mmio_* fields fully driven by the new
> completion value.
> ---
> I further think that the entire tail of _hvm_emulate_one() (everything
> past the code changed/added there by this patch) wants skipping in case
> a completion is needed, at the very least for the mmio and realmode
> cases, where we know we'll come back here.
>
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1683,9 +1683,11 @@ static int hvmemul_validate(
> const struct x86_emulate_state *state,
> struct x86_emulate_ctxt *ctxt)
> {
> -const struct hvm_emulate_ctxt *hvmemul_ctxt =
> +struct hvm_emulate_ctxt *hvmemul_ctxt =
> container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>
> +hvmemul_ctxt->is_mem_access = x86_insn_is_mem_access(state, ctxt);
> +
> return !hvmemul_ctxt->validate || hvmemul_ctxt->validate(state, ctxt)
> ? X86EMUL_OKAY : X86EMUL_UNHANDLEABLE;
> }
> @@ -2610,8 +2612,13 @@ static const struct x86_emulate_ops hvm_
> .vmfunc= hvmemul_vmfunc,
> };
>
> +/*
> + * Note that passing HVMIO_no_completion into this function serves as kind
> + * of (but not fully) an "auto select completion" indicator.
> + */
> static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
> -const struct x86_emulate_ops *ops)
> +const struct x86_emulate_ops *ops,
> +enum hvm_io_completion completion)
> {
> const struct cpu_user_regs *regs = hvmemul_ctxt->ctxt.regs;
> struct vcpu *curr = current;
> @@ -2642,16 +2649,31 @@ static int _hvm_emulate_one(struct hvm_e
> rc = X86EMUL_RETRY;
>
> if ( !hvm_ioreq_needs_completion(>io_req) )
> +completion = HVMIO_no_completion;
> +else if ( completion == HVMIO_no_completion )
> +completion = (vio->io_req.type != IOREQ_TYPE_PIO ||
> + hvmemul_ctxt->is_mem_access) ? HVMIO_mmio_completion
> + : HVMIO_pio_completion;
> +
> +switch (
RE: [PATCH v3] x86/HVM: more consistently set I/O completion
> -Original Message-
> From: Jan Beulich
> Sent: 07 September 2020 10:43
> To: p...@xen.org
> Cc: xen-devel@lists.xenproject.org; 'Andrew Cooper'
> ; 'Wei Liu'
> ; 'Roger Pau Monné' ; 'Jun Nakajima'
> ;
> 'Kevin Tian' ; 'George Dunlap'
>
> Subject: Re: [PATCH v3] x86/HVM: more consistently set I/O completion
>
> On 04.09.2020 18:17, Paul Durrant wrote:
> >> @@ -2610,8 +2612,13 @@ static const struct x86_emulate_ops hvm_
> >> .vmfunc= hvmemul_vmfunc,
> >> };
> >>
> >> +/*
> >> + * Note that passing HVMIO_no_completion into this function serves as kind
> >> + * of (but not fully) an "auto select completion" indicator.
> >> + */
> >> static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
> >> -const struct x86_emulate_ops *ops)
> >> +const struct x86_emulate_ops *ops,
> >> +enum hvm_io_completion completion)
> >> {
> >> const struct cpu_user_regs *regs = hvmemul_ctxt->ctxt.regs;
> >> struct vcpu *curr = current;
> >> @@ -2642,16 +2649,31 @@ static int _hvm_emulate_one(struct hvm_e
> >> rc = X86EMUL_RETRY;
> >>
> >> if ( !hvm_ioreq_needs_completion(>io_req) )
> >> +completion = HVMIO_no_completion;
> >
> > The comment doesn't mention that passing in something other than
> > HVMIO_no_completion could get overridden. Is that intentional?
>
> Well, it was, but since you seem to be asking for it I've added
> "When there's no completion needed, the passed in value will be
> ignored in any case."
That sounds ok.
>
> >> +else if ( completion == HVMIO_no_completion )
> >> +completion = (vio->io_req.type != IOREQ_TYPE_PIO ||
> >> + hvmemul_ctxt->is_mem_access) ? HVMIO_mmio_completion
> >> + : HVMIO_pio_completion;
> >> +
> >> +switch ( vio->io_completion = completion )
> >
> > I thought we tended to avoid assignments in control flow statements.
>
> In if() we do, because of the ambiguity whether == might have
> been meant instead. But in switch() there's imo no such
> ambiguity - if == was really meant, if() would better be used
> anyway. We have quite a few cases of this elsewhere, and I think
> some of them are reasonably recent introductions. As you're the
> maintainer of the file, if you strongly think I shouldn't do so,
> I'll switch of course.
No, that's ok; I was just seeking clarification of when this style is
acceptable.
>
> >> @@ -2727,8 +2752,8 @@ int hvm_emulate_one_mmio(unsigned long m
> >> hvm_emulate_init_once(, x86_insn_is_mem_write,
> >>guest_cpu_user_regs());
> >> ctxt.ctxt.data = _ro_ctxt;
> >> -rc = _hvm_emulate_one(, ops);
> >> -switch ( rc )
> >> +
> >> +switch ( rc = _hvm_emulate_one(, ops, HVMIO_no_completion) )
> >
> > Again, why move the assignment into the switch statement?
>
> For consistency with the other cases where this gets introduced
> in this patch, at least. I for one consider this the more concise
> way of writing such code.
>
> >> --- a/xen/arch/x86/hvm/vmx/realmode.c
> >> +++ b/xen/arch/x86/hvm/vmx/realmode.c
> >> @@ -97,15 +97,11 @@ static void realmode_deliver_exception(
> >> void vmx_realmode_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt)
> >> {
> >> struct vcpu *curr = current;
> >> -struct hvm_vcpu_io *vio = >arch.hvm.hvm_io;
> >> int rc;
> >>
> >> perfc_incr(realmode_emulations);
> >>
> >> -rc = hvm_emulate_one(hvmemul_ctxt);
> >> -
> >> -if ( hvm_ioreq_needs_completion(>io_req) )
> >> -vio->io_completion = HVMIO_realmode_completion;
> >> +rc = hvm_emulate_one(hvmemul_ctxt, HVMIO_realmode_completion);
> >
> > Ok, I guess the override of completion is intentional to deal with
> > this case. Perhaps expand the comment above _hvm_emulate_one() then.
>
> Right, done as per above. Let me know whether the text still isn't
> sufficient.
>
> >> --- a/xen/include/asm-x86/hvm/emulate.h
> >> +++ b/xen/include/asm-x86/hvm/emulate.h
> >> @@ -48,6 +48,8 @@ struct hvm_emulate_ctxt {
> >>
> >> uint32_t intr_shadow;
> >>
> >> +bool is_mem_access;
> >> +
> >
> > Whilst you mention in the commit comment why this is added, I don't
> > see any consumer if it in this patch. Will the come later?
>
> hvmemul_validate() sets the field, and _hvm_emulate_one() consumes
> it.
>
Oh yes, I see it now.
With the comment addition...
Reviewed-by: Paul Durrant
> Jan
Re: [PATCH v3] x86/HVM: more consistently set I/O completion
On 04.09.2020 18:17, Paul Durrant wrote:
>> @@ -2610,8 +2612,13 @@ static const struct x86_emulate_ops hvm_
>> .vmfunc= hvmemul_vmfunc,
>> };
>>
>> +/*
>> + * Note that passing HVMIO_no_completion into this function serves as kind
>> + * of (but not fully) an "auto select completion" indicator.
>> + */
>> static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
>> -const struct x86_emulate_ops *ops)
>> +const struct x86_emulate_ops *ops,
>> +enum hvm_io_completion completion)
>> {
>> const struct cpu_user_regs *regs = hvmemul_ctxt->ctxt.regs;
>> struct vcpu *curr = current;
>> @@ -2642,16 +2649,31 @@ static int _hvm_emulate_one(struct hvm_e
>> rc = X86EMUL_RETRY;
>>
>> if ( !hvm_ioreq_needs_completion(>io_req) )
>> +completion = HVMIO_no_completion;
>
> The comment doesn't mention that passing in something other than
> HVMIO_no_completion could get overridden. Is that intentional?
Well, it was, but since you seem to be asking for it I've added
"When there's no completion needed, the passed in value will be
ignored in any case."
>> +else if ( completion == HVMIO_no_completion )
>> +completion = (vio->io_req.type != IOREQ_TYPE_PIO ||
>> + hvmemul_ctxt->is_mem_access) ? HVMIO_mmio_completion
>> + : HVMIO_pio_completion;
>> +
>> +switch ( vio->io_completion = completion )
>
> I thought we tended to avoid assignments in control flow statements.
In if() we do, because of the ambiguity whether == might have
been meant instead. But in switch() there's imo no such
ambiguity - if == was really meant, if() would better be used
anyway. We have quite a few cases of this elsewhere, and I think
some of them are reasonably recent introductions. As you're the
maintainer of the file, if you strongly think I shouldn't do so,
I'll switch of course.
>> @@ -2727,8 +2752,8 @@ int hvm_emulate_one_mmio(unsigned long m
>> hvm_emulate_init_once(, x86_insn_is_mem_write,
>>guest_cpu_user_regs());
>> ctxt.ctxt.data = _ro_ctxt;
>> -rc = _hvm_emulate_one(, ops);
>> -switch ( rc )
>> +
>> +switch ( rc = _hvm_emulate_one(, ops, HVMIO_no_completion) )
>
> Again, why move the assignment into the switch statement?
For consistency with the other cases where this gets introduced
in this patch, at least. I for one consider this the more concise
way of writing such code.
>> --- a/xen/arch/x86/hvm/vmx/realmode.c
>> +++ b/xen/arch/x86/hvm/vmx/realmode.c
>> @@ -97,15 +97,11 @@ static void realmode_deliver_exception(
>> void vmx_realmode_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt)
>> {
>> struct vcpu *curr = current;
>> -struct hvm_vcpu_io *vio = >arch.hvm.hvm_io;
>> int rc;
>>
>> perfc_incr(realmode_emulations);
>>
>> -rc = hvm_emulate_one(hvmemul_ctxt);
>> -
>> -if ( hvm_ioreq_needs_completion(>io_req) )
>> -vio->io_completion = HVMIO_realmode_completion;
>> +rc = hvm_emulate_one(hvmemul_ctxt, HVMIO_realmode_completion);
>
> Ok, I guess the override of completion is intentional to deal with
> this case. Perhaps expand the comment above _hvm_emulate_one() then.
Right, done as per above. Let me know whether the text still isn't
sufficient.
>> --- a/xen/include/asm-x86/hvm/emulate.h
>> +++ b/xen/include/asm-x86/hvm/emulate.h
>> @@ -48,6 +48,8 @@ struct hvm_emulate_ctxt {
>>
>> uint32_t intr_shadow;
>>
>> +bool is_mem_access;
>> +
>
> Whilst you mention in the commit comment why this is added, I don't
> see any consumer if it in this patch. Will the come later?
hvmemul_validate() sets the field, and _hvm_emulate_one() consumes
it.
Jan
RE: [PATCH v3] x86/HVM: more consistently set I/O completion
> -Original Message-
> From: Jan Beulich
> Sent: 27 August 2020 08:09
> To: xen-devel@lists.xenproject.org
> Cc: Andrew Cooper ; Wei Liu ; Roger
> Pau Monné
> ; Paul Durrant ; Jun Nakajima
> ; Kevin Tian
> ; George Dunlap
> Subject: [PATCH v3] x86/HVM: more consistently set I/O completion
>
> 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
>
> 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
> Tested-by: Don Slutz
> ---
> v3: Add comment ahead of _hvm_emulate_one(). Add parentheses in a
> conditional expr. Justify why this does not need an XSA.
> v2: Make updating of vio->mmio_* fields fully driven by the new
> completion value.
> ---
> I further think that the entire tail of _hvm_emulate_one() (everything
> past the code changed/added there by this patch) wants skipping in case
> a completion is needed, at the very least for the mmio and realmode
> cases, where we know we'll come back here.
>
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1683,9 +1683,11 @@ static int hvmemul_validate(
> const struct x86_emulate_state *state,
> struct x86_emulate_ctxt *ctxt)
> {
> -const struct hvm_emulate_ctxt *hvmemul_ctxt =
> +struct hvm_emulate_ctxt *hvmemul_ctxt =
> container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>
> +hvmemul_ctxt->is_mem_access = x86_insn_is_mem_access(state, ctxt);
> +
> return !hvmemul_ctxt->validate || hvmemul_ctxt->validate(state, ctxt)
> ? X86EMUL_OKAY : X86EMUL_UNHANDLEABLE;
> }
> @@ -2610,8 +2612,13 @@ static const struct x86_emulate_ops hvm_
> .vmfunc= hvmemul_vmfunc,
> };
>
> +/*
> + * Note that passing HVMIO_no_completion into this function serves as kind
> + * of (but not fully) an "auto select completion" indicator.
> + */
> static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
> -const struct x86_emulate_ops *ops)
> +const struct x86_emulate_ops *ops,
> +enum hvm_io_completion completion)
> {
> const struct cpu_user_regs *regs = hvmemul_ctxt->ctxt.regs;
> struct vcpu *curr = current;
> @@ -2642,16 +2649,31 @@ static int _hvm_emulate_one(struct hvm_e
> rc = X86EMUL_RETRY;
>
> if ( !hvm_ioreq_needs_completion(>io_req) )
> +completion = HVMIO_no_completion;
The comment doesn't mention that passing in something other than
HVMIO_no_completion
