On 12/04/2024 1:33 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 > > Changed in v3: > > * Use -ENODEV instead of -ENOSYS.
Hmm - I've still got a double copy of patch 2. No idea what's going on, but I'll make do with what b4 thinks is on the list. A couple of things. 1) You introduced trailing whitespace in patch 1 in the middle line here: > + ASSERT(spin_is_locked(&iommu->intremap.lock)); + + old_ire = *entry; 2) In your commit messages, the grammar is a bit awkward. It would be clearer to say: "All hardware with VT-d/AMD-Vi has CMPXCHG16 support. Check this at initialisation time, and remove the effectively-dead logic for the non-cx16 case." just as you've done in the cover letter. 3) In patch 1, you shouldn't modify x2apic_bsp_setup() like that. x2APIC && no-IOMMU is a legal combination. Instead, you should put a cx16 check in both driver's supports_x2apic() hooks. 4) In patch 3, you should drop the Suggested-by me. You found that one all yourself. ~Andrew
