> -----Original Message-----
> From: Christoph Hellwig [mailto:[email protected]]
> Sent: Sunday, November 15, 2020 9:45 PM
> To: Song Bao Hua (Barry Song) <[email protected]>
> Cc: Christoph Hellwig <[email protected]>; [email protected];
> [email protected]; [email protected]; Linuxarm
> <[email protected]>; [email protected]; xuwei (O)
> <[email protected]>; Joerg Roedel <[email protected]>; Will Deacon
> <[email protected]>; Shuah Khan <[email protected]>
> Subject: Re: [PATCH v3 1/2] dma-mapping: add benchmark support for
> streaming DMA APIs
>
> On Sun, Nov 15, 2020 at 12:11:15AM +0000, Song Bao Hua (Barry Song)
> wrote:
> >
> > Checkpatch has changed 80 to 100. That's probably why my local checkpatch
> didn't report any warning:
> >
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=
> bdc48fa11e46f867ea4d
> >
> > I am happy to change them to be less than 80 if you like.
>
> Don't rely on checkpath, is is broken. Look at the codingstyle document.
>
> > > I think this needs to set a dma mask as behavior for unlimited dma
> > > mask vs the default 32-bit one can be very different.
> >
> > I actually prefer users bind real devices with real dma_mask to test rather
> than force to change
> > the dma_mask in this benchmark.
>
> The mask is set by the driver, not the device. So you need to set when
> when you bind, real device or not.
Yep while it is a little bit tricky.
Sometimes, it is done by "device" in architectures, e.g. there are lots of
dma_mask configuration code in arch/arm/mach-xxx.
arch/arm/mach-davinci/da850.c
static u64 da850_vpif_dma_mask = DMA_BIT_MASK(32);
static struct platform_device da850_vpif_dev = {
.name = "vpif",
.id = -1,
.dev = {
.dma_mask = &da850_vpif_dma_mask,
.coherent_dma_mask = DMA_BIT_MASK(32),
},
.resource = da850_vpif_resource,
.num_resources = ARRAY_SIZE(da850_vpif_resource),
};
Sometimes, it is done by "of" or "acpi", for example:
drivers/acpi/arm64/iort.c
void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
{
u64 end, mask, dmaaddr = 0, size = 0, offset = 0;
int ret;
...
ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
if (!ret) {
/*
* Limit coherent and dma mask based on size retrieved from
* firmware.
*/
end = dmaaddr + size - 1;
mask = DMA_BIT_MASK(ilog2(end) + 1);
dev->bus_dma_limit = end;
dev->coherent_dma_mask = mask;
*dev->dma_mask = mask;
}
...
}
Sometimes, it is done by "bus", for example, ISA:
isa_dev->dev.coherent_dma_mask = DMA_BIT_MASK(24);
isa_dev->dev.dma_mask = &isa_dev->dev.coherent_dma_mask;
error = device_register(&isa_dev->dev);
if (error) {
put_device(&isa_dev->dev);
break;
}
And in many cases, it is done by driver. On the ARM64 server platform I am
testing,
actually rarely drivers set dma_mask.
So to make the dma benchmark work on all platforms, it seems it is worth
to add a dma_mask_bit parameter. But, in order to avoid breaking the
dma_mask of those devices whose dma_mask are set by architectures,
acpi and bus, it seems we need to do the below in dma_benchmark:
u64 old_mask;
old_mask = dma_get_mask(dev);
dma_set_mask(dev, &new_mask);
do_map_benchmark();
/* restore old dma_mask so that the dma_mask of the device is not changed due to
benchmark when it is bound back to its original driver */
dma_set_mask(dev, &old_mask);
Thanks
Barry
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
