linux-kernel - Re: DMA masks

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 09 Aug 2013 12:34:11 +0200
From:	khalasa@...p.pl (Krzysztof Hałasa)
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	linux-kernel@...r.kernel.org,
	James Bottomley <JBottomley@...allels.com>,
	"David S. Miller" <davem@...emloft.net>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: DMA masks

Russell King - ARM Linux <linux@....linux.org.uk> writes:

>> struct device {
>>         ...
>> 
>>         u64             *dma_mask;      /* dma mask (if dma'able device) */
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> "If dma-able device" gives a clue.  It is now the case that the DMA API
> refuses DMA (via dma_supported) if the DMA mask pointer is NULL.

But precisely the same effect can be obtained by setting the (actual)
dma_mask to 0 (most preferably, using DMA API's dma_set_mask()).
This is what coherent_dma_mask does.

Actually, when the (pointer) dma_mask is NULL, the behaviour isn't
exactly simple. I guess this all works because devices without DMA
capability don't try to use DMA.

There is also that old saying that both the DMA masks should default to
0xFFFFFFFF (it does on some archs).

Perhaps I'm wrong, but wouldn't the conversion to simple u64 remove
a lot of unneeded code, make all the archs/platforms behave more
consistently, and generally make the world a bit better?

arch/unicore32/include/asm/dma-mapping.h:
static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
{
        if (dev && dev->dma_mask)
                return addr + size - 1 <= *dev->dma_mask;

        return 1;
	^^^^^^^^^
}

arch/microblaze/include/asm/dma-mapping.h:
static inline unsigned long device_to_mask(struct device *dev)
{
        if (dev->dma_mask && *dev->dma_mask)
                return *dev->dma_mask;
        /* Assume devices without mask can take 32 bit addresses */
        return 0xfffffffful;
}

arch/hexagon/kernel/dma.c:
static int check_addr(const char *name, struct device *hwdev,
                      dma_addr_t bus, size_t size)
{
        if (hwdev && hwdev->dma_mask && !dma_capable(hwdev, bus, size)) {
                if (*hwdev->dma_mask >= DMA_BIT_MASK(32))
                        printk(KERN_ERR
                                "%s: overflow %Lx+%zu of device mask %Lx\n",
                                name, (long long)bus, size,
                                (long long)*hwdev->dma_mask);
                return 0;
        }
        return 1;
}

arch/powerpc/include/asm/dma-mapping.h:
static inline unsigned long device_to_mask(struct device *dev)
{
        if (dev->dma_mask && *dev->dma_mask)
                return *dev->dma_mask;
        /* Assume devices without mask can take 32 bit addresses */
        return 0xfffffffful;
}

etc.


Things like this:
	pdev1->dev.dma_mask = &pdev1->dev.coherent_dma_mask;
	pdev1->dev.coherent_dma_mask = DMA_BIT_MASK(32);
(which are all over the place) make me wonder what happens when some
code requires and tries to set different masks for normal and coherent
DMA.

> However, placing the storage for the dma_mask pointer into struct device
> is something which I've recently discussed with Greg, and is something
> he'd like to see happen, so I'll be adding that to my DMA masks patch
> series in the very near future.

Does this mean the dma_mask would remain a pointer, but it would point
(usually) to a variable in struct device?
-- 
Krzysztof Halasa

Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland
--
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/