> -----Original Message-----
> From: Xen-devel [mailto:[email protected]] On Behalf
> Of Roger Pau Monne
> Sent: 27 July 2018 16:32
> To: [email protected]
> Cc: Kevin Tian <[email protected]>; Stefano Stabellini
> <[email protected]>; Wei Liu <[email protected]>; George Dunlap
> <[email protected]>; Andrew Cooper
> <[email protected]>; Ian Jackson <[email protected]>; Tim
> (Xen.org) <[email protected]>; Julien Grall <[email protected]>; Jan Beulich
> <[email protected]>; Roger Pau Monne <[email protected]>
> Subject: [Xen-devel] [PATCH 2/4] iommu: generalize
> iommu_inclusive_mapping
>
> Introduce a new iommu=inclusive generic option that supersedes
> iommu_inclusive_mapping. This should be a non-functional change on
> Intel hardware, while AMD hardware will gain the same functionality of
> mapping almost everything below the 4GB boundary.
>
> Note that is a noop for ARM hardware.
>
> Signed-off-by: Roger Pau Monné <[email protected]>
> ---
> Cc: Andrew Cooper <[email protected]>
> Cc: George Dunlap <[email protected]>
> Cc: Ian Jackson <[email protected]>
> Cc: Jan Beulich <[email protected]>
> Cc: Julien Grall <[email protected]>
> Cc: Konrad Rzeszutek Wilk <[email protected]>
> Cc: Stefano Stabellini <[email protected]>
> Cc: Tim Deegan <[email protected]>
> Cc: Wei Liu <[email protected]>
> Cc: Kevin Tian <[email protected]>
> ---
> docs/misc/xen-command-line.markdown | 14 ++++++
> xen/drivers/passthrough/arm/iommu.c | 4 ++
> xen/drivers/passthrough/iommu.c | 6 +++
> xen/drivers/passthrough/vtd/extern.h | 2 -
> xen/drivers/passthrough/vtd/iommu.c | 6 ---
> xen/drivers/passthrough/vtd/x86/vtd.c | 66 +------------------------
> xen/drivers/passthrough/x86/iommu.c | 70
> +++++++++++++++++++++++++++
> xen/include/xen/iommu.h | 2 +
> 8 files changed, 97 insertions(+), 73 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-
> command-line.markdown
> index 65b4754418..91a8bfc9a6 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1198,6 +1198,17 @@ detection of systems known to misbehave upon
> accesses to that port.
>
> >> Enable IOMMU debugging code (implies `verbose`).
>
> +> `inclusive`
This is a dom0 (or hwdom) specific setting so perhaps dom0-inclusive?
Actually the dom0 iommu options are starting to get unwieldy as they are
conflated with the general host iommu options so I think it may be worthwhile
splitting things out into a separate 'dom0-iommu=' top level parameter at this
stage. (My reasons are slightly selfish as I intend to add another dom0 iommu
option to give it just reserved regions, to avoid unnecessary set-up if we know
it will be using PV-IOMMU).
Cheers,
Paul
> +
> +> Default: `true`
> +
> +>> Use this to work around firmware issues providing incorrect RMRR or
> IVMD
> +>> entries. Rather than only mapping RAM pages for IOMMU accesses for
> Dom0,
> +>> with this option all pages up to 4GB, not marked as unusable in the E820
> +>> table, will get a mapping established. Note that this option is only
> +>> applicable to a PV dom0. Also note that if `dom0-strict` mode is enabled
> +>> then conventional RAM pages not assigned to dom0 will not be mapped.
> +
> ### iommu\_dev\_iotlb\_timeout
> > `= <integer>`
>
> @@ -1212,6 +1223,9 @@ wait descriptor timed out', try increasing this value.
>
> > Default: `true`
>
> +**WARNING: This command line option is deprecated, and superseded by
> +_iommu=inclusive_ - using both options in combination is undefined.**
> +
> Use this to work around firmware issues providing incorrect RMRR entries.
> Rather than only mapping RAM pages for IOMMU accesses for Dom0, with
> this
> option all pages up to 4GB, not marked as unusable in the E820 table, will
> diff --git a/xen/drivers/passthrough/arm/iommu.c
> b/xen/drivers/passthrough/arm/iommu.c
> index 95b1abb972..325997b19f 100644
> --- a/xen/drivers/passthrough/arm/iommu.c
> +++ b/xen/drivers/passthrough/arm/iommu.c
> @@ -73,3 +73,7 @@ int arch_iommu_populate_page_table(struct domain
> *d)
> /* The IOMMU shares the p2m with the CPU */
> return -ENOSYS;
> }
> +
> +void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> +{
> +}
> diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> index 70d218f910..3f3aa71b2c 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -47,6 +47,9 @@ integer_param("iommu_dev_iotlb_timeout",
> iommu_dev_iotlb_timeout);
> * no-igfx Disable VT-d for IGD devices (insecure)
> * no-amd-iommu-perdev-intremap Don't use per-device interrupt
> remapping
> * tables (insecure)
> + * inclusive Map additional regions into the IOMMU page
> + * tables in order to workaround bugs in ACPI
> + * tables.
> */
> custom_param("iommu", parse_iommu_param);
> bool_t __initdata iommu_enable = 1;
> @@ -60,6 +63,7 @@ bool_t __read_mostly iommu_passthrough;
> bool_t __read_mostly iommu_snoop = 1;
> bool_t __read_mostly iommu_qinval = 1;
> bool_t __read_mostly iommu_intremap = 1;
> +bool __hwdom_initdata iommu_inclusive = true;
>
> /*
> * In the current implementation of VT-d posted interrupts, in some
> extreme
> @@ -208,6 +212,8 @@ void __hwdom_init iommu_hwdom_init(struct
> domain *d)
> }
>
> hd->platform_ops->hwdom_init(d);
> +
> + arch_iommu_hwdom_init(d);
> }
>
> void iommu_teardown(struct domain *d)
> diff --git a/xen/drivers/passthrough/vtd/extern.h
> b/xen/drivers/passthrough/vtd/extern.h
> index fb7edfaef9..91cadc602e 100644
> --- a/xen/drivers/passthrough/vtd/extern.h
> +++ b/xen/drivers/passthrough/vtd/extern.h
> @@ -99,6 +99,4 @@ void pci_vtd_quirk(const struct pci_dev *);
> bool_t platform_supports_intremap(void);
> bool_t platform_supports_x2apic(void);
>
> -void vtd_set_hwdom_mapping(struct domain *d);
> -
> #endif // _VTD_EXTERN_H_
> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> index 1710256823..569ec4aec2 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1304,12 +1304,6 @@ static void __hwdom_init
> intel_iommu_hwdom_init(struct domain *d)
> {
> struct acpi_drhd_unit *drhd;
>
> - if ( !iommu_passthrough && is_pv_domain(d) )
> - {
> - /* Set up 1:1 page table for hardware domain. */
> - vtd_set_hwdom_mapping(d);
> - }
> -
> setup_hwdom_pci_devices(d, setup_hwdom_device);
> setup_hwdom_rmrr(d);
>
> diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c
> b/xen/drivers/passthrough/vtd/x86/vtd.c
> index cc2bfea162..55d74a97e2 100644
> --- a/xen/drivers/passthrough/vtd/x86/vtd.c
> +++ b/xen/drivers/passthrough/vtd/x86/vtd.c
> @@ -35,8 +35,7 @@
> * iommu_inclusive_mapping: when set, all memory below 4GB is included in
> dom0
> * 1:1 iommu mappings except xen and unusable regions.
> */
> -static bool_t __hwdom_initdata iommu_inclusive_mapping = 1;
> -boolean_param("iommu_inclusive_mapping", iommu_inclusive_mapping);
> +boolean_param("iommu_inclusive_mapping", iommu_inclusive);
>
> void *map_vtd_domain_page(u64 maddr)
> {
> @@ -108,66 +107,3 @@ void hvm_dpci_isairq_eoi(struct domain *d,
> unsigned int isairq)
> spin_unlock(&d->event_lock);
> }
>
> -void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
> -{
> - unsigned long i, j, tmp, top, max_pfn;
> -
> - BUG_ON(!is_hardware_domain(d));
> -
> - max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
> - top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
> -
> - for ( i = 0; i < top; i++ )
> - {
> - unsigned long pfn = pdx_to_pfn(i);
> - bool map;
> - int rc = 0;
> -
> - /*
> - * Set up 1:1 mapping for dom0. Default to include only
> - * conventional RAM areas and let RMRRs include needed reserved
> - * regions. When set, the inclusive mapping additionally maps in
> - * every pfn up to 4GB except those that fall in unusable ranges.
> - */
> - if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
> - continue;
> -
> - if ( iommu_inclusive_mapping && pfn <= max_pfn )
> - map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
> - else
> - map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
> -
> - if ( !map )
> - continue;
> -
> - /* Exclude Xen bits */
> - if ( xen_in_range(pfn) )
> - continue;
> -
> - /*
> - * If dom0-strict mode is enabled then exclude conventional RAM
> - * and let the common code map dom0's pages.
> - */
> - if ( iommu_dom0_strict &&
> - page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
> - continue;
> -
> - tmp = 1 << (PAGE_SHIFT - PAGE_SHIFT_4K);
> - for ( j = 0; j < tmp; j++ )
> - {
> - int ret = iommu_map_page(d, pfn * tmp + j, pfn * tmp + j,
> - IOMMUF_readable|IOMMUF_writable);
> -
> - if ( !rc )
> - rc = ret;
> - }
> -
> - if ( rc )
> - printk(XENLOG_WARNING VTDPREFIX " d%d: IOMMU mapping failed:
> %d\n",
> - d->domain_id, rc);
> -
> - if (!(i & (0xfffff >> (PAGE_SHIFT - PAGE_SHIFT_4K))))
> - process_pending_softirqs();
> - }
> -}
> -
> diff --git a/xen/drivers/passthrough/x86/iommu.c
> b/xen/drivers/passthrough/x86/iommu.c
> index 68182afd91..ba0bbd9a15 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -20,6 +20,8 @@
> #include <xen/softirq.h>
> #include <xsm/xsm.h>
>
> +#include <asm/setup.h>
> +
> void iommu_update_ire_from_apic(
> unsigned int apic, unsigned int reg, unsigned int value)
> {
> @@ -132,6 +134,74 @@ void arch_iommu_domain_destroy(struct domain
> *d)
> {
> }
>
> +void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> +{
> + unsigned long i, j, tmp, top, max_pfn;
> +
> + if ( iommu_passthrough || !is_pv_domain(d) )
> + return;
> +
> + BUG_ON(!is_hardware_domain(d));
> +
> + max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
> + top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
> +
> + for ( i = 0; i < top; i++ )
> + {
> + unsigned long pfn = pdx_to_pfn(i);
> + bool map;
> + int rc = 0;
> +
> + /*
> + * Set up 1:1 mapping for dom0. Default to include only
> + * conventional RAM areas and let RMRRs include needed reserved
> + * regions. When set, the inclusive mapping additionally maps in
> + * every pfn up to 4GB except those that fall in unusable ranges.
> + */
> + if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
> + continue;
> +
> + if ( iommu_inclusive && pfn <= max_pfn )
> + map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
> + else
> + map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
> +
> + if ( !map )
> + continue;
> +
> + /* Exclude Xen bits */
> + if ( xen_in_range(pfn) )
> + continue;
> +
> + /*
> + * If dom0-strict mode is enabled then exclude conventional RAM
> + * and let the common code map dom0's pages.
> + */
> + if ( iommu_dom0_strict &&
> + page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
> + continue;
> +
> + tmp = 1 << (PAGE_SHIFT - PAGE_SHIFT_4K);
> + for ( j = 0; j < tmp; j++ )
> + {
> + int ret = iommu_map_page(d, pfn * tmp + j, pfn * tmp + j,
> + IOMMUF_readable|IOMMUF_writable);
> +
> + if ( !rc )
> + rc = ret;
> + }
> +
> + if ( rc )
> + printk(XENLOG_WARNING "d%d: IOMMU mapping failed: %d\n",
> + d->domain_id, rc);
> +
> + if (!(i & (0xfffff >> (PAGE_SHIFT - PAGE_SHIFT_4K))))
> + process_pending_softirqs();
> + }
> +
> +
> +}
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 6b42e3b876..787566a4e7 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -35,6 +35,7 @@ extern bool_t iommu_snoop, iommu_qinval,
> iommu_intremap, iommu_intpost;
> extern bool_t iommu_hap_pt_share;
> extern bool_t iommu_debug;
> extern bool_t amd_iommu_perdev_intremap;
> +extern bool iommu_inclusive;
>
> extern unsigned int iommu_dev_iotlb_timeout;
>
> @@ -49,6 +50,7 @@ void arch_iommu_domain_destroy(struct domain *d);
> int arch_iommu_domain_init(struct domain *d);
> int arch_iommu_populate_page_table(struct domain *d);
> void arch_iommu_check_autotranslated_hwdom(struct domain *d);
> +void arch_iommu_hwdom_init(struct domain *d);
>
> int iommu_construct(struct domain *d);
>
> --
> 2.18.0
>
>
> _______________________________________________
> Xen-devel mailing list
> [email protected]
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
