Re: [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On Tue, Sep 08, 2020 at 06:36:19AM +0100, Christoph Hellwig wrote: > On Mon, Sep 07, 2020 at 09:18:50PM +0100, Tom Murphy wrote: > > Yeah we talked about passing an attr to map_sg to disable merging at > > the following microconfernce: > > https://linuxplumbersconf.org/event/7/contributions/846/ > > As far as I can remember everyone seemed happy with that solution. I > > won't be working on this though as I don't have any more time to > > dedicate to this. It seems Lu Baolu will take over this. > > I'm absolutely again passing a flag. Tha just invites further > abuse. We need a PCI ID based quirk or something else that can't > be as easily abused. Also, I looked at i915 and there are just three dma_map_sg callers. The two dmabuf related ones are fixed by Marek in his series, leaving just the one in i915_gem_gtt_prepare_pages, which does indeed look very fishy. But if that one is so hard to fix it can just be replaced by an open coded for_each_sg loop that contains manual dma_map_page calls. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On Mon, Sep 07, 2020 at 09:18:50PM +0100, Tom Murphy wrote: > Yeah we talked about passing an attr to map_sg to disable merging at > the following microconfernce: > https://linuxplumbersconf.org/event/7/contributions/846/ > As far as I can remember everyone seemed happy with that solution. I > won't be working on this though as I don't have any more time to > dedicate to this. It seems Lu Baolu will take over this. I'm absolutely again passing a flag. Tha just invites further abuse. We need a PCI ID based quirk or something else that can't be as easily abused. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/2] iommu/amd: Fix IOMMUv2 devices when SME is active
Am 2020-09-06 um 12:08 p.m. schrieb Deucher, Alexander: > [AMD Official Use Only - Internal Distribution Only] > >> -Original Message- >> From: Joerg Roedel >> Sent: Friday, September 4, 2020 6:06 AM >> To: Deucher, Alexander >> Cc: jroe...@suse.de; Kuehling, Felix ; >> iommu@lists.linux-foundation.org; Huang, Ray ; >> Koenig, Christian ; Lendacky, Thomas >> ; Suthikulpanit, Suravee >> ; linux-ker...@vger.kernel.org >> Subject: Re: [PATCH 0/2] iommu/amd: Fix IOMMUv2 devices when SME is >> active >> >> On Fri, Aug 28, 2020 at 03:47:07PM +, Deucher, Alexander wrote: >>> Ah, right, So CZ and ST are not an issue. Raven is paired with Zen based >> CPUs. >> >> Okay, so for the Raven case, can you add code to the amdgpu driver which >> makes it fail to initialize on Raven when SME is active? There is a global >> checking function for that, so that shouldn't be hard to do. >> > Sure. How about the attached patch? The patch is Acked-by: Felix Kuehling Thanks, Felix > > Alex > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On Mon, 7 Sep 2020 at 08:00, Christoph Hellwig wrote: > > On Thu, Sep 03, 2020 at 09:18:37PM +0100, Tom Murphy wrote: > > Disable combining sg segments in the dma-iommu api. > > Combining the sg segments exposes a bug in the intel i915 driver which > > causes visual artifacts and the screen to freeze. This is most likely > > because of how the i915 handles the returned list. It probably doesn't > > respect the returned value specifying the number of elements in the list > > and instead depends on the previous behaviour of the intel iommu driver > > which would return the same number of elements in the output list as in > > the input list. > > So what is the state of addressing this properly in i915? IF we can't I think this is the latest on addressing this issue: https://patchwork.kernel.org/cover/11306999/ tl;dr: some people seem to be looking at it but I'm not sure if it's being actively worked on > get it done ASAP I wonder if we need a runtime quirk to disable > merging instead of blocking this conversion.. Yeah we talked about passing an attr to map_sg to disable merging at the following microconfernce: https://linuxplumbersconf.org/event/7/contributions/846/ As far as I can remember everyone seemed happy with that solution. I won't be working on this though as I don't have any more time to dedicate to this. It seems Lu Baolu will take over this. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v16 00/20] iommu/arm-smmu + drm/msm: per-process GPU pgtables
On 2020-09-01 17:46, Rob Clark wrote: > From: Rob Clark > > NOTE: I have re-ordered the series, and propose that we could merge this >series in the following order: > > 1) 01-11 - merge via drm / msm-next > 2) 12-15 - merge via iommu, no dependency on msm-next pull req > 3) 16-18 - patch 16 has a dependency on 02 and 04, so it would >need to come post -rc1 or on following cycle, but I >think it would be unlikely to conflict with other >arm-smmu patches (other than Bjorn's smmu handover >series?) > 4) 19-20 - dt bits should be safe to land in any order without >breaking anything > > > > This series adds an Adreno SMMU implementation to arm-smmu to allow GPU > hardware > pagetable switching. > > The Adreno GPU has built in capabilities to switch the TTBR0 pagetable during > runtime to allow each individual instance or application to have its own > pagetable. In order to take advantage of the HW capabilities there are > certain > requirements needed of the SMMU hardware. > > This series adds support for an Adreno specific arm-smmu implementation. The > new > implementation 1) ensures that the GPU domain is always assigned context bank > 0, > 2) enables split pagetable support (TTBR1) so that the instance specific > pagetable can be swapped while the global memory remains in place and 3) > shares > the current pagetable configuration with the GPU driver to allow it to create > its own io-pgtable instances. > > The series then adds the drm/msm code to enable these features. For targets > that > support it allocate new pagetables using the io-pgtable configuration shared > by > the arm-smmu driver and swap them in during runtime. > > This version of the series merges the previous patchset(s) [1] and [2] > with the following improvements: > > v16: (Respin by Rob) >- Fix indentation >- Re-order series to split drm and iommu parts > v15: (Respin by Rob) >- Adjust dt bindings to keep SoC specific compatible (Doug) >- Add dts workaround for cheza fw limitation >- Add missing 'select IOMMU_IO_PGTABLE' (Guenter) > v14: (Respin by Rob) >- Minor update to 16/20 (only force ASID to zero in one place) >- Addition of sc7180 dtsi patch. > v13: (Respin by Rob) >- Switch to a private interface between adreno-smmu and GPU driver, > dropping the custom domain attr (Will Deacon) >- Rework the SCTLR.HUPCF patch to add new fields in smmu_domain->cfg > rather than adding new impl hook (Will Deacon) >- Drop for_each_cfg_sme() in favor of plain for() loop (Will Deacon) >- Fix context refcnt'ing issue which was causing problems with GPU > crash recover stress testing. >- Spiff up $debugfs/gem to show process information associated with > VMAs > v12: >- Nitpick cleanups in gpu/drm/msm/msm_iommu.c (Rob Clark) >- Reorg in gpu/drm/msm/msm_gpu.c (Rob Clark) >- Use the default asid for the context bank so that iommu_tlb_flush_all > works >- Flush the UCHE after a page switch >- Add the SCTLR.HUPCF patch at the end of the series > v11: >- Add implementation specific get_attr/set_attr functions (per Rob Clark) >- Fix context bank allocation (per Bjorn Andersson) > v10: >- arm-smmu: add implementation hook to allocate context banks >- arm-smmu: Match the GPU domain by stream ID instead of compatible string >- arm-smmu: Make DOMAIN_ATTR_PGTABLE_CFG bi-directional. The leaf driver > queries the configuration to create a pagetable and then sends the newly > created configuration back to the smmu-driver to enable TTBR0 >- drm/msm: Add context reference counting for submissions >- drm/msm: Use dummy functions to skip TLB operations on per-instance > pagetables > > [1] https://lists.linuxfoundation.org/pipermail/iommu/2020-June/045653.html > [2] https://lists.linuxfoundation.org/pipermail/iommu/2020-June/045659.html > > Jordan Crouse (12): >drm/msm: Add a context pointer to the submitqueue >drm/msm: Drop context arg to gpu->submit() >drm/msm: Set the global virtual address range from the IOMMU domain >drm/msm: Add support to create a local pagetable >drm/msm: Add support for private address space instances >drm/msm/a6xx: Add support for per-instance pagetables >iommu/arm-smmu: Pass io-pgtable config to implementation specific > function >iommu/arm-smmu: Add support for split pagetables >iommu/arm-smmu: Prepare for the adreno-smmu implementation >iommu/arm-smmu-qcom: Add implementation for the adreno GPU SMMU >dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU >arm: dts: qcom: sm845: Set the compatible string for the GPU SMMU > > Rob Clark (8): >drm/msm: Remove dangling submitqueue references >drm/msm: Add private interface for adreno-smmu >drm/msm/gpu: Add dev_to_gpu() helper >drm/msm:
Re: [PATCH v3 0/8] iommu/arm-smmu: Support maintaining bootloader mappings
On 2020-09-04 16:55, Bjorn Andersson wrote: > Based on previous attempts and discussions this is the latest attempt at > inheriting stream mappings set up by the bootloader, for e.g. boot splash or > efifb. > > Per Will's request this builds on the work by Jordan and Rob for the Adreno > SMMU support. It applies cleanly ontop of v16 of their series, which can be > found at > https://lore.kernel.org/linux-arm-msm/20200901164707.2645413-1-robdcl...@gmail.com/ > > Bjorn Andersson (8): >iommu/arm-smmu: Refactor context bank allocation >iommu/arm-smmu: Delay modifying domain during init >iommu/arm-smmu: Consult context bank allocator for identify domains >iommu/arm-smmu-qcom: Emulate bypass by using context banks >iommu/arm-smmu-qcom: Consistently initialize stream mappings >iommu/arm-smmu: Add impl hook for inherit boot mappings >iommu/arm-smmu: Provide helper for allocating identity domain >iommu/arm-smmu-qcom: Setup identity domain for boot mappings > > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 111 ++- > drivers/iommu/arm/arm-smmu/arm-smmu.c | 122 ++--- > drivers/iommu/arm/arm-smmu/arm-smmu.h | 14 ++- > 3 files changed, 205 insertions(+), 42 deletions(-) > Tested on the OnePlus 6 (SDM845), allows booting with display enabled. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
On 9/7/2020 10:43 AM, Jim Quinlan wrote: On Mon, Sep 7, 2020 at 5:16 AM Lorenzo Pieralisi wrote: On Thu, Aug 27, 2020 at 09:29:59AM -0400, Jim Quinlan wrote: On Thu, Aug 27, 2020 at 2:35 AM Christoph Hellwig wrote: On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote: Hi, On 8/24/2020 12:30 PM, Jim Quinlan wrote: Patchset Summary: Enhance a PCIe host controller driver. Because of its unusual design we are foced to change dev->dma_pfn_offset into a more general role allowing multiple offsets. See the 'v1' notes below for more info. We are version 11 and counting, and it is not clear to me whether there is any chance of getting these patches reviewed and hopefully merged for the 5.10 merge window. There are a lot of different files being touched, so what would be the ideal way of routing those changes towards inclusion? FYI, I offered to take the dma-mapping bits through the dma-mapping tree. I have a bit of a backlog, but plan to review and if Jim is ok with that apply the current version. Sounds good to me. Hi Jim, is the dependency now solved ? Should we review/take this series as is for v5.10 through the PCI tree ? Hello Lorenzo, We are still working out a regression with the DMA offset commit on the RaspberryPi. Nicolas has found the root cause and we are now devising a solution. Maybe we can parallelize the PCIe driver review while the DMA changes are being worked on in Christoph's branch. Lorenzo, are you fine with the PCIe changes proper? -- Florian ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, 2020-09-07 at 13:40 -0400, Jim Quinlan wrote:
> On Mon, Sep 7, 2020 at 11:01 AM Nicolas Saenz Julienne
> wrote:
> > >
> > > Hi Nicolas,
> > >
> > > Can you please help us out here? It appears that your commit
> >
> > It's dma_offset_from_dma_addr() that's causing trouble. It goes over all the
> > dma regions and, if no region matches the phys address range, it returns 0.
> > This isn't acceptable as is. In the past, we used to pass the offset
> > nonetheless, which would make the phys address grow out of the dma memory
> > area
> > and fail the dma_capable() test.
> >
> > For example, RPi4 devices attached to the old interconnect see phys [0x0
> > 0x3fff] at [0xc000 0x]. So say you're trying to convert
> > physical address 0xfa00, you'll get 0 from dma_offset_from_phys_addr()
> > (since your dma range only covers the first GB) to then test if 0xfa00
> > is
> > dma_capable(), which it is, but for the wrong reasons. Causing DMA issues
> > further down the line.
> >
> > I don't have a proper suggestion on how to solve this but here's the
> > solution I
> > used:
> >
> > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> > index 4c4646761afe..40fe3c97f2be 100644
> > --- a/include/linux/dma-mapping.h
> > +++ b/include/linux/dma-mapping.h
> > @@ -217,11 +217,19 @@ static inline u64 dma_offset_from_phys_addr(struct
> > device *dev,
> > {
> > const struct bus_dma_region *m = dev->dma_range_map;
> >
> > - if (m)
> > + if (m) {
> > for (; m->size; m++)
> > if (paddr >= m->cpu_start &&
> > paddr - m->cpu_start < m->size)
> > return m->offset;
> > +
> > + /*
> > +* The physical address doesn't fit any of the DMA regions,
> > +* return an impossible to fulfill offset.
> > +*/
> > + return -(1ULL << 46);
> > + }
> > +
> > return 0;
> > }
> Hi Nicolas,
>
> Thanks for looking into this. The concern I have with your solution
> is that returning an arbitrarily large offset might overlap with an
> improbable but valid usage. AFAIK there is nothing that disallows
> mapping a device to anywhere within the 64bit range of PCIe space,
> including up to say 0x.
Indeed, that's why I wasn't all that happy with my solution.
As an alternative, how about returning '-dev->bus_dma_limit' instead of 0? It's
always 0 for the devices without bus_dma_regions, and, I think, an always
unattainable offset for devices that do (I tested it on RPi4 with the 30bit
limitd mmc controller and it seems to work alright).
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -222,7 +222,8 @@ static inline u64 dma_offset_from_phys_addr(struct device
*dev,
if (paddr >= m->cpu_start &&
paddr - m->cpu_start < m->size)
return m->offset;
- return 0;
+
+ return -dev->bus_dma_limit;
}
> As an alternative perhaps changing dma_capable() so that if the
> dma_range_map is present then it validates that both ends of the
> prospective DMA region get "hits" on one of the offset regions in the
> map. Christoph, if you are okay with this I can quickly post a patch.
IIUC, by the time you enter dma_capable() you already converted the physical
address to DMA address and the damage is done.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
On Mon, Sep 7, 2020 at 5:16 AM Lorenzo Pieralisi wrote: > > On Thu, Aug 27, 2020 at 09:29:59AM -0400, Jim Quinlan wrote: > > On Thu, Aug 27, 2020 at 2:35 AM Christoph Hellwig wrote: > > > > > > On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote: > > > > Hi, > > > > > > > > On 8/24/2020 12:30 PM, Jim Quinlan wrote: > > > >> > > > >> Patchset Summary: > > > >>Enhance a PCIe host controller driver. Because of its unusual > > > >> design > > > >>we are foced to change dev->dma_pfn_offset into a more general role > > > >>allowing multiple offsets. See the 'v1' notes below for more info. > > > > > > > > We are version 11 and counting, and it is not clear to me whether there > > > > is > > > > any chance of getting these patches reviewed and hopefully merged for > > > > the > > > > 5.10 merge window. > > > > > > > > There are a lot of different files being touched, so what would be the > > > > ideal way of routing those changes towards inclusion? > > > > > > FYI, I offered to take the dma-mapping bits through the dma-mapping tree. > > > I have a bit of a backlog, but plan to review and if Jim is ok with that > > > apply the current version. > > Sounds good to me. > > Hi Jim, > > is the dependency now solved ? Should we review/take this series as > is for v5.10 through the PCI tree ? Hello Lorenzo, We are still working out a regression with the DMA offset commit on the RaspberryPi. Nicolas has found the root cause and we are now devising a solution. Thanks, Jim Quinlan Broadcom STB > > Thanks, > Lorenzo ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, Sep 7, 2020 at 11:01 AM Nicolas Saenz Julienne
wrote:
>
> Hi Jim, sorry I'm a little late to the party, but was on vacation.
>
> On Thu, 2020-09-03 at 13:32 -0400, Jim Quinlan wrote:
> > On Wed, Sep 2, 2020 at 8:52 PM Nathan Chancellor
> > wrote:
> > > On Wed, Sep 02, 2020 at 05:36:29PM -0700, Florian Fainelli wrote:
> > > >
> > > > On 9/2/2020 3:38 PM, Nathan Chancellor wrote:
> > > > [snip]
> > > > > > Hello Nathan,
> > > > > >
> > > > > > Can you tell me how much memory your RPI has and if all of it is
> > > > >
> > > > > This is the 4GB version.
> > > > >
> > > > > > accessible by the PCIe device? Could you also please include the
> > > > > > DTS
> > > > > > of the PCIe node? IIRC, the RPI firmware does some mangling of the
> > > > > > PCIe DT before Linux boots -- could you describe what is going on
> > > > > > there?
> > > > >
> > > > > Unfortunately, I am not familiar with how to get this information. If
> > > > > you could provide some instructions for how to do so, I am more than
> > > > > happy to. I am not very knowleagable about the inner working of the
> > > > > Pi,
> > > > > I mainly use it as a test platform for making sure that LLVM does not
> > > > > cause problems on real devices.
> > > >
> > > > Can you bring the dtc application to your Pi root filesystem, and if
> > > > so, can
> > > > you run the following:
> > > >
> > > > dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb
> > >
> > > Sure, the result is attached.
> > >
> > > > or cat /sys/firmware/fdt > device.dtb
> > > >
> > > > and attach the resulting file?
> > > >
> > > > > > Finally, can you attach the text of the full boot log?
> > > > >
> > > > > I have attached a working and broken boot log. Thank you for the quick
> > > > > response!
> > > >
> > > > Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by
> > > > any
> > > > chance?
> > >
> > > Of course. A new log is attached with the debug output from that config.
> >
> > Hi Nicolas,
> >
> > Can you please help us out here? It appears that your commit
>
> It's dma_offset_from_dma_addr() that's causing trouble. It goes over all the
> dma regions and, if no region matches the phys address range, it returns 0.
> This isn't acceptable as is. In the past, we used to pass the offset
> nonetheless, which would make the phys address grow out of the dma memory area
> and fail the dma_capable() test.
>
> For example, RPi4 devices attached to the old interconnect see phys [0x0
> 0x3fff] at [0xc000 0x]. So say you're trying to convert
> physical address 0xfa00, you'll get 0 from dma_offset_from_phys_addr()
> (since your dma range only covers the first GB) to then test if 0xfa00 is
> dma_capable(), which it is, but for the wrong reasons. Causing DMA issues
> further down the line.
>
> I don't have a proper suggestion on how to solve this but here's the solution
> I
> used:
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 4c4646761afe..40fe3c97f2be 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -217,11 +217,19 @@ static inline u64 dma_offset_from_phys_addr(struct
> device *dev,
> {
> const struct bus_dma_region *m = dev->dma_range_map;
>
> - if (m)
> + if (m) {
> for (; m->size; m++)
> if (paddr >= m->cpu_start &&
> paddr - m->cpu_start < m->size)
> return m->offset;
> +
> + /*
> +* The physical address doesn't fit any of the DMA regions,
> +* return an impossible to fulfill offset.
> +*/
> + return -(1ULL << 46);
> + }
> +
> return 0;
> }
Hi Nicolas,
Thanks for looking into this. The concern I have with your solution
is that returning an arbitrarily large offset might overlap with an
improbable but valid usage. AFAIK there is nothing that disallows
mapping a device to anywhere within the 64bit range of PCIe space,
including up to say 0x.
As an alternative perhaps changing dma_capable() so that if the
dma_range_map is present then it validates that both ends of the
prospective DMA region get "hits" on one of the offset regions in the
map. Christoph, if you are okay with this I can quickly post a patch.
Regards,
Jim Quinlan
Broadcom STB
>
> I didn't catch this on my previous tests as I was using a newer bcm2711 SoC
> revision which expanded emmc2's DMA capabilities (can do 32 bit DMA as opposed
> to 30 bit).
>
> Regards,
> Nicolas
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RESEND v9 09/13] iommu/arm-smmu-v3: Seize private ASID
Hi Jean,
On 8/17/20 7:15 PM, Jean-Philippe Brucker wrote:
> The SMMU has a single ASID space, the union of shared and private ASID
> sets. This means that the SMMU driver competes with the arch allocator
> for ASIDs. Shared ASIDs are those of Linux processes, allocated by the
> arch, and contribute in broadcast TLB maintenance. Private ASIDs are
> allocated by the SMMU driver and used for "classic" map/unmap DMA. They
> require command-queue TLB invalidations.
>
> When we pin down an mm_context and get an ASID that is already in use by
> the SMMU, it belongs to a private context. We used to simply abort the
> bind, but this is unfair to users that would be unable to bind a few
> seemingly random processes. Try to allocate a new private ASID for the
> context, and make the old ASID shared.
>
> Signed-off-by: Jean-Philippe Brucker
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 ++
> .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 36 +--
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 34 +++---
> 3 files changed, 58 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 6b06a6f19604..90c08f156b43 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -678,6 +678,9 @@ struct arm_smmu_domain {
> extern struct xarray arm_smmu_asid_xa;
> extern struct mutex arm_smmu_asid_lock;
>
> +int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
> + struct arm_smmu_ctx_desc *cd);
> +void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid);
> bool arm_smmu_free_asid(struct arm_smmu_ctx_desc *cd);
>
> #endif /* _ARM_SMMU_V3_H */
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> index 7a4f40565e06..e919ce894dd1 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> @@ -10,10 +10,19 @@
> #include "arm-smmu-v3.h"
> #include "../../io-pgtable-arm.h"
>
> +/*
> + * Try to reserve this ASID in the SMMU. If it is in use, try to steal it
> from
> + * the private entry. Careful here, we may be modifying the context tables of
> + * another SMMU!
Not sure I got what you meant by this comment.
> + */
> static struct arm_smmu_ctx_desc *
> arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
> {
> + int ret;
> + u32 new_asid;
> struct arm_smmu_ctx_desc *cd;
> + struct arm_smmu_device *smmu;
> + struct arm_smmu_domain *smmu_domain;
>
> cd = xa_load(_smmu_asid_xa, asid);
> if (!cd)
> @@ -27,8 +36,31 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
> return cd;
> }
>
> - /* Ouch, ASID is already in use for a private cd. */
> - return ERR_PTR(-EBUSY);
> + smmu_domain = container_of(cd, struct arm_smmu_domain, s1_cfg.cd);
> + smmu = smmu_domain->smmu;
> +
> + ret = xa_alloc(_smmu_asid_xa, _asid, cd,
> +XA_LIMIT(1, 1 << smmu->asid_bits), GFP_KERNEL);
XA_LIMIT(1, (1 << smmu->asid_bits) - 1), GFP_KERNEL)
> + if (ret)
> + return ERR_PTR(-ENOSPC);
> + /*
> + * Race with unmap: TLB invalidations will start targeting the new ASID,
> + * which isn't assigned yet. We'll do an invalidate-all on the old ASID
> + * later, so it doesn't matter.
> + */
> + cd->asid = new_asid;
> + /*
> + * Update ASID and invalidate CD in all associated masters. There will
> + * be some overlap between use of both ASIDs, until we invalidate the
> + * TLB.
> + */
> + arm_smmu_write_ctx_desc(smmu_domain, 0, cd);
> +
> + /* Invalidate TLB entries previously associated with that context */
> + arm_smmu_tlb_inv_asid(smmu, asid);
> +
> + xa_erase(_smmu_asid_xa, asid);
> + return NULL;
> }
>
> __maybe_unused
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 9e81615744de..9e755caea525 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -873,6 +873,17 @@ static int arm_smmu_cmdq_batch_submit(struct
> arm_smmu_device *smmu,
> }
>
> /* Context descriptor manipulation functions */
> +void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid)
> +{
> + struct arm_smmu_cmdq_ent cmd = {
> + .opcode = CMDQ_OP_TLBI_NH_ASID,
> + .tlbi.asid = asid,
> + };
> +
> + arm_smmu_cmdq_issue_cmd(smmu, );
> + arm_smmu_cmdq_issue_sync(smmu);
> +}
> +
> static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
>int ssid, bool leaf)
> {
> @@ -953,8 +964,8 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain
> *smmu_domain,
> return l1_desc->l2ptr + idx * CTXDESC_CD_DWORDS;
> }
>
[PATCH AUTOSEL 5.4 42/43] iommu/amd: Do not use IOMMUv2 functionality when SME is active
From: Joerg Roedel [ Upstream commit 2822e582501b65707089b097e773e6fd70774841 ] When memory encryption is active the device is likely not in a direct mapped domain. Forbid using IOMMUv2 functionality for now until finer grained checks for this have been implemented. Signed-off-by: Joerg Roedel Link: https://lore.kernel.org/r/20200824105415.21000-3-j...@8bytes.org Signed-off-by: Joerg Roedel Signed-off-by: Sasha Levin --- drivers/iommu/amd_iommu_v2.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c index d6d85debd01b0..05f3d93cf480c 100644 --- a/drivers/iommu/amd_iommu_v2.c +++ b/drivers/iommu/amd_iommu_v2.c @@ -741,6 +741,13 @@ int amd_iommu_init_device(struct pci_dev *pdev, int pasids) might_sleep(); + /* +* When memory encryption is active the device is likely not in a +* direct-mapped domain. Forbid using IOMMUv2 functionality for now. +*/ + if (mem_encrypt_active()) + return -ENODEV; + if (!amd_iommu_v2_supported()) return -ENODEV; -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.19 25/26] iommu/amd: Do not use IOMMUv2 functionality when SME is active
From: Joerg Roedel [ Upstream commit 2822e582501b65707089b097e773e6fd70774841 ] When memory encryption is active the device is likely not in a direct mapped domain. Forbid using IOMMUv2 functionality for now until finer grained checks for this have been implemented. Signed-off-by: Joerg Roedel Link: https://lore.kernel.org/r/20200824105415.21000-3-j...@8bytes.org Signed-off-by: Joerg Roedel Signed-off-by: Sasha Levin --- drivers/iommu/amd_iommu_v2.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c index 58da65df03f5e..7a59a8ebac108 100644 --- a/drivers/iommu/amd_iommu_v2.c +++ b/drivers/iommu/amd_iommu_v2.c @@ -776,6 +776,13 @@ int amd_iommu_init_device(struct pci_dev *pdev, int pasids) might_sleep(); + /* +* When memory encryption is active the device is likely not in a +* direct-mapped domain. Forbid using IOMMUv2 functionality for now. +*/ + if (mem_encrypt_active()) + return -ENODEV; + if (!amd_iommu_v2_supported()) return -ENODEV; -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 5.8 52/53] iommu/amd: Do not use IOMMUv2 functionality when SME is active
From: Joerg Roedel [ Upstream commit 2822e582501b65707089b097e773e6fd70774841 ] When memory encryption is active the device is likely not in a direct mapped domain. Forbid using IOMMUv2 functionality for now until finer grained checks for this have been implemented. Signed-off-by: Joerg Roedel Link: https://lore.kernel.org/r/20200824105415.21000-3-j...@8bytes.org Signed-off-by: Joerg Roedel Signed-off-by: Sasha Levin --- drivers/iommu/amd/iommu_v2.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/iommu/amd/iommu_v2.c b/drivers/iommu/amd/iommu_v2.c index e4b025c5637c4..5a188cac7a0f1 100644 --- a/drivers/iommu/amd/iommu_v2.c +++ b/drivers/iommu/amd/iommu_v2.c @@ -737,6 +737,13 @@ int amd_iommu_init_device(struct pci_dev *pdev, int pasids) might_sleep(); + /* +* When memory encryption is active the device is likely not in a +* direct-mapped domain. Forbid using IOMMUv2 functionality for now. +*/ + if (mem_encrypt_active()) + return -ENODEV; + if (!amd_iommu_v2_supported()) return -ENODEV; -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 5.8 51/53] iommu/amd: Do not force direct mapping when SME is active
From: Joerg Roedel [ Upstream commit 7cad554887f1c5fd77e57e6bf4be38370c2160cb ] Do not force devices supporting IOMMUv2 to be direct mapped when memory encryption is active. This might cause them to be unusable because their DMA mask does not include the encryption bit. Signed-off-by: Joerg Roedel Link: https://lore.kernel.org/r/20200824105415.21000-2-j...@8bytes.org Signed-off-by: Joerg Roedel Signed-off-by: Sasha Levin --- drivers/iommu/amd/iommu.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c index 2f22326ee4dfe..547b41e376574 100644 --- a/drivers/iommu/amd/iommu.c +++ b/drivers/iommu/amd/iommu.c @@ -2650,7 +2650,12 @@ static int amd_iommu_def_domain_type(struct device *dev) if (!dev_data) return 0; - if (dev_data->iommu_v2) + /* +* Do not identity map IOMMUv2 capable devices when memory encryption is +* active, because some of those devices (AMD GPUs) don't have the +* encryption bit in their DMA-mask and require remapping. +*/ + if (!mem_encrypt_active() && dev_data->iommu_v2) return IOMMU_DOMAIN_IDENTITY; return 0; -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 0/3] iommu/arm-smmu-v3: permit users to disable msi polling
On Thu, 27 Aug 2020 21:29:54 +1200, Barry Song wrote: > patch 1/3 and patch 2/3 are the preparation of patch 3/3 which permits users > to disable MSI-based polling by cmd line. > > -v5: > add Robin's reviewed-by > > -v4: > with respect to Robin's comments > * cleanup the code of the existing module parameter disable_bypass > * add ARM_SMMU_OPT_MSIPOLL flag. on the other hand, we only need to check > a bit in options rather than two bits in features > > [...] Applied to will (for-joerg/arm-smmu/updates), thanks! [1/3] iommu/arm-smmu-v3: replace symbolic permissions by octal permissions for module parameter https://git.kernel.org/will/c/fea9ae18cf77 [2/3] iommu/arm-smmu-v3: replace module_param_named by module_param for disable_bypass https://git.kernel.org/will/c/9305d02adcc1 [3/3] iommu/arm-smmu-v3: permit users to disable msi polling https://git.kernel.org/will/c/bd07a20a38b6 Cheers, -- Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu-v3: Fix l1 stream table size in the error message
On Wed, 26 Aug 2020 22:17:58 +0800, Zenghui Yu wrote:
> The actual size of level-1 stream table is l1size. This looks like an
> oversight on commit d2e88e7c081ef ("iommu/arm-smmu: Fix LOG2SIZE setting
> for 2-level stream tables") which forgot to update the @size in error
> message as well.
>
> As memory allocation failure is already bad enough, nothing worse would
> happen. But let's be careful.
Applied to will (for-joerg/arm-smmu/updates), thanks!
[1/1] iommu/arm-smmu-v3: Fix l1 stream table size in the error message
https://git.kernel.org/will/c/dc898eb84b25
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 30/30] videobuf2: use sgtable-based scatterlist wrappers
On Mon, Sep 7, 2020 at 4:02 PM Marek Szyprowski
wrote:
>
> Hi Tomasz,
>
> On 07.09.2020 15:07, Tomasz Figa wrote:
> > On Fri, Sep 4, 2020 at 3:35 PM Marek Szyprowski
> > wrote:
> >> Use recently introduced common wrappers operating directly on the struct
> >> sg_table objects and scatterlist page iterators to make the code a bit
> >> more compact, robust, easier to follow and copy/paste safe.
> >>
> >> No functional change, because the code already properly did all the
> >> scatterlist related calls.
> >>
> >> Signed-off-by: Marek Szyprowski
> >> Reviewed-by: Robin Murphy
> >> ---
> >> .../common/videobuf2/videobuf2-dma-contig.c | 34 ---
> >> .../media/common/videobuf2/videobuf2-dma-sg.c | 32 +++--
> >> .../common/videobuf2/videobuf2-vmalloc.c | 12 +++
> >> 3 files changed, 31 insertions(+), 47 deletions(-)
> >>
> > Thanks for the patch! Please see my comments inline.
> >
> >> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> >> b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> >> index ec3446cc45b8..1b242d844dde 100644
> >> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> >> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> >> @@ -58,10 +58,10 @@ static unsigned long vb2_dc_get_contiguous_size(struct
> >> sg_table *sgt)
> >> unsigned int i;
> >> unsigned long size = 0;
> >>
> >> - for_each_sg(sgt->sgl, s, sgt->nents, i) {
> >> + for_each_sgtable_dma_sg(sgt, s, i) {
> >> if (sg_dma_address(s) != expected)
> >> break;
> >> - expected = sg_dma_address(s) + sg_dma_len(s);
> >> + expected += sg_dma_len(s);
> >> size += sg_dma_len(s);
> >> }
> >> return size;
> >> @@ -103,8 +103,7 @@ static void vb2_dc_prepare(void *buf_priv)
> >> if (!sgt)
> >> return;
> >>
> >> - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
> >> - buf->dma_dir);
> >> + dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir);
> >> }
> >>
> >> static void vb2_dc_finish(void *buf_priv)
> >> @@ -115,7 +114,7 @@ static void vb2_dc_finish(void *buf_priv)
> >> if (!sgt)
> >> return;
> >>
> >> - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents,
> >> buf->dma_dir);
> >> + dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir);
> >> }
> >>
> >> /*/
> >> @@ -275,8 +274,8 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf
> >> *dbuf,
> >> * memory locations do not require any explicit cache
> >> * maintenance prior or after being used by the device.
> >> */
> >> - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl,
> >> sgt->orig_nents,
> >> - attach->dma_dir,
> >> DMA_ATTR_SKIP_CPU_SYNC);
> >> + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
> >> + DMA_ATTR_SKIP_CPU_SYNC);
> >> sg_free_table(sgt);
> >> kfree(attach);
> >> db_attach->priv = NULL;
> >> @@ -301,8 +300,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
> >>
> >> /* release any previous cache */
> >> if (attach->dma_dir != DMA_NONE) {
> >> - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl,
> >> sgt->orig_nents,
> >> - attach->dma_dir,
> >> DMA_ATTR_SKIP_CPU_SYNC);
> >> + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
> >> + DMA_ATTR_SKIP_CPU_SYNC);
> >> attach->dma_dir = DMA_NONE;
> >> }
> >>
> >> @@ -310,9 +309,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
> >> * mapping to the client with new direction, no cache sync
> >> * required see comment in vb2_dc_dmabuf_ops_detach()
> >> */
> >> - sgt->nents = dma_map_sg_attrs(db_attach->dev, sgt->sgl,
> >> sgt->orig_nents,
> >> - dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
> >> - if (!sgt->nents) {
> >> + if (dma_map_sgtable(db_attach->dev, sgt, dma_dir,
> >> + DMA_ATTR_SKIP_CPU_SYNC)) {
> >> pr_err("failed to map scatterlist\n");
> >> mutex_unlock(lock);
> >> return ERR_PTR(-EIO);
> > As opposed to dma_map_sg_attrs(), dma_map_sgtable() now returns an
> > error code on its own. Is it expected to ignore it and return -EIO?
>
> Those errors are more or less propagated to userspace and -EIO has been
> already widely documented in V4L2 documentation as the error code for
> the most of the V4L2 ioctls. I don't want to change it. A possible
> -EINVAL returned from dma_map_sgtable() was just one of the 'generic'
> error codes, not very descriptive in that case.
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
Hi Christoph, a small fix to your fixes: On Tue, 2020-09-01 at 10:24 +0200, Christoph Hellwig wrote: > I've applied this to the dma-mapping tree. > > I had to resolve a conflict in drivers/of/address.c with a recent > mainline commit. I also applied the minor tweaks Andy pointed out > plus a few more style changes. A real change is that I changed the > prototype for dma_copy_dma_range_map to require less boilerplate code. diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 0b5f8d62f251..4cd012817af6 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -610,7 +610,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent, * mask for the entire HCD, so don't do that. */ dev->dev.dma_mask = bus->sysdev->dma_mask; - if (!dma_copy_dma_range_map(>dev, bus->sysdev)) + if (dma_copy_dma_range_map(>dev, bus->sysdev)) dev_err(>dev, "failed to copy DMA map\n"); set_dev_node(>dev, dev_to_node(bus->sysdev)); dev->state = USB_STATE_ATTACHED; Regards, Nicolas signature.asc Description: This is a digitally signed message part ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/sun50i: Fix set-but-not-used variable warning
On Fri, Sep 04, 2020 at 01:39:06PM +0200, Joerg Roedel wrote: > From: Joerg Roedel > > Fix the following warning the the SUN50I driver: > >drivers/iommu/sun50i-iommu.c: In function 'sun50i_iommu_irq': >drivers/iommu/sun50i-iommu.c:890:14: warning: variable 'iova' set but not > used [-Wunused-but-set-variable] > 890 | phys_addr_t iova; > | ^~~~ > > Reported-by: kernel test robot > Signed-off-by: Joerg Roedel Acked-by: Maxime Ripard Thanks! Maxime ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
Hi Jim, sorry I'm a little late to the party, but was on vacation.
On Thu, 2020-09-03 at 13:32 -0400, Jim Quinlan wrote:
> On Wed, Sep 2, 2020 at 8:52 PM Nathan Chancellor
> wrote:
> > On Wed, Sep 02, 2020 at 05:36:29PM -0700, Florian Fainelli wrote:
> > >
> > > On 9/2/2020 3:38 PM, Nathan Chancellor wrote:
> > > [snip]
> > > > > Hello Nathan,
> > > > >
> > > > > Can you tell me how much memory your RPI has and if all of it is
> > > >
> > > > This is the 4GB version.
> > > >
> > > > > accessible by the PCIe device? Could you also please include the DTS
> > > > > of the PCIe node? IIRC, the RPI firmware does some mangling of the
> > > > > PCIe DT before Linux boots -- could you describe what is going on
> > > > > there?
> > > >
> > > > Unfortunately, I am not familiar with how to get this information. If
> > > > you could provide some instructions for how to do so, I am more than
> > > > happy to. I am not very knowleagable about the inner working of the Pi,
> > > > I mainly use it as a test platform for making sure that LLVM does not
> > > > cause problems on real devices.
> > >
> > > Can you bring the dtc application to your Pi root filesystem, and if so,
> > > can
> > > you run the following:
> > >
> > > dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb
> >
> > Sure, the result is attached.
> >
> > > or cat /sys/firmware/fdt > device.dtb
> > >
> > > and attach the resulting file?
> > >
> > > > > Finally, can you attach the text of the full boot log?
> > > >
> > > > I have attached a working and broken boot log. Thank you for the quick
> > > > response!
> > >
> > > Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by any
> > > chance?
> >
> > Of course. A new log is attached with the debug output from that config.
>
> Hi Nicolas,
>
> Can you please help us out here? It appears that your commit
It's dma_offset_from_dma_addr() that's causing trouble. It goes over all the
dma regions and, if no region matches the phys address range, it returns 0.
This isn't acceptable as is. In the past, we used to pass the offset
nonetheless, which would make the phys address grow out of the dma memory area
and fail the dma_capable() test.
For example, RPi4 devices attached to the old interconnect see phys [0x0
0x3fff] at [0xc000 0x]. So say you're trying to convert
physical address 0xfa00, you'll get 0 from dma_offset_from_phys_addr()
(since your dma range only covers the first GB) to then test if 0xfa00 is
dma_capable(), which it is, but for the wrong reasons. Causing DMA issues
further down the line.
I don't have a proper suggestion on how to solve this but here's the solution I
used:
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4c4646761afe..40fe3c97f2be 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -217,11 +217,19 @@ static inline u64 dma_offset_from_phys_addr(struct device
*dev,
{
const struct bus_dma_region *m = dev->dma_range_map;
- if (m)
+ if (m) {
for (; m->size; m++)
if (paddr >= m->cpu_start &&
paddr - m->cpu_start < m->size)
return m->offset;
+
+ /*
+* The physical address doesn't fit any of the DMA regions,
+* return an impossible to fulfill offset.
+*/
+ return -(1ULL << 46);
+ }
+
return 0;
}
I didn't catch this on my previous tests as I was using a newer bcm2711 SoC
revision which expanded emmc2's DMA capabilities (can do 32 bit DMA as opposed
to 30 bit).
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 30/30] videobuf2: use sgtable-based scatterlist wrappers
Hi Tomasz,
On 07.09.2020 15:07, Tomasz Figa wrote:
> On Fri, Sep 4, 2020 at 3:35 PM Marek Szyprowski
> wrote:
>> Use recently introduced common wrappers operating directly on the struct
>> sg_table objects and scatterlist page iterators to make the code a bit
>> more compact, robust, easier to follow and copy/paste safe.
>>
>> No functional change, because the code already properly did all the
>> scatterlist related calls.
>>
>> Signed-off-by: Marek Szyprowski
>> Reviewed-by: Robin Murphy
>> ---
>> .../common/videobuf2/videobuf2-dma-contig.c | 34 ---
>> .../media/common/videobuf2/videobuf2-dma-sg.c | 32 +++--
>> .../common/videobuf2/videobuf2-vmalloc.c | 12 +++
>> 3 files changed, 31 insertions(+), 47 deletions(-)
>>
> Thanks for the patch! Please see my comments inline.
>
>> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>> b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>> index ec3446cc45b8..1b242d844dde 100644
>> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
>> @@ -58,10 +58,10 @@ static unsigned long vb2_dc_get_contiguous_size(struct
>> sg_table *sgt)
>> unsigned int i;
>> unsigned long size = 0;
>>
>> - for_each_sg(sgt->sgl, s, sgt->nents, i) {
>> + for_each_sgtable_dma_sg(sgt, s, i) {
>> if (sg_dma_address(s) != expected)
>> break;
>> - expected = sg_dma_address(s) + sg_dma_len(s);
>> + expected += sg_dma_len(s);
>> size += sg_dma_len(s);
>> }
>> return size;
>> @@ -103,8 +103,7 @@ static void vb2_dc_prepare(void *buf_priv)
>> if (!sgt)
>> return;
>>
>> - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
>> - buf->dma_dir);
>> + dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir);
>> }
>>
>> static void vb2_dc_finish(void *buf_priv)
>> @@ -115,7 +114,7 @@ static void vb2_dc_finish(void *buf_priv)
>> if (!sgt)
>> return;
>>
>> - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents,
>> buf->dma_dir);
>> + dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir);
>> }
>>
>> /*/
>> @@ -275,8 +274,8 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf
>> *dbuf,
>> * memory locations do not require any explicit cache
>> * maintenance prior or after being used by the device.
>> */
>> - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents,
>> - attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
>> + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
>> + DMA_ATTR_SKIP_CPU_SYNC);
>> sg_free_table(sgt);
>> kfree(attach);
>> db_attach->priv = NULL;
>> @@ -301,8 +300,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
>>
>> /* release any previous cache */
>> if (attach->dma_dir != DMA_NONE) {
>> - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents,
>> - attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
>> + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
>> + DMA_ATTR_SKIP_CPU_SYNC);
>> attach->dma_dir = DMA_NONE;
>> }
>>
>> @@ -310,9 +309,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
>> * mapping to the client with new direction, no cache sync
>> * required see comment in vb2_dc_dmabuf_ops_detach()
>> */
>> - sgt->nents = dma_map_sg_attrs(db_attach->dev, sgt->sgl,
>> sgt->orig_nents,
>> - dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
>> - if (!sgt->nents) {
>> + if (dma_map_sgtable(db_attach->dev, sgt, dma_dir,
>> + DMA_ATTR_SKIP_CPU_SYNC)) {
>> pr_err("failed to map scatterlist\n");
>> mutex_unlock(lock);
>> return ERR_PTR(-EIO);
> As opposed to dma_map_sg_attrs(), dma_map_sgtable() now returns an
> error code on its own. Is it expected to ignore it and return -EIO?
Those errors are more or less propagated to userspace and -EIO has been
already widely documented in V4L2 documentation as the error code for
the most of the V4L2 ioctls. I don't want to change it. A possible
-EINVAL returned from dma_map_sgtable() was just one of the 'generic'
error codes, not very descriptive in that case. Probably the main
problem here is that dma_map_sg() and friend doesn't return any error
codes...
> ...
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
___
iommu mailing list
iommu@lists.linux-foundation.org
Re: [PATCH v10 30/30] videobuf2: use sgtable-based scatterlist wrappers
Hi Marek,
On Fri, Sep 4, 2020 at 3:35 PM Marek Szyprowski
wrote:
>
> Use recently introduced common wrappers operating directly on the struct
> sg_table objects and scatterlist page iterators to make the code a bit
> more compact, robust, easier to follow and copy/paste safe.
>
> No functional change, because the code already properly did all the
> scatterlist related calls.
>
> Signed-off-by: Marek Szyprowski
> Reviewed-by: Robin Murphy
> ---
> .../common/videobuf2/videobuf2-dma-contig.c | 34 ---
> .../media/common/videobuf2/videobuf2-dma-sg.c | 32 +++--
> .../common/videobuf2/videobuf2-vmalloc.c | 12 +++
> 3 files changed, 31 insertions(+), 47 deletions(-)
>
Thanks for the patch! Please see my comments inline.
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> index ec3446cc45b8..1b242d844dde 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> @@ -58,10 +58,10 @@ static unsigned long vb2_dc_get_contiguous_size(struct
> sg_table *sgt)
> unsigned int i;
> unsigned long size = 0;
>
> - for_each_sg(sgt->sgl, s, sgt->nents, i) {
> + for_each_sgtable_dma_sg(sgt, s, i) {
> if (sg_dma_address(s) != expected)
> break;
> - expected = sg_dma_address(s) + sg_dma_len(s);
> + expected += sg_dma_len(s);
> size += sg_dma_len(s);
> }
> return size;
> @@ -103,8 +103,7 @@ static void vb2_dc_prepare(void *buf_priv)
> if (!sgt)
> return;
>
> - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
> - buf->dma_dir);
> + dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir);
> }
>
> static void vb2_dc_finish(void *buf_priv)
> @@ -115,7 +114,7 @@ static void vb2_dc_finish(void *buf_priv)
> if (!sgt)
> return;
>
> - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents,
> buf->dma_dir);
> + dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir);
> }
>
> /*/
> @@ -275,8 +274,8 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf,
> * memory locations do not require any explicit cache
> * maintenance prior or after being used by the device.
> */
> - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents,
> - attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
> + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
> + DMA_ATTR_SKIP_CPU_SYNC);
> sg_free_table(sgt);
> kfree(attach);
> db_attach->priv = NULL;
> @@ -301,8 +300,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
>
> /* release any previous cache */
> if (attach->dma_dir != DMA_NONE) {
> - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents,
> - attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
> + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir,
> + DMA_ATTR_SKIP_CPU_SYNC);
> attach->dma_dir = DMA_NONE;
> }
>
> @@ -310,9 +309,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
> * mapping to the client with new direction, no cache sync
> * required see comment in vb2_dc_dmabuf_ops_detach()
> */
> - sgt->nents = dma_map_sg_attrs(db_attach->dev, sgt->sgl,
> sgt->orig_nents,
> - dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
> - if (!sgt->nents) {
> + if (dma_map_sgtable(db_attach->dev, sgt, dma_dir,
> + DMA_ATTR_SKIP_CPU_SYNC)) {
> pr_err("failed to map scatterlist\n");
> mutex_unlock(lock);
> return ERR_PTR(-EIO);
As opposed to dma_map_sg_attrs(), dma_map_sgtable() now returns an
error code on its own. Is it expected to ignore it and return -EIO?
> @@ -455,8 +453,8 @@ static void vb2_dc_put_userptr(void *buf_priv)
> * No need to sync to CPU, it's already synced to the CPU
> * since the finish() memop will have been called before this.
> */
> - dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
> - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
> + dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir,
> + DMA_ATTR_SKIP_CPU_SYNC);
> pages = frame_vector_pages(buf->vec);
> /* sgt should exist only if vector contains pages... */
> BUG_ON(IS_ERR(pages));
> @@ -553,9 +551,8 @@ static void *vb2_dc_get_userptr(struct device *dev,
> unsigned long vaddr,
>
Re: [PATCH 0/2] iommu/amd: Fix IOMMUv2 devices when SME is active
Am 07.09.20 um 12:44 schrieb Joerg Roedel:
On Sun, Sep 06, 2020 at 04:08:58PM +, Deucher, Alexander wrote:
From f479b9da353c2547c26ebac8930a5dcd9a134eb7 Mon Sep 17 00:00:00 2001
From: Alex Deucher
Date: Sun, 6 Sep 2020 12:05:12 -0400
Subject: [PATCH] drm/amdgpu: Fail to load on RAVEN if SME is active
Due to hardware bugs, scatter/gather display on raven requires
a 1:1 IOMMU mapping, however, SME (System Memory Encryption)
requires an indirect IOMMU mapping because the encryption bit
is beyond the DMA mask of the chip. As such, the two are
incompatible.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 12e16445df7c..d87d37c25329 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1102,6 +1102,16 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
return -ENODEV;
}
+ /* Due to hardware bugs, S/G Display on raven requires a 1:1 IOMMU mapping,
+* however, SME requires an indirect IOMMU mapping because the
encryption
+* bit is beyond the DMA mask of the chip.
+*/
+ if (mem_encrypt_active() && ((flags & AMD_ASIC_MASK) == CHIP_RAVEN)) {
+ dev_info(>dev,
+"SME is not compatible with RAVEN\n");
+ return -ENOTSUPP;
+ }
+
#ifdef CONFIG_DRM_AMDGPU_SI
if (!amdgpu_si_support) {
switch (flags & AMD_ASIC_MASK) {
--
2.25.4
Looks good to me, thanks.
Acked-by: Joerg Roedel
This is really unfortunate, but I don't see any other solution either.
Reviewed-by: Christian König
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 9/9] x86/mmu: Allocate/free PASID
On Thu, Aug 27, 2020 at 08:06:34AM -0700, Fenghua Yu wrote:
> A PASID is allocated for an "mm" the first time any thread binds
> to an SVM capable device and is freed from the "mm" when the SVM is
> unbound by the last thread. It's possible for the "mm" to have different
> PASID values in different binding/unbinding SVM cycles.
>
> The mm's PASID (non-zero for valid PASID or 0 for invalid PASID) is
> propagated to per-thread PASID MSR for all threads within the mm through
> through IPI, context switch, or inherit to ensure a running thread has
> the right PASID MSR matching the mm's PASID.
That sentence has grown too large and confused. Pls fix.
> Suggested-by: Andy Lutomirski
> Signed-off-by: Fenghua Yu
> Reviewed-by: Tony Luck
> ---
> v7:
> - Don't fix up PASID in #GP. Instead, update the PASID MSR by IPI and
> context switch after PASID allocation and free. Inherit PASID from
> parent. (Andy)
>
> Before v7:
> - Allocate a PASID for the mm and free it until mm exit.
>
> arch/x86/include/asm/disabled-features.h | 2 +-
> arch/x86/include/asm/fpu/api.h | 12 +
> arch/x86/include/asm/fpu/internal.h | 2 +
> arch/x86/kernel/fpu/xstate.c | 56
> drivers/iommu/intel/svm.c| 28 +++-
> 5 files changed, 98 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/disabled-features.h
> b/arch/x86/include/asm/disabled-features.h
> index 588d83e9da49..5861d34f9771 100644
> --- a/arch/x86/include/asm/disabled-features.h
> +++ b/arch/x86/include/asm/disabled-features.h
> @@ -56,7 +56,7 @@
> # define DISABLE_PTI (1 << (X86_FEATURE_PTI & 31))
> #endif
>
> -#ifdef CONFIG_INTEL_IOMMU_SVM
> +#ifdef CONFIG_IOMMU_SUPPORT
> # define DISABLE_ENQCMD 0
> #else
> # define DISABLE_ENQCMD (1 << (X86_FEATURE_ENQCMD & 31))
That hunk belongs with the previous patch.
> diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
> index b774c52e5411..dcd9503b1098 100644
> --- a/arch/x86/include/asm/fpu/api.h
> +++ b/arch/x86/include/asm/fpu/api.h
> @@ -62,4 +62,16 @@ extern void switch_fpu_return(void);
> */
> extern int cpu_has_xfeatures(u64 xfeatures_mask, const char **feature_name);
>
> +/*
> + * Tasks that are not using SVA have mm->pasid set to zero to note that they
> + * will not have the valid bit set in MSR_IA32_PASID while they are running.
> + */
> +#define PASID_DISABLED 0
> +
> +#ifdef CONFIG_IOMMU_SUPPORT
> +/* Update current's PASID MSR/state by mm's PASID. */
> +void update_pasid(void);
> +#else
> +static inline void update_pasid(void) { }
> +#endif
> #endif /* _ASM_X86_FPU_API_H */
> diff --git a/arch/x86/include/asm/fpu/internal.h
> b/arch/x86/include/asm/fpu/internal.h
> index 0a460f2a3f90..2d737e02b59a 100644
> --- a/arch/x86/include/asm/fpu/internal.h
> +++ b/arch/x86/include/asm/fpu/internal.h
> @@ -583,6 +583,8 @@ static inline void switch_fpu_finish(struct fpu *new_fpu)
> pkru_val = pk->pkru;
> }
> __write_pkru(pkru_val);
> +
> + update_pasid();
> }
>
> /*
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 67f1a03b9b23..556040e14f1c 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -1402,3 +1402,59 @@ int proc_pid_arch_status(struct seq_file *m, struct
> pid_namespace *ns,
> return 0;
> }
> #endif /* CONFIG_PROC_PID_ARCH_STATUS */
> +
> +#ifdef CONFIG_IOMMU_SUPPORT
> +void update_pasid(void)
> +{
> + u64 pasid_state;
> + u32 pasid;
> +
> + if (!cpu_feature_enabled(X86_FEATURE_ENQCMD))
> + return;
> +
> + if (!current->mm)
> + return;
> +
> + pasid = READ_ONCE(current->mm->pasid);
> + /* Set the valid bit in the PASID MSR/state only for valid pasid. */
> + pasid_state = pasid == PASID_DISABLED ?
> + pasid : pasid | MSR_IA32_PASID_VALID;
> +
> + /*
> + * No need to hold fregs_lock() since the task's fpstate won't
> + * be changed by others (e.g. ptrace) while the task is being
> + * switched to or is in IPI.
> + */
> + if (!test_thread_flag(TIF_NEED_FPU_LOAD)) {
> + /* The MSR is active and can be directly updated. */
> + wrmsrl(MSR_IA32_PASID, pasid_state);
> + } else {
> + struct fpu *fpu = >thread.fpu;
> + struct ia32_pasid_state *ppasid_state;
> + struct xregs_state *xsave;
> +
> + /*
> + * The CPU's xstate registers are not currently active. Just
> + * update the PASID state in the memory buffer here. The
> + * PASID MSR will be loaded when returning to user mode.
> + */
> + xsave = >state.xsave;
> + xsave->header.xfeatures |= XFEATURE_MASK_PASID;
> + ppasid_state = get_xsave_addr(xsave, XFEATURE_PASID);
> + if (ppasid_state) {
> + /*
> +
Re: [PATCH 0/2] iommu/amd: Fix IOMMUv2 devices when SME is active
On Sun, Sep 06, 2020 at 04:08:58PM +, Deucher, Alexander wrote:
> From f479b9da353c2547c26ebac8930a5dcd9a134eb7 Mon Sep 17 00:00:00 2001
> From: Alex Deucher
> Date: Sun, 6 Sep 2020 12:05:12 -0400
> Subject: [PATCH] drm/amdgpu: Fail to load on RAVEN if SME is active
>
> Due to hardware bugs, scatter/gather display on raven requires
> a 1:1 IOMMU mapping, however, SME (System Memory Encryption)
> requires an indirect IOMMU mapping because the encryption bit
> is beyond the DMA mask of the chip. As such, the two are
> incompatible.
>
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 12e16445df7c..d87d37c25329 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1102,6 +1102,16 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> return -ENODEV;
> }
>
> + /* Due to hardware bugs, S/G Display on raven requires a 1:1 IOMMU
> mapping,
> + * however, SME requires an indirect IOMMU mapping because the
> encryption
> + * bit is beyond the DMA mask of the chip.
> + */
> + if (mem_encrypt_active() && ((flags & AMD_ASIC_MASK) == CHIP_RAVEN)) {
> + dev_info(>dev,
> + "SME is not compatible with RAVEN\n");
> + return -ENOTSUPP;
> + }
> +
> #ifdef CONFIG_DRM_AMDGPU_SI
> if (!amdgpu_si_support) {
> switch (flags & AMD_ASIC_MASK) {
> --
> 2.25.4
>
Looks good to me, thanks.
Acked-by: Joerg Roedel
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 8/9] x86/cpufeatures: Mark ENQCMD as disabled when configured out
On Thu, Aug 27, 2020 at 08:06:33AM -0700, Fenghua Yu wrote: > Currently the ENQCMD feature cannot be used if CONFIG_INTEL_IOMMU_SVM > is not set. IOW, "Currently, the ENQCMD feature depends on CONFIG_INTEL_IOMMU_SVM." ? No need for a "cannot ... if not" formulation. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 1/3] dt-bindings: iommu: Add binding for MediaTek MT8167 IOMMU
This commit adds IOMMU binding documentation and larb port definitions for the MT8167 SoC. Signed-off-by: Fabien Parent Acked-by: Rob Herring --- V4: * Added path to mt8167 larb header file * Added Honghui Zhang in copyright header V3: Added mt8167-larb-port.h file for iommu port definitions V2: no change --- .../bindings/iommu/mediatek,iommu.txt | 2 + include/dt-bindings/memory/mt8167-larb-port.h | 51 +++ 2 files changed, 53 insertions(+) create mode 100644 include/dt-bindings/memory/mt8167-larb-port.h diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt index c1ccd8582eb2..ac949f7fe3d4 100644 --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt @@ -61,6 +61,7 @@ Required properties: "mediatek,mt6779-m4u" for mt6779 which uses generation two m4u HW. "mediatek,mt7623-m4u", "mediatek,mt2701-m4u" for mt7623 which uses generation one m4u HW. + "mediatek,mt8167-m4u" for mt8167 which uses generation two m4u HW. "mediatek,mt8173-m4u" for mt8173 which uses generation two m4u HW. "mediatek,mt8183-m4u" for mt8183 which uses generation two m4u HW. - reg : m4u register base and size. @@ -80,6 +81,7 @@ Required properties: dt-binding/memory/mt2701-larb-port.h for mt2701, mt7623 dt-binding/memory/mt2712-larb-port.h for mt2712, dt-binding/memory/mt6779-larb-port.h for mt6779, + dt-binding/memory/mt8167-larb-port.h for mt8167, dt-binding/memory/mt8173-larb-port.h for mt8173, and dt-binding/memory/mt8183-larb-port.h for mt8183. diff --git a/include/dt-bindings/memory/mt8167-larb-port.h b/include/dt-bindings/memory/mt8167-larb-port.h new file mode 100644 index ..000fb299a408 --- /dev/null +++ b/include/dt-bindings/memory/mt8167-larb-port.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2020 MediaTek Inc. + * Copyright (c) 2020 BayLibre, SAS + * Author: Honghui Zhang + * Author: Fabien Parent + */ +#ifndef __DTS_IOMMU_PORT_MT8167_H +#define __DTS_IOMMU_PORT_MT8167_H + +#define MTK_M4U_ID(larb, port) (((larb) << 5) | (port)) + +#define M4U_LARB0_ID 0 +#define M4U_LARB1_ID 1 +#define M4U_LARB2_ID 2 + +/* larb0 */ +#define M4U_PORT_DISP_OVL0 MTK_M4U_ID(M4U_LARB0_ID, 0) +#define M4U_PORT_DISP_RDMA0MTK_M4U_ID(M4U_LARB0_ID, 1) +#define M4U_PORT_DISP_WDMA0MTK_M4U_ID(M4U_LARB0_ID, 2) +#define M4U_PORT_DISP_RDMA1MTK_M4U_ID(M4U_LARB0_ID, 3) +#define M4U_PORT_MDP_RDMA MTK_M4U_ID(M4U_LARB0_ID, 4) +#define M4U_PORT_MDP_WDMA MTK_M4U_ID(M4U_LARB0_ID, 5) +#define M4U_PORT_MDP_WROT MTK_M4U_ID(M4U_LARB0_ID, 6) +#define M4U_PORT_DISP_FAKE MTK_M4U_ID(M4U_LARB0_ID, 7) + +/* larb1*/ +#define M4U_PORT_CAM_IMGO MTK_M4U_ID(M4U_LARB1_ID, 0) +#define M4U_PORT_CAM_IMG2O MTK_M4U_ID(M4U_LARB1_ID, 1) +#define M4U_PORT_CAM_LSCI MTK_M4U_ID(M4U_LARB1_ID, 2) +#define M4U_PORT_CAM_ESFKO MTK_M4U_ID(M4U_LARB1_ID, 3) +#define M4U_PORT_CAM_AAO MTK_M4U_ID(M4U_LARB1_ID, 4) +#define M4U_PORT_VENC_REC MTK_M4U_ID(M4U_LARB1_ID, 5) +#define M4U_PORT_VENC_BSDMAMTK_M4U_ID(M4U_LARB1_ID, 6) +#define M4U_PORT_VENC_RD_COMV MTK_M4U_ID(M4U_LARB1_ID, 7) +#define M4U_PORT_CAM_IMGI MTK_M4U_ID(M4U_LARB1_ID, 8) +#define M4U_PORT_VENC_CUR_LUMA MTK_M4U_ID(M4U_LARB1_ID, 9) +#define M4U_PORT_VENC_CUR_CHROMA MTK_M4U_ID(M4U_LARB1_ID, 10) +#define M4U_PORT_VENC_REF_LUMA MTK_M4U_ID(M4U_LARB1_ID, 11) +#define M4U_PORT_VENC_REF_CHROMA MTK_M4U_ID(M4U_LARB1_ID, 12) + +/* larb2*/ +#define M4U_PORT_HW_VDEC_MC_EXTMTK_M4U_ID(M4U_LARB2_ID, 0) +#define M4U_PORT_HW_VDEC_PP_EXTMTK_M4U_ID(M4U_LARB2_ID, 1) +#define M4U_PORT_HW_VDEC_VLD_EXT MTK_M4U_ID(M4U_LARB2_ID, 2) +#define M4U_PORT_HW_VDEC_AVC_MV_EXTMTK_M4U_ID(M4U_LARB2_ID, 3) +#define M4U_PORT_HW_VDEC_PRED_RD_EXT MTK_M4U_ID(M4U_LARB2_ID, 4) +#define M4U_PORT_HW_VDEC_PRED_WR_EXT MTK_M4U_ID(M4U_LARB2_ID, 5) +#define M4U_PORT_HW_VDEC_PPWRAP_EXTMTK_M4U_ID(M4U_LARB2_ID, 6) + +#endif -- 2.28.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 2/3] iommu/mediatek: add flag for legacy ivrp paddr
Add a new flag in order to select which IVRP_PADDR format is used
by an SoC.
Signed-off-by: Fabien Parent
Reviewed-by: Yong Wu
---
v4: no change
v3: set LEGACY_IVRP_PADDR as a flag instead of platform data
v2: new patch
---
drivers/iommu/mtk_iommu.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 785b228d39a6..b1f85a7e9346 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -116,6 +116,7 @@
#define OUT_ORDER_WR_ENBIT(4)
#define HAS_SUB_COMM BIT(5)
#define WR_THROT_ENBIT(6)
+#define HAS_LEGACY_IVRP_PADDR BIT(7)
#define MTK_IOMMU_HAS_FLAG(pdata, _x) \
pdata)->flags) & (_x)) == (_x))
@@ -582,7 +583,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data
*data)
F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL);
- if (data->plat_data->m4u_plat == M4U_MT8173)
+ if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_LEGACY_IVRP_PADDR))
regval = (data->protect_base >> 1) | (data->enable_4GB << 31);
else
regval = lower_32_bits(data->protect_base) |
@@ -818,7 +819,8 @@ static const struct mtk_iommu_plat_data mt6779_data = {
static const struct mtk_iommu_plat_data mt8173_data = {
.m4u_plat = M4U_MT8173,
- .flags= HAS_4GB_MODE | HAS_BCLK | RESET_AXI,
+ .flags= HAS_4GB_MODE | HAS_BCLK | RESET_AXI |
+ HAS_LEGACY_IVRP_PADDR,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
.larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear mapping. */
};
--
2.28.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 3/3] iommu/mediatek: add support for MT8167
Add support for the IOMMU on MT8167
Signed-off-by: Fabien Parent
---
V4;
* Removed HAS_4GB_MODE flag since this SoC does not seem to support it
V3:
* use LEGACY_IVRP_PADDR flag instead of using a platform data member
V2:
* removed if based on m4u_plat, and using instead the new
has_legacy_ivrp_paddr member that was introduced in patch 2.
---
drivers/iommu/mtk_iommu.c | 8
drivers/iommu/mtk_iommu.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index b1f85a7e9346..4ff071eb5279 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -817,6 +817,13 @@ static const struct mtk_iommu_plat_data mt6779_data = {
.larbid_remap = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}},
};
+static const struct mtk_iommu_plat_data mt8167_data = {
+ .m4u_plat = M4U_MT8167,
+ .flags= RESET_AXI | HAS_LEGACY_IVRP_PADDR,
+ .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
+ .larbid_remap = {{0}, {1}, {2}}, /* Linear mapping. */
+};
+
static const struct mtk_iommu_plat_data mt8173_data = {
.m4u_plat = M4U_MT8173,
.flags= HAS_4GB_MODE | HAS_BCLK | RESET_AXI |
@@ -835,6 +842,7 @@ static const struct mtk_iommu_plat_data mt8183_data = {
static const struct of_device_id mtk_iommu_of_ids[] = {
{ .compatible = "mediatek,mt2712-m4u", .data = _data},
{ .compatible = "mediatek,mt6779-m4u", .data = _data},
+ { .compatible = "mediatek,mt8167-m4u", .data = _data},
{ .compatible = "mediatek,mt8173-m4u", .data = _data},
{ .compatible = "mediatek,mt8183-m4u", .data = _data},
{}
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index 122925dbe547..df32b3e3408b 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -39,6 +39,7 @@ enum mtk_iommu_plat {
M4U_MT2701,
M4U_MT2712,
M4U_MT6779,
+ M4U_MT8167,
M4U_MT8173,
M4U_MT8183,
};
--
2.28.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 3/3] iommu/mediatek: add support for MT8167
> > +static const struct mtk_iommu_plat_data mt8167_data = {
> > + .m4u_plat = M4U_MT8167,
> > + .flags= HAS_4GB_MODE | RESET_AXI | HAS_LEGACY_IVRP_PADDR,
>
> The 4GB mode flow was improved at[1] which has just been applied.
>
> If you add 4gb_mode flag but don't have "mt8167-infracfg", the probe may
> be failed.
Looking back at the datasheet I don't think HAS_4GB_MODE should have
been enabled for MT8167 anyway. I just removed it and retested the
patch. I will fix it in v4. Thanks
> [1]
> https://lore.kernel.org/linux-iommu/20200904112117.gc16...@8bytes.org/T/#m613e9926735d07ad004fddbbcedaa50b5afacca1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips
On Thu, Aug 27, 2020 at 09:29:59AM -0400, Jim Quinlan wrote: > On Thu, Aug 27, 2020 at 2:35 AM Christoph Hellwig wrote: > > > > On Tue, Aug 25, 2020 at 10:40:27AM -0700, Florian Fainelli wrote: > > > Hi, > > > > > > On 8/24/2020 12:30 PM, Jim Quinlan wrote: > > >> > > >> Patchset Summary: > > >>Enhance a PCIe host controller driver. Because of its unusual design > > >>we are foced to change dev->dma_pfn_offset into a more general role > > >>allowing multiple offsets. See the 'v1' notes below for more info. > > > > > > We are version 11 and counting, and it is not clear to me whether there is > > > any chance of getting these patches reviewed and hopefully merged for the > > > 5.10 merge window. > > > > > > There are a lot of different files being touched, so what would be the > > > ideal way of routing those changes towards inclusion? > > > > FYI, I offered to take the dma-mapping bits through the dma-mapping tree. > > I have a bit of a backlog, but plan to review and if Jim is ok with that > > apply the current version. > Sounds good to me. Hi Jim, is the dependency now solved ? Should we review/take this series as is for v5.10 through the PCI tree ? Thanks, Lorenzo ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/9] iommu/ioasid: Introduce ioasid_set APIs
Hi Jacob,
On 9/3/20 11:07 PM, Jacob Pan wrote:
> On Tue, 1 Sep 2020 13:51:26 +0200
> Auger Eric wrote:
>
>> Hi Jacob,
>>
>> On 8/22/20 6:35 AM, Jacob Pan wrote:
>>> ioasid_set was introduced as an arbitrary token that are shared by
>>> a
>> that is
> got it
>
>>> group of IOASIDs. For example, if IOASID #1 and #2 are allocated
>>> via the same ioasid_set*, they are viewed as to belong to the same
>>> set.
>> two IOASIDs allocated via the same ioasid_set pointer belong to the
>> same set?
>>>
> yes, better.
>
>>> For guest SVA usages, system-wide IOASID resources need to be
>>> partitioned such that VMs can have its own quota and being managed
>> their own
> right,
>
>>> separately. ioasid_set is the perfect candidate for meeting such
>>> requirements. This patch redefines and extends ioasid_set with the
>>> following new fields:
>>> - Quota
>>> - Reference count
>>> - Storage of its namespace
>>> - The token is stored in the new ioasid_set but with optional types
>>>
>>> ioasid_set level APIs are introduced that wires up these new data.
>> that wire
> right
>
>>> Existing users of IOASID APIs are converted where a host IOASID set
>>> is allocated for bare-metal usage.
>>>
>>> Signed-off-by: Liu Yi L
>>> Signed-off-by: Jacob Pan
>>> ---
>>> drivers/iommu/intel/iommu.c | 27 ++-
>>> drivers/iommu/intel/pasid.h | 1 +
>>> drivers/iommu/intel/svm.c | 8 +-
>>> drivers/iommu/ioasid.c | 390
>>> +---
>>> include/linux/ioasid.h | 82 -- 5 files changed, 465
>>> insertions(+), 43 deletions(-)
>>>
>>> diff --git a/drivers/iommu/intel/iommu.c
>>> b/drivers/iommu/intel/iommu.c index a3a0b5c8921d..5813eeaa5edb
>>> 100644 --- a/drivers/iommu/intel/iommu.c
>>> +++ b/drivers/iommu/intel/iommu.c
>>> @@ -42,6 +42,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>> #include
>>> #include
>>> @@ -103,6 +104,9 @@
>>> */
>>> #define INTEL_IOMMU_PGSIZES(~0xFFFUL)
>>>
>>> +/* PASIDs used by host SVM */
>>> +struct ioasid_set *host_pasid_set;
>>> +
>>> static inline int agaw_to_level(int agaw)
>>> {
>>> return agaw + 2;
>>> @@ -3103,8 +3107,8 @@ static void intel_vcmd_ioasid_free(ioasid_t
>>> ioasid, void *data)
>>> * Sanity check the ioasid owner is done at upper layer,
>>> e.g. VFIO
>>> * We can only free the PASID when all the devices are
>>> unbound. */
>>> - if (ioasid_find(NULL, ioasid, NULL)) {
>>> - pr_alert("Cannot free active IOASID %d\n", ioasid);
>>> + if (IS_ERR(ioasid_find(host_pasid_set, ioasid, NULL))) {
>>> + pr_err("Cannot free IOASID %d, not in system
>>> set\n", ioasid);
>> not sure the change in the trace is worth. Also you may be more
>> explicit like IOASID %d to be freed cannot be found in the system
>> ioasid set.
> Yes, better. will do.
>
>> shouldn't it be rate_limited as it is originated from
>> user space?
> virtual command is only used in the guest kernel, not from userspace
> though. But I should add ratelimited to all user originated calls.
Sure I mixed things up. Sorry for the noise
Eric
>
>>> return;
>>> }
>>> vcmd_free_pasid(iommu, ioasid);
>>> @@ -3288,6 +3292,19 @@ static int __init init_dmars(void)
>>> if (ret)
>>> goto free_iommu;
>>>
>>> + /* PASID is needed for scalable mode irrespective to SVM */
>>> + if (intel_iommu_sm) {
>>> + ioasid_install_capacity(intel_pasid_max_id);
>>> + /* We should not run out of IOASIDs at boot */
>>> + host_pasid_set = ioasid_alloc_set(NULL,
>>> PID_MAX_DEFAULT,
>> s/PID_MAX_DEFAULT/intel_pasid_max_id?
> Not really, when both baremetal and guest SVA are used on the same
> system, we want to to limit the baremetal SVM PASID to the number of
> host processes. host_pasid_set is for baremetal only.
>
> intel_pasid_max_id would take up the entire PASID resource and leave no
> PASIDs for guest usages.
>
>>> +
>>> IOASID_SET_TYPE_NULL);
>> as suggested by jean-Philippe ioasid_set_alloc
>>> + if (IS_ERR_OR_NULL(host_pasid_set)) {
>>> + pr_err("Failed to enable host PASID
>>> allocator %lu\n",
>>> + PTR_ERR(host_pasid_set));
>> does not sound like the correct error message? failed to allocate the
>> system ioasid_set?
> right
>
>>> + intel_iommu_sm = 0;
>>> + }
>>> + }
>>> +
>>> /*
>>> * for each drhd
>>> * enable fault log
>>> @@ -5149,7 +5166,7 @@ static void auxiliary_unlink_device(struct
>>> dmar_domain *domain, domain->auxd_refcnt--;
>>>
>>> if (!domain->auxd_refcnt && domain->default_pasid > 0)
>>> - ioasid_free(domain->default_pasid);
>>> + ioasid_free(host_pasid_set, domain->default_pasid);
>>> }
>>>
>>> static int aux_domain_add_dev(struct dmar_domain *domain,
>>> @@ -5167,7 +5184,7 @@ static int aux_domain_add_dev(struct
>>> dmar_domain *domain, int pasid;
>>>
>>>
Re: [PATCH v2 1/9] docs: Document IO Address Space ID (IOASID) APIs
Hi Jacob, On 9/1/20 6:56 PM, Jacob Pan wrote: > Hi Eric, > > On Thu, 27 Aug 2020 18:21:07 +0200 > Auger Eric wrote: > >> Hi Jacob, >> On 8/24/20 12:32 PM, Jean-Philippe Brucker wrote: >>> On Fri, Aug 21, 2020 at 09:35:10PM -0700, Jacob Pan wrote: IOASID is used to identify address spaces that can be targeted by device DMA. It is a system-wide resource that is essential to its many users. This document is an attempt to help developers from all vendors navigate the APIs. At this time, ARM SMMU and Intel’s Scalable IO Virtualization (SIOV) enabled platforms are the primary users of IOASID. Examples of how SIOV components interact with IOASID APIs are provided in that many APIs are driven by the requirements from SIOV. Signed-off-by: Liu Yi L Signed-off-by: Wu Hao Signed-off-by: Jacob Pan --- Documentation/ioasid.rst | 618 +++ 1 file changed, 618 insertions(+) create mode 100644 Documentation/ioasid.rst diff --git a/Documentation/ioasid.rst b/Documentation/ioasid.rst >>> >>> Thanks for writing this up. Should it go to >>> Documentation/driver-api/, or Documentation/driver-api/iommu/? I >>> think this also needs to Cc linux-...@vger.kernel.org and >>> cor...@lwn.net new file mode 100644 index ..b6a8cdc885ff --- /dev/null +++ b/Documentation/ioasid.rst @@ -0,0 +1,618 @@ +.. ioasid: + += +IO Address Space ID += + +IOASID is a generic name for PCIe Process Address ID (PASID) or ARM +SMMU sub-stream ID. An IOASID identifies an address space that DMA >>> >>> "SubstreamID" >> On ARM if we don't use PASIDs we have streamids (SID) which can also >> identify address spaces that DMA requests can target. So maybe this >> definition is not sufficient. >> > According to SMMU spec, the SubstreamID is equivalent to PASID. My > understanding is that SID is equivalent to PCI requester ID that > identifies stage 2. Do you plan to use IOASID for stage 2? No. So actually if PASID is not used we still have a default single IOASID matching the single context. So that may be fine as a definition. > IOASID is mostly for SVA and DMA request w/ PASID. > >>> +requests can target. + +The primary use cases for IOASID are Shared Virtual Address (SVA) and +IO Virtual Address (IOVA). However, the requirements for IOASID >>> >>> IOVA alone isn't a use case, maybe "multiple IOVA spaces per >>> device"? +management can vary among hardware architectures. + +This document covers the generic features supported by IOASID +APIs. Vendor-specific use cases are also illustrated with Intel's VT-d +based platforms as the first example. + +.. contents:: :local: + +Glossary + +PASID - Process Address Space ID + +IOASID - IO Address Space ID (generic term for PCIe PASID and +sub-stream ID in SMMU) >>> >>> "SubstreamID" >>> + +SVA/SVM - Shared Virtual Addressing/Memory + +ENQCMD - New Intel X86 ISA for efficient workqueue submission [1] >>> >>> Maybe drop the "New", to keep the documentation perennial. It might >>> be good to add internal links here to the specifications URLs at >>> the bottom. + +DSA - Intel Data Streaming Accelerator [2] + +VDCM - Virtual device composition module [3] + +SIOV - Intel Scalable IO Virtualization + + +Key Concepts + + +IOASID Set +--- +An IOASID set is a group of IOASIDs allocated from the system-wide +IOASID pool. An IOASID set is created and can be identified by a +token of u64. Refer to IOASID set APIs for more details. >>> >>> Identified either by an u64 or an mm_struct, right? Maybe just >>> drop the second sentence if it's detailed in the IOASID set section >>> below. + +IOASID set is particularly useful for guest SVA where each guest could +have its own IOASID set for security and efficiency reasons. + +IOASID Set Private ID (SPID) + +SPIDs are introduced as IOASIDs within its set. Each SPID maps to a +system-wide IOASID but the namespace of SPID is within its IOASID +set. >>> >>> The intro isn't super clear. Perhaps this is simpler: >>> "Each IOASID set has a private namespace of SPIDs. An SPID maps to a >>> single system-wide IOASID." >> or, "within an ioasid set, each ioasid can be associated with an alias >> ID, named SPID." > I don't have strong opinion, I feel it is good to explain the > relationship between SPID and IOASID in both directions, how about add? > " Conversely, each IOASID is associated with an alias ID, named SPID." yep. I amy suggest: each IOASID may be associated with an alias ID,
Re: [PATCH V2 5/5] DO NOT MERGE: iommu: disable list appending in dma-iommu
On Thu, Sep 03, 2020 at 09:18:37PM +0100, Tom Murphy wrote: > Disable combining sg segments in the dma-iommu api. > Combining the sg segments exposes a bug in the intel i915 driver which > causes visual artifacts and the screen to freeze. This is most likely > because of how the i915 handles the returned list. It probably doesn't > respect the returned value specifying the number of elements in the list > and instead depends on the previous behaviour of the intel iommu driver > which would return the same number of elements in the output list as in > the input list. So what is the state of addressing this properly in i915? IF we can't get it done ASAP I wonder if we need a runtime quirk to disable merging instead of blocking this conversion.. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
