| Message ID | 20230104084502.61734-6-burzalodowa@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Make x86 IOMMU driver support configurable | expand |
On 04.01.2023 09:44, Xenia Ragiadakou wrote: > The variable untrusted_msi indicates whether the system is vulnerable to > CVE-2011-1898. This vulnerablity is VT-d specific. As per the reply by Andrew to v1, this vulnerability is generic to intremap- incapable or intremap-disabled configurations. You want to say so. In turn I wonder whether instead of the changes you're making you wouldn't want to move the definition of the variable to xen/drivers/passthrough/x86/iommu.c. A useful further step might be to guard its definition (not necessarily its declaration; see replies to earlier patches) by CONFIG_PV instead (of course I understand that's largely orthogonal to your series here, yet it would fit easily with moving the definition). > --- a/xen/arch/x86/include/asm/iommu.h > +++ b/xen/arch/x86/include/asm/iommu.h > @@ -127,7 +127,9 @@ int iommu_identity_mapping(struct domain *d, p2m_access_t p2ma, > unsigned int flag); > void iommu_identity_map_teardown(struct domain *d); > > +#ifdef CONFIG_INTEL_IOMMU > extern bool untrusted_msi; > +#endif As per above / earlier comments I don't think this part is needed in any event. > --- a/xen/arch/x86/pv/hypercall.c > +++ b/xen/arch/x86/pv/hypercall.c > @@ -193,8 +193,10 @@ void pv_ring1_init_hypercall_page(void *p) > > void do_entry_int82(struct cpu_user_regs *regs) > { > +#ifdef CONFIG_INTEL_IOMMU > if ( unlikely(untrusted_msi) ) > check_for_unexpected_msi((uint8_t)regs->entry_vector); > +#endif > > _pv_hypercall(regs, true /* compat */); > } > diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S > index ae01285181..8f2fb36770 100644 > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -406,11 +406,13 @@ ENTRY(int80_direct_trap) > .Lint80_cr3_okay: > sti > > +#ifdef CONFIG_INTEL_IOMMU > cmpb $0,untrusted_msi(%rip) > UNLIKELY_START(ne, msi_check) > movl $0x80,%edi > call check_for_unexpected_msi > UNLIKELY_END(msi_check) > +#endif > > movq STACK_CPUINFO_FIELD(current_vcpu)(%rbx), %rbx >
On 1/12/23 14:01, Jan Beulich wrote: > On 04.01.2023 09:44, Xenia Ragiadakou wrote: >> The variable untrusted_msi indicates whether the system is vulnerable to >> CVE-2011-1898. This vulnerablity is VT-d specific. > > As per the reply by Andrew to v1, this vulnerability is generic to intremap- > incapable or intremap-disabled configurations. You want to say so. In turn > I wonder whether instead of the changes you're making you wouldn't want to > move the definition of the variable to xen/drivers/passthrough/x86/iommu.c. > A useful further step might be to guard its definition (not necessarily > its declaration; see replies to earlier patches) by CONFIG_PV instead (of > course I understand that's largely orthogonal to your series here, yet it > would fit easily with moving the definition). Sure I can do that. > >> --- a/xen/arch/x86/include/asm/iommu.h >> +++ b/xen/arch/x86/include/asm/iommu.h >> @@ -127,7 +127,9 @@ int iommu_identity_mapping(struct domain *d, p2m_access_t p2ma, >> unsigned int flag); >> void iommu_identity_map_teardown(struct domain *d); >> >> +#ifdef CONFIG_INTEL_IOMMU >> extern bool untrusted_msi; >> +#endif > > As per above / earlier comments I don't think this part is needed in any > event. > >> --- a/xen/arch/x86/pv/hypercall.c >> +++ b/xen/arch/x86/pv/hypercall.c >> @@ -193,8 +193,10 @@ void pv_ring1_init_hypercall_page(void *p) >> >> void do_entry_int82(struct cpu_user_regs *regs) >> { >> +#ifdef CONFIG_INTEL_IOMMU >> if ( unlikely(untrusted_msi) ) >> check_for_unexpected_msi((uint8_t)regs->entry_vector); >> +#endif >> >> _pv_hypercall(regs, true /* compat */); >> } >> diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S >> index ae01285181..8f2fb36770 100644 >> --- a/xen/arch/x86/x86_64/entry.S >> +++ b/xen/arch/x86/x86_64/entry.S >> @@ -406,11 +406,13 @@ ENTRY(int80_direct_trap) >> .Lint80_cr3_okay: >> sti >> >> +#ifdef CONFIG_INTEL_IOMMU >> cmpb $0,untrusted_msi(%rip) >> UNLIKELY_START(ne, msi_check) >> movl $0x80,%edi >> call check_for_unexpected_msi >> UNLIKELY_END(msi_check) >> +#endif >> >> movq STACK_CPUINFO_FIELD(current_vcpu)(%rbx), %rbx >> >
diff --git a/xen/arch/x86/include/asm/iommu.h b/xen/arch/x86/include/asm/iommu.h index fc0afe35bf..fb5fe4e1bf 100644 --- a/xen/arch/x86/include/asm/iommu.h +++ b/xen/arch/x86/include/asm/iommu.h @@ -127,7 +127,9 @@ int iommu_identity_mapping(struct domain *d, p2m_access_t p2ma, unsigned int flag); void iommu_identity_map_teardown(struct domain *d); +#ifdef CONFIG_INTEL_IOMMU extern bool untrusted_msi; +#endif int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq, const uint8_t gvec); diff --git a/xen/arch/x86/pv/hypercall.c b/xen/arch/x86/pv/hypercall.c index 2eedfbfae8..9d616a0fc5 100644 --- a/xen/arch/x86/pv/hypercall.c +++ b/xen/arch/x86/pv/hypercall.c @@ -193,8 +193,10 @@ void pv_ring1_init_hypercall_page(void *p) void do_entry_int82(struct cpu_user_regs *regs) { +#ifdef CONFIG_INTEL_IOMMU if ( unlikely(untrusted_msi) ) check_for_unexpected_msi((uint8_t)regs->entry_vector); +#endif _pv_hypercall(regs, true /* compat */); } diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S index ae01285181..8f2fb36770 100644 --- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -406,11 +406,13 @@ ENTRY(int80_direct_trap) .Lint80_cr3_okay: sti +#ifdef CONFIG_INTEL_IOMMU cmpb $0,untrusted_msi(%rip) UNLIKELY_START(ne, msi_check) movl $0x80,%edi call check_for_unexpected_msi UNLIKELY_END(msi_check) +#endif movq STACK_CPUINFO_FIELD(current_vcpu)(%rbx), %rbx
The variable untrusted_msi indicates whether the system is vulnerable to CVE-2011-1898. This vulnerablity is VT-d specific. Place the code that addresses the issue under CONFIG_INTEL_IOMMU. No functional change intended. Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> --- Changes in v2: - replace CONFIG_INTEL_VTD with CONFIG_INTEL_IOMMU xen/arch/x86/include/asm/iommu.h | 2 ++ xen/arch/x86/pv/hypercall.c | 2 ++ xen/arch/x86/x86_64/entry.S | 2 ++ 3 files changed, 6 insertions(+)