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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jun 05, 2019 at 02:02:43AM -0600, Jan Beulich wrote:
> >>> 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.

AFAICT this was an error on my side, the commit that fixed the
update_paging_mode related issue is 4250a4416225c, and b6bd02b7a877 is
plain wrong unless I'm missing something.

PVH dom0 AMD support in 4.11 is not working and it's not easy to
bisect the changes between 4.11 and 4.12 in that state.

For the revert:

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks, Roger.

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