| Message ID | 20221219063456.2017996-7-burzalodowa@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Proposal to make x86 IOMMU driver support configurable | expand |
On 19.12.2022 07:34, Xenia Ragiadakou wrote: > @@ -774,6 +779,16 @@ static inline void hvm_set_nonreg_state(struct vcpu *v, > alternative_vcall(hvm_funcs.set_nonreg_state, v, nrs); > } > > +static inline int hvm_pi_update_irte(const struct vcpu *v, > + const struct pirq *pirq, uint8_t gvec) Why "int" as return type when both call sites ignore the return value? > @@ -893,6 +908,13 @@ static inline void hvm_set_reg(struct vcpu *v, unsigned int reg, uint64_t val) > ASSERT_UNREACHABLE(); > } > > +static inline int hvm_pi_update_irte(const struct vcpu *v, > + const struct pirq *pirq, uint8_t gvec) > +{ > + ASSERT_UNREACHABLE(); > + return -EOPNOTSUPP; > +} I don't think you need this stub - both callers live in a file which is built only for HVM=y anyway. Jan
On 12/21/22 12:13, Jan Beulich wrote: > On 19.12.2022 07:34, Xenia Ragiadakou wrote: >> @@ -774,6 +779,16 @@ static inline void hvm_set_nonreg_state(struct vcpu *v, >> alternative_vcall(hvm_funcs.set_nonreg_state, v, nrs); >> } >> >> +static inline int hvm_pi_update_irte(const struct vcpu *v, >> + const struct pirq *pirq, uint8_t gvec) > > Why "int" as return type when both call sites ignore the return value? Because the original function returned int. I 'm not sure though why the returned value is ignored. > >> @@ -893,6 +908,13 @@ static inline void hvm_set_reg(struct vcpu *v, unsigned int reg, uint64_t val) >> ASSERT_UNREACHABLE(); >> } >> >> +static inline int hvm_pi_update_irte(const struct vcpu *v, >> + const struct pirq *pirq, uint8_t gvec) >> +{ >> + ASSERT_UNREACHABLE(); >> + return -EOPNOTSUPP; >> +} > > I don't think you need this stub - both callers live in a file which > is built only for HVM=y anyway. That's true. I will remove it. > > Jan
On 21.12.2022 12:09, Xenia Ragiadakou wrote: > > On 12/21/22 12:13, Jan Beulich wrote: >> On 19.12.2022 07:34, Xenia Ragiadakou wrote: >>> @@ -774,6 +779,16 @@ static inline void hvm_set_nonreg_state(struct vcpu *v, >>> alternative_vcall(hvm_funcs.set_nonreg_state, v, nrs); >>> } >>> >>> +static inline int hvm_pi_update_irte(const struct vcpu *v, >>> + const struct pirq *pirq, uint8_t gvec) >> >> Why "int" as return type when both call sites ignore the return value? > > Because the original function returned int. Hmm, indeed - looking more closely there can actually be errors, and those shouldn't really be ignored in all cases. At the very least an assertion would seem on order. > I 'm not sure though why the returned value is ignored. Kevin, thoughts? Jan
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 7c81b80710..3080f1ef41 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -2134,6 +2134,14 @@ static bool cf_check vmx_test_pir(const struct vcpu *v, uint8_t vec) return pi_test_pir(vec, &v->arch.hvm.vmx.pi_desc); } +static int cf_check vmx_pi_update_irte(const struct vcpu *v, + const struct pirq *pirq, uint8_t gvec) +{ + const struct pi_desc *pi_desc = v ? &v->arch.hvm.vmx.pi_desc : NULL; + + return pi_update_irte(pi_desc, pirq, gvec); +} + static void cf_check vmx_handle_eoi(uint8_t vector, int isr) { uint8_t old_svi = set_svi(isr); @@ -2582,6 +2590,8 @@ static struct hvm_function_table __initdata_cf_clobber vmx_function_table = { .tsc_scaling = { .max_ratio = VMX_TSC_MULTIPLIER_MAX, }, + + .pi_update_irte = vmx_pi_update_irte, }; /* Handle VT-d posted-interrupt when VCPU is blocked. */ diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h index 93254651f2..f16b9edeaf 100644 --- a/xen/arch/x86/include/asm/hvm/hvm.h +++ b/xen/arch/x86/include/asm/hvm/hvm.h @@ -28,6 +28,8 @@ #include <asm/x86_emulate.h> #include <asm/hvm/asid.h> +struct pirq; /* needed by pi_update_irte */ + #ifdef CONFIG_HVM_FEP /* Permit use of the Forced Emulation Prefix in HVM guests */ extern bool_t opt_hvm_fep; @@ -250,6 +252,9 @@ struct hvm_function_table { /* Architecture function to setup TSC scaling ratio */ void (*setup)(struct vcpu *v); } tsc_scaling; + + int (*pi_update_irte)(const struct vcpu *v, + const struct pirq *pirq, uint8_t gvec); }; extern struct hvm_function_table hvm_funcs; @@ -774,6 +779,16 @@ static inline void hvm_set_nonreg_state(struct vcpu *v, alternative_vcall(hvm_funcs.set_nonreg_state, v, nrs); } +static inline int hvm_pi_update_irte(const struct vcpu *v, + const struct pirq *pirq, uint8_t gvec) +{ + if ( hvm_funcs.pi_update_irte ) + return alternative_call(hvm_funcs.pi_update_irte, v, pirq, gvec); + + return -EOPNOTSUPP; +} + + #else /* CONFIG_HVM */ #define hvm_enabled false @@ -893,6 +908,13 @@ static inline void hvm_set_reg(struct vcpu *v, unsigned int reg, uint64_t val) ASSERT_UNREACHABLE(); } +static inline int hvm_pi_update_irte(const struct vcpu *v, + const struct pirq *pirq, uint8_t gvec) +{ + ASSERT_UNREACHABLE(); + return -EOPNOTSUPP; +} + #define is_viridian_domain(d) ((void)(d), false) #define is_viridian_vcpu(v) ((void)(v), false) #define has_viridian_time_ref_count(d) ((void)(d), false) diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmx.h b/xen/arch/x86/include/asm/hvm/vmx/vmx.h index 8eedf59155..17f13e79a0 100644 --- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h +++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h @@ -155,6 +155,17 @@ static inline void pi_clear_sn(struct pi_desc *pi_desc) clear_bit(POSTED_INTR_SN, &pi_desc->control); } +#ifdef CONFIG_INTEL_VTD +int pi_update_irte(const struct pi_desc *pi_desc, + const struct pirq *pirq, const uint8_t gvec); +#else +static inline int pi_update_irte(const struct pi_desc *pi_desc, + const struct pirq *pirq, const uint8_t gvec) +{ + return -EOPNOTSUPP; +} +#endif + /* * Exit Reasons */ diff --git a/xen/arch/x86/include/asm/iommu.h b/xen/arch/x86/include/asm/iommu.h index 41bd1b9e05..c433c75b76 100644 --- a/xen/arch/x86/include/asm/iommu.h +++ b/xen/arch/x86/include/asm/iommu.h @@ -131,9 +131,6 @@ void iommu_identity_map_teardown(struct domain *d); extern bool untrusted_msi; #endif -int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq, - const uint8_t gvec); - extern bool iommu_non_coherent, iommu_superpages; static inline void iommu_sync_cache(const void *addr, unsigned int size) diff --git a/xen/drivers/passthrough/x86/hvm.c b/xen/drivers/passthrough/x86/hvm.c index a16e0e5344..e720461a14 100644 --- a/xen/drivers/passthrough/x86/hvm.c +++ b/xen/drivers/passthrough/x86/hvm.c @@ -381,8 +381,7 @@ int pt_irq_create_bind( /* Use interrupt posting if it is supported. */ if ( iommu_intpost ) - pi_update_irte(vcpu ? &vcpu->arch.hvm.vmx.pi_desc : NULL, - info, pirq_dpci->gmsi.gvec); + hvm_pi_update_irte(vcpu, info, pirq_dpci->gmsi.gvec); if ( pt_irq_bind->u.msi.gflags & XEN_DOMCTL_VMSI_X86_UNMASKED ) { @@ -672,7 +671,7 @@ int pt_irq_destroy_bind( what = "bogus"; } else if ( pirq_dpci && pirq_dpci->gmsi.posted ) - pi_update_irte(NULL, pirq, 0); + hvm_pi_update_irte(NULL, pirq, 0); if ( pirq_dpci && (pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) && list_empty(&pirq_dpci->digl_list) )
Posted interrupt support in Xen is currently implemented only for the Intel platforms. Instead of calling directly pi_update_irte() from the common hvm code, add a pi_update_irte callback to the hvm_function_table. Then, create a wrapper function hvm_pi_update_irte() to be used by the common hvm code. In the pi_update_irte callback prototype, pass the vcpu as first parameter instead of the posted-interrupt descriptor that is platform specific, and remove the const qualifier from the parameter gvec since it is not needed and because it does not compile with the alternative code patching in use. Move the declaration of pi_update_irte() from asm/iommu.h to asm/hvm/vmx/vmx.h since it is hvm and Intel specific. No functional change intended. Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> --- xen/arch/x86/hvm/vmx/vmx.c | 10 ++++++++++ xen/arch/x86/include/asm/hvm/hvm.h | 22 ++++++++++++++++++++++ xen/arch/x86/include/asm/hvm/vmx/vmx.h | 11 +++++++++++ xen/arch/x86/include/asm/iommu.h | 3 --- xen/drivers/passthrough/x86/hvm.c | 5 ++--- 5 files changed, 45 insertions(+), 6 deletions(-)