Re: [Xen-devel] [PATCH] amd/iommu: remove hidden AMD inclusive mappings
Roger,
I'm still catching up on the map-inclusive and map-reserved stuff.
I have a couple questions below.
On 9/21/18 10:20 PM, Roger Pau Monne wrote:
And just rely on arch_iommu_hwdom_init to setup the correct inclusive
mappings as it's done for Intel.
AMD has code in amd_iommu_hwdom_init to setup inclusive mappings up to
max_pdx, remove this since it's now a duplication of
arch_iommu_hwdom_init. Note that AMD mapped every page with a valid
mfn up to max_pdx, arch_iommu_hwdom_init will only do so for memory
below 4GB, so this is a functional change for AMD.
Is there any reasons why limit to only below 4GB?
Move the default setting of iommu_hwdom_{inclusive/reserved} to
arch_iommu_hwdom_init since the defaults are now the same for both
Intel and AMD.
Reported-by: Paul Durrant
Signed-off-by: Roger Pau Monné
---
Cc: Suravee Suthikulpanit
Cc: Brian Woods
Cc: Kevin Tian
Cc: Jan Beulich
Cc: Paul Durrant
---
xen/drivers/passthrough/amd/pci_amd_iommu.c | 39 -
xen/drivers/passthrough/vtd/iommu.c | 7
xen/drivers/passthrough/x86/iommu.c | 8 -
3 files changed, 7 insertions(+), 47 deletions(-)
...
>
diff --git a/xen/drivers/passthrough/x86/iommu.c
b/xen/drivers/passthrough/x86/iommu.c
index b7c8b5be41..2de8822c59 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -210,7 +210,13 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
BUG_ON(!is_hardware_domain(d));
-ASSERT(iommu_hwdom_inclusive != -1 && iommu_hwdom_inclusive != -1);
Not sure if this was a typo. The logic looks strange. Anyhow, it is removed.
However, I notice in the xen/drivers/passthrough/iommu.c that
the parsing logic for the map-reserved option is setting the
iommu_hwdom_inclusive
instead of the iommu_hwdom_reserved. Is that intentional?
Also, what's the difference b/w the option map-inclusive parameter in
drivers/passthrough/iommu.c:
parse_dom0_iommu_param() and the option iommu_inclusive_mapping in
drivers/passthrough/vtd/x86/vtd.c?
+/* Inclusive mappings are enabled by default for PV. */
+if ( iommu_hwdom_inclusive == -1 )
+iommu_hwdom_inclusive = is_pv_domain(d);
+/* Reserved IOMMU mappings are enabled by default. */
+if ( iommu_hwdom_reserved == -1 )
+iommu_hwdom_reserved = 1;
+
if ( iommu_hwdom_inclusive && !is_pv_domain(d) )
{
printk(XENLOG_WARNING
Thanks,
Suravee
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 6/6] x86/iommu: add map-reserved dom0-iommu option to map reserved memory ranges
On 9/7/18 4:07 PM, Roger Pau Monne wrote: Several people have reported hardware issues (malfunctioning USB controllers) due to iommu page faults on Intel hardware. Those faults are caused by missing RMRR (VTd) entries in the ACPI tables. Those can be worked around on VTd hardware by manually adding RMRR entries on the command line, this is however limited to Intel hardware and quite cumbersome to do. In order to solve those issues add a new dom0-iommu=map-reserved option that identity maps all regions marked as reserved in the memory map. Note that regions used by devices emulated by Xen (LAPIC, IO-APIC or PCIe MCFG regions) are specifically avoided. Note that this option is available to all Dom0 modes (as opposed to the inclusive option which only works for PV Dom0). Signed-off-by: Roger Pau Monné Reviewed-by: Kevin Tian Reviewed-by: Wei Liu Acked-by: Jan Beulich --- Changes since v7: - Don't use true/false with int8_t. - Print a warning message if map-reserved is set on ARM. Changes since v6: - Reword the map-reserved help to make it clear it's available to both PV and PVH Dom0. - Assign type inside of the switch expression. - Remove the comment about IO-APIC MMIO relocation, this is not supported ATM. Changes since v5: - Merge with the vpci MMCFG helper patch. - Add a TODO item about the issues with relocating the LAPIC or IOAPIC MMIO regions. - Use the newly introduced page_get_ram_type that returns all the types that fall between a page. - Use paging_mode_translate instead of iommu_use_hap_pt when deciding whether to use set_identity_p2m_entry or iommu_map_page. Changes since v4: - Use pfn_to_paddr. - Rebase on top of previous changes. - Change the default option setting to use if instead of a ternary operator. - Rename to map-reserved. Changes since v3: - Add mappings if the iommu page tables are shared. Changes since v2: - Fix comment regarding dom0-strict. - Change documentation style of xen command line. - Rename iommu_map to hwdom_iommu_map. - Move all the checks to hwdom_iommu_map. Changes since v1: - Introduce a new reserved option instead of abusing the inclusive option. - Use the same helper function for PV and PVH in order to decide if a page should be added to the domain page tables. - Use the data inside of the domain struct to detect overlaps with emulated MMIO regions. --- Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu Cc: Paul Durrant Cc: Suravee Suthikulpanit Cc: Brian Woods Cc: Kevin Tian For AMD-related changes. Acked-by: Suravee Suthikulpanit Thank you, Suravee ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 3/6] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu
On 9/7/18 4:07 PM, Roger Pau Monne wrote:
Introduce a new dom0-iommu=map-inclusive generic option that
supersedes iommu_inclusive_mapping. The previous behavior is preserved
and the option should only be enabled by default on Intel hardware.
Signed-off-by: Roger Pau Monné
Reviewed-by: Paul Durrant
Reviewed-by: Jan Beulich
Reviewed-by: Kevin Tian
---
Changes since v7:
- Use -1, 0, 1 in iommu_hwdom_inclusive.
- Print a warning message if map-inclusive is set on ARM.
Changes since v4:
- Use an if to set the default option value.
- Set the option to false unconditionally on ARM.
Changes since v2:
- Fix typo in commit message.
- Change style and text of the documentation in xen command line.
- Set the defaults in {intel/amd}_iommu_hwdom_init for inclusive.
- Re-add the iommu_dom0_passthrough || !is_pv_domain(d) check.
Changes since v1:
- Use dom0-iommu instead of the iommu option.
- Only enable by default on Intel hardware.
---
Cc: Andrew Cooper
Cc: George Dunlap
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Julien Grall
Cc: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini
Cc: Tim Deegan
Cc: Wei Liu
Cc: Suravee Suthikulpanit
Cc: Brian Woods
Cc: Kevin Tian
For AMD-related changes.
Acked-by: Suravee Suthikulpanit
Thank you,
Suravee
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 1/6] iommu: rename iommu_dom0_strict and iommu_passthrough
Roger, On 9/7/18 4:07 PM, Roger Pau Monne wrote: To iommu_hwdom_strict and iommu_hwdom_passthrough which is more descriptive of their usage. Also change their type from bool_t to bool. No functional change. Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich Reviewed-by: Kevin Tian Reviewed-by: Wei Liu --- Cc: Jan Beulich Cc: Andrew Cooper Cc: Suravee Suthikulpanit Cc: Brian Woods Cc: Kevin Tian --- Changes since v5: - Remove unneeded !!. Changes since v4: - New in this version Acked-by: Suravee Suthikulpanit Thank you, Suravee ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC 07/10] passthrough/amd: split out hvm code from iommu_map.c
Wei, On 2/21/18 3:46 PM, Wei Liu wrote: Move and rename update_paging_mode. Create a local header file for this and other functions that need exporting. Signed-off-by: Wei Liu --- Cc: Suravee Suthikulpanit --- xen/drivers/passthrough/x86/amd/Makefile| 1 + xen/drivers/passthrough/x86/amd/hvm.c | 108 xen/drivers/passthrough/x86/amd/iommu.h | 32 + xen/drivers/passthrough/x86/amd/iommu_map.c | 103 ++ 4 files changed, 148 insertions(+), 96 deletions(-) create mode 100644 xen/drivers/passthrough/x86/amd/hvm.c create mode 100644 xen/drivers/passthrough/x86/amd/iommu.h I am still not sure why the update_paging_mode need to be moved out of iommu_map.c. Could you please elaborate? Thanks, Suravee ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC 06/10] passthrough/amd: remove guest iommu support
Wei, On 2/21/18 3:46 PM, Wei Liu wrote: It is never used and it is getting in the way of cleaning up. The only callsite of guest_iommu_add_ppr_log has no effect because guest iommu is not initialised. Signed-off-by: Wei Liu --- Cc: Suravee Suthikulpanit --- xen/drivers/passthrough/x86/amd/Makefile | 1 - xen/drivers/passthrough/x86/amd/iommu_guest.c | 927 -- xen/drivers/passthrough/x86/amd/iommu_init.c | 21 +- xen/include/asm-x86/amd-iommu.h | 51 -- xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 8 - xen/include/asm-x86/iommu.h | 1 - 6 files changed, 6 insertions(+), 1003 deletions(-) delete mode 100644 xen/drivers/passthrough/x86/amd/iommu_guest.c Would it be too much trouble if we keep this code for now. There is a plan to support IOMMU in guest in the near future. This could be clean up at that point if no longer needed. Thanks, Suravee ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
