[RFC 09/10] x86/ioreq: guard VIO_realmode_completion with INTEL_VMX

Xenia Ragiadakou posted 10 patches 2 years, 12 months ago
[RFC 09/10] x86/ioreq: guard VIO_realmode_completion with INTEL_VMX
Posted by Xenia Ragiadakou 2 years, 12 months ago
VIO_realmode_completion is specific to vmx realmode, so guard the completion
handling code with INTEL_VMX.

No functional change intended.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 xen/arch/x86/hvm/ioreq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 0bdcca1e1a..1b0c0f1b12 100644
--- 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
 
     default:
         ASSERT_UNREACHABLE();
-- 
2.37.2
Re: [RFC 09/10] x86/ioreq: guard VIO_realmode_completion with INTEL_VMX
Posted by Jan Beulich 2 years, 11 months ago
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.

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.

Jan
Re: [RFC 09/10] x86/ioreq: guard VIO_realmode_completion with INTEL_VMX
Posted by Xenia Ragiadakou 2 years, 11 months ago
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