Antwort: [PATCH v2 18/39] acpi: Add support for DMAR

[Wolfgang Wallner]

Hi Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> 
> The DMA Remapping Reporting (DMAR) table contains information about DMA
> remapping.
> 
> Add a version simple version of this table with only the minimum fields
> filled out. i.e. no entries.
> 
> Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> Changes in v2:
> - Drop two unnecessary __packed
> - Move __packed to after struct
> 
>  include/acpi_table.h  | 57 +++++++++++++++++++++++++++++++++++++++++++
>  lib/acpi/acpi_table.c | 26 ++++++++++++++++++++
>  test/dm/acpi.c        | 14 +++++++++++
>  3 files changed, 97 insertions(+)
> 
> diff --git a/include/acpi_table.h b/include/acpi_table.h
> index ccf6fa04db..db84b79be5 100644
> --- a/include/acpi_table.h
> +++ b/include/acpi_table.h
> @@ -21,6 +21,9 @@
>  #define ACPI_RSDP_REV_ACPI_1_0	0
>  #define ACPI_RSDP_REV_ACPI_2_0	2
>  
> +/* TODO(sjg at chromium.org): Figure out how to get compiler revision */
> +#define ASL_REVISION	0
> +
>  #if !defined(__ACPI__)
>  
>  /*
> @@ -360,6 +363,51 @@ struct acpi_csrt_shared_info {
>  	u32 max_block_size;
>  };
>  
> +enum dmar_type {
> +	DMAR_DRHD = 0,
> +	DMAR_RMRR = 1,
> +	DMAR_ATSR = 2,
> +	DMAR_RHSA = 3,
> +	DMAR_ANDD = 4
> +};
> +
> +enum {
> +	DRHD_INCLUDE_PCI_ALL = 1

Could you use "1 << 0" or "BIT(0)" ?
Similar as the flag definitions below.

> +};
> +
> +enum dmar_flags {
> +	DMAR_INTR_REMAP			= 1 << 0,
> +	DMAR_X2APIC_OPT_OUT		= 1 << 1,
> +	DMA_CTRL_PLATFORM_OPT_IN_FLAG	= 1 << 2,

The first two entries are prefixed with DMAR_, the last one is not.
Is this on purpose?

Could you use BIT(x) here?

> +};
> +
> +struct dmar_entry {
> +	u16 type;
> +	u16 length;
> +	u8 flags;
> +	u8 reserved;
> +	u16 segment;
> +	u64 bar;
> +};
> +
> +struct dmar_rmrr_entry {
> +	u16 type;
> +	u16 length;
> +	u16 reserved;
> +	u16 segment;
> +	u64 bar;
> +	u64 limit;
> +};
> +
> +/* DMAR (DMA Remapping Reporting Structure) */
> +struct __packed acpi_dmar {
> +	struct acpi_table_header header;
> +	u8 host_address_width;
> +	u8 flags;
> +	u8 reserved[10];
> +	struct dmar_entry structure[0];
> +};
> +
>  /* DBG2 definitions are partially used for SPCR interface_type */
>  
>  /* Types for port_type field */
> @@ -452,6 +500,15 @@ enum acpi_tables {
>   */
>  int acpi_get_table_revision(enum acpi_tables table);
>  
> +/**
> + * acpi_create_dmar() - Create a DMA Remapping Reporting (DMAR) table
> + *
> + * @dmar: Place to put the table
> + * @flags: DMAR flags to use
> + * @return 0 if OK, -ve on error
> + */
> +int acpi_create_dmar(struct acpi_dmar *dmar, enum dmar_flags flags);
> +
>  #endif /* !__ACPI__*/
>  
>  #include <asm/acpi_table.h>
> diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> index 197f965c08..ed312ac663 100644
> --- a/lib/acpi/acpi_table.c
> +++ b/lib/acpi/acpi_table.c
> @@ -6,7 +6,33 @@
>   */
>  
>  #include <common.h>
> +#include <dm.h>
>  #include <acpi_table.h>
> +#include <cpu.h>
> +
> +int acpi_create_dmar(struct acpi_dmar *dmar, enum dmar_flags flags)
> +{
> +	struct acpi_table_header *header = &dmar->header;
> +	struct cpu_info info;
> +	struct udevice *cpu;
> +	int ret;
> +
> +	ret = uclass_first_device(UCLASS_CPU, &cpu);
> +	if (ret)
> +		return log_msg_ret("cpu", ret);
> +	ret = cpu_get_info(cpu, &info);

Here the return value of cpu_get_info() is ignored.

> +	memset((void *)dmar, 0, sizeof(struct acpi_dmar));
> +
> +	/* Fill out header fields. */
> +	acpi_fill_header(&dmar->header, "DMAR");
> +	header->length = sizeof(struct acpi_dmar);
> +	header->revision = acpi_get_table_revision(ACPITAB_DMAR);
> +
> +	dmar->host_address_width = info.address_width - 1;
> +	dmar->flags = flags;
> +
> +	return 0;
> +}
>  
>  int acpi_get_table_revision(enum acpi_tables table)
>  {
> diff --git a/test/dm/acpi.c b/test/dm/acpi.c
> index e65295b7ca..2737896643 100644
> --- a/test/dm/acpi.c
> +++ b/test/dm/acpi.c
> @@ -67,3 +67,17 @@ static int dm_test_acpi_get_table_revision(struct unit_test_state *uts)
>  }
>  DM_TEST(dm_test_acpi_get_table_revision,
>  	DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> +
> +/* Test acpi_create_dmar() */
> +static int dm_test_acpi_create_dmar(struct unit_test_state *uts)
> +{
> +	struct acpi_dmar dmar;
> +
> +	ut_assertok(acpi_create_dmar(&dmar, DMAR_INTR_REMAP));
> +	ut_asserteq(DMAR_INTR_REMAP, dmar.flags);
> +	ut_asserteq(DMAR_INTR_REMAP, dmar.flags);

This test case is inserted twice.

> +	ut_asserteq(32 - 1, dmar.host_address_width);
> +
> +	return 0;
> +}
> +DM_TEST(dm_test_acpi_create_dmar, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> -- 
> 2.25.1.481.gfbce0eb801-goog

regards, Wolfgang