[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] iommu/amd-vi: do not error if device referenced in IVMD is not behind any IOMMU
Hello, Le 09/10/2024 à 14:09, Jan Beulich a écrit : > On 09.10.2024 13:47, Roger Pau Monné wrote: >> On Wed, Oct 09, 2024 at 01:28:19PM +0200, Jan Beulich wrote: >>> On 09.10.2024 13:13, Roger Pau Monné wrote: >>>> I also think returning an error when no device in the IVMD range is >>>> covered by an IOMMU is dubious. Xen will already print warning >>>> messages about such firmware inconsistencies, but refusing to boot is >>>> too strict. >>> >>> I disagree. We shouldn't enable DMA remapping in such an event. Whereas >> >> I'm not sure I understand why you would go as far as refusing to >> enable DMA remapping. How is a IVMD block having references to some >> devices not assigned to any IOMMU different to all devices referenced >> not assigned to any IOMMU? We should deal with both in the same >> way. > > Precisely because of ... > >> If all devices in the IVMD block are not covered by an IOMMU, the >> IVMD block is useless. > > ... this. We simply can't judge whether such a useless block really was > meant to cover something. If it was, we're hosed. Or maybe we screwed up > and wrongly conclude it's useless. > >> But there's nothing for Xen to action, due to >> the devices not having an IOMMU assigned. IOW: it would be the same >> as booting natively without parsing the IVRS in the first place. > > Not really, no. Not parsing IVRS means not turning on any IOMMU. We > then know we can't pass through any devices. We can't assess the > security of passing through devices (as far as it's under our control) > if we enable the IOMMU in perhaps a flawed way. > > A formally valid IVMD we can't make sense of is imo no different from > a formally invalid IVMD, for which we would return ENODEV as well (and > hence fail to enable the IOMMU). Whereas what you're suggesting is, if > I take it further, to basically ignore (almost) all errors in table > parsing, and enable the IOMMU(s) in a best effort manner, no matter > whether that leads to a functional (let alone secure [to the degree > possible]) system. > > What I don't really understand is why you want to relax our checking > beyond what's necessary for the one issue at hand. > >>> the "refusing to boot" is interrupt remapping related iirc, if x2APIC >>> is already enabled. We need to properly separate the two (and the >>> discussion there was started quite a long time ago, but it got stuck at >>> some point); until such time it is simply an undesirable side effect of >>> the inappropriate implementation that in certain case we fail boot when >>> we shouldn't. >> >> Yes, but that's a different topic, and not something I plan to fix as >> the scope of this patch :). > > Sure, I'm merely asking to accept that, until that's resolved, issues > with boot failure can result here, and need to be lived with. > > Jan > Would it be possible to find a middle-ground by adding a "non-security supported" xen command-line option to allow a workaround on this issue ? Something like iommu=amd-skip-unknown-ivmd ? And preventing boot otherwise. Teddy Teddy Astie | Vates XCP-ng Intern XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |