Re: [PATCH] iommu/dma: Fix calculation overflow in __finalise_sg()
Hi Robin,
On Mon, Jul 01, 2019 at 01:39:55PM +0100, Robin Murphy wrote:
> > > The max_len is a u32 type variable so the calculation on the
> > > left hand of the last if-condition will potentially overflow
> > > when a cur_len gets closer to UINT_MAX -- note that there're
> > > drivers setting max_seg_size to UINT_MAX:
> > >drivers/dma/dw-edma/dw-edma-core.c:745:
> > > dma_set_max_seg_size(dma->dev, U32_MAX);
> > >drivers/dma/dma-axi-dmac.c:871:
> > > dma_set_max_seg_size(>dev, UINT_MAX);
> > >drivers/mmc/host/renesas_sdhi_internal_dmac.c:338:
> > > dma_set_max_seg_size(dev, 0x);
> > >drivers/nvme/host/pci.c:2520:
> > > dma_set_max_seg_size(dev->dev, 0x);
> > >
> > > So this patch just casts the cur_len in the calculation to a
> > > size_t type to fix the overflow issue, as it's not necessary
> > > to change the type of cur_len after all.
> > >
> > > Fixes: 809eac54cdd6 ("iommu/dma: Implement scatterlist segment merging")
> > > Cc: sta...@vger.kernel.org
> > > Signed-off-by: Nicolin Chen
> >
> > Looks good to me, but I let Robin take a look too before I apply it,
> > Robin?
> I'll need to take a closer look at how exactly an overflow would happen here
It was triggered by a test case that was trying to map a 4GB
dma_buf (1000+ nents in the scatterlist). This function then
seemed to reduce the nents by merging most of them, probably
because they were contiguous.
> (just got back off some holiday), but my immediate thought is that if this
> is a real problem, then what about 32-bit builds where size_t would still
> overflow?
I think most of callers are also using size_t type for their
size parameters like dma_buf, so the cur_len + s_length will
unlikely go higher than UINT_MAX. But just in case that some
driver allocates a large sg with a size parameter defined in
64-bit and uses this map() function, so it might be safer to
change to "size_t" here to "u64"?
Thank you
Nicolin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: DMA-API attr - DMA_ATTR_NO_KERNEL_MAPPING
On Mon, Jul 1, 2019 at 11:17 PM Pankaj Suryawanshi
wrote:
>
>
>
>
> On Mon, Jul 1, 2019 at 7:39 PM Robin Murphy wrote:
>>
>> On 28/06/2019 17:29, Pankaj Suryawanshi wrote:
>> > On Wed, Jun 26, 2019 at 11:21 PM Christoph Hellwig
>> > wrote:
>> >>
>> >> On Wed, Jun 26, 2019 at 10:12:45PM +0530, Pankaj Suryawanshi wrote:
>> >>> [CC: linux kernel and Vlastimil Babka]
>> >>
>> >> The right list is the list for the DMA mapping subsystem, which is
>> >> iommu@lists.linux-foundation.org. I've also added that.
>> >>
>> I am writing driver in which I used DMA_ATTR_NO_KERNEL_MAPPING attribute
>> for cma allocation using dma_alloc_attr(), as per kernel docs
>> https://www.kernel.org/doc/Documentation/DMA-attributes.txt buffers
>> allocated with this attribute can be only passed to user space by
>> calling
>> dma_mmap_attrs().
>>
>> how can I mapped in kernel space (after dma_alloc_attr with
>> DMA_ATTR_NO_KERNEL_MAPPING ) ?
>> >>
>> >> You can't. And that is the whole point of that API.
>> >
>> > 1. We can again mapped in kernel space using dma_remap() api , because
>> > when we are using DMA_ATTR_NO_KERNEL_MAPPING for dma_alloc_attr it
>> > returns the page as virtual address(in case of CMA) so we can mapped
>> > it again using dma_remap().
>>
>> No, you really can't. A caller of dma_alloc_attrs(...,
>> DMA_ATTR_NO_KERNEL_MAPPING) cannot make any assumptions about the void*
>> it returns, other than that it must be handed back to dma_free_attrs()
>> later. The implementation is free to ignore the flag and give back a
>> virtual mapping anyway. Any driver which depends on how one particular
>> implementation on one particular platform happens to behave today is,
>> essentially, wrong.
>
>
> Here is the example that i have tried in my driver.
> ///code
> snippet/
>
> For CMA allocation using DMA API with DMA_ATTR_NO_KERNEL_MAPPING :-
>
> if(strcmp("video",info->name) == 0)
> {
> printk("Testing CMA Alloc %s\n", info->name);
> info->dma_virt = dma_alloc_attrs(pmap_device, info->size, ,
> GFP_KERNEL,
> DMA_ATTR_WRITE_COMBINE | DMA_ATTR_FORCE_CONTIGUOUS |
> DMA_ATTR_NO_KERNEL_MAPPING);
> if (!info->dma_virt) {
> pr_err("\x1b[31m" "pmap: cma: failed to alloc %s" "\x1b[0m\n",
> info->name);
> return 0;
> }
> __dma_remap(info->dma_virt, info->size, PAGE_KERNEL); // /*TO
> DO pgprot we will be taken from attr */ // we will use this only when
> virtual mapping is required.
> virt = page_address(info->dma_virt); // will use this virtual
> when kernel mapping needed.
> }
>
> For CMA free using DMA api with DMA_ATTR_NO_KERNEL_MAPPING:-
>
> if(strcmp("video",info->name) == 0)
> {
> printk("Testing CMA Release\n");
> __dma_remap(info->dma_virt, info->size, PAGE_KERNEL);
> dma_free_attrs(pmap_device, info->size, info->dma_virt, phys,
> DMA_ATTR_WRITE_COMBINE | DMA_ATTR_FORCE_CONTIGUOUS |
> DMA_ATTR_NO_KERNEL_MAPPING);
> }
>
> Flow of Function calls :-
>
> 1. static void *__dma_alloc() // .want_vaddr = ((attrs &
> DMA_ATTR_NO_KERNEL_MAPPING) == 0)
>
> 2.cma_allocator :-
> i. static void *cma_allocator_alloc ()
> ii. static void *__alloc_from_contiguous() //
> file name :- ./arch/arm/mm/dma-mapping.c
> if
> (!want_vaddr)
>
> goto out; // condition true for DMA_ATTR_NO_KERNEL_MAPPING
>
> if
> (PageHighMem(page)) {
> ptr =
> __dma_alloc_remap(page, size, GFP_KERNEL, prot, caller);
> if
> (!ptr) {
>
> dma_release_from_contiguous(dev, page, count);
>
> return NULL;
> }
> } else {
>
> __dma_remap(page, size, prot);
> ptr =
> page_address(page);
> }
>
>out:
> *ret_page =
> page; // return page
>
Re: DMA-API attr - DMA_ATTR_NO_KERNEL_MAPPING
On Mon, Jul 1, 2019 at 11:24 PM Robin Murphy wrote: > > On 01/07/2019 18:47, Pankaj Suryawanshi wrote: > >> If you want a kernel mapping, *don't* explicitly request not to have a > >> kernel mapping in the first place. It's that simple. > >> > > > > Do you mean do not use dma-api ? because if i used dma-api it will give you > > mapped virtual address. > > or i have to use directly cma_alloc() in my driver. // if i used this > > approach i need to reserved more vmalloc area. > > No, I mean just call dma_alloc_attrs() normally *without* adding the > DMA_ATTR_NO_KERNEL_MAPPING flag. That flag means "I never ever want to > make CPU accesses to this buffer from the kernel" - that is clearly not > the case for your code, so it is utterly nonsensical to still pass the > flag but try to hack around it later. Actually my use case is that i want virtual mapping only when i will play video as my vpu/gpu driver is design like that. and i am using 32-bit so virtual memory is splitted as 3G/1G so dont have enough memory for all the time to mapped with kernel space. Lets say i am allocating 400MB for driver but i want only 30MB for virtual mapping (not everytime) that is the case. > > > Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Is IOMMU a generic name for Intel VT-d and AMD IOV?
Good morning from Singapore, Is IOMMU a generic name for Intel VT-d and AMD IOV? Thank you. -BEGIN EMAIL SIGNATURE- The Gospel for all Targeted Individuals (TIs): [The New York Times] Microwave Weapons Are Prime Suspect in Ills of U.S. Embassy Workers Link: https://www.nytimes.com/2018/09/01/science/sonic-attack-cuba-microwave.html Singaporean Mr. Turritopsis Dohrnii Teo En Ming's Academic Qualifications as at 14 Feb 2019 [1] https://tdtemcerts.wordpress.com/ [2] https://tdtemcerts.blogspot.sg/ [3] https://www.scribd.com/user/270125049/Teo-En-Ming -END EMAIL SIGNATURE- ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: DMA-API attr - DMA_ATTR_NO_KERNEL_MAPPING
On 01/07/2019 18:47, Pankaj Suryawanshi wrote: If you want a kernel mapping, *don't* explicitly request not to have a kernel mapping in the first place. It's that simple. Do you mean do not use dma-api ? because if i used dma-api it will give you mapped virtual address. or i have to use directly cma_alloc() in my driver. // if i used this approach i need to reserved more vmalloc area. No, I mean just call dma_alloc_attrs() normally *without* adding the DMA_ATTR_NO_KERNEL_MAPPING flag. That flag means "I never ever want to make CPU accesses to this buffer from the kernel" - that is clearly not the case for your code, so it is utterly nonsensical to still pass the flag but try to hack around it later. Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: DMA-API attr - DMA_ATTR_NO_KERNEL_MAPPING
[CC: pankaj.suryawan...@einfochips.com]
On Mon, Jul 1, 2019 at 11:17 PM Pankaj Suryawanshi <
pankajssuryawan...@gmail.com> wrote:
>
>
>
>
> On Mon, Jul 1, 2019 at 7:39 PM Robin Murphy wrote:
>>
>> On 28/06/2019 17:29, Pankaj Suryawanshi wrote:
>> > On Wed, Jun 26, 2019 at 11:21 PM Christoph Hellwig
wrote:
>> >>
>> >> On Wed, Jun 26, 2019 at 10:12:45PM +0530, Pankaj Suryawanshi wrote:
>> >>> [CC: linux kernel and Vlastimil Babka]
>> >>
>> >> The right list is the list for the DMA mapping subsystem, which is
>> >> iommu@lists.linux-foundation.org. I've also added that.
>> >>
>> I am writing driver in which I used DMA_ATTR_NO_KERNEL_MAPPING
attribute
>> for cma allocation using dma_alloc_attr(), as per kernel docs
>> https://www.kernel.org/doc/Documentation/DMA-attributes.txt buffers
>> allocated with this attribute can be only passed to user space by
calling
>> dma_mmap_attrs().
>>
>> how can I mapped in kernel space (after dma_alloc_attr with
>> DMA_ATTR_NO_KERNEL_MAPPING ) ?
>> >>
>> >> You can't. And that is the whole point of that API.
>> >
>> > 1. We can again mapped in kernel space using dma_remap() api , because
>> > when we are using DMA_ATTR_NO_KERNEL_MAPPING for dma_alloc_attr it
>> > returns the page as virtual address(in case of CMA) so we can mapped
>> > it again using dma_remap().
>>
>> No, you really can't. A caller of dma_alloc_attrs(...,
>> DMA_ATTR_NO_KERNEL_MAPPING) cannot make any assumptions about the void*
>> it returns, other than that it must be handed back to dma_free_attrs()
>> later. The implementation is free to ignore the flag and give back a
>> virtual mapping anyway. Any driver which depends on how one particular
>> implementation on one particular platform happens to behave today is,
>> essentially, wrong.
>
>
> Here is the example that i have tried in my driver.
> ///code
snippet/
>
> For CMA allocation using DMA API with DMA_ATTR_NO_KERNEL_MAPPING :-
>
> if(strcmp("video",info->name) == 0)
> {
> printk("Testing CMA Alloc %s\n", info->name);
> info->dma_virt = dma_alloc_attrs(pmap_device, info->size, ,
GFP_KERNEL,
> DMA_ATTR_WRITE_COMBINE |
DMA_ATTR_FORCE_CONTIGUOUS | DMA_ATTR_NO_KERNEL_MAPPING);
> if (!info->dma_virt) {
> pr_err("\x1b[31m" "pmap: cma: failed to alloc %s"
"\x1b[0m\n",
> info->name);
> return 0;
> }
> __dma_remap(info->dma_virt, info->size, PAGE_KERNEL); //
pgprot we will take from attr
> virt = page_address(info->dma_virt);
> }
>
> For CMA free using DMA api with DMA_ATTR_NO_KERNEL_MAPPING:-
>
> if(strcmp("video",info->name) == 0)
> {
> printk("Testing CMA Release\n");
> __dma_remap(info->dma_virt, info->size, PAGE_KERNEL);
> dma_free_attrs(pmap_device, info->size, info->dma_virt, phys,
> DMA_ATTR_WRITE_COMBINE |
DMA_ATTR_FORCE_CONTIGUOUS | DMA_ATTR_NO_KERNEL_MAPPING);
> }
>
> Flow of Function calls :-
>
> 1. static void *__dma_alloc() // .want_vaddr = ((attrs &
DMA_ATTR_NO_KERNEL_MAPPING) == 0)
>
> 2.cma_allocator :-
> i. static void *cma_allocator_alloc ()
> ii. static void *__alloc_from_contiguous()
// file name :- ./arch/arm/mm/dma-mapping.c
> if
(!want_vaddr)
>
goto out; // condition true for DMA_ATTR_NO_KERNEL_MAPPING
>
> if
(PageHighMem(page)) {
> ptr
= __dma_alloc_remap(page, size, GFP_KERNEL, prot, caller);
> if
(!ptr) {
>
dma_release_from_contiguous(dev, page, count);
>
return NULL;
> }
> }
else {
>
__dma_remap(page, size, prot);
> ptr =
page_address(page);
> }
>
>out:
>
*ret_page = page; // return page
>return
ptr; // nothing in ptr
> }
> iii. struct page *dma_alloc_from_contiguous()
> iv. cma_alloc()
> 3. dma_alloc () // returns
> return args.want_vaddr ? addr : page; // returns page which is return by
alloc_from_contiguous().
>
> What wrong with this if we already know page is returning
dma_alloc_attr().
> we
Re: DMA-API attr - DMA_ATTR_NO_KERNEL_MAPPING
On Mon, Jul 1, 2019 at 7:39 PM Robin Murphy wrote:
> On 28/06/2019 17:29, Pankaj Suryawanshi wrote:
> > On Wed, Jun 26, 2019 at 11:21 PM Christoph Hellwig
> wrote:
> >>
> >> On Wed, Jun 26, 2019 at 10:12:45PM +0530, Pankaj Suryawanshi wrote:
> >>> [CC: linux kernel and Vlastimil Babka]
> >>
> >> The right list is the list for the DMA mapping subsystem, which is
> >> iommu@lists.linux-foundation.org. I've also added that.
> >>
> I am writing driver in which I used DMA_ATTR_NO_KERNEL_MAPPING
> attribute
> for cma allocation using dma_alloc_attr(), as per kernel docs
> https://www.kernel.org/doc/Documentation/DMA-attributes.txt buffers
> allocated with this attribute can be only passed to user space by
> calling
> dma_mmap_attrs().
>
> how can I mapped in kernel space (after dma_alloc_attr with
> DMA_ATTR_NO_KERNEL_MAPPING ) ?
> >>
> >> You can't. And that is the whole point of that API.
> >
> > 1. We can again mapped in kernel space using dma_remap() api , because
> > when we are using DMA_ATTR_NO_KERNEL_MAPPING for dma_alloc_attr it
> > returns the page as virtual address(in case of CMA) so we can mapped
> > it again using dma_remap().
>
> No, you really can't. A caller of dma_alloc_attrs(...,
> DMA_ATTR_NO_KERNEL_MAPPING) cannot make any assumptions about the void*
> it returns, other than that it must be handed back to dma_free_attrs()
> later. The implementation is free to ignore the flag and give back a
> virtual mapping anyway. Any driver which depends on how one particular
> implementation on one particular platform happens to behave today is,
> essentially, wrong.
>
Here is the example that i have tried in my driver.
///code
snippet/
For CMA allocation using DMA API with DMA_ATTR_NO_KERNEL_MAPPING :-
if(strcmp("video",info->name) == 0)
{
printk("Testing CMA Alloc %s\n", info->name);
info->dma_virt = dma_alloc_attrs(pmap_device, info->size, ,
GFP_KERNEL,
DMA_ATTR_WRITE_COMBINE | DMA_ATTR_FORCE_CONTIGUOUS
| DMA_ATTR_NO_KERNEL_MAPPING);
if (!info->dma_virt) {
pr_err("\x1b[31m" "pmap: cma: failed to alloc %s"
"\x1b[0m\n",
info->name);
return 0;
}
__dma_remap(info->dma_virt, info->size, PAGE_KERNEL); //
pgprot we will take from attr
virt = page_address(info->dma_virt);
}
For CMA free using DMA api with DMA_ATTR_NO_KERNEL_MAPPING:-
if(strcmp("video",info->name) == 0)
{
printk("Testing CMA Release\n");
__dma_remap(info->dma_virt, info->size, PAGE_KERNEL);
dma_free_attrs(pmap_device, info->size, info->dma_virt, phys,
DMA_ATTR_WRITE_COMBINE | DMA_ATTR_FORCE_CONTIGUOUS
| DMA_ATTR_NO_KERNEL_MAPPING);
}
Flow of Function calls :-
1. static void *__dma_alloc() // .want_vaddr = ((attrs &
DMA_ATTR_NO_KERNEL_MAPPING) == 0)
2.cma_allocator :-
i. static void *cma_allocator_alloc ()
ii. static void *__alloc_from_contiguous() //
file name :- ./arch/arm/mm/dma-mapping.c
if
(!want_vaddr)
goto
out; // condition true for DMA_ATTR_NO_KERNEL_MAPPING
if
(PageHighMem(page)) {
ptr =
__dma_alloc_remap(page, size, GFP_KERNEL, prot, caller);
if
(!ptr) {
dma_release_from_contiguous(dev, page, count);
return NULL;
}
} else
{
__dma_remap(page, size, prot);
ptr =
page_address(page);
}
out:
*ret_page
= page; // return page
return
ptr; // nothing in ptr
}
iii. struct page *dma_alloc_from_contiguous()
iv. cma_alloc()
3. dma_alloc () // returns
return args.want_vaddr ? addr : page; // returns page which is return by
alloc_from_contiguous().
What wrong with this if we already know page is returning dma_alloc_attr().
we can use dma_remap in our driver and free as freed in static void
__free_from_contiguous ().
Please let me
Re: [PATCH v3 8/9] iommu/arm-smmu-v3: Add support for PCI ATS
Hi Jean-Philippe, I realise it's a bit late for a "review", but digging up the original patch seemed as good a place as any to raise this... On 17/04/2019 19:24, Jean-Philippe Brucker wrote: [...] @@ -1740,6 +1906,9 @@ static void arm_smmu_detach_dev(struct arm_smmu_master *master) master->domain = NULL; arm_smmu_install_ste_for_dev(master); + + /* Disabling ATS invalidates all ATC entries */ + arm_smmu_disable_ats(master); } Is that actually true? I had initially overlooked this entirely while diagnosing something else and thought that we were missing any ATC invalidation on detach at all, but even having looked again I'm not entirely convinced it's bulletproof. Firstly, the ATS spec only seems to say that *enabling* the ATS capability invalidates all ATC entries, although I think any corner cases that that alone opens up should be at best theoretical. More importantly though, pci_disable_ats() might not actually touch the capability - given that, it seems possible to move a VF to a new domain, and if it's not reset, end up preserving now-bogus ATC entries despite the old domain being torn down and freed. Do we need an explicit ATC invalidation here to be 100% safe, or is there something else I'm missing? Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
On Thu, Jun 27, 2019 at 10:58:40PM -0300, Thiago Jung Bauermann wrote: > > Michael S. Tsirkin writes: > > > On Mon, Jun 03, 2019 at 10:13:59PM -0300, Thiago Jung Bauermann wrote: > >> > >> > >> Michael S. Tsirkin writes: > >> > >> > On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote: > >> >> I rephrased it in terms of address translation. What do you think of > >> >> this version? The flag name is slightly different too: > >> >> > >> >> > >> >> VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION This feature has the same > >> >> meaning as VIRTIO_F_ACCESS_PLATFORM both when set and when not set, > >> >> with the exception that address translation is guaranteed to be > >> >> unnecessary when accessing memory addresses supplied to the device > >> >> by the driver. Which is to say, the device will always use physical > >> >> addresses matching addresses used by the driver (typically meaning > >> >> physical addresses used by the CPU) and not translated further. This > >> >> flag should be set by the guest if offered, but to allow for > >> >> backward-compatibility device implementations allow for it to be > >> >> left unset by the guest. It is an error to set both this flag and > >> >> VIRTIO_F_ACCESS_PLATFORM. > >> > > >> > > >> > > >> > > >> > OK so VIRTIO_F_ACCESS_PLATFORM is designed to allow unpriveledged > >> > drivers. This is why devices fail when it's not negotiated. > >> > >> Just to clarify, what do you mean by unprivileged drivers? Is it drivers > >> implemented in guest userspace such as with VFIO? Or unprivileged in > >> some other sense such as needing to use bounce buffers for some reason? > > > > I had drivers in guest userspace in mind. > > Great. Thanks for clarifying. > > I don't think this flag would work for guest userspace drivers. Should I > add a note about that in the flag definition? I think you need to clarify access protection rules. Is it only translation that is bypassed or is any platform-specific protection mechanism bypassed too? > >> > This confuses me. > >> > If driver is unpriveledged then what happens with this flag? > >> > It can supply any address it wants. Will that corrupt kernel > >> > memory? > >> > >> Not needing address translation doesn't necessarily mean that there's no > >> IOMMU. On powerpc we don't use VIRTIO_F_ACCESS_PLATFORM but there's > >> always an IOMMU present. And we also support VFIO drivers. The VFIO API > >> for pseries (sPAPR section in Documentation/vfio.txt) has extra ioctls > >> to program the IOMMU. > >> > >> For our use case, we don't need address translation because we set up an > >> identity mapping in the IOMMU so that the device can use guest physical > >> addresses. OK so I think I am beginning to see it in a different light. Right now the specific platform creates an identity mapping. That in turn means DMA API can be fast - it does not need to do anything. What you are looking for is a way to tell host it's an identity mapping - just as an optimization. Is that right? So this is what I would call this option: VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS and the explanation should state that all device addresses are translated by the platform to identical addresses. In fact this option then becomes more, not less restrictive than VIRTIO_F_ACCESS_PLATFORM - it's a promise by guest to only create identity mappings, and only before driver_ok is set. This option then would always be negotiated together with VIRTIO_F_ACCESS_PLATFORM. Host then must verify that 1. full 1:1 mappings are created before driver_ok or can we make sure this happens before features_ok? that would be ideal as we could require that features_ok fails 2. mappings are not modified between driver_ok and reset i guess attempts to change them will fail - possibly by causing a guest crash or some other kind of platform-specific error So far so good, but now a question: how are we handling guest address width limitations? Is VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS subject to guest address width limitations? I am guessing we can make them so ... This needs to be documented. > > > > And can it access any guest physical address? > > Sorry, I was mistaken. We do support VFIO in guests but not for virtio > devices, only for regular PCI devices. In which case they will use > address translation. Not sure how this answers the question. > >> If the guest kernel is concerned that an unprivileged driver could > >> jeopardize its integrity it should not negotiate this feature flag. > > > > Unfortunately flag negotiation is done through config space > > and so can be overwritten by the driver. > > Ok, so the guest kernel has to forbid VFIO access on devices where this > flag is advertised. That's possible in theory but in practice we did not yet teach VFIO not to attach to legacy devices without VIRTIO_F_ACCESS_PLATFORM. So all security relies on host denying driver_ok without
Re: DMA-API attr - DMA_ATTR_NO_KERNEL_MAPPING
On 28/06/2019 17:29, Pankaj Suryawanshi wrote: On Wed, Jun 26, 2019 at 11:21 PM Christoph Hellwig wrote: On Wed, Jun 26, 2019 at 10:12:45PM +0530, Pankaj Suryawanshi wrote: [CC: linux kernel and Vlastimil Babka] The right list is the list for the DMA mapping subsystem, which is iommu@lists.linux-foundation.org. I've also added that. I am writing driver in which I used DMA_ATTR_NO_KERNEL_MAPPING attribute for cma allocation using dma_alloc_attr(), as per kernel docs https://www.kernel.org/doc/Documentation/DMA-attributes.txt buffers allocated with this attribute can be only passed to user space by calling dma_mmap_attrs(). how can I mapped in kernel space (after dma_alloc_attr with DMA_ATTR_NO_KERNEL_MAPPING ) ? You can't. And that is the whole point of that API. 1. We can again mapped in kernel space using dma_remap() api , because when we are using DMA_ATTR_NO_KERNEL_MAPPING for dma_alloc_attr it returns the page as virtual address(in case of CMA) so we can mapped it again using dma_remap(). No, you really can't. A caller of dma_alloc_attrs(..., DMA_ATTR_NO_KERNEL_MAPPING) cannot make any assumptions about the void* it returns, other than that it must be handed back to dma_free_attrs() later. The implementation is free to ignore the flag and give back a virtual mapping anyway. Any driver which depends on how one particular implementation on one particular platform happens to behave today is, essentially, wrong. 2. We can mapped in kernel space using vmap() as used for ion-cma https://github.com/torvalds/linux/tree/master/drivers/staging/android/ion as used in function ion_heap_map_kernel(). Please let me know if i am missing anything. If you want a kernel mapping, *don't* explicitly request not to have a kernel mapping in the first place. It's that simple. Robin.
Re: [PATCH] iommu/dma: Fix calculation overflow in __finalise_sg()
On 01/07/2019 13:21, Joerg Roedel wrote:
On Fri, Jun 21, 2019 at 09:38:14PM -0700, Nicolin Chen wrote:
The max_len is a u32 type variable so the calculation on the
left hand of the last if-condition will potentially overflow
when a cur_len gets closer to UINT_MAX -- note that there're
drivers setting max_seg_size to UINT_MAX:
drivers/dma/dw-edma/dw-edma-core.c:745:
dma_set_max_seg_size(dma->dev, U32_MAX);
drivers/dma/dma-axi-dmac.c:871:
dma_set_max_seg_size(>dev, UINT_MAX);
drivers/mmc/host/renesas_sdhi_internal_dmac.c:338:
dma_set_max_seg_size(dev, 0x);
drivers/nvme/host/pci.c:2520:
dma_set_max_seg_size(dev->dev, 0x);
So this patch just casts the cur_len in the calculation to a
size_t type to fix the overflow issue, as it's not necessary
to change the type of cur_len after all.
Fixes: 809eac54cdd6 ("iommu/dma: Implement scatterlist segment merging")
Cc: sta...@vger.kernel.org
Signed-off-by: Nicolin Chen
Looks good to me, but I let Robin take a look too before I apply it,
Robin?
I'll need to take a closer look at how exactly an overflow would happen
here (just got back off some holiday), but my immediate thought is that
if this is a real problem, then what about 32-bit builds where size_t
would still overflow?
Robin.
Re: [PATCH] iommu: io-pgtable: Support non-coherent page tables
On 25/06/2019 12:56, Will Deacon wrote:
On Mon, Jun 24, 2019 at 12:53:49PM +0100, Will Deacon wrote:
On Tue, Jun 18, 2019 at 06:39:33PM +0100, Will Deacon wrote:
On Wed, May 15, 2019 at 04:32:34PM -0700, Bjorn Andersson wrote:
Describe the memory related to page table walks as non-cachable for iommu
instances that are not DMA coherent.
Signed-off-by: Bjorn Andersson
---
drivers/iommu/io-pgtable-arm.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 4e21efbc4459..68ff22ffd2cb 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -803,9 +803,15 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg,
void *cookie)
return NULL;
/* TCR */
- reg = (ARM_LPAE_TCR_SH_IS << ARM_LPAE_TCR_SH0_SHIFT) |
- (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_IRGN0_SHIFT) |
- (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_ORGN0_SHIFT);
+ if (cfg->quirks & IO_PGTABLE_QUIRK_NO_DMA) {
+ reg = (ARM_LPAE_TCR_SH_IS << ARM_LPAE_TCR_SH0_SHIFT) |
+ (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_IRGN0_SHIFT) |
+ (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_ORGN0_SHIFT);
+ } else {
+ reg = (ARM_LPAE_TCR_SH_IS << ARM_LPAE_TCR_SH0_SHIFT) |
Nit: this should be outer-shareable (ARM_LPAE_TCR_SH_OS).
+ (ARM_LPAE_TCR_RGN_NC << ARM_LPAE_TCR_IRGN0_SHIFT) |
+ (ARM_LPAE_TCR_RGN_NC << ARM_LPAE_TCR_ORGN0_SHIFT);
+ }>>
Should we also be doing something similar for the short-descriptor code
in io-pgtable-arm-v7s.c? Looks like you just need to use ARM_V7S_RGN_NC
instead of ARM_V7S_RGN_WBWA when initialising ttbr0 for non-coherent
SMMUs.
Do you plan to respin this? I'll need it this week if you're shooting for
5.3.
I started having a crack at this myself, but in doing so I realised that
using IO_PGTABLE_QUIRK_NO_DMA for this isn't quite right based on its
current description. When that flag is set, we can rely on the page-table walker
being coherent, but I don't think it works the other way around: you can't
rely on the flag being clear meaning that the page-table walker is not
coherent.
Ideally, we'd use something like is_device_dma_coherent, but that's a Xen
thing and it doesn't look reliable for the IOMMU.
Looking at the users of io-pgtable, we have:
panfrost_mmu.c - I can't see where the page-table even gets installed...
arm-smmu.c - IO_PGTABLE_QUIRK_NO_DMA is reliable
arm-smmu-v3.c - IO_PGTABLE_QUIRK_NO_DMA is reliable
ipmmu-vmsa.c- Always sets TTBCR as cacheable (ignores tcr)
msm_iommu.c - Always non-coherent
mtk_iommu.c - Ignores tcr
qcom_iommu.c- Always non-coherent
so we could go ahead and change IO_PGTABLE_QUIRK_NO_DMA to do what you want
without breaking any drivers. In any case, the driver is free to override
the control register if it knows better, as seems to be the case for some
of these already.
See patch below. I'll rework your patch on top.
Will
--->8
From 4f41845b340783eaec9cc2840fe3cb9a00574054 Mon Sep 17 00:00:00 2001
From: Will Deacon
Date: Tue, 25 Jun 2019 12:51:25 +0100
Subject: [PATCH] iommu/io-pgtable: Replace IO_PGTABLE_QUIRK_NO_DMA with
specific flag
IO_PGTABLE_QUIRK_NO_DMA is a bit of a misnomer, since it's really just
an indication of whether or not the page-table walker for the IOMMU is
coherent with the CPU caches. Since cache coherency is more than just a
quirk, replace the flag with its own field in the io_pgtable_cfg
structure.
The comment may have ended up being a bit misleading, but the name
itself wasn't a misnomer - it specifically meant "skip DMA API calls",
which served two purposes:
- Firstly, for the selftests where we didn't have a valid DMA API
device available.
- Secondly, as a performance hack to short-circuit the DMA API call
overhead (and fiddly intermediate-table-flush-flag logic) when, due to
other assumptions elsewhere, we could be sure that they would be no-ops.
I don't massively object to this change having been merged (after all,
the original reasoning was mine[1]), but I don't believe it really makes
anything better either - it's mostly just moving the goalposts. As it
stands, now it looks like you can make a coherent SMMU do non-cacheable
walks on a per-context basis by choosing not to set this flag, but
anyone trying that will quickly find it subtly and fundamentally broken.
Robin.
[1]
https://lore.kernel.org/linux-arm-kernel/a508a6a5-8e1f-3660-51ef-e65bc3829...@arm.com/
Cc: Bjorn Andersson
Signed-off-by: Will Deacon
---
drivers/iommu/arm-smmu-v3.c| 4 +---
drivers/iommu/arm-smmu.c | 4 +---
drivers/iommu/io-pgtable-arm-v7s.c | 10 +-
drivers/iommu/io-pgtable-arm.c | 19 ---
drivers/iommu/ipmmu-vmsa.c | 1 +
include/linux/io-pgtable.h | 11 ---
6 files
Re: [PATCH] irq_remapping/vt-d: cleanup unused variable
On Mon, Jun 24, 2019 at 01:17:42PM -0700, Jacob Pan wrote: > drivers/iommu/intel_irq_remapping.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dma: Fix calculation overflow in __finalise_sg()
On Fri, Jun 21, 2019 at 09:38:14PM -0700, Nicolin Chen wrote:
> The max_len is a u32 type variable so the calculation on the
> left hand of the last if-condition will potentially overflow
> when a cur_len gets closer to UINT_MAX -- note that there're
> drivers setting max_seg_size to UINT_MAX:
> drivers/dma/dw-edma/dw-edma-core.c:745:
> dma_set_max_seg_size(dma->dev, U32_MAX);
> drivers/dma/dma-axi-dmac.c:871:
> dma_set_max_seg_size(>dev, UINT_MAX);
> drivers/mmc/host/renesas_sdhi_internal_dmac.c:338:
> dma_set_max_seg_size(dev, 0x);
> drivers/nvme/host/pci.c:2520:
> dma_set_max_seg_size(dev->dev, 0x);
>
> So this patch just casts the cur_len in the calculation to a
> size_t type to fix the overflow issue, as it's not necessary
> to change the type of cur_len after all.
>
> Fixes: 809eac54cdd6 ("iommu/dma: Implement scatterlist segment merging")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Nicolin Chen
Looks good to me, but I let Robin take a look too before I apply it,
Robin?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 0/5] iommu/amd: Convert the AMD iommu driver to the dma-iommu api
Hi, On Sun, Jun 23, 2019 at 11:19:45PM -0700, Christoph Hellwig wrote: > Joerg, any chance you could review this? Toms patches to convert the > AMD and Intel IOMMU drivers to the dma-iommu code are going to make my > life in DMA land significantly easier, so I have a vested interest > in this series moving forward :) I really appreciate Toms work on this. Tom, please rebase and resubmit this series after the next merge window and I will do more performance testing on it. If all goes well I and no other issues show up I can apply it for v5.4. Regards, Joerg
Re: [PATCH v3] iommu/amd: Flush not present cache in iommu_map_page
On Thu, Jun 13, 2019 at 11:04:55PM +0100, Tom Murphy wrote: > drivers/iommu/amd_iommu.c | 20 > 1 file changed, 16 insertions(+), 4 deletions(-) Applied, thanks.
Re: [PATCH 0/3] handle init errors more gracefully in amd_iommu
Hi Kevin, On Wed, Jun 12, 2019 at 02:52:01PM -0700, Kevin Mitchell wrote: > I have tested this series on a variety of AMD CPUs with firmware > exhibiting the issue. I have additionally tested on platforms where the > firmware has been fixed. I tried both with and without amd_iommu=off. I > have also tested on older CPUs where no IOMMU is detected and even one > where the GART driver ends up running. Thanks for the fixes, applied them all.
Re: [GIT PULL] iommu/arm-smmu: Updates for 5.3
On Fri, Jun 28, 2019 at 12:04:40PM +0100, Will Deacon wrote: > git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git > for-joerg/arm-smmu/updates Pulled, thanks Will. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] m68k: use the generic dma coherent remap allocator
Hi Christoph,
On Tue, Jun 25, 2019 at 11:01 AM Christoph Hellwig wrote:
> This switche to using common code for the DMA allocations, including
switches m68k
> potential use of the CMA allocator if configure. Also add a
configured
> comment where the existing behavior seems to be lacking.
>
> Switching to the generic code enables DMA allocations from atomic
> context, which is required by the DMA API documentation, and also
> adds various other minor features drivers start relying upon. It
> also makes sure we have on tested code base for all architectures
a tested code base
> that require uncached pte bits for coherent DMA allocations.
>
> Signed-off-by: Christoph Hellwig
Thanks, applying and queueing for v5.3.
> --- a/arch/m68k/kernel/dma.c
> +++ b/arch/m68k/kernel/dma.c
> @@ -18,57 +18,20 @@
> #include
>
> #if defined(CONFIG_MMU) && !defined(CONFIG_COLDFIRE)
> -
> -void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
> - gfp_t flag, unsigned long attrs)
> +pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
> + unsigned long attrs)
> {
> - struct page *page, **map;
> - pgprot_t pgprot;
> - void *addr;
> - int i, order;
> -
> - pr_debug("dma_alloc_coherent: %d,%x\n", size, flag);
> -
> - size = PAGE_ALIGN(size);
> - order = get_order(size);
> -
> - page = alloc_pages(flag | __GFP_ZERO, order);
> - if (!page)
> - return NULL;
> -
> - *handle = page_to_phys(page);
> - map = kmalloc(sizeof(struct page *) << order, flag & ~__GFP_DMA);
> - if (!map) {
> - __free_pages(page, order);
> - return NULL;
> + /*
> +* XXX: this doesn't seem to handle the sun3 MMU at all.
Correct. This file is not compiled on Sun-3, which selects NO_DMA, so
I'll drop the comment while applying.
> +*/
> + if (CPU_IS_040_OR_060) {
> + pgprot_val(prot) &= ~_PAGE_CACHE040;
> + pgprot_val(prot) |= _PAGE_GLOBAL040 | _PAGE_NOCACHE_S;
> + } else {
> + pgprot_val(prot) |= _PAGE_NOCACHE030;
> }
> - split_page(page, order);
> -
> - order = 1 << order;
> - size >>= PAGE_SHIFT;
> - map[0] = page;
> - for (i = 1; i < size; i++)
> - map[i] = page + i;
> - for (; i < order; i++)
> - __free_page(page + i);
> - pgprot = __pgprot(_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_DIRTY);
> - if (CPU_IS_040_OR_060)
> - pgprot_val(pgprot) |= _PAGE_GLOBAL040 | _PAGE_NOCACHE_S;
> - else
> - pgprot_val(pgprot) |= _PAGE_NOCACHE030;
> - addr = vmap(map, size, VM_MAP, pgprot);
> - kfree(map);
> -
> - return addr;
> + return prot;
> }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: Device specific pass through in host systems - discuss user interface
On Tue, Jun 11, 2019 at 05:27:15PM +, Prakhya, Sai Praneeth wrote: > 1. Since we already have "type" file, which is "read-only", we could make it > R/W. > > The present value shows the existing type of default domain. > If user wants to change it (Eg: from DMA to IDENTITY or vice versa), he > attempts to write the new value. > Kernel performs checks to make sure that the driver in unbinded and it's safe > to change the default domain type. > After successfully changing the default_domain type internally, kernel > reflects the new value in the file. > Ay errors in the process will be reported in dmesg. I prefer this way. Writing to the file should fail with -EBUSY when it is not safe to change the default domain-type. Writing should only succeed when no device in the group is assigned to a device driver. Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: use exact allocation for dma coherent memory
On Fri, Jun 14, 2019 at 03:47:10PM +0200, Christoph Hellwig wrote: > Switching to a slightly cleaned up alloc_pages_exact is pretty easy, > but it turns out that because we didn't filter valid gfp_t flags > on the DMA allocator, a bunch of drivers were passing __GFP_COMP > to it, which is rather bogus in too many ways to explain. Arm has > been filtering it for a while, but this series instead tries to fix > the drivers and warn when __GFP_COMP is passed, which makes it much > larger than just adding the functionality. Dear driver maintainers, can you look over the patches touching your drivers, please? I'd like to get as much as possible of the driver patches into this merge window, so that it can you through your maintainer trees. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v7 0/5] treewide: improve R-Car SDHI performance
Any comments from the block, iommu and mmc maintainers? I'd be happy to queue this up in the dma-mapping tree, but I'll need some ACKs for that fast. Alternatively I can just queue up the DMA API bits, leaving the rest for the next merge window, but would drag things out far too long IMHO.
RE: Re: CMA in AMD IOMMU driver
Hi Christoph, Im able to reserve CMA memory during boot up in AMD. But how can i use/allocate that memory in my driver. I dont know if this is the right forum to ask such question. Any help will be appriciated. Thanks and regards, Sathya - Original Message - Sender : Christoph Hellwig Date : 2019-06-26 12:28 (GMT+5:30) Title : Re: CMA in AMD IOMMU driver To : Sathyavathi M CC : null Toms conversion of the AMD IOMMU driver to use dma-iommu adds CMA support :) Regards, Sathya ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
