On 28/02/2020 12:12, Jan Beulich wrote:
> The wider cluster mode APIC IDs aren't generally representable. Convert
> the iommu_intremap variable into a tristate, allowing the AMD IOMMU
> driver to signal this special restriction to the apic_x2apic_probe().
> (Note: assignments to the variable get adjusted, while existing
> consumers - all assuming a boolean property - are left alone.)
I think it would be helpful to state that while we are not aware of any
hardware with this as a restriction, it is a situation which can be
created on fully x2apic-capable systems via firmware settings.
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
> ---
> v2: New.
>
> --- a/xen/arch/x86/genapic/x2apic.c
> +++ b/xen/arch/x86/genapic/x2apic.c
> @@ -236,12 +236,21 @@ const struct genapic *__init apic_x2apic
> x2apic_phys = !iommu_intremap ||
> (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL);
> }
> - else if ( !x2apic_phys && !iommu_intremap )
> - {
> - printk("WARNING: x2APIC cluster mode is not supported without
> interrupt remapping\n"
> - "x2APIC: forcing phys mode\n");
> - x2apic_phys = true;
> - }
> + else if ( !x2apic_phys )
> + switch ( iommu_intremap )
> + {
> + case iommu_intremap_off:
> + case iommu_intremap_restricted:
> + printk("WARNING: x2APIC cluster mode is not supported %s
> interrupt remapping\n"
> + "x2APIC: forcing phys mode\n",
Any chance to fold this into a single line with "- forcing phys mode\n"
as a suffix?
> + iommu_intremap == iommu_intremap_off ? "without"
> + : "with restricted");
> + x2apic_phys = true;
> + break;
> +
> + case iommu_intremap_full:
> + break;
> + }
>
> if ( x2apic_phys )
> return &apic_x2apic_phys;
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -54,7 +54,18 @@ static inline bool_t dfn_eq(dfn_t x, dfn
>
> extern bool_t iommu_enable, iommu_enabled;
> extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
> -extern bool_t iommu_snoop, iommu_qinval, iommu_intremap, iommu_intpost;
> +extern bool_t iommu_snoop, iommu_qinval, iommu_intpost;
> +extern enum __packed iommu_intremap {
> + /*
> + * In order to allow traditional boolean uses of the iommu_intremap
> + * variable, the "off" value has to come first (yielding a value of zero).
> + */
> + iommu_intremap_off,
> +#ifdef CONFIG_X86
> + iommu_intremap_restricted,
This needs a note about its meaning. How about this?
/* Interrupt remapping enabled, but only able to generate interrupts
with an 8-bit APIC ID. */
Otherwise, Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>
Not an issue for now, but "restricted" might become ambiguous with
future extensions.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
