Re: [Xen-devel] [PATCH v2] AMD IOMMU: Introduce support for IVHD block type 11h
Thanks for these review comment. I'll update this patch and send out V3 Suravee ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] AMD IOMMU: Introduce support for IVHD block type 11h
On 22/05/16 01:20, suravee.suthikulpa...@amd.com 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_TYPEACPI_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@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] AMD IOMMU: Introduce support for IVHD block type 11h
From: Suravee SuthikulpanitAlong with the IVHD block type 10h, newer AMD platforms also come with types 11h, which is a superset of the older one. Having multiple IVHD block types in the same platform allows backward compatibility of newer systems to work with existing drivers. The driver should only parse the highest-level (newest) type of IVHD block that it can support. However, the current driver returns error when encounters with unknown IVHD block type. This causes existing driver to unnecessarily fail IOMMU initialization on new systems. This patch introduces a new logic, which scans through IVRS table looking for the highest-level supporsted IVHD block type. It also adds support for the new IVHD block type 11h. More information about the IVHD type 11h can be found in the AMD I/O Virtualization Technology (IOMMU) Specification rev 2.62. http://support.amd.com/TechDocs/48882_IOMMU.pdf Signed-off-by: Suravee Suthikulpanit --- Changes from V1: * Update get_ivhd_header_size() to use switch statment. * Update logic in get_supported_ivhd_type(). * Move ACPI_FADT_NO_MSI flag check to amd_iommu_init() since it should be checked before any IVRS parsing. * Clean up various styling issues. xen/drivers/passthrough/amd/iommu_acpi.c | 137 ++ xen/drivers/passthrough/amd/iommu_init.c | 10 +- xen/include/acpi/actbl2.h | 7 +- xen/include/asm-x86/amd-iommu.h | 1 + xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 1 + 5 files changed, 111 insertions(+), 45 deletions(-) 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 +get_ivhd_header_size(const struct acpi_ivrs_hardware *ivhd_block) +{ +int ret = 0; + +switch ( ivhd_block->header.type ) +{ +case ACPI_IVRS_TYPE_HARDWARE: +ret = offsetof(struct acpi_ivrs_hardware, efr_image); +break; +case ACPI_IVRS_TYPE_HARDWARE_11H: +ret = sizeof(struct acpi_ivrs_hardware); +break; +default: +break; +} +return ret; +} + static int __init parse_ivhd_block(const struct acpi_ivrs_hardware *ivhd_block) { const union acpi_ivhd_device *ivhd_device; u16 block_length, dev_length; +unsigned int hdr_size = get_ivhd_header_size(ivhd_block) ; struct amd_iommu *iommu; -if ( ivhd_block->header.length < sizeof(*ivhd_block) ) +if ( ivhd_block->header.length < hdr_size ) { AMD_IOMMU_DEBUG("IVHD Error: Invalid Block Length!\n"); return -ENODEV; @@ -845,7 +865,7 @@ static int __init parse_ivhd_block(const struct acpi_ivrs_hardware *ivhd_block) } /* parse Device Entries */ -block_length = sizeof(*ivhd_block); +block_length = hdr_size; while ( ivhd_block->header.length >= (block_length + sizeof(struct acpi_ivrs_de_header)) ) { @@ -914,34 +934,6 @@ static int __init parse_ivhd_block(const struct acpi_ivrs_hardware *ivhd_block) return 0; } -static int __init parse_ivrs_block(const struct acpi_ivrs_header *ivrs_block) -{ -const struct acpi_ivrs_hardware *ivhd_block; -const struct acpi_ivrs_memory *ivmd_block; - -switch ( ivrs_block->type ) -{ -case ACPI_IVRS_TYPE_HARDWARE: -ivhd_block = container_of(ivrs_block, const struct acpi_ivrs_hardware, - header); -return parse_ivhd_block(ivhd_block); - -case ACPI_IVRS_TYPE_MEMORY_ALL: -case ACPI_IVRS_TYPE_MEMORY_ONE: -case ACPI_IVRS_TYPE_MEMORY_RANGE: -case ACPI_IVRS_TYPE_MEMORY_IOMMU: -ivmd_block = container_of(ivrs_block, const struct acpi_ivrs_memory, - header); -return parse_ivmd_block(ivmd_block); - -default: -AMD_IOMMU_DEBUG("IVRS Error: Invalid Block Type!\n"); -return -ENODEV; -} - -return 0; -} - static void __init dump_acpi_table_header(struct acpi_table_header *table) { int i; @@ -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 ) && \ + (( x == ACPI_IVRS_TYPE_HARDWARE ) || \ + ( x == ACPI_IVRS_TYPE_HARDWARE_11H ))) + +#define is_ivmd_block(x) \ +(( x == ACPI_IVRS_TYPE_MEMORY_ALL ) || \ + ( x == ACPI_IVRS_TYPE_MEMORY_ONE ) || \ + ( x == ACPI_IVRS_TYPE_MEMORY_RANGE ) || \ + ( x == ACPI_IVRS_TYPE_MEMORY_IOMMU )) + static int __init