Re: [Xen-devel] [PATCH] AMD/IOMMU: revert "amd/iommu: assign iommu devices to Xen"

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.06.19 at 09:55, <JBeulich@xxxxxxxx> wrote:
>>>> On 04.06.19 at 18:20, <roger.pau@xxxxxxxxxx> wrote:
>> On Tue, Jun 04, 2019 at 07:02:06AM -0600, Jan Beulich wrote:
>>> >>> On 04.06.19 at 10:48, <roger.pau@xxxxxxxxxx> wrote:
>>> > On Mon, Jun 03, 2019 at 07:00:25AM -0600, Jan Beulich wrote:
>>> >> This reverts commit b6bd02b7a877f9fac2de69e64d8245d56f92ab25. The change
>>> >> was redundant with amd_iommu_detect_one_acpi() already calling
>>> >> pci_ro_device().
>>> >> 
>>> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> > 
>>> > I think this needs to be squashed together with your `AMD/IOMMU: don't
>>> > "add" IOMMUs` patch, or else PVH dom0 will break because
>>> > update_paging_mode will find devices not behind an IOMMU assigned to
>>> > dom0, thus returning an error and crashing dom0.
>>> 
>>> I've taken another look (while correcting the other patch, now sent
>>> as v2): update_paging_mode() iterates over all devices the domain
>>> owns. The IOMMU ones, having been subject of an early
>>> pci_ro_device(), should never end up on Dom0's list. And looking at
>>> the code I also can't - for now at least - see how they could get
>>> moved there. In fact I've verified that they take the "override"
>>> path in _setup_hwdom_pci_devices().
>> 
>> As you realized this commit was indeed papering over an existing issue
>> elsewhere. When I did this patch I didn't realize
>> amd_iommu_detect_one_acpi was calling pci_ro_device.
>> 
>> The issue is that when pci_ro_device gets called by
>> amd_iommu_detect_one_acpi dom_xen has not been created yet, so
>> pdev->domain ends up being NULL.
> 
> Well, that's being fixed by "adjust system domain creation (and call it
> earlier on x86)" (note that it's "special" rather than "system" in the
> posted version). pdev->domain remaining to be NULL really is the
> smaller of the problems; accessing dom_xen->arch.pdev_list is the
> worse part here.
> 
> One thing is curious though: So far I thought I would have screwed
> up things by having amd_iommu_detect_one_acpi() called earlier,
> as mentioned in that patch's description. Your change that I'd like
> to revert predates that though by several months, so I'm getting
> the impression the issue has been older. As a result the range of
> versions to backport this to may have to grow.

And no, I cannot confirm this as the original (4.12) behavior: There
I see iommu_setup() getting called a few lines after
arch_init_memory(). Therefore I'm still unclear what exact problem
the pci_hide_device() addition was really meant to fix.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel