| Message ID | 20230213145751.1047236-10-burzalodowa@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: Make cpu virtualization support configurable | expand |
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
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
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();
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(+)