| 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: <21d7e9970907282328l4a44b9a2of4500c5c84a3586b@mail.gmail.com>
Date: Wed, 29 Jul 2009 16:28:02 +1000
From: Dave Airlie <airlied@...il.com>
To: David Woodhouse <dwmw2@...radead.org>
Cc: Zhenyu Wang <zhenyu.z.wang@...el.com>,
linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org,
Joerg Roedel <joerg.roedel@....com>
Subject: Re: [RFC] Make AGP work with IOMMU
> Based on some patches from Zhenyu that I found at
> http://people.freedesktop.org/~zhen/agp-mm-*, this set of patches stops
> the Intel graphics drivers from being utterly broken when the IOMMU is
> enabled...
>
> http://git.infradead.org/users/dwmw2/iommu-agp.git
>
> I'm still seeing write faults to seemingly random addresses at startup
> when I first enable the IOMMU (and before the gfx driver actually
> starts). Those happen even when I don't use any graphics drivers at all,
> though.
>
> And I'm slightly confused about the way we use the scratch page without
> mask_memory().
Looks good, some comments below:
> commit d8450c7be46afd0b51d2dc171c9d2eb60366ce05
> Author: Zhenyu Wang <zhenyu.z.wang@...el.com>
> Date: Mon Jul 27 12:59:57 2009 +0100
>
> intel_agp: Use PCI DMA API correctly on chipsets new enough to have IOMMU
>
> When graphics dma remapping engine is active, we must fill
> gart table with dma address from dmar engine, as now graphics
> device access to graphics memory must go through dma remapping
> table to get real physical address.
>
> Add this support to all drivers which use intel_i915_insert_entries()
>
> Signed-off-by: Zhenyu Wang <zhenyu.z.wang@...el.com>
> Signed-off-by: David Woodhouse <David.Woodhouse@...el.com>
>
> commit ca8cdb1f983c508f586b90870703e85aab87659a
> Author: Zhenyu Wang <zhenyu.z.wang@...el.com>
> Date: Thu Jul 23 17:25:49 2009 +0100
>
> agp: Add generic support for graphics dma remapping
>
> New driver hooks for support graphics memory dma remapping
> are introduced in this patch. It makes generic code can
> tell if current device needs dma remapping, then call driver
> provided interfaces for mapping and unmapping. Change has
> also been made to handle scratch_page in remapping case.
>
> Signed-off-by: Zhenyu Wang <zhenyu.z.wang@...el.com>
> Signed-off-by: David Woodhouse <David.Woodhouse@...el.com>
>
> commit 42a509b9cd513ddd536c38cc302c7457a3c811d7
> Author: David Woodhouse <David.Woodhouse@...el.com>
> Date: Mon Jul 27 10:27:29 2009 +0100
>
> agp: Switch mask_memory() method to take address argument again, not page
>
> In commit 07613ba2 ("agp: switch AGP to use page array instead of
> unsigned long array") we switched the mask_memory() method to take a
> 'struct page *' instead of an address. This is painful, because in some
> cases it has to be an IOMMU-mapped virtual bus address (in fact,
> shouldn't it _always_ be a dma_addr_t returned from pci_map_xxx(), and
> we just happen to get lucky most of the time?)
Yup pretty much we always got lucky, its not like AGP and IOMMU systems
are a huge item, its really only Intel IGPs which use the AGP
subsystem these days.
>
> struct agp_bridge_data *agp_generic_find_bridge(struct pci_dev *pdev);
> diff --git a/drivers/char/agp/amd-k7-agp.c b/drivers/char/agp/amd-k7-agp.c
> index ba9bde7..542a878 100644
> --- a/drivers/char/agp/amd-k7-agp.c
> +++ b/drivers/char/agp/amd-k7-agp.c
> @@ -325,7 +325,9 @@ static int amd_insert_memory(struct agp_memory *mem, off_t pg_start, int type)
> addr = (j * PAGE_SIZE) + agp_bridge->gart_bus_addr;
> cur_gatt = GET_GATT(addr);
> writel(agp_generic_mask_memory(agp_bridge,
> - mem->pages[i], mem->type), cur_gatt+GET_GATT_OFF(addr));
> + phys_to_gart(page_to_phys(mem->pages[i])),
don't suppose we want page_to_gart or is the double function nicer?
> diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
> index cfa5a64..19ac366 100644
> --- a/drivers/char/agp/backend.c
> +++ b/drivers/char/agp/backend.c
> @@ -150,8 +150,17 @@ static int agp_backend_initialize(struct agp_bridge_data *bridge)
> }
>
> bridge->scratch_page_real = phys_to_gart(page_to_phys(page));
> - bridge->scratch_page =
> - bridge->driver->mask_memory(bridge, page, 0);
> + bridge->scratch_page = bridge->driver->mask_memory(bridge,
> + phys_to_gart(page_to_phys(page)), 0);
> +
> + if (bridge->driver->agp_map_page &&
> + bridge->driver->agp_map_page(phys_to_virt(page_to_phys(page)),
and maybe page_to_virt.
>
> +#ifdef USE_PCI_DMA_API
> +static int intel_agp_map_page(void *addr, dma_addr_t *ret)
> +{
> + *ret = pci_map_single(intel_private.pcidev, addr,
> + PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
> + if (pci_dma_mapping_error(intel_private.pcidev, *ret))
> + return -EINVAL;
> + return 0;
> +}
> +
> +static void intel_agp_unmap_page(void *addr, dma_addr_t dma)
> +{
> + pci_unmap_single(intel_private.pcidev, dma,
> + PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
> +}
> +
> +static int intel_agp_map_memory(struct agp_memory *mem)
> +{
> + struct scatterlist *sg;
> + int i;
> +
> + DBG("try mapping %lu pages\n", (unsigned long)mem->page_count);
> +
> + if ((mem->page_count * sizeof(*mem->sg_list)) < 2*PAGE_SIZE)
> + mem->sg_list = kcalloc(mem->page_count, sizeof(*mem->sg_list),
> + GFP_KERNEL);
> +
> + if (mem->sg_list == NULL) {
> + mem->sg_list = vmalloc(mem->page_count * sizeof(*mem->sg_list));
> + mem->sg_vmalloc_flag = 1;
Can we drop vmalloc_flag and use is_vmalloc_addr on the free function?
(aside: yet another place that wants a kmalloc/vmalloc allocator. I suspect
vmalloc here to be slow but I suppose there isn't much we can do.)
> + }
> +
> + if (!mem->sg_list) {
> + mem->sg_vmalloc_flag = 0;
> + return -ENOMEM;
> + }
> + sg_init_table(mem->sg_list, mem->page_count);
> +
> + sg = mem->sg_list;
> + for (i = 0 ; i < mem->page_count; i++, sg = sg_next(sg))
> + sg_set_page(sg, mem->pages[i], PAGE_SIZE, 0);
> +
> + mem->num_sg = pci_map_sg(intel_private.pcidev, mem->sg_list,
> + mem->page_count, PCI_DMA_BIDIRECTIONAL);
> + if (!mem->num_sg) {
> + if (mem->sg_vmalloc_flag)
> + vfree(mem->sg_list);
> + else
> + kfree(mem->sg_list);
> + mem->sg_list = NULL;
> + mem->sg_vmalloc_flag = 0;
some common cleanup function?
> + return -ENOMEM;
> + }
> + return 0;
> +}
> +
> +static void intel_agp_unmap_memory(struct agp_memory *mem)
> +{
> + DBG("try unmapping %lu pages\n", (unsigned long)mem->page_count);
> +
> + pci_unmap_sg(intel_private.pcidev, mem->sg_list,
> + mem->page_count, PCI_DMA_BIDIRECTIONAL);
> + if (mem->sg_vmalloc_flag)
> + vfree(mem->sg_list);
> + else
> + kfree(mem->sg_list);
> + mem->sg_vmalloc_flag = 0;
> + mem->sg_list = NULL;
since we reproduce it here.
Dave.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists