Re: [Xen-devel] [PATCH v4 3/4] amd-iommu: disable iommu_hap_pt_share with AMD IOMMUs

[Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>]

Roger,

I am not quite sure why you would disable "iommu_hap_pt_share" for AMD 
IOMMU. The current implementation assumes that the p2m can be shared.
Also, I feel that simply just set iommu_hap_pt_share = 0 (while still 
having several places in the AMD iommu drivers and p2m-pt.c assuming 
that it can be shared) seems a bit messy.
Thanks,

Suravee

On 06/04/2014 05:06 AM, Roger Pau Monne wrote:
> This message has been archived. View the original item
> <http://ausev2.amd.com/EnterpriseVault/ViewMessage.asp?VaultId=1EBD12133601C6E47868220A36CFE2B201110000amdvault.amd.com&SavesetId=201407064668470~201406041006380000~Z~B0E2C95E06CD0962996EB51CFC709411>
> According to the comment in p2m.h, AMD IOMMUs don't work correctly
> with page types different than p2m_ram_rw when the p2m is shared
> between HAP and IOMMU, so disable this sharing when using AMD IOMMUs.
>
> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> Cc: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> I have not tested this patch on AMD hardware, so I would like some
> confirmation that this actually works.
> ---
> Changes since v1:
>   - Add a debug message when disabling iommu_hap_pt_share.
> ---
>   xen/drivers/passthrough/amd/iommu_init.c |    8 ++++++++
>   1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/xen/drivers/passthrough/amd/iommu_init.c
> b/xen/drivers/passthrough/amd/iommu_init.c
> index b2f74ef..56bda00 100644
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -1255,6 +1255,14 @@ int __init amd_iommu_init(void)
>       if ( iterate_ivrs_mappings(amd_iommu_setup_device_table) != 0 )
>           goto error_out;
>
> +    /*
> +     * Disable sharing HAP page tables with AMD IOMMU,
> +     * since it only supports p2m_ram_rw, and this would
> +     * prevent doing IO to/from mapped grant frames.
> +     */
> +    iommu_hap_pt_share = 0;
> +    printk(XENLOG_DEBUG "AMD-Vi: Disabled HAP memory map sharing with
> IOMMU");
> +
>       /* per iommu initialization  */
>       for_each_amd_iommu ( iommu )
>           if ( amd_iommu_init_one(iommu) != 0 )
> --
> 1.7.7.5 (Apple Git-26)
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel