diff mbox series

[1/7] iommu/vt-d: Support size of the register set in DRHD

Message ID 20230111202504.378258-2-kan.liang@linux.intel.com
State New
Headers
Series iommu/vt-d: Support performance monitoring for IOMMU |

Commit Message

Liang, Kan Jan. 11, 2023, 8:24 p.m. UTC 
  From: Kan Liang <kan.liang@linux.intel.com>

A new field, which indicates the size of the remapping hardware register
set for this remapping unit, is introduced in the DMA-remapping hardware
unit definition (DRHD) structure with the VT-d Spec 4.0.
With the information, SW doesn't need to 'guess' the size of the
register set anymore.

Update the struct acpi_dmar_hardware_unit to reflect the field.

Store the size of the register set in struct dmar_drhd_unit for each
dmar device.

The 'size' information is ResvZ for the old BIOS and platforms. Fall
back to the old guessing method. There is nothing changed.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 drivers/iommu/intel/dmar.c | 25 ++++++++++++++++++-------
 include/acpi/actbl1.h      |  2 +-
 include/linux/dmar.h       |  1 +
 3 files changed, 20 insertions(+), 8 deletions(-)

Comments

Baolu Lu Jan. 12, 2023, 12:42 p.m. UTC | #1 
On 2023/1/12 4:24, kan.liang@linux.intel.com wrote:
> From: Kan Liang <kan.liang@linux.intel.com>
> 
> A new field, which indicates the size of the remapping hardware register
> set for this remapping unit, is introduced in the DMA-remapping hardware
> unit definition (DRHD) structure with the VT-d Spec 4.0.
> With the information, SW doesn't need to 'guess' the size of the
> register set anymore.
> 
> Update the struct acpi_dmar_hardware_unit to reflect the field.
> 
> Store the size of the register set in struct dmar_drhd_unit for each
> dmar device.
> 
> The 'size' information is ResvZ for the old BIOS and platforms. Fall
> back to the old guessing method. There is nothing changed.
> 
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> ---
>   drivers/iommu/intel/dmar.c | 25 ++++++++++++++++++-------
>   include/acpi/actbl1.h      |  2 +-
>   include/linux/dmar.h       |  1 +
>   3 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index b00a0ceb2d13..6a411d964474 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -399,6 +399,13 @@ dmar_find_dmaru(struct acpi_dmar_hardware_unit *drhd)
>   	return NULL;
>   }
>   
> +/* The size of the register set is 2 ^ N 4 KB pages. */
> +static unsigned long
> +dmar_get_drhd_reg_size(u8 npages)

No need to divide it into two lines. Or put this line of code directly
where it is called?

> +{
> +	return 1UL << (npages + 12);
> +}
> +
>   /*
>    * dmar_parse_one_drhd - parses exactly one DMA remapping hardware definition
>    * structure which uniquely represent one DMA remapping hardware unit
> @@ -427,6 +434,7 @@ static int dmar_parse_one_drhd(struct acpi_dmar_header *header, void *arg)
>   	memcpy(dmaru->hdr, header, header->length);
>   	dmaru->reg_base_addr = drhd->address;
>   	dmaru->segment = drhd->segment;
> +	dmaru->reg_size = dmar_get_drhd_reg_size(drhd->size);
>   	dmaru->include_all = drhd->flags & 0x1; /* BIT0: INCLUDE_ALL */
>   	dmaru->devices = dmar_alloc_dev_scope((void *)(drhd + 1),
>   					      ((void *)drhd) + drhd->header.length,
> @@ -880,6 +888,7 @@ dmar_validate_one_drhd(struct acpi_dmar_header *entry, void *arg)
>   	struct acpi_dmar_hardware_unit *drhd;
>   	void __iomem *addr;
>   	u64 cap, ecap;
> +	unsigned long size;
>   
>   	drhd = (void *)entry;
>   	if (!drhd->address) {
> @@ -887,10 +896,11 @@ dmar_validate_one_drhd(struct acpi_dmar_header *entry, void *arg)
>   		return -EINVAL;
>   	}
>   
> +	size = dmar_get_drhd_reg_size(drhd->size);
>   	if (arg)
> -		addr = ioremap(drhd->address, VTD_PAGE_SIZE);
> +		addr = ioremap(drhd->address, size);
>   	else
> -		addr = early_ioremap(drhd->address, VTD_PAGE_SIZE);
> +		addr = early_ioremap(drhd->address, size);
>   	if (!addr) {
>   		pr_warn("Can't validate DRHD address: %llx\n", drhd->address);
>   		return -EINVAL;
> @@ -902,7 +912,7 @@ dmar_validate_one_drhd(struct acpi_dmar_header *entry, void *arg)
>   	if (arg)
>   		iounmap(addr);
>   	else
> -		early_iounmap(addr, VTD_PAGE_SIZE);
> +		early_iounmap(addr, size);
>   
>   	if (cap == (uint64_t)-1 && ecap == (uint64_t)-1) {
>   		warn_invalid_dmar(drhd->address, " returns all ones");

The cap and ecap registers are always in the first page. Just map one
4K page is enough. There is no need to change it?

> @@ -956,17 +966,18 @@ static void unmap_iommu(struct intel_iommu *iommu)
>   /**
>    * map_iommu: map the iommu's registers
>    * @iommu: the iommu to map
> - * @phys_addr: the physical address of the base resgister
> + * @drhd: DMA remapping hardware definition structure
>    *
>    * Memory map the iommu's registers.  Start w/ a single page, and
>    * possibly expand if that turns out to be insufficent.
>    */
> -static int map_iommu(struct intel_iommu *iommu, u64 phys_addr)
> +static int map_iommu(struct intel_iommu *iommu, struct dmar_drhd_unit *drhd)
>   {
> +	u64 phys_addr = drhd->reg_base_addr;
>   	int map_size, err=0;
>   
>   	iommu->reg_phys = phys_addr;
> -	iommu->reg_size = VTD_PAGE_SIZE;
> +	iommu->reg_size = drhd->reg_size;
>   
>   	if (!request_mem_region(iommu->reg_phys, iommu->reg_size, iommu->name)) {
>   		pr_err("Can't reserve memory\n");
> @@ -1050,7 +1061,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
>   	}
>   	sprintf(iommu->name, "dmar%d", iommu->seq_id);
>   
> -	err = map_iommu(iommu, drhd->reg_base_addr);
> +	err = map_iommu(iommu, drhd);
>   	if (err) {
>   		pr_err("Failed to map %s\n", iommu->name);
>   		goto error_free_seq_id;
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index 4175dce3967c..bdded0ac46eb 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -802,7 +802,7 @@ struct acpi_dmar_pci_path {
>   struct acpi_dmar_hardware_unit {
>   	struct acpi_dmar_header header;
>   	u8 flags;
> -	u8 reserved;
> +	u8 size;		/* Size of the register set */
>   	u16 segment;
>   	u64 address;		/* Register Base Address */
>   };
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index d81a51978d01..51d498f0a02b 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -46,6 +46,7 @@ struct dmar_drhd_unit {
>   	u8	include_all:1;
>   	u8	gfx_dedicated:1;	/* graphic dedicated	*/
>   	struct intel_iommu *iommu;
> +	unsigned long reg_size;		/* size of register set */

Move it up and pair it with reg_base_addr.

>   };
>   
>   struct dmar_pci_path {

--
Best regards,
baolu
Liang, Kan Jan. 12, 2023, 4:42 p.m. UTC | #2 
On 2023-01-12 7:42 a.m., Baolu Lu wrote:
> On 2023/1/12 4:24, kan.liang@linux.intel.com wrote:
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> A new field, which indicates the size of the remapping hardware register
>> set for this remapping unit, is introduced in the DMA-remapping hardware
>> unit definition (DRHD) structure with the VT-d Spec 4.0.
>> With the information, SW doesn't need to 'guess' the size of the
>> register set anymore.
>>
>> Update the struct acpi_dmar_hardware_unit to reflect the field.
>>
>> Store the size of the register set in struct dmar_drhd_unit for each
>> dmar device.
>>
>> The 'size' information is ResvZ for the old BIOS and platforms. Fall
>> back to the old guessing method. There is nothing changed.
>>
>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>> ---
>>   drivers/iommu/intel/dmar.c | 25 ++++++++++++++++++-------
>>   include/acpi/actbl1.h      |  2 +-
>>   include/linux/dmar.h       |  1 +
>>   3 files changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
>> index b00a0ceb2d13..6a411d964474 100644
>> --- a/drivers/iommu/intel/dmar.c
>> +++ b/drivers/iommu/intel/dmar.c
>> @@ -399,6 +399,13 @@ dmar_find_dmaru(struct acpi_dmar_hardware_unit
>> *drhd)
>>       return NULL;
>>   }
>>   +/* The size of the register set is 2 ^ N 4 KB pages. */
>> +static unsigned long
>> +dmar_get_drhd_reg_size(u8 npages)
> 
> No need to divide it into two lines. Or put this line of code directly
> where it is called?
> 
>> +{
>> +    return 1UL << (npages + 12);
>> +}
>> +
>>   /*
>>    * dmar_parse_one_drhd - parses exactly one DMA remapping hardware
>> definition
>>    * structure which uniquely represent one DMA remapping hardware unit
>> @@ -427,6 +434,7 @@ static int dmar_parse_one_drhd(struct
>> acpi_dmar_header *header, void *arg)
>>       memcpy(dmaru->hdr, header, header->length);
>>       dmaru->reg_base_addr = drhd->address;
>>       dmaru->segment = drhd->segment;
>> +    dmaru->reg_size = dmar_get_drhd_reg_size(drhd->size);
>>       dmaru->include_all = drhd->flags & 0x1; /* BIT0: INCLUDE_ALL */
>>       dmaru->devices = dmar_alloc_dev_scope((void *)(drhd + 1),
>>                             ((void *)drhd) + drhd->header.length,
>> @@ -880,6 +888,7 @@ dmar_validate_one_drhd(struct acpi_dmar_header
>> *entry, void *arg)
>>       struct acpi_dmar_hardware_unit *drhd;
>>       void __iomem *addr;
>>       u64 cap, ecap;
>> +    unsigned long size;
>>         drhd = (void *)entry;
>>       if (!drhd->address) {
>> @@ -887,10 +896,11 @@ dmar_validate_one_drhd(struct acpi_dmar_header
>> *entry, void *arg)
>>           return -EINVAL;
>>       }
>>   +    size = dmar_get_drhd_reg_size(drhd->size);
>>       if (arg)
>> -        addr = ioremap(drhd->address, VTD_PAGE_SIZE);
>> +        addr = ioremap(drhd->address, size);
>>       else
>> -        addr = early_ioremap(drhd->address, VTD_PAGE_SIZE);
>> +        addr = early_ioremap(drhd->address, size);
>>       if (!addr) {
>>           pr_warn("Can't validate DRHD address: %llx\n", drhd->address);
>>           return -EINVAL;
>> @@ -902,7 +912,7 @@ dmar_validate_one_drhd(struct acpi_dmar_header
>> *entry, void *arg)
>>       if (arg)
>>           iounmap(addr);
>>       else
>> -        early_iounmap(addr, VTD_PAGE_SIZE);
>> +        early_iounmap(addr, size);
>>         if (cap == (uint64_t)-1 && ecap == (uint64_t)-1) {
>>           warn_invalid_dmar(drhd->address, " returns all ones");
> 
> The cap and ecap registers are always in the first page. Just map one
> 4K page is enough. There is no need to change it?

Right, one page should be enough for cap and ecap.

The perf cap will be checked in the alloc_iommu_pmu() of patch 2. If it
fails, it should not impact the existing features. No need to change
here. I will update it in V2.

After the change, there will be only one place which invokes
dmar_get_drhd_reg_size(). I will move it to the place as well.

> 
>> @@ -956,17 +966,18 @@ static void unmap_iommu(struct intel_iommu *iommu)
>>   /**
>>    * map_iommu: map the iommu's registers
>>    * @iommu: the iommu to map
>> - * @phys_addr: the physical address of the base resgister
>> + * @drhd: DMA remapping hardware definition structure
>>    *
>>    * Memory map the iommu's registers.  Start w/ a single page, and
>>    * possibly expand if that turns out to be insufficent.
>>    */
>> -static int map_iommu(struct intel_iommu *iommu, u64 phys_addr)
>> +static int map_iommu(struct intel_iommu *iommu, struct dmar_drhd_unit
>> *drhd)
>>   {
>> +    u64 phys_addr = drhd->reg_base_addr;
>>       int map_size, err=0;
>>         iommu->reg_phys = phys_addr;
>> -    iommu->reg_size = VTD_PAGE_SIZE;
>> +    iommu->reg_size = drhd->reg_size;
>>         if (!request_mem_region(iommu->reg_phys, iommu->reg_size,
>> iommu->name)) {
>>           pr_err("Can't reserve memory\n");
>> @@ -1050,7 +1061,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
>>       }
>>       sprintf(iommu->name, "dmar%d", iommu->seq_id);
>>   -    err = map_iommu(iommu, drhd->reg_base_addr);
>> +    err = map_iommu(iommu, drhd);
>>       if (err) {
>>           pr_err("Failed to map %s\n", iommu->name);
>>           goto error_free_seq_id;
>> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
>> index 4175dce3967c..bdded0ac46eb 100644
>> --- a/include/acpi/actbl1.h
>> +++ b/include/acpi/actbl1.h
>> @@ -802,7 +802,7 @@ struct acpi_dmar_pci_path {
>>   struct acpi_dmar_hardware_unit {
>>       struct acpi_dmar_header header;
>>       u8 flags;
>> -    u8 reserved;
>> +    u8 size;        /* Size of the register set */
>>       u16 segment;
>>       u64 address;        /* Register Base Address */
>>   };
>> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
>> index d81a51978d01..51d498f0a02b 100644
>> --- a/include/linux/dmar.h
>> +++ b/include/linux/dmar.h
>> @@ -46,6 +46,7 @@ struct dmar_drhd_unit {
>>       u8    include_all:1;
>>       u8    gfx_dedicated:1;    /* graphic dedicated    */
>>       struct intel_iommu *iommu;
>> +    unsigned long reg_size;        /* size of register set */
> 
> Move it up and pair it with reg_base_addr.

Sure.

Thanks,
Kan
> 
>>   };
>>     struct dmar_pci_path {
> 
> -- 
> Best regards,
> baolu
diff mbox series

Patch

diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index b00a0ceb2d13..6a411d964474 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -399,6 +399,13 @@  dmar_find_dmaru(struct acpi_dmar_hardware_unit *drhd)
 	return NULL;
 }
 
+/* The size of the register set is 2 ^ N 4 KB pages. */
+static unsigned long
+dmar_get_drhd_reg_size(u8 npages)
+{
+	return 1UL << (npages + 12);
+}
+
 /*
  * dmar_parse_one_drhd - parses exactly one DMA remapping hardware definition
  * structure which uniquely represent one DMA remapping hardware unit
@@ -427,6 +434,7 @@  static int dmar_parse_one_drhd(struct acpi_dmar_header *header, void *arg)
 	memcpy(dmaru->hdr, header, header->length);
 	dmaru->reg_base_addr = drhd->address;
 	dmaru->segment = drhd->segment;
+	dmaru->reg_size = dmar_get_drhd_reg_size(drhd->size);
 	dmaru->include_all = drhd->flags & 0x1; /* BIT0: INCLUDE_ALL */
 	dmaru->devices = dmar_alloc_dev_scope((void *)(drhd + 1),
 					      ((void *)drhd) + drhd->header.length,
@@ -880,6 +888,7 @@  dmar_validate_one_drhd(struct acpi_dmar_header *entry, void *arg)
 	struct acpi_dmar_hardware_unit *drhd;
 	void __iomem *addr;
 	u64 cap, ecap;
+	unsigned long size;
 
 	drhd = (void *)entry;
 	if (!drhd->address) {
@@ -887,10 +896,11 @@  dmar_validate_one_drhd(struct acpi_dmar_header *entry, void *arg)
 		return -EINVAL;
 	}
 
+	size = dmar_get_drhd_reg_size(drhd->size);
 	if (arg)
-		addr = ioremap(drhd->address, VTD_PAGE_SIZE);
+		addr = ioremap(drhd->address, size);
 	else
-		addr = early_ioremap(drhd->address, VTD_PAGE_SIZE);
+		addr = early_ioremap(drhd->address, size);
 	if (!addr) {
 		pr_warn("Can't validate DRHD address: %llx\n", drhd->address);
 		return -EINVAL;
@@ -902,7 +912,7 @@  dmar_validate_one_drhd(struct acpi_dmar_header *entry, void *arg)
 	if (arg)
 		iounmap(addr);
 	else
-		early_iounmap(addr, VTD_PAGE_SIZE);
+		early_iounmap(addr, size);
 
 	if (cap == (uint64_t)-1 && ecap == (uint64_t)-1) {
 		warn_invalid_dmar(drhd->address, " returns all ones");
@@ -956,17 +966,18 @@  static void unmap_iommu(struct intel_iommu *iommu)
 /**
  * map_iommu: map the iommu's registers
  * @iommu: the iommu to map
- * @phys_addr: the physical address of the base resgister
+ * @drhd: DMA remapping hardware definition structure
  *
  * Memory map the iommu's registers.  Start w/ a single page, and
  * possibly expand if that turns out to be insufficent.
  */
-static int map_iommu(struct intel_iommu *iommu, u64 phys_addr)
+static int map_iommu(struct intel_iommu *iommu, struct dmar_drhd_unit *drhd)
 {
+	u64 phys_addr = drhd->reg_base_addr;
 	int map_size, err=0;
 
 	iommu->reg_phys = phys_addr;
-	iommu->reg_size = VTD_PAGE_SIZE;
+	iommu->reg_size = drhd->reg_size;
 
 	if (!request_mem_region(iommu->reg_phys, iommu->reg_size, iommu->name)) {
 		pr_err("Can't reserve memory\n");
@@ -1050,7 +1061,7 @@  static int alloc_iommu(struct dmar_drhd_unit *drhd)
 	}
 	sprintf(iommu->name, "dmar%d", iommu->seq_id);
 
-	err = map_iommu(iommu, drhd->reg_base_addr);
+	err = map_iommu(iommu, drhd);
 	if (err) {
 		pr_err("Failed to map %s\n", iommu->name);
 		goto error_free_seq_id;
diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 4175dce3967c..bdded0ac46eb 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -802,7 +802,7 @@  struct acpi_dmar_pci_path {
 struct acpi_dmar_hardware_unit {
 	struct acpi_dmar_header header;
 	u8 flags;
-	u8 reserved;
+	u8 size;		/* Size of the register set */
 	u16 segment;
 	u64 address;		/* Register Base Address */
 };
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index d81a51978d01..51d498f0a02b 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -46,6 +46,7 @@  struct dmar_drhd_unit {
 	u8	include_all:1;
 	u8	gfx_dedicated:1;	/* graphic dedicated	*/
 	struct intel_iommu *iommu;
+	unsigned long reg_size;		/* size of register set */
 };
 
 struct dmar_pci_path {