On 2/14/23 18:46, Jan Beulich wrote:
> On 13.02.2023 15:57, Xenia Ragiadakou wrote:
>> VIO_realmode_completion is specific to vmx realmode, so guard the completion
>> handling code with INTEL_VMX.
>
> While it means slightly bigger a code change, personally I'd prefer if
> VIO_realmode_completion simply didn't exist as enumerator when !VMX.
> Besides ...
>
>> --- a/xen/arch/x86/hvm/ioreq.c
>> +++ b/xen/arch/x86/hvm/ioreq.c
>> @@ -44,6 +44,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion)
>> {
>> switch ( completion )
>> {
>> +#ifdef CONFIG_INTEL_VMX
>> case VIO_realmode_completion:
>> {
>> struct hvm_emulate_ctxt ctxt;
>> @@ -54,6 +55,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion)
>>
>> break;
>> }
>> +#endif
>
> ... this use there's exactly one further one which would need #ifdef-ing.
> I think that's tolerable.
Sure. I will fix it.
>
> Thinking further (not necessarily for you to carry out) the x86 version
> is identical to Arm's when !VMX, so it would look better to me if the
> entire x86 function became conditional, the Arm one was dropped, and
> common/ioreq.c provided a fallback for the case where no arch one exists.
It may look awkward to have arch_vcpu_ioreq_completion() in VMX guards
in common ioreq code because the connection is not obvious immediately.
>
> Jan
--
Xenia