Re: [PATCH 1/7] iommu/vt-d: Support size of the register set in DRHD
From: Liang, Kan
Date: Thu Jan 12 2023 - 12:09:50 EST
On 2023-01-12 7:42 a.m., Baolu Lu wrote:
> On 2023/1/12 4:24, kan.liang@xxxxxxxxxxxxxxx wrote:
>> From: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxxx>
>> ---
>> 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