lkml.org 
[lkml]   [2009]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [RFC] Make AGP work with IOMMU
    From
    > 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@intel.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@intel.com>
    >    Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
    >
    > commit ca8cdb1f983c508f586b90870703e85aab87659a
    > Author: Zhenyu Wang <zhenyu.z.wang@intel.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@intel.com>
    >    Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
    >
    > commit 42a509b9cd513ddd536c38cc302c7457a3c811d7
    > Author: David Woodhouse <David.Woodhouse@intel.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@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2009-07-29 08:31    [W:0.037 / U:30.076 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site