Re: [Xen-devel] [PATCH v2] AMD IOMMU: Introduce support for IVHD block type 11h

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/05/16 01:20, suravee.suthikulpanit@xxxxxxx wrote:
> diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c 
> b/xen/drivers/passthrough/amd/iommu_acpi.c
> index 79c1f8c..c4eec50 100644
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -821,13 +821,33 @@ static u16 __init parse_ivhd_device_special(
>      return dev_length;
>  }
>  
> +static inline unsigned int

size_t

> +get_ivhd_header_size(const struct acpi_ivrs_hardware *ivhd_block)
> +{
> +    int ret = 0;

You don't need this variable at all.  Also, you have signed/unsigned
mismatch.

> +
> +    switch ( ivhd_block->header.type )
> +    {
> +    case ACPI_IVRS_TYPE_HARDWARE:
> +        ret = offsetof(struct acpi_ivrs_hardware, efr_image);

Just return straight from here.

> @@ -978,6 +970,22 @@ static void __init dump_acpi_table_header(struct 
> acpi_table_header *table)
>  
>  }
>  
> +#define to_ivhd_block(hdr) \
> +    container_of(hdr, const struct acpi_ivrs_hardware, header)
> +#define to_ivmd_block(hdr) \
> +    container_of(hdr, const struct acpi_ivrs_memory, header)
> +
> +#define is_ivhd_block(x) \
> +    (( x <= IVHD_HIGHEST_SUPPORT_TYPE ) && \

This <= calculation is redundant with the following exact tests.

Please use static inlines here, which will also fix the current side
effect and bracketing issues.

> @@ -1207,8 +1215,51 @@ int __init amd_iommu_get_ivrs_dev_entries(void)
>  
>  int __init amd_iommu_update_ivrs_mapping_acpi(void)
>  {
> -    if ( unlikely(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_MSI) )
> -        return -EPERM;
> -
>      return acpi_table_parse(ACPI_SIG_IVRS, parse_ivrs_table);
>  }
> +
> +static int __init
> +get_supported_ivhd_type(struct acpi_table_header *table)
> +{
> +    unsigned long length = sizeof(struct acpi_table_ivrs);

size_t

> diff --git a/xen/include/acpi/actbl2.h b/xen/include/acpi/actbl2.h
> index 4341a30..a25ed48 100644
> --- a/xen/include/acpi/actbl2.h
> +++ b/xen/include/acpi/actbl2.h
> @@ -589,12 +589,15 @@ struct acpi_ivrs_header {
>  
>  enum acpi_ivrs_type {
>       ACPI_IVRS_TYPE_HARDWARE = 0x10,
> +     ACPI_IVRS_TYPE_HARDWARE_11H = 0x11,
>       ACPI_IVRS_TYPE_MEMORY_ALL /* _MEMORY1 */ = 0x20,
>       ACPI_IVRS_TYPE_MEMORY_ONE /* _MEMORY2 */ = 0x21,
>       ACPI_IVRS_TYPE_MEMORY_RANGE /* _MEMORY3 */ = 0x22,
>       ACPI_IVRS_TYPE_MEMORY_IOMMU = 0x23
>  };
>  
> +#define IVHD_HIGHEST_SUPPORT_TYPE    ACPI_IVRS_TYPE_HARDWARE_11H

I don't think you need this any more.

> diff --git a/xen/include/asm-x86/amd-iommu.h b/xen/include/asm-x86/amd-iommu.h
> index e9fa9c2..18c9f8e 100644
> --- a/xen/include/asm-x86/amd-iommu.h
> +++ b/xen/include/asm-x86/amd-iommu.h
> @@ -126,6 +126,7 @@ struct ivrs_mappings {
>  };
>  
>  extern unsigned int ivrs_bdf_entries;
> +extern int ivhd_type;

The type field in an ivrs header is u8.

~Andrew

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