Re: [PATCH] AMD/guest_iommu: Disable guest iommu support

[Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>]

On 10/02/2014 09:02 AM, Andrew Cooper wrote:
> AMD Guest IOMMU support was added to allow correct use of PASID and PRI
> hardware support with an ATS-aware guest driver.
>
> However, support cannot possibly function as guest_iommu_set_base() has no
> callers.  This means that its MMIO region's P2M pages are not set to
> p2m_mmio_dm, preventing any invocation of the MMIO read/write handlers.
>
> c/s fd186384 "x86/HVM: extend LAPIC shortcuts around P2M lookups" introduces a
> path (via hvm_mmio_internal()) where iommu_mmio_handler claims its MMIO range,
> and causes __hvm_copy() to fail with HVMCOPY_bad_gfn_to_mfn.
>
> iommu->mmio_base defaults to 0, with a range of 8 pages, and is unilaterally
> enabled in any HVM guests when the host IOMMU(s) supports any extended
> features.
>
> Unfortunately, HVMLoader's AP boot trampoline executes an `lmsw` instruction
> at linear address 0x100c which unconditionally requires emulation.  The
> instruction fetch in turn fails as __hvm_copy() fails with
> HVMCOPY_bad_gfn_to_mfn.
>
> The result is that multi-vcpu HVM guests do not work on newer AMD hardware, if
> IOMMU support is enabled in the BIOS.
>
> Change the default mmio_base address to ~0ULL.  This prevents
> guest_iommu_mmio_range() from actually claiming any physical range
> whatsoever, which allows the emulation of `lmsw` to succeed.
>
> Reported-by: Roberto Luongo <rluongo@ready.it>
> Suggested-by: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Tested-by: Roberto Luongo <rluongo@ready.it>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> CC: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> ---
>   xen/drivers/passthrough/amd/iommu_guest.c |    1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/xen/drivers/passthrough/amd/iommu_guest.c b/xen/drivers/passthrough/amd/iommu_guest.c
> index 5660020..98e7b38 100644
> --- a/xen/drivers/passthrough/amd/iommu_guest.c
> +++ b/xen/drivers/passthrough/amd/iommu_guest.c
> @@ -885,6 +885,7 @@ int guest_iommu_init(struct domain* d)
>       }
>
>       guest_iommu_reg_init(iommu);
> +    iommu->mmio_base = ~0ULL;
>       iommu->domain = d;
>       hd->arch.g_iommu = iommu;
>
>
Thank you, Andrew.

Acked-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Jan,

The function guest_iommu_set_base() was added by Wei before my time. I 
checked the log and didn't see the evidence that this code has ever been 
used. Do you remember why he added this code to begin with, and how this 
was planned to be used?

Suravee