* [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported
@ 2024-04-08 13:02 Teddy Astie
2024-04-08 13:02 ` [XEN PATCH v2 1/3] VT-d: Disable IOMMU if " Teddy Astie
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Teddy Astie @ 2024-04-08 13:02 UTC (permalink / raw)
To: xen-devel; +Cc: Teddy Astie, Jan Beulich, Andrew Cooper, Roger Pau Monné
All hardware that supports VT-d/AMD-Vi that exists also supports cx16 (aside
specifically crafted virtual machines).
Some IOMMU code paths in Xen consider cases where VT-d/AMD-Vi is supported
while cx16 isn't, those paths may be bugged and are barely tested, dead code
in practice.
Disable IOMMU in case we have IOMMU hardware but no cx16, then cleanup
no-cx16 handling logic from VT-d and AMD-Vi drivers.
Teddy
Changed in v2:
* Added cleanup no-cx16 code for x2APIC
* Fixed commit and code formatting
* Added missing Suggested-by note
Teddy Astie (3):
VT-d: Disable IOMMU if cx16 isn't supported
AMD-Vi: Disable IOMMU if cx16 isn't supported
VT-d: Cleanup MAP_SINGLE_DEVICE and related code
xen/arch/x86/apic.c | 6 ++
xen/drivers/passthrough/amd/iommu_map.c | 42 ++++------
xen/drivers/passthrough/amd/pci_amd_iommu.c | 6 ++
xen/drivers/passthrough/vtd/intremap.c | 65 ++++-----------
xen/drivers/passthrough/vtd/iommu.c | 92 +++++++--------------
xen/drivers/passthrough/vtd/vtd.h | 5 +-
6 files changed, 71 insertions(+), 145 deletions(-)
--
2.44.0
Teddy Astie | Vates XCP-ng Intern
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
^ permalink raw reply [flat|nested] 8+ messages in thread* [XEN PATCH v2 1/3] VT-d: Disable IOMMU if cx16 isn't supported 2024-04-08 13:02 [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported Teddy Astie @ 2024-04-08 13:02 ` Teddy Astie 2024-04-08 13:02 ` [XEN PATCH v2 2/3] AMD-Vi: " Teddy Astie ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Teddy Astie @ 2024-04-08 13:02 UTC (permalink / raw) To: xen-devel; +Cc: Teddy Astie, Jan Beulich, Andrew Cooper, Roger Pau Monné No hardware has VT-d support while not having cx16 support, disable IOMMU in this case to avoid potentially buggy code. Now that IOMMU is only enabled if cx16 is supported, drop dead code that handles cases where cx16 isn't supported. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Teddy Astie <teddy.astie@vates.tech> --- xen/arch/x86/apic.c | 6 ++ xen/drivers/passthrough/vtd/intremap.c | 65 +++++---------------- xen/drivers/passthrough/vtd/iommu.c | 80 +++++++++----------------- 3 files changed, 46 insertions(+), 105 deletions(-) diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c index 592b78e11e..91d7f2b248 100644 --- a/xen/arch/x86/apic.c +++ b/xen/arch/x86/apic.c @@ -836,6 +836,12 @@ void __init x2apic_bsp_setup(void) if ( !cpu_has_x2apic ) return; + if ( unlikely(!cpu_has_cx16) ) + { + printk("x2APIC: CPU doesn't support CMPXCHG16B, disabling\n"); + return; + } + if ( !opt_x2apic ) { if ( !x2apic_enabled ) diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c index c504852eb8..b0a0dbdbc2 100644 --- a/xen/drivers/passthrough/vtd/intremap.c +++ b/xen/drivers/passthrough/vtd/intremap.c @@ -173,47 +173,26 @@ bool __init cf_check intel_iommu_supports_eim(void) * Assume iremap_lock has been acquired. It is to make sure software will not * change the same IRTE behind us. With this assumption, if only high qword or * low qword in IRTE is to be updated, this function's atomic variant can - * present an atomic update to VT-d hardware even when cmpxchg16b - * instruction is not supported. + * present an atomic update to VT-d hardware. */ static void update_irte(struct vtd_iommu *iommu, struct iremap_entry *entry, const struct iremap_entry *new_ire, bool atomic) { - ASSERT(spin_is_locked(&iommu->intremap.lock)); - - if ( cpu_has_cx16 ) - { - __uint128_t ret; - struct iremap_entry old_ire; + __uint128_t ret; + struct iremap_entry old_ire; - old_ire = *entry; - ret = cmpxchg16b(entry, &old_ire, new_ire); + ASSERT(spin_is_locked(&iommu->intremap.lock)); + + old_ire = *entry; + ret = cmpxchg16b(entry, &old_ire, new_ire); - /* - * In the above, we use cmpxchg16 to atomically update the 128-bit - * IRTE, and the hardware cannot update the IRTE behind us, so - * the return value of cmpxchg16 should be the same as old_ire. - * This ASSERT validate it. - */ - ASSERT(ret == old_ire.val); - } - else - { - /* - * VT-d hardware doesn't update IRTEs behind us, nor the software - * since we hold iremap_lock. If the caller wants VT-d hardware to - * always see a consistent entry, but we can't meet it, a bug will - * be raised. - */ - if ( entry->lo == new_ire->lo ) - write_atomic(&entry->hi, new_ire->hi); - else if ( entry->hi == new_ire->hi ) - write_atomic(&entry->lo, new_ire->lo); - else if ( !atomic ) - *entry = *new_ire; - else - BUG(); - } + /* + * In the above, we use cmpxchg16 to atomically update the 128-bit + * IRTE, and the hardware cannot update the IRTE behind us, so + * the return value of cmpxchg16 should be the same as old_ire. + * This ASSERT validate it. + */ + ASSERT(ret == old_ire.val); } /* Mark specified intr remap entry as free */ @@ -395,7 +374,6 @@ static int ioapic_rte_to_remap_entry(struct vtd_iommu *iommu, /* Indicate remap format. */ remap_rte->format = 1; - /* If cmpxchg16b is not available the caller must mask the IO-APIC pin. */ update_irte(iommu, iremap_entry, &new_ire, !init && !masked); iommu_sync_cache(iremap_entry, sizeof(*iremap_entry)); iommu_flush_iec_index(iommu, 0, index); @@ -437,21 +415,6 @@ void cf_check io_apic_write_remap_rte( bool masked = true; int rc; - if ( !cpu_has_cx16 ) - { - /* - * Cannot atomically update the IRTE entry: mask the IO-APIC pin to - * avoid interrupts seeing an inconsistent IRTE entry. - */ - old_rte = __ioapic_read_entry(apic, pin, true); - if ( !old_rte.mask ) - { - masked = false; - old_rte.mask = 1; - __ioapic_write_entry(apic, pin, true, old_rte); - } - } - /* Not the initializer, for old gcc to cope. */ new_rte.raw = rte; diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index c7110af7c9..47b56f37a9 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -1482,7 +1482,7 @@ int domain_context_mapping_one( { struct domain_iommu *hd = dom_iommu(domain); struct context_entry *context, *context_entries, lctxt; - __uint128_t old; + __uint128_t res, old; uint64_t maddr; uint16_t seg = iommu->drhd->segment, prev_did = 0; struct domain *prev_dom = NULL; @@ -1580,55 +1580,23 @@ int domain_context_mapping_one( ASSERT(!context_fault_disable(lctxt)); } - if ( cpu_has_cx16 ) - { - __uint128_t res = cmpxchg16b(context, &old, &lctxt.full); + res = cmpxchg16b(context, &old, &lctxt.full); - /* - * Hardware does not update the context entry behind our backs, - * so the return value should match "old". - */ - if ( res != old ) - { - if ( pdev ) - check_cleanup_domid_map(domain, pdev, iommu); - printk(XENLOG_ERR - "%pp: unexpected context entry %016lx_%016lx (expected %016lx_%016lx)\n", - &PCI_SBDF(seg, bus, devfn), - (uint64_t)(res >> 64), (uint64_t)res, - (uint64_t)(old >> 64), (uint64_t)old); - rc = -EILSEQ; - goto unlock; - } - } - else if ( !prev_dom || !(mode & MAP_WITH_RMRR) ) - { - context_clear_present(*context); - iommu_sync_cache(context, sizeof(*context)); - - write_atomic(&context->hi, lctxt.hi); - /* No barrier should be needed between these two. */ - write_atomic(&context->lo, lctxt.lo); - } - else /* Best effort, updating DID last. */ + /* + * Hardware does not update the context entry behind our backs, + * so the return value should match "old". + */ + if ( res != old ) { - /* - * By non-atomically updating the context entry's DID field last, - * during a short window in time TLB entries with the old domain ID - * but the new page tables may be inserted. This could affect I/O - * of other devices using this same (old) domain ID. Such updating - * therefore is not a problem if this was the only device associated - * with the old domain ID. Diverting I/O of any of a dying domain's - * devices to the quarantine page tables is intended anyway. - */ - if ( !(mode & (MAP_OWNER_DYING | MAP_SINGLE_DEVICE)) ) - printk(XENLOG_WARNING VTDPREFIX - " %pp: reassignment may cause %pd data corruption\n", - &PCI_SBDF(seg, bus, devfn), prev_dom); - - write_atomic(&context->lo, lctxt.lo); - /* No barrier should be needed between these two. */ - write_atomic(&context->hi, lctxt.hi); + if ( pdev ) + check_cleanup_domid_map(domain, pdev, iommu); + printk(XENLOG_ERR + "%pp: unexpected context entry %016lx_%016lx (expected %016lx_%016lx)\n", + &PCI_SBDF(seg, bus, devfn), + (uint64_t)(res >> 64), (uint64_t)res, + (uint64_t)(old >> 64), (uint64_t)old); + rc = -EILSEQ; + goto unlock; } iommu_sync_cache(context, sizeof(struct context_entry)); @@ -2630,6 +2598,15 @@ static int __init cf_check vtd_setup(void) int ret; bool reg_inval_supported = true; + if ( unlikely(!cpu_has_cx16) ) + { + printk(XENLOG_ERR VTDPREFIX + "IOMMU: CPU doesn't support CMPXCHG16B, disabling\n"); + + ret = -ENOSYS; + goto error; + } + if ( list_empty(&acpi_drhd_units) ) { ret = -ENODEV; @@ -2692,12 +2669,7 @@ static int __init cf_check vtd_setup(void) iommu_intremap = iommu_intremap_off; #ifndef iommu_intpost - /* - * We cannot use posted interrupt if X86_FEATURE_CX16 is - * not supported, since we count on this feature to - * atomically update 16-byte IRTE in posted format. - */ - if ( !cap_intr_post(iommu->cap) || !iommu_intremap || !cpu_has_cx16 ) + if ( !cap_intr_post(iommu->cap) || !iommu_intremap ) iommu_intpost = false; #endif -- 2.44.0 Teddy Astie | Vates XCP-ng Intern XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [XEN PATCH v2 2/3] AMD-Vi: Disable IOMMU if cx16 isn't supported 2024-04-08 13:02 [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported Teddy Astie 2024-04-08 13:02 ` [XEN PATCH v2 1/3] VT-d: Disable IOMMU if " Teddy Astie @ 2024-04-08 13:02 ` Teddy Astie 2024-04-08 13:02 ` [XEN PATCH v2 3/3] VT-d: Cleanup MAP_SINGLE_DEVICE and related code Teddy Astie 2024-04-11 20:05 ` [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported Andrew Cooper 3 siblings, 0 replies; 8+ messages in thread From: Teddy Astie @ 2024-04-08 13:02 UTC (permalink / raw) To: xen-devel; +Cc: Teddy Astie, Jan Beulich, Andrew Cooper, Roger Pau Monné No hardware has AMD-Vi support while not having cx16 support, disable IOMMU in this case to avoid potentially buggy code. Now that IOMMU is only enabled if cx16 is supported, drop dead code that handles cases where cx16 isn't supported. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Teddy Astie <teddy.astie@vates.tech> --- xen/drivers/passthrough/amd/iommu_map.c | 42 +++++++-------------- xen/drivers/passthrough/amd/pci_amd_iommu.c | 6 +++ 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/amd/iommu_map.c index e0f4fe736a..f67975e700 100644 --- a/xen/drivers/passthrough/amd/iommu_map.c +++ b/xen/drivers/passthrough/amd/iommu_map.c @@ -167,15 +167,14 @@ int amd_iommu_set_root_page_table(struct amd_iommu_dte *dte, { bool valid = flags & SET_ROOT_VALID; - if ( dte->v && dte->tv && - (cpu_has_cx16 || (flags & SET_ROOT_WITH_UNITY_MAP)) ) + if ( dte->v && dte->tv ) { union { struct amd_iommu_dte dte; uint64_t raw64[4]; __uint128_t raw128[2]; } ldte = { .dte = *dte }; - __uint128_t old = ldte.raw128[0]; + __uint128_t res, old = ldte.raw128[0]; int ret = 0; ldte.dte.domain_id = domain_id; @@ -185,33 +184,20 @@ int amd_iommu_set_root_page_table(struct amd_iommu_dte *dte, ldte.dte.paging_mode = paging_mode; ldte.dte.v = valid; - if ( cpu_has_cx16 ) - { - __uint128_t res = cmpxchg16b(dte, &old, &ldte.raw128[0]); + res = cmpxchg16b(dte, &old, &ldte.raw128[0]); - /* - * Hardware does not update the DTE behind our backs, so the - * return value should match "old". - */ - if ( res != old ) - { - printk(XENLOG_ERR - "Dom%d: unexpected DTE %016lx_%016lx (expected %016lx_%016lx)\n", - domain_id, - (uint64_t)(res >> 64), (uint64_t)res, - (uint64_t)(old >> 64), (uint64_t)old); - ret = -EILSEQ; - } - } - else /* Best effort, updating domain_id last. */ + /* + * Hardware does not update the DTE behind our backs, so the + * return value should match "old". + */ + if ( res != old ) { - uint64_t *ptr = (void *)dte; - - write_atomic(ptr + 0, ldte.raw64[0]); - /* No barrier should be needed between these two. */ - write_atomic(ptr + 1, ldte.raw64[1]); - - ret = 1; + printk(XENLOG_ERR + "Dom%d: unexpected DTE %016lx_%016lx (expected %016lx_%016lx)\n", + domain_id, + (uint64_t)(res >> 64), (uint64_t)res, + (uint64_t)(old >> 64), (uint64_t)old); + ret = -EILSEQ; } return ret; diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c index f6efd88e36..656c5eda5d 100644 --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -312,6 +312,12 @@ static int __init cf_check iov_detect(void) return -ENODEV; } + if ( unlikely(!cpu_has_cx16) ) + { + printk("AMD-Vi: CPU doesn't support CMPXCHG16B, disabling\n"); + return -ENOSYS; + } + init_done = 1; if ( !amd_iommu_perdev_intremap ) -- 2.44.0 Teddy Astie | Vates XCP-ng Intern XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [XEN PATCH v2 3/3] VT-d: Cleanup MAP_SINGLE_DEVICE and related code 2024-04-08 13:02 [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported Teddy Astie 2024-04-08 13:02 ` [XEN PATCH v2 1/3] VT-d: Disable IOMMU if " Teddy Astie 2024-04-08 13:02 ` [XEN PATCH v2 2/3] AMD-Vi: " Teddy Astie @ 2024-04-08 13:02 ` Teddy Astie 2024-04-11 20:05 ` [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported Andrew Cooper 3 siblings, 0 replies; 8+ messages in thread From: Teddy Astie @ 2024-04-08 13:02 UTC (permalink / raw) To: xen-devel; +Cc: Teddy Astie, Jan Beulich, Andrew Cooper, Roger Pau Monné This flag was only used in case cx16 is not available, as those code paths no longer exist, this flag now does basically nothing. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Teddy Astie <teddy.astie@vates.tech> --- xen/drivers/passthrough/vtd/iommu.c | 12 +++--------- xen/drivers/passthrough/vtd/vtd.h | 5 ++--- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index 47b56f37a9..4b15e6da79 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -1692,15 +1692,9 @@ static int domain_context_mapping(struct domain *domain, u8 devfn, break; } - if ( domain != pdev->domain && pdev->domain != dom_io ) - { - if ( pdev->domain->is_dying ) - mode |= MAP_OWNER_DYING; - else if ( drhd && - !any_pdev_behind_iommu(pdev->domain, pdev, drhd->iommu) && - !pdev->phantom_stride ) - mode |= MAP_SINGLE_DEVICE; - } + if ( domain != pdev->domain && pdev->domain != dom_io && + pdev->domain->is_dying ) + mode |= MAP_OWNER_DYING; switch ( pdev->type ) { diff --git a/xen/drivers/passthrough/vtd/vtd.h b/xen/drivers/passthrough/vtd/vtd.h index cb2df76eed..43f06a353d 100644 --- a/xen/drivers/passthrough/vtd/vtd.h +++ b/xen/drivers/passthrough/vtd/vtd.h @@ -28,9 +28,8 @@ */ #define MAP_WITH_RMRR (1u << 0) #define MAP_OWNER_DYING (1u << 1) -#define MAP_SINGLE_DEVICE (1u << 2) -#define MAP_ERROR_RECOVERY (1u << 3) -#define UNMAP_ME_PHANTOM_FUNC (1u << 4) +#define MAP_ERROR_RECOVERY (1u << 2) +#define UNMAP_ME_PHANTOM_FUNC (1u << 3) /* Allow for both IOAPIC and IOSAPIC. */ #define IO_xAPIC_route_entry IO_APIC_route_entry -- 2.44.0 Teddy Astie | Vates XCP-ng Intern XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported 2024-04-08 13:02 [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported Teddy Astie ` (2 preceding siblings ...) 2024-04-08 13:02 ` [XEN PATCH v2 3/3] VT-d: Cleanup MAP_SINGLE_DEVICE and related code Teddy Astie @ 2024-04-11 20:05 ` Andrew Cooper 2024-04-11 23:54 ` Marek Marczykowski-Górecki 2024-04-12 8:31 ` Teddy Astie 3 siblings, 2 replies; 8+ messages in thread From: Andrew Cooper @ 2024-04-11 20:05 UTC (permalink / raw) To: Teddy Astie, xen-devel; +Cc: Jan Beulich, Roger Pau Monné On 08/04/2024 2:02 pm, Teddy Astie wrote: > All hardware that supports VT-d/AMD-Vi that exists also supports cx16 (aside > specifically crafted virtual machines). > > Some IOMMU code paths in Xen consider cases where VT-d/AMD-Vi is supported > while cx16 isn't, those paths may be bugged and are barely tested, dead code > in practice. > > Disable IOMMU in case we have IOMMU hardware but no cx16, then cleanup > no-cx16 handling logic from VT-d and AMD-Vi drivers. > > Teddy > > Changed in v2: > > * Added cleanup no-cx16 code for x2APIC > * Fixed commit and code formatting > * Added missing Suggested-by note > > Teddy Astie (3): > VT-d: Disable IOMMU if cx16 isn't supported > AMD-Vi: Disable IOMMU if cx16 isn't supported > VT-d: Cleanup MAP_SINGLE_DEVICE and related code > > xen/arch/x86/apic.c | 6 ++ > xen/drivers/passthrough/amd/iommu_map.c | 42 ++++------ > xen/drivers/passthrough/amd/pci_amd_iommu.c | 6 ++ > xen/drivers/passthrough/vtd/intremap.c | 65 ++++----------- > xen/drivers/passthrough/vtd/iommu.c | 92 +++++++-------------- > xen/drivers/passthrough/vtd/vtd.h | 5 +- > 6 files changed, 71 insertions(+), 145 deletions(-) > Sorry, but you've sent out two copies of each patch in this series, and it's not clear if they're identical or not. Please could you send out another version, making sure there's only one of each patch. Also, you need to swap ENOSYS with ENODEV, as per Jan's review on v1. Thanks, ~Andrew ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported 2024-04-11 20:05 ` [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported Andrew Cooper @ 2024-04-11 23:54 ` Marek Marczykowski-Górecki 2024-04-12 8:31 ` Teddy Astie 1 sibling, 0 replies; 8+ messages in thread From: Marek Marczykowski-Górecki @ 2024-04-11 23:54 UTC (permalink / raw) To: Andrew Cooper; +Cc: Teddy Astie, xen-devel, Jan Beulich, Roger Pau Monné [-- Attachment #1: Type: text/plain, Size: 292 bytes --] On Thu, Apr 11, 2024 at 09:05:08PM +0100, Andrew Cooper wrote: > Sorry, but you've sent out two copies of each patch in this series, and > it's not clear if they're identical or not. FWIW I've got just one copy. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported 2024-04-11 20:05 ` [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported Andrew Cooper 2024-04-11 23:54 ` Marek Marczykowski-Górecki @ 2024-04-12 8:31 ` Teddy Astie 2024-04-12 9:39 ` Andrew Cooper 1 sibling, 1 reply; 8+ messages in thread From: Teddy Astie @ 2024-04-12 8:31 UTC (permalink / raw) To: Andrew Cooper; +Cc: Jan Beulich, Roger Pau Monné, xen-devel Le 11/04/2024 à 22:05, Andrew Cooper a écrit : > On 08/04/2024 2:02 pm, Teddy Astie wrote: >> All hardware that supports VT-d/AMD-Vi that exists also supports cx16 (aside >> specifically crafted virtual machines). >> >> Some IOMMU code paths in Xen consider cases where VT-d/AMD-Vi is supported >> while cx16 isn't, those paths may be bugged and are barely tested, dead code >> in practice. >> >> Disable IOMMU in case we have IOMMU hardware but no cx16, then cleanup >> no-cx16 handling logic from VT-d and AMD-Vi drivers. >> >> Teddy >> >> Changed in v2: >> >> * Added cleanup no-cx16 code for x2APIC >> * Fixed commit and code formatting >> * Added missing Suggested-by note >> >> Teddy Astie (3): >> VT-d: Disable IOMMU if cx16 isn't supported >> AMD-Vi: Disable IOMMU if cx16 isn't supported >> VT-d: Cleanup MAP_SINGLE_DEVICE and related code >> >> xen/arch/x86/apic.c | 6 ++ >> xen/drivers/passthrough/amd/iommu_map.c | 42 ++++------ >> xen/drivers/passthrough/amd/pci_amd_iommu.c | 6 ++ >> xen/drivers/passthrough/vtd/intremap.c | 65 ++++----------- >> xen/drivers/passthrough/vtd/iommu.c | 92 +++++++-------------- >> xen/drivers/passthrough/vtd/vtd.h | 5 +- >> 6 files changed, 71 insertions(+), 145 deletions(-) >> > > Sorry, but you've sent out two copies of each patch in this series, and > it's not clear if they're identical or not. > > Please could you send out another version, making sure there's only one > of each patch. > > Also, you need to swap ENOSYS with ENODEV, as per Jan's review on v1. > > Thanks, > > ~Andrew Hello, Not entirely sure why it got sent twice, as marek said he only received it once. Will double-check next time to avoid this issue in case I wrongfully sent it twice. Will also swap ENOSYS with ENODEV in the next version. Thanks, Teddy Teddy Astie | Vates XCP-ng Intern XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported 2024-04-12 8:31 ` Teddy Astie @ 2024-04-12 9:39 ` Andrew Cooper 0 siblings, 0 replies; 8+ messages in thread From: Andrew Cooper @ 2024-04-12 9:39 UTC (permalink / raw) To: Teddy Astie; +Cc: Jan Beulich, Roger Pau Monné, xen-devel On 12/04/2024 9:31 am, Teddy Astie wrote: > Le 11/04/2024 à 22:05, Andrew Cooper a écrit : >> On 08/04/2024 2:02 pm, Teddy Astie wrote: >>> All hardware that supports VT-d/AMD-Vi that exists also supports cx16 (aside >>> specifically crafted virtual machines). >>> >>> Some IOMMU code paths in Xen consider cases where VT-d/AMD-Vi is supported >>> while cx16 isn't, those paths may be bugged and are barely tested, dead code >>> in practice. >>> >>> Disable IOMMU in case we have IOMMU hardware but no cx16, then cleanup >>> no-cx16 handling logic from VT-d and AMD-Vi drivers. >>> >>> Teddy >>> >>> Changed in v2: >>> >>> * Added cleanup no-cx16 code for x2APIC >>> * Fixed commit and code formatting >>> * Added missing Suggested-by note >>> >>> Teddy Astie (3): >>> VT-d: Disable IOMMU if cx16 isn't supported >>> AMD-Vi: Disable IOMMU if cx16 isn't supported >>> VT-d: Cleanup MAP_SINGLE_DEVICE and related code >>> >>> xen/arch/x86/apic.c | 6 ++ >>> xen/drivers/passthrough/amd/iommu_map.c | 42 ++++------ >>> xen/drivers/passthrough/amd/pci_amd_iommu.c | 6 ++ >>> xen/drivers/passthrough/vtd/intremap.c | 65 ++++----------- >>> xen/drivers/passthrough/vtd/iommu.c | 92 +++++++-------------- >>> xen/drivers/passthrough/vtd/vtd.h | 5 +- >>> 6 files changed, 71 insertions(+), 145 deletions(-) >>> >> Sorry, but you've sent out two copies of each patch in this series, and >> it's not clear if they're identical or not. >> >> Please could you send out another version, making sure there's only one >> of each patch. >> >> Also, you need to swap ENOSYS with ENODEV, as per Jan's review on v1. >> >> Thanks, >> >> ~Andrew > Hello, > > Not entirely sure why it got sent twice, as marek said he only received > it once. Will double-check next time to avoid this issue in case I > wrongfully sent it twice. Huh, lore agrees. I seem to have both a direct and list copy which weren't correctly deduplicated. Sorry for the noise. > Will also swap ENOSYS with ENODEV in the next version. Thanks. ~Andrew ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-04-12 9:40 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-08 13:02 [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported Teddy Astie 2024-04-08 13:02 ` [XEN PATCH v2 1/3] VT-d: Disable IOMMU if " Teddy Astie 2024-04-08 13:02 ` [XEN PATCH v2 2/3] AMD-Vi: " Teddy Astie 2024-04-08 13:02 ` [XEN PATCH v2 3/3] VT-d: Cleanup MAP_SINGLE_DEVICE and related code Teddy Astie 2024-04-11 20:05 ` [XEN PATCH v2 0/3] x86/iommu: Drop IOMMU support when cx16 isn't supported Andrew Cooper 2024-04-11 23:54 ` Marek Marczykowski-Górecki 2024-04-12 8:31 ` Teddy Astie 2024-04-12 9:39 ` Andrew Cooper
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.