Messages in this thread | | | Date | Thu, 2 Apr 2009 12:54:15 +0200 | From | Joerg Roedel <> | Subject | Re: [Bug 487894] New: sky2 0000:06:00.0: DMA-API: device driver frees DMA memory with different size |
| |
On Wed, Apr 01, 2009 at 11:02:45AM -0700, Stephen Hemminger wrote: > > The sky2 driver uses pci_unmap_len and pci_unmap_len_set which on 32 bit > platforms are meaningless so they are stubbed out. > Basically, DMA-API checks are wrong/bogus to enforce on 32bit x86 as is.
As far as I know the VT-d driver is available on 32 bit x86 too. So this should not always be a nop. The other question is why pci_unmap_* functions are not defined as a nop too and call dma_unmap_* instead? This looks inconsisent to me.
> ======================================================== > Unstub pci_unmap macros if doing DMA-API checks > > If doing device driver DMA-API tests then need to keep track of address/length > even on 32-bit x86 where the information is not normally needed. > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> > > > --- a/arch/x86/include/asm/pci_32.h 2009-04-01 10:52:07.117504355 -0700 > +++ b/arch/x86/include/asm/pci_32.h 2009-04-01 10:56:02.249066174 -0700 > @@ -17,16 +17,25 @@ struct pci_dev; > */ > #define PCI_DMA_BUS_IS_PHYS (1) > > +#ifdef CONFIG_DMA_API_DEBUG > +/* keep real values */ > +#define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME; > +#define DECLARE_PCI_UNMAP_LEN(LEN_NAME) __u32 LEN_NAME; > +#define pci_unmap_addr(PTR, ADDR_NAME) ((PTR)->ADDR_NAME) > +#define pci_unmap_addr_set(PTR, ADDR_NAME, VAL) (((PTR)->ADDR_NAME) = (VAL)) > +#define pci_unmap_len(PTR, LEN_NAME) ((PTR)->LEN_NAME) > +#define pci_unmap_len_set(PTR, LEN_NAME, VAL) (((PTR)->LEN_NAME) = (VAL)) > +#else > /* pci_unmap_{page,single} is a nop so... */ > #define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME[0]; > -#define DECLARE_PCI_UNMAP_LEN(LEN_NAME) unsigned LEN_NAME[0]; > -#define pci_unmap_addr(PTR, ADDR_NAME) sizeof((PTR)->ADDR_NAME) > +#define DECLARE_PCI_UNMAP_LEN(LEN_NAME) __u32 LEN_NAME[0]; > +#define pci_unmap_addr(PTR, ADDR_NAME) sizeof((PTR)->ADDR_NAME) > #define pci_unmap_addr_set(PTR, ADDR_NAME, VAL) \ > do { break; } while (pci_unmap_addr(PTR, ADDR_NAME)) > #define pci_unmap_len(PTR, LEN_NAME) sizeof((PTR)->LEN_NAME) > #define pci_unmap_len_set(PTR, LEN_NAME, VAL) \ > do { break; } while (pci_unmap_len(PTR, LEN_NAME)) > - > +#endif
I think we need another solution which takes into account that there might be VT-d enabled and which defines the pci_unmap_* functions also as a nop when it is not. I suggest to make a CONFIG option for these functions nit being a nop and let DMA_API_DEBUG select it on x86. So we get proper checking for drivers on 32bit x86 too which helps other architectures where these drivers can be used.
Joerg
-- | Advanced Micro Devices GmbH Operating | Karl-Hammerschmidt-Str. 34, 85609 Dornach bei München System | Research | Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni Center | Sitz: Dornach, Gemeinde Aschheim, Landkreis München | Registergericht München, HRB Nr. 43632
-- 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/
| |