| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <367345ad-33cc-2268-63e1-979299d42c2e@linux.intel.com>
Date: Thu, 12 Jan 2023 20:42:53 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: kan.liang@...ux.intel.com, joro@...tes.org, will@...nel.org,
dwmw2@...radead.org, robin.murphy@....com, robert.moore@...el.com,
rafael.j.wysocki@...el.com, lenb@...nel.org, iommu@...ts.linux.dev,
linux-kernel@...r.kernel.org
Cc: baolu.lu@...ux.intel.com
Subject: Re: [PATCH 1/7] iommu/vt-d: Support size of the register set in DRHD
On 2023/1/12 4:24, kan.liang@...ux.intel.com wrote:
> From: Kan Liang <kan.liang@...ux.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@...ux.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
Powered by blists - more mailing lists