Re: [Xen-devel] [PATCH 2/2] VT-d: section placement and type adjustments

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Jan Beulich wrote on 2015-09-29:
> With x2APIC requiring iommu_supports_eim() to return true, we can
> adjust a few conditonals such that both it and
> platform_supports_x2apic() can be marked __init. For the latter as
> well as for platform_supports_intremap() also change the return types
> to bool_t.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -901,16 +901,17 @@ void acpi_dmar_zap(void)
>          write_atomic((uint32_t*)&dmar_table->signature[0], sig);
>  }
> -int platform_supports_intremap(void)
> +bool_t platform_supports_intremap(void)
>  {
> -    unsigned int mask = ACPI_DMAR_INTR_REMAP;
> +    const unsigned int mask = ACPI_DMAR_INTR_REMAP;
> 
>      return (dmar_flags & mask) == ACPI_DMAR_INTR_REMAP;
>  }
> -int platform_supports_x2apic(void)
> +bool_t __init platform_supports_x2apic(void)
>  {
> -    unsigned int mask = ACPI_DMAR_INTR_REMAP |
> ACPI_DMAR_X2APIC_OPT_OUT; +    const unsigned int mask =
> ACPI_DMAR_INTR_REMAP | ACPI_DMAR_X2APIC_OPT_OUT; +
>      return cpu_has_x2apic && ((dmar_flags & mask) ==
> ACPI_DMAR_INTR_REMAP);
>  }
> --- a/xen/drivers/passthrough/vtd/extern.h
> +++ b/xen/drivers/passthrough/vtd/extern.h
> @@ -93,8 +93,8 @@ void vtd_ops_preamble_quirk(struct iommu
>  void vtd_ops_postamble_quirk(struct iommu* iommu);
>  void me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map);
>  void pci_vtd_quirk(const struct pci_dev *);
> -int platform_supports_intremap(void);
> -int platform_supports_x2apic(void);
> +bool_t platform_supports_intremap(void);
> +bool_t platform_supports_x2apic(void);
> 
>  void vtd_set_hwdom_mapping(struct domain *d);
> --- a/xen/drivers/passthrough/vtd/intremap.c
> +++ b/xen/drivers/passthrough/vtd/intremap.c
> @@ -143,7 +143,7 @@ static void set_hpet_source_id(unsigned
>      set_ire_sid(ire, SVT_VERIFY_SID_SQ, SQ_13_IGNORE_3,
> hpetid_to_bdf(id));
>  }
> -bool_t iommu_supports_eim(void)
> +bool_t __init iommu_supports_eim(void)
>  {
>      struct acpi_drhd_unit *drhd; unsigned int apic; @@ -832,11 +832,16
>      @@ int iommu_enable_x2apic_IR(void) struct acpi_drhd_unit *drhd;
>      struct iommu *iommu;
> -    if ( !iommu_supports_eim() )
> -        return -EOPNOTSUPP;
> +    if ( system_state < SYS_STATE_active )
> +    {
> +        if ( !iommu_supports_eim() )
> +            return -EOPNOTSUPP;
> 
> -    if ( !platform_supports_x2apic() )
> -        return -ENXIO;
> +        if ( !platform_supports_x2apic() )
> +            return -ENXIO;
> +    }
> +    else if ( !x2apic_enabled )
> +        return -EOPNOTSUPP;

Why need the last check here? From the code, this check is called only in 
resume_x2apic() which already has an assert there: ASSERT(x2apic_enabled) .

Best regards,
Yang



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel