On 09.10.2024 15:37, Roger Pau Monné wrote: > On Wed, Oct 09, 2024 at 02:09:33PM +0200, Jan Beulich wrote: >> 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. > > The same could be stated about devices in a IVMD block that are not > assigned to an IOMMU: it could also be Xen that screwed up and wrongly > concluded they are not assigned to an IOMMU. > >>> 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. > > No, don't take it further: not ignore all errors, I think we should > ignore errors when the device in the IVMD is not behind an IOMMU. And > I think that should apply globally, regardless of whether all devices > in the IVMD block fall in that category. > > That will bring AMD-Vi inline with VT-d RMRR, as from what I can see > acpi_parse_one_rmrr() doesn't care whether the device scope in the > entry is behind an IOMMU or not, or whether the whole RMRR doesn't > effectively apply to any device because none of them is covered by an > IOMMU. > >> What I don't really understand is why you want to relax our checking >> beyond what's necessary for the one issue at hand. > > This issue has been reported to us and we have been able to debug. > However, I worry what other malformed IVMD blocks might be out there, > for example an IVMD block that applies to a single device (type 21h), > but such single device doesn't exist (or it's not behind an IOMMU). > Maybe next time we simply won't get a report, the user will try Xen, > see it's not working and move to something else. > > I've taken a quick look at Linux, and when parsing the IVMD blocks > there's no checking that referred devices are behind an IOMMU, the > regions are just added to a list.
Hmm, okay, after some more chewing on it on this basis Acked-by: Jan Beulich <jbeul...@suse.com> Jan
