| Message ID | 9e64fa33b298f789d8340cf1046a9fbf683dd2b7.1715761386.git.Sergiy_Kibrik@epam.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: make cpu virtualization support configurable | expand |
On Wed, 15 May 2024, Sergiy Kibrik wrote: > From: Xenia Ragiadakou <burzalodowa@gmail.com> > > VIO_realmode_completion is specific to vmx realmode, so guard the completion > handling code with CONFIG_VMX. Also, guard VIO_realmode_completion itself by > CONFIG_VMX, instead of generic CONFIG_X86. > > No functional change intended. > > Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
On 15.05.2024 11:24, Sergiy Kibrik wrote: > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -2667,7 +2667,9 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt, > break; > > case VIO_mmio_completion: > +#ifdef CONFIG_VMX > case VIO_realmode_completion: > +#endif > BUILD_BUG_ON(sizeof(hvio->mmio_insn) < sizeof(hvmemul_ctxt->insn_buf)); > hvio->mmio_insn_bytes = hvmemul_ctxt->insn_buf_bytes; > memcpy(hvio->mmio_insn, hvmemul_ctxt->insn_buf, hvio->mmio_insn_bytes); This change doesn't buy us anything, does it? > --- a/xen/arch/x86/hvm/ioreq.c > +++ b/xen/arch/x86/hvm/ioreq.c > @@ -33,6 +33,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion) > { > switch ( completion ) > { > +#ifdef CONFIG_VMX > case VIO_realmode_completion: > { > struct hvm_emulate_ctxt ctxt; > @@ -43,6 +44,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion) > > break; > } > +#endif > > default: > ASSERT_UNREACHABLE(); And while this change is needed for the goal of the series, I wonder whether it wouldn't better be arch_vcpu_ioreq_completion() as whole that then gets stubbed out. Jan
16.05.24 15:11, Jan Beulich: > On 15.05.2024 11:24, Sergiy Kibrik wrote: >> --- a/xen/arch/x86/hvm/emulate.c >> +++ b/xen/arch/x86/hvm/emulate.c >> @@ -2667,7 +2667,9 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt, >> break; >> >> case VIO_mmio_completion: >> +#ifdef CONFIG_VMX >> case VIO_realmode_completion: >> +#endif >> BUILD_BUG_ON(sizeof(hvio->mmio_insn) < sizeof(hvmemul_ctxt->insn_buf)); >> hvio->mmio_insn_bytes = hvmemul_ctxt->insn_buf_bytes; >> memcpy(hvio->mmio_insn, hvmemul_ctxt->insn_buf, hvio->mmio_insn_bytes); > > This change doesn't buy us anything, does it? why not? Code won't compile w/o it. Or do you mean hiding the whole VIO_realmode_completion enum under CONFIG_VMX wasn't really useful? >> --- a/xen/arch/x86/hvm/ioreq.c >> +++ b/xen/arch/x86/hvm/ioreq.c >> @@ -33,6 +33,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion) >> { >> switch ( completion ) >> { >> +#ifdef CONFIG_VMX >> case VIO_realmode_completion: >> { >> struct hvm_emulate_ctxt ctxt; >> @@ -43,6 +44,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion) >> >> break; >> } >> +#endif >> >> default: >> ASSERT_UNREACHABLE(); > > And while this change is needed for the goal of the series, I wonder whether > it wouldn't better be arch_vcpu_ioreq_completion() as whole that then gets > stubbed out. > I'll post a patch to this thread to confirm whether I got your point correctly. -Sergiy
On 31.05.2024 10:05, Sergiy Kibrik wrote: > 16.05.24 15:11, Jan Beulich: >> On 15.05.2024 11:24, Sergiy Kibrik wrote: >>> --- a/xen/arch/x86/hvm/emulate.c >>> +++ b/xen/arch/x86/hvm/emulate.c >>> @@ -2667,7 +2667,9 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt, >>> break; >>> >>> case VIO_mmio_completion: >>> +#ifdef CONFIG_VMX >>> case VIO_realmode_completion: >>> +#endif >>> BUILD_BUG_ON(sizeof(hvio->mmio_insn) < sizeof(hvmemul_ctxt->insn_buf)); >>> hvio->mmio_insn_bytes = hvmemul_ctxt->insn_buf_bytes; >>> memcpy(hvio->mmio_insn, hvmemul_ctxt->insn_buf, hvio->mmio_insn_bytes); >> >> This change doesn't buy us anything, does it? > > why not? Code won't compile w/o it. > Or do you mean hiding the whole VIO_realmode_completion enum under > CONFIG_VMX wasn't really useful? That's what I meant, by implication. To me it's extra #ifdef-ary without real gain. Jan
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index ab1bc51683..d60b1f6f4d 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -2667,7 +2667,9 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt, break; case VIO_mmio_completion: +#ifdef CONFIG_VMX case VIO_realmode_completion: +#endif BUILD_BUG_ON(sizeof(hvio->mmio_insn) < sizeof(hvmemul_ctxt->insn_buf)); hvio->mmio_insn_bytes = hvmemul_ctxt->insn_buf_bytes; memcpy(hvio->mmio_insn, hvmemul_ctxt->insn_buf, hvio->mmio_insn_bytes); diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c index 4eb7a70182..b37bbd660b 100644 --- a/xen/arch/x86/hvm/ioreq.c +++ b/xen/arch/x86/hvm/ioreq.c @@ -33,6 +33,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion) { switch ( completion ) { +#ifdef CONFIG_VMX case VIO_realmode_completion: { struct hvm_emulate_ctxt ctxt; @@ -43,6 +44,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion) break; } +#endif default: ASSERT_UNREACHABLE(); diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 132b841995..50a58fe428 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -152,7 +152,7 @@ enum vio_completion { VIO_no_completion, VIO_mmio_completion, VIO_pio_completion, -#ifdef CONFIG_X86 +#ifdef CONFIG_VMX VIO_realmode_completion, #endif };