RE: (proposal) RE: [PATCH v7 00/16] vfio: expose virtual Shared Virtual Addressing to VMs

2020-10-13 Thread Tian, Kevin
Hi, Alex and Jason (G),

How about your opinion for this new proposal? For now looks both
Jason (W) and Jean are OK with this direction and more discussions
are possibly required for the new /dev/ioasid interface. Internally 
we're doing a quick prototype to see any unforeseen issue with this
separation. 

Please let us know your thoughts.

Thanks
Kevin

> From: Tian, Kevin 
> Sent: Monday, October 12, 2020 4:39 PM
> 
> > From: Jason Wang 
> > Sent: Monday, September 14, 2020 12:20 PM
> >
> [...]
>  > If it's possible, I would suggest a generic uAPI instead of a VFIO
> > specific one.
> >
> > Jason suggest something like /dev/sva. There will be a lot of other
> > subsystems that could benefit from this (e.g vDPA).
> >
> > Have you ever considered this approach?
> >
> 
> Hi, Jason,
> 
> We did some study on this approach and below is the output. It's a
> long writing but I didn't find a way to further abstract w/o losing
> necessary context. Sorry about that.
> 
> Overall the real purpose of this series is to enable IOMMU nested
> translation capability with vSVA as one major usage, through
> below new uAPIs:
>   1) Report/enable IOMMU nested translation capability;
>   2) Allocate/free PASID;
>   3) Bind/unbind guest page table;
>   4) Invalidate IOMMU cache;
>   5) Handle IOMMU page request/response (not in this series);
> 1/3/4) is the minimal set for using IOMMU nested translation, with
> the other two optional. For example, the guest may enable vSVA on
> a device without using PASID. Or, it may bind its gIOVA page table
> which doesn't require page fault support. Finally, all operations can
> be applied to either physical device or subdevice.
> 
> Then we evaluated each uAPI whether generalizing it is a good thing
> both in concept and regarding to complexity.
> 
> First, unlike other uAPIs which are all backed by iommu_ops, PASID
> allocation/free is through the IOASID sub-system. From this angle
> we feel generalizing PASID management does make some sense.
> First, PASID is just a number and not related to any device before
> it's bound to a page table and IOMMU domain. Second, PASID is a
> global resource (at least on Intel VT-d), while having separate VFIO/
> VDPA allocation interfaces may easily cause confusion in userspace,
> e.g. which interface to be used if both VFIO/VDPA devices exist.
> Moreover, an unified interface allows centralized control over how
> many PASIDs are allowed per process.
> 
> One unclear part with this generalization is about the permission.
> Do we open this interface to any process or only to those which
> have assigned devices? If the latter, what would be the mechanism
> to coordinate between this new interface and specific passthrough
> frameworks? A more tricky case, vSVA support on ARM (Eric/Jean
> please correct me) plans to do per-device PASID namespace which
> is built on a bind_pasid_table iommu callback to allow guest fully
> manage its PASIDs on a given passthrough device. I'm not sure
> how such requirement can be unified w/o involving passthrough
> frameworks, or whether ARM could also switch to global PASID
> style...
> 
> Second, IOMMU nested translation is a per IOMMU domain
> capability. Since IOMMU domains are managed by VFIO/VDPA
>  (alloc/free domain, attach/detach device, set/get domain attribute,
> etc.), reporting/enabling the nesting capability is an natural
> extension to the domain uAPI of existing passthrough frameworks.
> Actually, VFIO already includes a nesting enable interface even
> before this series. So it doesn't make sense to generalize this uAPI
> out.
> 
> Then the tricky part comes with the remaining operations (3/4/5),
> which are all backed by iommu_ops thus effective only within an
> IOMMU domain. To generalize them, the first thing is to find a way
> to associate the sva_FD (opened through generic /dev/sva) with an
> IOMMU domain that is created by VFIO/VDPA. The second thing is
> to replicate {domain<->device/subdevice} association in /dev/sva
> path because some operations (e.g. page fault) is triggered/handled
> per device/subdevice. Therefore, /dev/sva must provide both per-
> domain and per-device uAPIs similar to what VFIO/VDPA already
> does. Moreover, mapping page fault to subdevice requires pre-
> registering subdevice fault data to IOMMU layer when binding
> guest page table, while such fault data can be only retrieved from
> parent driver through VFIO/VDPA.
> 
> However, we failed to find a good way even at the 1st step about
> domain association. The iommu domains are not exposed to the
> userspace, and there is no 1:1 mapping between domain and device.
> In VFIO, all devices within the same VFIO container share the address
> space but they may be organized in multiple IOMMU domains based
> on their bus type. How (should we let) the userspace know the
> domain information and open an sva_FD for each domain is the main
> problem here.
> 
> In the end we just realized that doing such 

RE: (proposal) RE: [PATCH v7 00/16] vfio: expose virtual Shared Virtual Addressing to VMs

2020-10-13 Thread Tian, Kevin
> From: Jason Wang 
> Sent: Tuesday, October 13, 2020 2:22 PM
> 
> 
> On 2020/10/12 下午4:38, Tian, Kevin wrote:
> >> From: Jason Wang 
> >> Sent: Monday, September 14, 2020 12:20 PM
> >>
> > [...]
> >   > If it's possible, I would suggest a generic uAPI instead of a VFIO
> >> specific one.
> >>
> >> Jason suggest something like /dev/sva. There will be a lot of other
> >> subsystems that could benefit from this (e.g vDPA).
> >>
> >> Have you ever considered this approach?
> >>
> > Hi, Jason,
> >
> > We did some study on this approach and below is the output. It's a
> > long writing but I didn't find a way to further abstract w/o losing
> > necessary context. Sorry about that.
> >
> > Overall the real purpose of this series is to enable IOMMU nested
> > translation capability with vSVA as one major usage, through
> > below new uAPIs:
> > 1) Report/enable IOMMU nested translation capability;
> > 2) Allocate/free PASID;
> > 3) Bind/unbind guest page table;
> > 4) Invalidate IOMMU cache;
> > 5) Handle IOMMU page request/response (not in this series);
> > 1/3/4) is the minimal set for using IOMMU nested translation, with
> > the other two optional. For example, the guest may enable vSVA on
> > a device without using PASID. Or, it may bind its gIOVA page table
> > which doesn't require page fault support. Finally, all operations can
> > be applied to either physical device or subdevice.
> >
> > Then we evaluated each uAPI whether generalizing it is a good thing
> > both in concept and regarding to complexity.
> >
> > First, unlike other uAPIs which are all backed by iommu_ops, PASID
> > allocation/free is through the IOASID sub-system.
> 
> 
> A question here, is IOASID expected to be the single management
> interface for PASID?

yes

> 
> (I'm asking since there're already vendor specific IDA based PASID
> allocator e.g amdgpu_pasid_alloc())

That comes before IOASID core was introduced. I think it should be
changed to use the new generic interface. Jacob/Jean can better
comment if other reason exists for this exception.

> 
> 
> >   From this angle
> > we feel generalizing PASID management does make some sense.
> > First, PASID is just a number and not related to any device before
> > it's bound to a page table and IOMMU domain. Second, PASID is a
> > global resource (at least on Intel VT-d),
> 
> 
> I think we need a definition of "global" here. It looks to me for vt-d
> the PASID table is per device.

PASID table is per device, thus VT-d could support per-device PASIDs
in concept. However on Intel platform we require PASIDs to be managed 
in system-wide (cross host and guest) when combining vSVA, SIOV, SR-IOV 
and ENQCMD together. Thus the host creates only one 'global' PASID 
namespace but do use per-device PASID table to assure isolation between 
devices on Intel platforms. But ARM does it differently as Jean explained. 
They have a global namespace for host processes on all host-owned 
devices (same as Intel), but then per-device namespace when a device 
(and its PASID table) is assigned to userspace.

> 
> Another question, is this possible to have two DMAR hardware unit(at
> least I can see two even in my laptop). In this case, is PASID still a
> global resource?

yes

> 
> 
> >   while having separate VFIO/
> > VDPA allocation interfaces may easily cause confusion in userspace,
> > e.g. which interface to be used if both VFIO/VDPA devices exist.
> > Moreover, an unified interface allows centralized control over how
> > many PASIDs are allowed per process.
> 
> 
> Yes.
> 
> 
> >
> > One unclear part with this generalization is about the permission.
> > Do we open this interface to any process or only to those which
> > have assigned devices? If the latter, what would be the mechanism
> > to coordinate between this new interface and specific passthrough
> > frameworks?
> 
> 
> I'm not sure, but if you just want a permission, you probably can
> introduce new capability (CAP_XXX) for this.
> 
> 
> >   A more tricky case, vSVA support on ARM (Eric/Jean
> > please correct me) plans to do per-device PASID namespace which
> > is built on a bind_pasid_table iommu callback to allow guest fully
> > manage its PASIDs on a given passthrough device.
> 
> 
> I see, so I think the answer is to prepare for the namespace support
> from the start. (btw, I don't see how namespace is handled in current
> IOASID module?)

The PASID table is based on GPA when nested translation is enabled 
on ARM SMMU. This design implies that the guest manages PASID
table thus PASIDs instead of going through host-side API on assigned 
device. From this angle we don't need explicit namespace in the host 
API. Just need a way to control how many PASIDs a process is allowed 
to allocate in the global namespace. btw IOASID module already has 
'set' concept per-process and PASIDs are managed per-set. Then the 
quota control can be easily introduced in the 'set' level.

> 
> 
> >   I'm not sure
> > how such requirement can be 

RE: (proposal) RE: [PATCH v7 00/16] vfio: expose virtual Shared Virtual Addressing to VMs

2020-10-13 Thread Tian, Kevin
> From: Jean-Philippe Brucker 
> Sent: Tuesday, October 13, 2020 6:28 PM
> 
> On Mon, Oct 12, 2020 at 08:38:54AM +, Tian, Kevin wrote:
> > > From: Jason Wang 
> > > Sent: Monday, September 14, 2020 12:20 PM
> > >
> > [...]
> >  > If it's possible, I would suggest a generic uAPI instead of a VFIO
> > > specific one.
> > >
> > > Jason suggest something like /dev/sva. There will be a lot of other
> > > subsystems that could benefit from this (e.g vDPA).
> > >
> > > Have you ever considered this approach?
> > >
> >
> > Hi, Jason,
> >
> > We did some study on this approach and below is the output. It's a
> > long writing but I didn't find a way to further abstract w/o losing
> > necessary context. Sorry about that.
> >
> > Overall the real purpose of this series is to enable IOMMU nested
> > translation capability with vSVA as one major usage, through
> > below new uAPIs:
> > 1) Report/enable IOMMU nested translation capability;
> > 2) Allocate/free PASID;
> > 3) Bind/unbind guest page table;
> > 4) Invalidate IOMMU cache;
> > 5) Handle IOMMU page request/response (not in this series);
> > 1/3/4) is the minimal set for using IOMMU nested translation, with
> > the other two optional. For example, the guest may enable vSVA on
> > a device without using PASID. Or, it may bind its gIOVA page table
> > which doesn't require page fault support. Finally, all operations can
> > be applied to either physical device or subdevice.
> >
> > Then we evaluated each uAPI whether generalizing it is a good thing
> > both in concept and regarding to complexity.
> >
> > First, unlike other uAPIs which are all backed by iommu_ops, PASID
> > allocation/free is through the IOASID sub-system. From this angle
> > we feel generalizing PASID management does make some sense.
> > First, PASID is just a number and not related to any device before
> > it's bound to a page table and IOMMU domain. Second, PASID is a
> > global resource (at least on Intel VT-d), while having separate VFIO/
> > VDPA allocation interfaces may easily cause confusion in userspace,
> > e.g. which interface to be used if both VFIO/VDPA devices exist.
> > Moreover, an unified interface allows centralized control over how
> > many PASIDs are allowed per process.
> >
> > One unclear part with this generalization is about the permission.
> > Do we open this interface to any process or only to those which
> > have assigned devices? If the latter, what would be the mechanism
> > to coordinate between this new interface and specific passthrough
> > frameworks? A more tricky case, vSVA support on ARM (Eric/Jean
> > please correct me) plans to do per-device PASID namespace which
> > is built on a bind_pasid_table iommu callback to allow guest fully
> > manage its PASIDs on a given passthrough device.
> 
> Yes we need a bind_pasid_table. The guest needs to allocate the PASID
> tables because they are accessed via guest-physical addresses by the HW
> SMMU.
> 
> With bind_pasid_table, the invalidation message also requires a scope to
> invalidate a whole PASID context, in addition to invalidating a mappings
> ranges.
> 
> > I'm not sure
> > how such requirement can be unified w/o involving passthrough
> > frameworks, or whether ARM could also switch to global PASID
> > style...
> 
> Not planned at the moment, sorry. It requires a PV IOMMU to do PASID
> allocation, which is possible with virtio-iommu but not with a vSMMU
> emulation. The VM will manage its own PASID space. The upside is that we
> don't need userspace access to IOASID, so I won't pester you with comments
> on that part of the API :)

It makes sense. Possibly in the future when you plan to support 
SIOV-like capability then you may have to convert PASID table
to use host physical address then the same API could be reused. :)

Thanks
Kevin

> 
> > Second, IOMMU nested translation is a per IOMMU domain
> > capability. Since IOMMU domains are managed by VFIO/VDPA
> >  (alloc/free domain, attach/detach device, set/get domain attribute,
> > etc.), reporting/enabling the nesting capability is an natural
> > extension to the domain uAPI of existing passthrough frameworks.
> > Actually, VFIO already includes a nesting enable interface even
> > before this series. So it doesn't make sense to generalize this uAPI
> > out.
> 
> Agree for enabling, but for reporting we did consider adding a sysfs
> interface in /sys/class/iommu/ describing an IOMMU's properties. Then
> opted for VFIO capabilities to keep the API nice and contained, but if
> we're breaking up the API, sysfs might be more convenient to use and
> extend.
> 
> > Then the tricky part comes with the remaining operations (3/4/5),
> > which are all backed by iommu_ops thus effective only within an
> > IOMMU domain. To generalize them, the first thing is to find a way
> > to associate the sva_FD (opened through generic /dev/sva) with an
> > IOMMU domain that is created by VFIO/VDPA. The second thing is
> > to replicate 

Re: [PATCH next] iommu: intel: don't dereference iommu_device if IOMMU_API is not built

2020-10-13 Thread Lu Baolu

On 10/13/20 3:30 PM, Bartosz Golaszewski wrote:

From: Bartosz Golaszewski 

Since commit c40c1018 ("iommu/vt-d: Gracefully handle DMAR units
with no supported address widths") dmar.c needs struct iommu_device to
be selected. We can drop this dependency by not dereferencing struct
iommu_device if IOMMU_API is not selected and by reusing the information
stored in iommu->drhd->ignored instead.

This fixes the following build error when IOMMU_API is not selected:

drivers/iommu/intel/dmar.c: In function ‘free_iommu’:
drivers/iommu/intel/dmar.c:1139:41: error: ‘struct iommu_device’ has no member 
named ‘ops’
  1139 |  if (intel_iommu_enabled && iommu->iommu.ops) {
 ^

Fixes: c40c1018 ("iommu/vt-d: Gracefully handle DMAR units with no supported 
address widths")
Signed-off-by: Bartosz Golaszewski 


With commit title adjusted to "iommu/vt-d: Don't dereference
iommu_device if IOMMU_API is not built",

Acked-by: Lu Baolu 

Best regards,
baolu


---
  drivers/iommu/intel/dmar.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 2d70d56d8e0d..404b40af31cb 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1136,7 +1136,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
  
  static void free_iommu(struct intel_iommu *iommu)

  {
-   if (intel_iommu_enabled && iommu->iommu.ops) {
+   if (intel_iommu_enabled && !iommu->drhd->ignored) {
iommu_device_unregister(>iommu);
iommu_device_sysfs_remove(>iommu);
}


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 6/8] iommu/arm-smmu: Add impl hook for inherit boot mappings

2020-10-13 Thread Robin Murphy

On 2020-10-12 08:31, Bjorn Andersson wrote:

On Mon 21 Sep 23:08 CEST 2020, Will Deacon wrote:


On Sat, Sep 12, 2020 at 10:25:59PM -0500, Bjorn Andersson wrote:

On Fri 11 Sep 12:13 CDT 2020, Robin Murphy wrote:

On 2020-09-04 16:55, Bjorn Andersson wrote:

Add a new operation to allow platform implementations to inherit any
stream mappings from the boot loader.


Is there a reason we need an explicit step for this? The aim of the
cfg_probe hook is that the SMMU software state should all be set up by then,
and you can mess about with it however you like before arm_smmu_reset()
actually commits anything to hardware. I would have thought you could
permanently steal a context bank, configure it as your bypass hole, read out
the previous SME configuration and tweak smmu->smrs and smmu->s2crs
appropriately all together "invisibly" at that point.


I did this because as of 6a79a5a3842b ("iommu/arm-smmu: Call
configuration impl hook before consuming features") we no longer have
setup pgsize_bitmap as we hit cfg_probe, which means that I need to
replicate this logic to set up the iommu_domain.

If I avoid setting up an iommu_domain for the identity context, as you
request in patch 8, this shouldn't be needed anymore.


If that can't work, I'm very curious as to what I've overlooked.



I believe that will work, I will rework the patches and try it out.


Did you get a chance to rework this?



Finally got a chance to dig through this properly.

Initial results where positive and with an implementation of cfg_probe
in qcom_smmu_impl I'm able to probe the arm-smmu driver just fine - and
display (e.g. efifb) stays alive.

Unfortunately as the display driver (drivers/gpu/drm/msm) is about to
probe a new iommu domain is created, which due to its match against
qcom_smmu_client_of_match[] becomes of type IOMMU_DOMAIN_IDENTITY.
This results in a S2CR of BYPASS type, which the firmware intercepts and
turns the stream into a type FAULT.

So while the cfg_probe looks very reasonable we're still in need of a
mechanism to use the fake identity context for the iommu domain
associated with the display controller.


Yes, we'll still need some kind of hook somewhere to make identity 
domains work at all - my point about cfg_probe was to keep the 
reservation and configuration of the special identity context, plus the 
handling of the initial SME state, simple and entirely internal to the 
impl. In terms of where said hook should be, TBH it might actually work 
out pretty clean to simply hook GR0 register accesses so you can rewrite 
between S2CR bypass entries and translation entries targeting your 
reserved context on-the-fly. Failing that, something to massage "type" 
and "cbndx" in arm_smmu_domain_add_master() would be the next best 
option, I think.


Robin.


The workings of the display driver is that it gets the iommu domain
setup for byass and then after that creates a translation context for
this same stream where it maps the framebuffer.

For testing purposes I made def_domain_type always return 0 in the qcom
impl and the result is that we get a few page faults while probing the
display driver, but these are handled somewhat gracefully and the
initialization did proceed and the system comes up nicely (but in the
case that the display driver would probe defer this leads to an storm of
faults as the screen continues to be refreshed).

TL;DR I think we still need to have a way to get the arm-smmu driver to
allow the qcom implementation to configure identity domains to use
translation - but we can make the setup of the identity context a detail
of the qcom driver.

Regards,
Bjorn


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 2/3] drm/msm: add DRM_MSM_GEM_SYNC_CACHE for non-coherent cache maintenance

2020-10-13 Thread Rob Clark
On Tue, Oct 13, 2020 at 6:42 AM Robin Murphy  wrote:
>
> On 2020-10-07 07:25, Christoph Hellwig wrote:
> > On Tue, Oct 06, 2020 at 09:19:32AM -0400, Jonathan Marek wrote:
> >> One example why drm/msm can't use DMA API is multiple page table support
> >> (that is landing in 5.10), which is something that definitely couldn't work
> >> with DMA API.
> >>
> >> Another one is being able to choose the address for mappings, which AFAIK
> >> DMA API can't do (somewhat related to this: qcom hardware often has ranges
> >> of allowed addresses, which the dma_mask mechanism fails to represent, what
> >> I see is drivers using dma_mask as a "maximum address", and since addresses
> >> are allocated from the top it generally works)
> >
> > That sounds like a good enough rason to use the IOMMU API.  I just
> > wanted to make sure this really makes sense.
>
> I still think this situation would be best handled with a variant of
> dma_ops_bypass that also guarantees to bypass SWIOTLB, and can be set
> automatically when attaching to an unmanaged IOMMU domain. That way the
> device driver can make DMA API calls in the appropriate places that do
> the right thing either way, and only needs logic to decide whether to
> use the returned DMA addresses directly or ignore them if it knows
> they're overridden by its own IOMMU mapping.
>

That would be pretty ideal from my PoV

BR,
-R
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[git pull] IOMMU Updates for Linux v5.10

2020-10-13 Thread Joerg Roedel
Hi Linus,

there is a minor conflict this time in include/linux/iommu.h which
should be easy to resolve. I would attach my resolution, but somehow git
[show|log] didn't show it to me.

The conflict is in a function signature, specifically the type of the
pasid parameter. My resolution used iopasid_t.

With that in mind:

The following changes since commit 549738f15da0e5a00275977623be199fbbf7df50:

  Linux 5.9-rc8 (2020-10-04 16:04:34 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git 
tags/iommu-updates-v5.10

for you to fetch changes up to 7e3c3883c381aeda903778d7e99fc4cd523be610:

  Merge branches 'arm/allwinner', 'arm/mediatek', 'arm/renesas', 'arm/tegra', 
'arm/qcom', 'arm/smmu', 'ppc/pamu', 'x86/amd', 'x86/vt-d' and 'core' into next 
(2020-10-07 11:51:59 +0200)


IOMMU Updates for Linux v5.10

Including:

- ARM-SMMU Updates from Will:

  - Continued SVM enablement, where page-table is shared with
CPU

  - Groundwork to support integrated SMMU with Adreno GPU

  - Allow disabling of MSI-based polling on the kernel
command-line

  - Minor driver fixes and cleanups (octal permissions, error
messages, ...)

- Secure Nested Paging Support for AMD IOMMU. The IOMMU will
  fault when a device tries DMA on memory owned by a guest. This
  needs new fault-types as well as a rewrite of the IOMMU memory
  semaphore for command completions.

- Allow broken Intel IOMMUs (wrong address widths reported) to
  still be used for interrupt remapping.

- IOMMU UAPI updates for supporting vSVA, where the IOMMU can
  access address spaces of processes running in a VM.

- Support for the MT8167 IOMMU in the Mediatek IOMMU driver.

- Device-tree updates for the Renesas driver to support r8a7742.

- Several smaller fixes and cleanups all over the place.


Alex Dewar (1):
  iommu/pamu: Replace use of kzfree with kfree_sensitive

Andy Shevchenko (1):
  iommu/vt-d: Move intel_iommu_gfx_mapped to Intel IOMMU header

Barry Song (3):
  iommu/arm-smmu-v3: replace symbolic permissions by octal permissions for 
module parameter
  iommu/arm-smmu-v3: replace module_param_named by module_param for 
disable_bypass
  iommu/arm-smmu-v3: permit users to disable msi polling

David Woodhouse (1):
  iommu/vt-d: Gracefully handle DMAR units with no supported address widths

Dmitry Osipenko (1):
  iommu/tegra-smmu: Add locking around mapping operations

Fabien Parent (3):
  dt-bindings: iommu: Add binding for MediaTek MT8167 IOMMU
  iommu/mediatek: Add flag for legacy ivrp paddr
  iommu/mediatek: Add support for MT8167

Jacob Pan (6):
  docs: IOMMU user API
  iommu/uapi: Add argsz for user filled data
  iommu/uapi: Use named union for user data
  iommu/uapi: Rename uapi functions
  iommu/uapi: Handle data and argsz filled by users
  iommu/vt-d: Check UAPI data processed by IOMMU core

Jean-Philippe Brucker (9):
  iommu/arm-smmu-v3: Fix endianness annotations
  arm64: mm: Pin down ASIDs for sharing mm with devices
  arm64: cpufeature: Export symbol read_sanitised_ftr_reg()
  iommu/io-pgtable-arm: Move some definitions to a header
  iommu/arm-smmu-v3: Move definitions to a header
  iommu/arm-smmu-v3: Share process page tables
  iommu/arm-smmu-v3: Seize private ASID
  iommu/arm-smmu-v3: Check for SVA features
  iommu/arm-smmu-v3: Add SVA device feature

Joerg Roedel (3):
  iommu/sun50i: Fix set-but-not-used variable warning
  Merge tag 'arm-smmu-updates' of git://git.kernel.org/.../will/linux into 
arm/smmu
  Merge branches 'arm/allwinner', 'arm/mediatek', 'arm/renesas', 
'arm/tegra', 'arm/qcom', 'arm/smmu', 'ppc/pamu', 'x86/amd', 'x86/vt-d' and 
'core' into next

Jordan Crouse (3):
  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

Krzysztof Kozlowski (5):
  iommu/mediatek: Drop of_match_ptr to fix -Wunused-const-variable
  iommu/amd: Add missing function prototypes to fix -Wmissing-prototypes
  iommu/amd: Fix kerneldoc comments
  iommu/vt-d: Drop kerneldoc marker from regular comment
  iommu/qcom: Drop of_match_ptr to fix -Wunused-const-variable

Lad Prabhakar (4):
  dt-bindings: iommu: renesas,ipmmu-vmsa: Sort compatible string in 
increasing number of the SoC
  dt-bindings: iommu: renesas,ipmmu-vmsa: Add r8a7742 support
  ARM: dts: r8a7742: Add IPMMU DT nodes
  iommu/renesas: Update help description for IPMMU_VMSA config

Lu Baolu (1):
  iommu/vt-d: Use device numa domain if RHSA is missing

Miles Chen 

Re: [PATCH 2/3] drm/msm: add DRM_MSM_GEM_SYNC_CACHE for non-coherent cache maintenance

2020-10-13 Thread Robin Murphy

On 2020-10-07 07:25, Christoph Hellwig wrote:

On Tue, Oct 06, 2020 at 09:19:32AM -0400, Jonathan Marek wrote:

One example why drm/msm can't use DMA API is multiple page table support
(that is landing in 5.10), which is something that definitely couldn't work
with DMA API.

Another one is being able to choose the address for mappings, which AFAIK
DMA API can't do (somewhat related to this: qcom hardware often has ranges
of allowed addresses, which the dma_mask mechanism fails to represent, what
I see is drivers using dma_mask as a "maximum address", and since addresses
are allocated from the top it generally works)


That sounds like a good enough rason to use the IOMMU API.  I just
wanted to make sure this really makes sense.


I still think this situation would be best handled with a variant of 
dma_ops_bypass that also guarantees to bypass SWIOTLB, and can be set 
automatically when attaching to an unmanaged IOMMU domain. That way the 
device driver can make DMA API calls in the appropriate places that do 
the right thing either way, and only needs logic to decide whether to 
use the returned DMA addresses directly or ignore them if it knows 
they're overridden by its own IOMMU mapping.


Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 0/4] Add system mmu support for Armada-806

2020-10-13 Thread Robin Murphy

On 2020-10-06 16:16, Denis Odintsov wrote:

Hi,


Am 15.07.2020 um 09:06 schrieb Tomasz Nowicki :

The series is meant to support SMMU for AP806 and a workaround
for accessing ARM SMMU 64bit registers is the gist of it.

For the record, AP-806 can't access SMMU registers with 64bit width.
This patches split the readq/writeq into two 32bit accesses instead
and update DT bindings.

The series was successfully tested on a vanilla v5.8-rc3 kernel and
Intel e1000e PCIe NIC. The same for platform devices like SATA and USB.

For reference, previous versions are listed below:
V1: https://lkml.org/lkml/2018/10/15/373
V2: https://lkml.org/lkml/2019/7/11/426
V3: https://lkml.org/lkml/2020/7/2/1114



1) After enabling SMMU on Armada 8040, and ARM_SMMU_DISABLE_BYPASS_BY_DEFAUL=y 
by default in kernel since 954a03be033c7cef80ddc232e7cbdb17df735663,
internal eMMC is prevented from being initialised (as there is no iommus 
property for ap_sdhci0)
Disabling "Disable bypass by default" make it work, but the patch highly 
suggest doing it properly.
I wasn't able to find correct path for ap_sdhci for iommus in any publicly 
available documentation,
would be highly appreciated addressed properly, thank you!


FWIW the SMMU tells you the offending unmatched Stream ID, so if faults 
can reasonably be correlated with a particular device making accesses, 
you can effectively discover the Stream ID assignment by trial and 
error. Often that can be easier than trying to find formal documentation 
anyway ;)



2) Second issue I got (btw I have ClearFog GT 8k armada-8040 based board) is 
mpci ath10k card.
It is found, it is enumerated, it is visible in lspci, but it fails to be 
initialised. Here is the log:

[1.743754] armada8k-pcie f260.pcie: host bridge /cp0/pcie@f260 
ranges:
[1.751116] armada8k-pcie f260.pcie:  MEM 0x00f600..0x00f6ef 
-> 0x00f600
[1.964690] armada8k-pcie f260.pcie: Link up
[1.969379] armada8k-pcie f260.pcie: PCI host bridge to bus :00
[1.976026] pci_bus :00: root bus resource [bus 00-ff]
[1.981537] pci_bus :00: root bus resource [mem 0xf600-0xf6ef]
[1.988462] pci :00:00.0: [11ab:0110] type 01 class 0x060400
[1.994504] pci :00:00.0: reg 0x10: [mem 0x-0x000f]
[2.000843] pci :00:00.0: supports D1 D2
[2.005132] pci :00:00.0: PME# supported from D0 D1 D3hot
[2.011853] pci :01:00.0: [168c:003c] type 00 class 0x028000
[2.018001] pci :01:00.0: reg 0x10: [mem 0x-0x001f 64bit]
[2.025002] pci :01:00.0: reg 0x30: [mem 0x-0x pref]
[2.032111] pci :01:00.0: supports D1 D2
[2.049409] pci :00:00.0: BAR 14: assigned [mem 0xf600-0xf61f]
[2.056322] pci :00:00.0: BAR 0: assigned [mem 0xf620-0xf62f]
[2.063142] pci :00:00.0: BAR 15: assigned [mem 0xf630-0xf63f 
pref]
[2.070484] pci :01:00.0: BAR 0: assigned [mem 0xf600-0xf61f 
64bit]
[2.077880] pci :01:00.0: BAR 6: assigned [mem 0xf630-0xf630 
pref]
[2.085135] pci :00:00.0: PCI bridge to [bus 01-ff]
[2.090384] pci :00:00.0:   bridge window [mem 0xf600-0xf61f]
[2.097202] pci :00:00.0:   bridge window [mem 0xf630-0xf63f 
pref]
[2.104539] pcieport :00:00.0: Adding to iommu group 4
[2.110232] pcieport :00:00.0: PME: Signaling with IRQ 38
[2.116141] pcieport :00:00.0: AER: enabled with IRQ 38
[8.131135] ath10k_pci :01:00.0: Adding to iommu group 4
[8.131874] ath10k_pci :01:00.0: enabling device ( -> 0002)
[8.132203] ath10k_pci :01:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 
reset_mode 0

up to that point the log is the same as without SMMU enabled, except "Adding to 
iommu group N" lines, and IRQ being 37


Does forcing ath10k to use legacy interrupts rather than MSIs make a 
difference?


Judging by the DT it looks like MSIs ought to be targeting the GICv2M 
widget, but if things somehow end up trying to use the PCIe controller's 
internal MSI doorbell (upstream of SMMU translation) instead, then that 
might account for general interrupt-related weirdness.


Robin.


[8.221328] ath10k_pci :01:00.0: failed to poke copy engine: -16
[8.313362] ath10k_pci :01:00.0: failed to poke copy engine: -16
[8.409373] ath10k_pci :01:00.0: failed to poke copy engine: -16
[8.553433] ath10k_pci :01:00.0: failed to poke copy engine: -16
[8.641370] ath10k_pci :01:00.0: failed to poke copy engine: -16
[8.737979] ath10k_pci :01:00.0: failed to poke copy engine: -16
[8.807356] ath10k_pci :01:00.0: Failed to get pcie state addr: -16
[8.814032] ath10k_pci :01:00.0: failed to setup init config: -16
[8.820605] ath10k_pci :01:00.0: could not power on hif bus (-16)
[8.827111] ath10k_pci :01:00.0: could not probe fw (-16)

Thank you!


v3 -> v4
- call cfg_probe() impl hook a bit earlier 

Re: [PATCH 1/9] dma-mapping: split

2020-10-13 Thread Christoph Hellwig
Thanks for the report, I've commited the obvious fix.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: (proposal) RE: [PATCH v7 00/16] vfio: expose virtual Shared Virtual Addressing to VMs

2020-10-13 Thread Jean-Philippe Brucker
On Mon, Oct 12, 2020 at 08:38:54AM +, Tian, Kevin wrote:
> > From: Jason Wang 
> > Sent: Monday, September 14, 2020 12:20 PM
> >
> [...]
>  > If it's possible, I would suggest a generic uAPI instead of a VFIO
> > specific one.
> > 
> > Jason suggest something like /dev/sva. There will be a lot of other
> > subsystems that could benefit from this (e.g vDPA).
> > 
> > Have you ever considered this approach?
> > 
> 
> Hi, Jason,
> 
> We did some study on this approach and below is the output. It's a
> long writing but I didn't find a way to further abstract w/o losing 
> necessary context. Sorry about that.
> 
> Overall the real purpose of this series is to enable IOMMU nested
> translation capability with vSVA as one major usage, through
> below new uAPIs:
>   1) Report/enable IOMMU nested translation capability;
>   2) Allocate/free PASID;
>   3) Bind/unbind guest page table;
>   4) Invalidate IOMMU cache;
>   5) Handle IOMMU page request/response (not in this series);
> 1/3/4) is the minimal set for using IOMMU nested translation, with 
> the other two optional. For example, the guest may enable vSVA on 
> a device without using PASID. Or, it may bind its gIOVA page table 
> which doesn't require page fault support. Finally, all operations can 
> be applied to either physical device or subdevice.
> 
> Then we evaluated each uAPI whether generalizing it is a good thing 
> both in concept and regarding to complexity.
> 
> First, unlike other uAPIs which are all backed by iommu_ops, PASID 
> allocation/free is through the IOASID sub-system. From this angle
> we feel generalizing PASID management does make some sense. 
> First, PASID is just a number and not related to any device before 
> it's bound to a page table and IOMMU domain. Second, PASID is a 
> global resource (at least on Intel VT-d), while having separate VFIO/
> VDPA allocation interfaces may easily cause confusion in userspace,
> e.g. which interface to be used if both VFIO/VDPA devices exist. 
> Moreover, an unified interface allows centralized control over how 
> many PASIDs are allowed per process.
> 
> One unclear part with this generalization is about the permission.
> Do we open this interface to any process or only to those which
> have assigned devices? If the latter, what would be the mechanism
> to coordinate between this new interface and specific passthrough 
> frameworks? A more tricky case, vSVA support on ARM (Eric/Jean
> please correct me) plans to do per-device PASID namespace which
> is built on a bind_pasid_table iommu callback to allow guest fully 
> manage its PASIDs on a given passthrough device.

Yes we need a bind_pasid_table. The guest needs to allocate the PASID
tables because they are accessed via guest-physical addresses by the HW
SMMU.

With bind_pasid_table, the invalidation message also requires a scope to
invalidate a whole PASID context, in addition to invalidating a mappings
ranges.

> I'm not sure 
> how such requirement can be unified w/o involving passthrough
> frameworks, or whether ARM could also switch to global PASID 
> style...

Not planned at the moment, sorry. It requires a PV IOMMU to do PASID
allocation, which is possible with virtio-iommu but not with a vSMMU
emulation. The VM will manage its own PASID space. The upside is that we
don't need userspace access to IOASID, so I won't pester you with comments
on that part of the API :)

> Second, IOMMU nested translation is a per IOMMU domain
> capability. Since IOMMU domains are managed by VFIO/VDPA
>  (alloc/free domain, attach/detach device, set/get domain attribute,
> etc.), reporting/enabling the nesting capability is an natural 
> extension to the domain uAPI of existing passthrough frameworks. 
> Actually, VFIO already includes a nesting enable interface even 
> before this series. So it doesn't make sense to generalize this uAPI 
> out.

Agree for enabling, but for reporting we did consider adding a sysfs
interface in /sys/class/iommu/ describing an IOMMU's properties. Then
opted for VFIO capabilities to keep the API nice and contained, but if
we're breaking up the API, sysfs might be more convenient to use and
extend.

> Then the tricky part comes with the remaining operations (3/4/5),
> which are all backed by iommu_ops thus effective only within an 
> IOMMU domain. To generalize them, the first thing is to find a way 
> to associate the sva_FD (opened through generic /dev/sva) with an 
> IOMMU domain that is created by VFIO/VDPA. The second thing is 
> to replicate {domain<->device/subdevice} association in /dev/sva 
> path because some operations (e.g. page fault) is triggered/handled 
> per device/subdevice. Therefore, /dev/sva must provide both per-
> domain and per-device uAPIs similar to what VFIO/VDPA already 
> does. Moreover, mapping page fault to subdevice requires pre-
> registering subdevice fault data to IOMMU layer when binding 
> guest page table, while such fault data can be only retrieved from 
> parent 

答复: [PATCH v3 0/2] Add support for ACPI device in RMRR

2020-10-13 Thread FelixCui-oc
hi baolu,


>By the way, I guess the problem you are facing can still be handled well
>under current RMRR mechanism by simple putting the device in the
>ACPI/ANDD table. It's worth trying.


I understand what you mean is that  just put the device in the ANDD table and 
don't use RMRR, right?

But this can't solve my problem. Can you explain it in detail how to solve my 
problem?


In addition, the latest VT-D spec still has support for the use of acpi device 
under RMRR and ANDD.

From the perpective of supporting spec , the driver should support the case we 
submitted.


Best regards

Felixcui-oc



发件人: Lu Baolu 
发送时间: 2020年10月12日 10:31:40
收件人: FelixCui-oc; Joerg Roedel; iommu@lists.linux-foundation.org; 
linux-ker...@vger.kernel.org; David Woodhouse; Dan Carpenter; 
kbu...@lists.01.org
抄送: baolu...@linux.intel.com; CobeChen-oc; RaymondPang-oc; Tony W Wang-oc
主题: Re: [PATCH v3 0/2] Add support for ACPI device in RMRR

Hi Felix,

On 10/10/20 4:02 PM, FelixCuioc wrote:
> BIOS allocate reserved memory ranges that may be DMA targets.
> BIOS may report each such reserved memory region through the
> RMRR structures,along with the devices that requires access to
> the specified reserved memory region.
>
> The purpose of this series is to achieve ACPI device in RMRR
> access reserved memory.Therefore,it is necessary to increase
> the analysis of acpi device in RMRR and establish a mapping
> for this device.
>
> The first patch adds interfaces for detecting ACPI device
> in RMRR and in order to distinguish it from pci device,
> some interface functions are modified.
>
> The second patch adds support for probing ACPI device in RMRR.
> In probe_acpi_namespace_devices(),add support for direct mapping
> of ACPI device and add support for physical node of acpi device
> to be NULL.

Thanks for your patches. As I explained in the previous reply, RMRRs
were added as work around for certain legacy device and we have been
working hard to fix those legacy devices so that RMRR are no longer
needed. Any new use case of RMRR is not encouraged.

By the way, I guess the problem you are facing can still be handled well
under current RMRR mechanism by simple putting the device in the
ACPI/ANDD table. It's worth trying.

Best regards,
baolu

>
> v2->v3:
> - Add the blank line between functions.
> - Make dmar_acpi_insert_dev_scope() bool,change the 1/0 to true/false
>   and add a comment explaining.
> - Delete unused initialization.
> - if dmar_acpi_insert_dev_scope() always returns zero,will not
>   call dmar_rmrr_add_acpi_dev().
> - Use a proper error code.
> - Use if(!pdev).
> - Use goto unlock instead of mutex_unlock().
>
>
> FelixCuioc (2):
>iommu/vt-d:Add support for detecting ACPI device in RMRR
>iommu/vt-d:Add support for probing ACPI device in RMRR
>
>   drivers/iommu/intel/dmar.c  | 76 +
>   drivers/iommu/intel/iommu.c | 52 -
>   drivers/iommu/iommu.c   |  6 +++
>   include/linux/dmar.h| 12 +-
>   include/linux/iommu.h   |  2 +
>   5 files changed, 113 insertions(+), 35 deletions(-)
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH next] iommu: intel: don't dereference iommu_device if IOMMU_API is not built

2020-10-13 Thread David Woodhouse
On Tue, 2020-10-13 at 09:30 +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski 
> 
> Since commit c40c1018 ("iommu/vt-d: Gracefully handle DMAR units
> with no supported address widths") dmar.c needs struct iommu_device to
> be selected. We can drop this dependency by not dereferencing struct
> iommu_device if IOMMU_API is not selected and by reusing the information
> stored in iommu->drhd->ignored instead.
> 
> This fixes the following build error when IOMMU_API is not selected:
> 
> drivers/iommu/intel/dmar.c: In function ‘free_iommu’:
> drivers/iommu/intel/dmar.c:1139:41: error: ‘struct iommu_device’ has no 
> member named ‘ops’
>  1139 |  if (intel_iommu_enabled && iommu->iommu.ops) {
> ^
> 
> Fixes: c40c1018 ("iommu/vt-d: Gracefully handle DMAR units with no 
> supported address widths")
> Signed-off-by: Bartosz Golaszewski 

Oops, apologies for that. Thanks for fixing it.

Acked-by: David Woodhouse 




smime.p7s
Description: S/MIME cryptographic signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 6/9] x86/hpet: Use irq_find_matching_fwspec() to find remapping irqdomain

2020-10-13 Thread David Woodhouse
From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
 arch/x86/kernel/hpet.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index 3b8b12769f3b..fb7736ca7b5b 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -543,8 +543,8 @@ static struct irq_domain *hpet_create_irq_domain(int 
hpet_id)
 {
struct msi_domain_info *domain_info;
struct irq_domain *parent, *d;
-   struct irq_alloc_info info;
struct fwnode_handle *fn;
+   struct irq_fwspec fwspec;
 
if (x86_vector_domain == NULL)
return NULL;
@@ -556,15 +556,6 @@ static struct irq_domain *hpet_create_irq_domain(int 
hpet_id)
*domain_info = hpet_msi_domain_info;
domain_info->data = (void *)(long)hpet_id;
 
-   init_irq_alloc_info(, NULL);
-   info.type = X86_IRQ_ALLOC_TYPE_HPET_GET_PARENT;
-   info.devid = hpet_id;
-   parent = irq_remapping_get_irq_domain();
-   if (parent == NULL)
-   parent = x86_vector_domain;
-   else
-   hpet_msi_controller.name = "IR-HPET-MSI";
-
fn = irq_domain_alloc_named_id_fwnode(hpet_msi_controller.name,
  hpet_id);
if (!fn) {
@@ -572,6 +563,18 @@ static struct irq_domain *hpet_create_irq_domain(int 
hpet_id)
return NULL;
}
 
+   fwspec.fwnode = fn;
+   fwspec.param_count = 1;
+   fwspec.param[0] = hpet_id;
+   parent = irq_find_matching_fwspec(, DOMAIN_BUS_ANY);
+   if (!parent) {
+   irq_domain_free_fwnode(fn);
+   kfree(domain_info);
+   return NULL;
+   }
+   if (parent != x86_vector_domain)
+   hpet_msi_controller.name = "IR-HPET-MSI";
+
d = msi_create_irq_domain(fn, domain_info, parent);
if (!d) {
irq_domain_free_fwnode(fn);
-- 
2.26.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 4/9] iommu/vt-d: Implement select() method on remapping irqdomain

2020-10-13 Thread David Woodhouse
From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
 drivers/iommu/intel/irq_remapping.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/iommu/intel/irq_remapping.c 
b/drivers/iommu/intel/irq_remapping.c
index 511dfb4884bc..40c2fec122b8 100644
--- a/drivers/iommu/intel/irq_remapping.c
+++ b/drivers/iommu/intel/irq_remapping.c
@@ -1435,7 +1435,20 @@ static void intel_irq_remapping_deactivate(struct 
irq_domain *domain,
modify_irte(>irq_2_iommu, );
 }
 
+static int intel_irq_remapping_select(struct irq_domain *d,
+ struct irq_fwspec *fwspec,
+ enum irq_domain_bus_token bus_token)
+{
+   if (x86_fwspec_is_ioapic(fwspec))
+   return d == map_ioapic_to_ir(fwspec->param[0]);
+   else if (x86_fwspec_is_hpet(fwspec))
+   return d == map_hpet_to_ir(fwspec->param[0]);
+
+   return 0;
+}
+
 static const struct irq_domain_ops intel_ir_domain_ops = {
+   .select = intel_irq_remapping_select,
.alloc = intel_irq_remapping_alloc,
.free = intel_irq_remapping_free,
.activate = intel_irq_remapping_activate,
-- 
2.26.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 0/9] Remove irq_remapping_get_irq_domain()

2020-10-13 Thread David Woodhouse
I didn't much like the I/OAPIC and HPET drivers having magical knowledge
that they had to substitute x86_vector_domain if their call to
irq_remapping_get_irq_domain() returned NULL.

When Thomas tried to make it handle error returns from …get_irq_domain() 
distinctly from the NULL case too, it made me even sadder. So I killed 
it with fire.

Now they just use irq_find_matching_fwspec() to find an appropriate
irqdomain. Each remapping irqdomain just needs to say 'yep, that's me'
for the HPETs or I/OAPICs which are within their scope, while the
x86_vector_domain accepts them all but only if interrupt remapping
is *disabled*. No more special knowledge in the caller.

If IR is enabled and there's a child device which escapes the scope of
all remapping units, it gets NULL for its parent irqdomain and will
fail to initialise, which is the correct thing to do in that "should
never happen" case. For HPET that'll mean that it just doesn't support
MSI, while I/OAPIC will refuse to initialise and trigger a BUG_ON
because Linux quite likes it when *all* the I/OAPICs it knows about get
initialised successfully.

This is on top of the previous 'ext_dest_id' series at
https://patchwork.kernel.org/project/kvm/list/?series=362037

https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/ext_dest_id

David Woodhouse (9):
  genirq/irqdomain: Implement get_name() method on irqchip fwnodes
  x86/apic: Add select() method on vector irqdomain
  iommu/amd: Implement select() method on remapping irqdomain
  iommu/vt-d: Implement select() method on remapping irqdomain
  iommu/hyper-v: Implement select() method on remapping irqdomain
  x86/hpet: Use irq_find_matching_fwspec() to find remapping irqdomain
  x86/ioapic: Use irq_find_matching_fwspec() to find remapping irqdomain
  x86: Kill all traces of irq_remapping_get_irq_domain()
  iommu/vt-d: Simplify intel_irq_remapping_select()

 arch/x86/include/asm/hw_irq.h|  2 --
 arch/x86/include/asm/irq_remapping.h |  9 -
 arch/x86/include/asm/irqdomain.h |  3 +++
 arch/x86/kernel/apic/io_apic.c   | 24 
 arch/x86/kernel/apic/vector.c| 43 
+++
 arch/x86/kernel/hpet.c   | 23 +--
 drivers/iommu/amd/iommu.c| 53 
+++--
 drivers/iommu/hyperv-iommu.c | 18 +-
 drivers/iommu/intel/irq_remapping.c  | 43 
+--
 drivers/iommu/irq_remapping.c| 14 --
 drivers/iommu/irq_remapping.h|  3 ---
 kernel/irq/irqdomain.c   | 11 ++-
 12 files changed, 126 insertions(+), 120 deletions(-)



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 7/9] x86/ioapic: Use irq_find_matching_fwspec() to find remapping irqdomain

2020-10-13 Thread David Woodhouse
From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
 arch/x86/kernel/apic/io_apic.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index ca2da19d5c55..73cacc92c3bb 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2305,36 +2305,36 @@ static inline void __init check_timer(void)
 
 static int mp_irqdomain_create(int ioapic)
 {
-   struct irq_alloc_info info;
struct irq_domain *parent;
int hwirqs = mp_ioapic_pin_count(ioapic);
struct ioapic *ip = [ioapic];
struct ioapic_domain_cfg *cfg = >irqdomain_cfg;
struct mp_ioapic_gsi *gsi_cfg = mp_ioapic_gsi_routing(ioapic);
struct fwnode_handle *fn;
-   char *name = "IO-APIC";
+   struct irq_fwspec fwspec;
 
if (cfg->type == IOAPIC_DOMAIN_INVALID)
return 0;
 
-   init_irq_alloc_info(, NULL);
-   info.type = X86_IRQ_ALLOC_TYPE_IOAPIC_GET_PARENT;
-   info.devid = mpc_ioapic_id(ioapic);
-   parent = irq_remapping_get_irq_domain();
-   if (!parent)
-   parent = x86_vector_domain;
-   else
-   name = "IO-APIC-IR";
-
/* Handle device tree enumerated APICs proper */
if (cfg->dev) {
fn = of_node_to_fwnode(cfg->dev);
} else {
-   fn = irq_domain_alloc_named_id_fwnode(name, ioapic);
+   fn = irq_domain_alloc_named_id_fwnode("IO-APIC", ioapic);
if (!fn)
return -ENOMEM;
}
 
+   fwspec.fwnode = fn;
+   fwspec.param_count = 1;
+   fwspec.param[0] = ioapic;
+   parent = irq_find_matching_fwspec(, DOMAIN_BUS_ANY);
+   if (!parent) {
+   if (!cfg->dev)
+   irq_domain_free_fwnode(fn);
+   return -ENODEV;
+   }
+
ip->irqdomain = irq_domain_create_linear(fn, hwirqs, cfg->ops,
 (void *)(long)ioapic);
 
-- 
2.26.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 1/9] genirq/irqdomain: Implement get_name() method on irqchip fwnodes

2020-10-13 Thread David Woodhouse
From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
 kernel/irq/irqdomain.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 76cd7ebd1178..6440f97c412e 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -42,7 +42,16 @@ static inline void debugfs_add_domain_dir(struct irq_domain 
*d) { }
 static inline void debugfs_remove_domain_dir(struct irq_domain *d) { }
 #endif
 
-const struct fwnode_operations irqchip_fwnode_ops;
+static const char *irqchip_fwnode_get_name(const struct fwnode_handle *fwnode)
+{
+   struct irqchip_fwid *fwid = container_of(fwnode, struct irqchip_fwid, 
fwnode);
+
+   return fwid->name;
+}
+
+const struct fwnode_operations irqchip_fwnode_ops = {
+   .get_name = irqchip_fwnode_get_name,
+};
 EXPORT_SYMBOL_GPL(irqchip_fwnode_ops);
 
 /**
-- 
2.26.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 5/9] iommu/hyper-v: Implement select() method on remapping irqdomain

2020-10-13 Thread David Woodhouse
From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
 drivers/iommu/hyperv-iommu.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
index 37dd485a5640..6a8966fbc3bd 100644
--- a/drivers/iommu/hyperv-iommu.c
+++ b/drivers/iommu/hyperv-iommu.c
@@ -61,6 +61,14 @@ static struct irq_chip hyperv_ir_chip = {
.irq_set_affinity   = hyperv_ir_set_affinity,
 };
 
+static int hyperv_irq_remapping_select(struct irq_domain *d,
+  struct irq_fwspec *fwspec,
+  enum irq_domain_bus_token bus_token)
+{
+   /* Claim only the first (and only) I/OAPIC */
+   return x86_fwspec_is_ioapic(fwspec) && fwspec->param[0] == 0;
+}
+
 static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
 unsigned int virq, unsigned int nr_irqs,
 void *arg)
@@ -102,6 +110,7 @@ static void hyperv_irq_remapping_free(struct irq_domain 
*domain,
 }
 
 static const struct irq_domain_ops hyperv_ir_domain_ops = {
+   .select = hyperv_irq_remapping_select,
.alloc = hyperv_irq_remapping_alloc,
.free = hyperv_irq_remapping_free,
 };
-- 
2.26.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 9/9] iommu/vt-d: Simplify intel_irq_remapping_select()

2020-10-13 Thread David Woodhouse
From: David Woodhouse 

Now that the old get_irq_domain() method has gone, we can consolidate on
just the map_XXX_to_iommu() functions.

Signed-off-by: David Woodhouse 
---
 drivers/iommu/intel/irq_remapping.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel/irq_remapping.c 
b/drivers/iommu/intel/irq_remapping.c
index ccf61cd18f69..8a41bcb10e26 100644
--- a/drivers/iommu/intel/irq_remapping.c
+++ b/drivers/iommu/intel/irq_remapping.c
@@ -204,13 +204,13 @@ static int modify_irte(struct irq_2_iommu *irq_iommu,
return rc;
 }
 
-static struct irq_domain *map_hpet_to_ir(u8 hpet_id)
+static struct intel_iommu *map_hpet_to_iommu(u8 hpet_id)
 {
int i;
 
for (i = 0; i < MAX_HPET_TBS; i++) {
if (ir_hpet[i].id == hpet_id && ir_hpet[i].iommu)
-   return ir_hpet[i].iommu->ir_domain;
+   return ir_hpet[i].iommu;
}
return NULL;
 }
@@ -226,13 +226,6 @@ static struct intel_iommu *map_ioapic_to_iommu(int apic)
return NULL;
 }
 
-static struct irq_domain *map_ioapic_to_ir(int apic)
-{
-   struct intel_iommu *iommu = map_ioapic_to_iommu(apic);
-
-   return iommu ? iommu->ir_domain : NULL;
-}
-
 static struct irq_domain *map_dev_to_ir(struct pci_dev *dev)
 {
struct dmar_drhd_unit *drhd = dmar_find_matched_drhd_unit(dev);
@@ -1422,12 +1415,14 @@ static int intel_irq_remapping_select(struct irq_domain 
*d,
  struct irq_fwspec *fwspec,
  enum irq_domain_bus_token bus_token)
 {
+   struct intel_iommu *iommu = NULL;
+
if (x86_fwspec_is_ioapic(fwspec))
-   return d == map_ioapic_to_ir(fwspec->param[0]);
+   iommu = map_ioapic_to_iommu(fwspec->param[0]);
else if (x86_fwspec_is_hpet(fwspec))
-   return d == map_hpet_to_ir(fwspec->param[0]);
+   iommu = map_hpet_to_iommu(fwspec->param[0]);
 
-   return 0;
+   return iommu && d == iommu->ir_domain;
 }
 
 static const struct irq_domain_ops intel_ir_domain_ops = {
-- 
2.26.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 3/9] iommu/amd: Implement select() method on remapping irqdomain

2020-10-13 Thread David Woodhouse
From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
 drivers/iommu/amd/iommu.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 13d0a8f42d56..7ecebc5d255f 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3590,6 +3590,24 @@ struct irq_remap_ops amd_iommu_irq_ops = {
.get_irq_domain = get_irq_domain,
 };
 
+static int irq_remapping_select(struct irq_domain *d, struct irq_fwspec 
*fwspec,
+   enum irq_domain_bus_token bus_token)
+{
+   struct amd_iommu *iommu;
+   int devid = -1;
+
+   if (x86_fwspec_is_ioapic(fwspec))
+   devid = get_ioapic_devid(fwspec->param[0]);
+   else if (x86_fwspec_is_ioapic(fwspec))
+   devid = get_hpet_devid(fwspec->param[0]);
+
+   if (devid < 0)
+   return 0;
+
+   iommu = amd_iommu_rlookup_table[devid];
+   return iommu && iommu->ir_domain == d;
+}
+
 static void irq_remapping_prepare_irte(struct amd_ir_data *data,
   struct irq_cfg *irq_cfg,
   struct irq_alloc_info *info,
@@ -3813,6 +3831,7 @@ static void irq_remapping_deactivate(struct irq_domain 
*domain,
 }
 
 static const struct irq_domain_ops amd_ir_domain_ops = {
+   .select = irq_remapping_select,
.alloc = irq_remapping_alloc,
.free = irq_remapping_free,
.activate = irq_remapping_activate,
-- 
2.26.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 8/9] x86: Kill all traces of irq_remapping_get_irq_domain()

2020-10-13 Thread David Woodhouse
From: David Woodhouse 

Signed-off-by: David Woodhouse 
---
 arch/x86/include/asm/hw_irq.h|  2 --
 arch/x86/include/asm/irq_remapping.h |  9 
 drivers/iommu/amd/iommu.c| 34 
 drivers/iommu/hyperv-iommu.c |  9 
 drivers/iommu/intel/irq_remapping.c  | 17 --
 drivers/iommu/irq_remapping.c| 14 
 drivers/iommu/irq_remapping.h|  3 ---
 7 files changed, 88 deletions(-)

diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
index aabd8f1b6bb0..aef795f17478 100644
--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -40,8 +40,6 @@ enum irq_alloc_type {
X86_IRQ_ALLOC_TYPE_PCI_MSIX,
X86_IRQ_ALLOC_TYPE_DMAR,
X86_IRQ_ALLOC_TYPE_UV,
-   X86_IRQ_ALLOC_TYPE_IOAPIC_GET_PARENT,
-   X86_IRQ_ALLOC_TYPE_HPET_GET_PARENT,
 };
 
 struct ioapic_alloc_info {
diff --git a/arch/x86/include/asm/irq_remapping.h 
b/arch/x86/include/asm/irq_remapping.h
index af4a151d70b3..7cc49432187f 100644
--- a/arch/x86/include/asm/irq_remapping.h
+++ b/arch/x86/include/asm/irq_remapping.h
@@ -44,9 +44,6 @@ extern int irq_remapping_reenable(int);
 extern int irq_remap_enable_fault_handling(void);
 extern void panic_if_irq_remap(const char *msg);
 
-extern struct irq_domain *
-irq_remapping_get_irq_domain(struct irq_alloc_info *info);
-
 /* Create PCI MSI/MSIx irqdomain, use @parent as the parent irqdomain. */
 extern struct irq_domain *
 arch_create_remap_msi_irq_domain(struct irq_domain *par, const char *n, int 
id);
@@ -71,11 +68,5 @@ static inline void panic_if_irq_remap(const char *msg)
 {
 }
 
-static inline struct irq_domain *
-irq_remapping_get_irq_domain(struct irq_alloc_info *info)
-{
-   return NULL;
-}
-
 #endif /* CONFIG_IRQ_REMAP */
 #endif /* __X86_IRQ_REMAPPING_H */
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 7ecebc5d255f..16adbf9d8fbb 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3536,10 +3536,8 @@ static int get_devid(struct irq_alloc_info *info)
 {
switch (info->type) {
case X86_IRQ_ALLOC_TYPE_IOAPIC:
-   case X86_IRQ_ALLOC_TYPE_IOAPIC_GET_PARENT:
return get_ioapic_devid(info->devid);
case X86_IRQ_ALLOC_TYPE_HPET:
-   case X86_IRQ_ALLOC_TYPE_HPET_GET_PARENT:
return get_hpet_devid(info->devid);
case X86_IRQ_ALLOC_TYPE_PCI_MSI:
case X86_IRQ_ALLOC_TYPE_PCI_MSIX:
@@ -3550,44 +3548,12 @@ static int get_devid(struct irq_alloc_info *info)
}
 }
 
-static struct irq_domain *get_irq_domain_for_devid(struct irq_alloc_info *info,
-  int devid)
-{
-   struct amd_iommu *iommu = amd_iommu_rlookup_table[devid];
-
-   if (!iommu)
-   return NULL;
-
-   switch (info->type) {
-   case X86_IRQ_ALLOC_TYPE_IOAPIC_GET_PARENT:
-   case X86_IRQ_ALLOC_TYPE_HPET_GET_PARENT:
-   return iommu->ir_domain;
-   default:
-   WARN_ON_ONCE(1);
-   return NULL;
-   }
-}
-
-static struct irq_domain *get_irq_domain(struct irq_alloc_info *info)
-{
-   int devid;
-
-   if (!info)
-   return NULL;
-
-   devid = get_devid(info);
-   if (devid < 0)
-   return NULL;
-   return get_irq_domain_for_devid(info, devid);
-}
-
 struct irq_remap_ops amd_iommu_irq_ops = {
.prepare= amd_iommu_prepare,
.enable = amd_iommu_enable,
.disable= amd_iommu_disable,
.reenable   = amd_iommu_reenable,
.enable_faulting= amd_iommu_enable_faulting,
-   .get_irq_domain = get_irq_domain,
 };
 
 static int irq_remapping_select(struct irq_domain *d, struct irq_fwspec 
*fwspec,
diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
index 6a8966fbc3bd..e7ed2bb83358 100644
--- a/drivers/iommu/hyperv-iommu.c
+++ b/drivers/iommu/hyperv-iommu.c
@@ -160,18 +160,9 @@ static int __init hyperv_enable_irq_remapping(void)
return IRQ_REMAP_X2APIC_MODE;
 }
 
-static struct irq_domain *hyperv_get_irq_domain(struct irq_alloc_info *info)
-{
-   if (info->type == X86_IRQ_ALLOC_TYPE_IOAPIC_GET_PARENT)
-   return ioapic_ir_domain;
-   else
-   return NULL;
-}
-
 struct irq_remap_ops hyperv_irq_remap_ops = {
.prepare= hyperv_prepare_irq_remapping,
.enable = hyperv_enable_irq_remapping,
-   .get_irq_domain = hyperv_get_irq_domain,
 };
 
 #endif
diff --git a/drivers/iommu/intel/irq_remapping.c 
b/drivers/iommu/intel/irq_remapping.c
index 40c2fec122b8..ccf61cd18f69 100644
--- a/drivers/iommu/intel/irq_remapping.c
+++ b/drivers/iommu/intel/irq_remapping.c
@@ -1128,29 +1128,12 @@ static void prepare_irte(struct irte *irte, int vector, 
unsigned int dest)
irte->redir_hint = 1;
 }
 
-static 

[PATCH 2/9] x86/apic: Add select() method on vector irqdomain

2020-10-13 Thread David Woodhouse
From: David Woodhouse 

This will be used to select the irqdomain for I/OAPIC and HPET.

Signed-off-by: David Woodhouse 
---
 arch/x86/include/asm/irqdomain.h |  3 +++
 arch/x86/kernel/apic/vector.c| 43 
 2 files changed, 46 insertions(+)

diff --git a/arch/x86/include/asm/irqdomain.h b/arch/x86/include/asm/irqdomain.h
index cd684d45cb5f..125c23b7bad3 100644
--- a/arch/x86/include/asm/irqdomain.h
+++ b/arch/x86/include/asm/irqdomain.h
@@ -12,6 +12,9 @@ enum {
X86_IRQ_ALLOC_LEGACY= 0x2,
 };
 
+extern int x86_fwspec_is_ioapic(struct irq_fwspec *fwspec);
+extern int x86_fwspec_is_hpet(struct irq_fwspec *fwspec);
+
 extern struct irq_domain *x86_vector_domain;
 
 extern void init_irq_alloc_info(struct irq_alloc_info *info,
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index bb2e2a2488a5..b9b05caa28a4 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -636,7 +636,50 @@ static void x86_vector_debug_show(struct seq_file *m, 
struct irq_domain *d,
 }
 #endif
 
+int x86_fwspec_is_ioapic(struct irq_fwspec *fwspec)
+{
+   if (fwspec->param_count != 1)
+   return 0;
+
+   if (is_fwnode_irqchip(fwspec->fwnode)) {
+   const char *fwname = fwnode_get_name(fwspec->fwnode);
+   return fwname && !strncmp(fwname, "IO-APIC-", 8) &&
+   simple_strtol(fwname+8, NULL, 10) == fwspec->param[0];
+   }
+   return to_of_node(fwspec->fwnode) &&
+   of_device_is_compatible(to_of_node(fwspec->fwnode),
+   "intel,ce4100-ioapic");
+}
+
+int x86_fwspec_is_hpet(struct irq_fwspec *fwspec)
+{
+   if (fwspec->param_count != 1)
+   return 0;
+
+   if (is_fwnode_irqchip(fwspec->fwnode)) {
+   const char *fwname = fwnode_get_name(fwspec->fwnode);
+   return fwname && !strncmp(fwname, "HPET-MSI-", 9) &&
+   simple_strtol(fwname+9, NULL, 10) == fwspec->param[0];
+   }
+   return 0;
+}
+
+static int x86_vector_select(struct irq_domain *d, struct irq_fwspec *fwspec,
+enum irq_domain_bus_token bus_token)
+{
+   /*
+* HPET and I/OAPIC cannot be parented in the vector domain
+* if IRQ remapping is enabled. APIC IDs above 15 bits are
+* only permitted if IRQ remapping is enabled, so check that.
+*/
+   if (apic->apic_id_valid(32768))
+   return 0;
+
+   return x86_fwspec_is_ioapic(fwspec) || x86_fwspec_is_hpet(fwspec);
+}
+
 static const struct irq_domain_ops x86_vector_domain_ops = {
+   .select = x86_vector_select,
.alloc  = x86_vector_alloc_irqs,
.free   = x86_vector_free_irqs,
.activate   = x86_vector_activate,
-- 
2.26.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 02/24] dt-bindings: memory: mediatek: Convert SMI to DT schema

2020-10-13 Thread Yong Wu
On Mon, 2020-10-12 at 15:26 +0200, Krzysztof Kozlowski wrote:
> On Mon, 12 Oct 2020 at 14:02, Yong Wu  wrote:
> >
> > On Mon, 2020-10-12 at 09:18 +0200, Krzysztof Kozlowski wrote:
> > > On Sat, Oct 10, 2020 at 02:18:11PM +0800, Yong Wu wrote:
> > > > On Tue, 2020-10-06 at 09:15 +0200, Krzysztof Kozlowski wrote:
> > > > > On Tue, 6 Oct 2020 at 06:27, Yong Wu  wrote:
> > > > > >
> > > > > > On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote:
> > > > > > > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> > > > > > > > Convert MediaTek SMI to DT schema.
> > > > > > > >
> > > > > > > > Signed-off-by: Yong Wu 
> > > > > > > > ---
> > > > > > > >  .../mediatek,smi-common.txt   |  49 -
> > > > > > > >  .../mediatek,smi-common.yaml  | 100 
> > > > > > > > ++
> > > > > > > >  .../memory-controllers/mediatek,smi-larb.txt  |  49 -
> > > > > > > >  .../memory-controllers/mediatek,smi-larb.yaml |  91 
> > > > > > > > 
> > > > > > > >  4 files changed, 191 insertions(+), 98 deletions(-)
> > > > > > > >  delete mode 100644 
> > > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> > > > > > > >  create mode 100644 
> > > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > > > > > > >  delete mode 100644 
> > > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> > > > > > > >  create mode 100644 
> > > > > > > > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> > > > > > ...
> > > > > > > > +properties:
> > > > > > > > +  compatible:
> > > > > > > > +oneOf:
> > > > > > > > +  - enum:
> > > > > > > > +  - mediatek,mt2701-smi-common
> > > > > > > > +  - mediatek,mt2712-smi-common
> > > > > > > > +  - mediatek,mt6779-smi-common
> > > > > > > > +  - mediatek,mt8173-smi-common
> > > > > > > > +  - mediatek,mt8183-smi-common
> > > > > > > > +
> > > > > > > > +  - description: for mt7623
> > > > > > > > +items:
> > > > > > > > +  - const: mediatek,mt7623-smi-common
> > > > > > > > +  - const: mediatek,mt2701-smi-common
> > > > > > > > +
> > > > > > > > +  reg:
> > > > > > > > +maxItems: 1
> > > > > > > > +
> > > > > > > > +  clocks:
> > > > > > > > +description: |
> > > > > > > > +  apb and smi are mandatory. the async is only for 
> > > > > > > > generation 1 smi HW.
> > > > > > > > +  gals(global async local sync) also is optional, here is 
> > > > > > > > the list which
> > > > > > > > +  require gals: mt6779 and mt8183.
> > > > > > > > +minItems: 2
> > > > > > > > +maxItems: 4
> > > > > > > > +items:
> > > > > > > > +  - description: apb is Advanced Peripheral Bus clock, 
> > > > > > > > It's the clock for
> > > > > > > > +  setting the register.
> > > > > > > > +  - description: smi is the clock for transfer data and 
> > > > > > > > command.
> > > > > > > > +  - description: async is asynchronous clock, it help 
> > > > > > > > transform the smi clock
> > > > > > > > +  into the emi clock domain.
> > > > > > > > +  - description: gals0 is the path0 clock of gals.
> > > > > > > > +  - description: gals1 is the path1 clock of gals.
> > > > > > > > +
> > > > > > > > +  clock-names:
> > > > > > > > +oneOf:
> > > > > > > > +  - items:
> > > > > > > > +  - const: apb
> > > > > > > > +  - const: smi
> > > > > > > > +  - items:
> > > > > > > > +  - const: apb
> > > > > > > > +  - const: smi
> > > > > > > > +  - const: async
> > > > > > > > +  - items:
> > > > > > > > +  - const: apb
> > > > > > > > +  - const: smi
> > > > > > > > +  - const: gals0
> > > > > > > > +  - const: gals1
> > > > > > >
> > > > > > > Similarly to my comment to other properties, this requirement per
> > > > > > > compatible should be part of the schema within 'if-then'.
> > > > > >
> > > > > > I'm not so familiar with this format. Do this has "if-then-'else
> > > > > > if'-then-else"?
> > > > >
> > > > > These are mutually exclusive conditions, so you can skip else:
> > > > >  - if-then
> > > > >  - if-then
> > > > >  - if-then
> > > > > It will be more readable then stacking 'if' under 'else'
> > > >
> > > > Thanks. I will use something like this:
> > > >
> > > >  anyOf:
> > >
> > > Then it should be oneOf as only one condition can be valid.
> >
> > I did do this at the beginning. But I get a warning log when
> > dt_binding_check.
> 
> Mhmm, right, since "if-else" matches in either of arms, then oneOf
> will complain as it expects only one of items to match.  Then just go
> with allOf. anyOf might match zero of items, so it would not catch
> actual errors, I think.

Thanks for the confirm. I will use "allOf" in next version.

> 
> Best regards,
> Krzysztof


Re: [PATCH v3 01/24] dt-bindings: iommu: mediatek: Convert IOMMU to DT schema

2020-10-13 Thread Yong Wu
On Mon, 2020-10-12 at 19:08 +0200, Krzysztof Kozlowski wrote:
> On Tue, 6 Oct 2020 at 06:27, Yong Wu  wrote:
> >
> > On Fri, 2020-10-02 at 13:07 +0200, Krzysztof Kozlowski wrote:
> > > On Wed, Sep 30, 2020 at 03:06:24PM +0800, Yong Wu wrote:
> > > > Convert MediaTek IOMMU to DT schema.
> > > >
> > > > Signed-off-by: Yong Wu 
> > > > ---
> > > >  .../bindings/iommu/mediatek,iommu.txt | 103 
> > > >  .../bindings/iommu/mediatek,iommu.yaml| 154 ++
> > > >  2 files changed, 154 insertions(+), 103 deletions(-)
> > > >  delete mode 100644 
> > > > Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> > > >  create mode 100644 
> > > > Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> > > >
> >
> > [...]
> >
> > > > +properties:
> > > > +  compatible:
> > > > +oneOf:
> > > > +  - enum:
> > > > +  - mediatek,mt2701-m4u # mt2701 generation one HW
> > > > +  - mediatek,mt2712-m4u # mt2712 generation two HW
> > > > +  - mediatek,mt6779-m4u # mt6779 generation two HW
> > > > +  - mediatek,mt8173-m4u # mt8173 generation two HW
> > > > +  - mediatek,mt8183-m4u # mt8183 generation two HW
> > > > +
> > > > +  - description: mt7623 generation one HW
> > > > +items:
> > > > +  - const: mediatek,mt7623-m4u
> > > > +  - const: mediatek,mt2701-m4u
> > > > +
> > > > +  reg:
> > > > +maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +description: |
> > > > +  bclk is optional. here is the list which require this bclk:
> > > > +  mt2701, mt2712, mt7623 and mt8173.
> > >
> > > Similarly to my comment in other patch, this should be part of schema
> > > within 'if-then'.
> >
> > Thanks for the review.
> >
> > I will change like this:
> >
> > =
> >   clocks:
> > items:
> >   - description: bclk is the block clock.
> >
> >   clock-names:
> > items:
> >   - const: bclk
> >
> > required:
> >   - compatible
> >   - reg
> >   - interrupts
> >   - mediatek,larbs
> >   - '#iommu-cells'
> > if:
> >   properties:
> > compatible:
> >   contains:
> > enum:
> >   - mediatek,mt2701-m4u
> >   - mediatek,mt2712-m4u
> >   - mediatek,mt8173-m4u
> >
> > then:
> >  required:
> >- clocks
> > ==
> >
> > If this is not right, please tell me.
> > (dt_binding_check is ok.)
> 
> Looks fine, except "if" should be part of some "allOf" block.

Thanks very much for the review.
I will add "allOf" in next version.

> 
> Best regards,
> Krzysztof

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH next] iommu: intel: make DMAR_TABLE select IOMMU_API

2020-10-13 Thread Bartosz Golaszewski
On Tue, Oct 13, 2020 at 3:33 AM Lu Baolu  wrote:
>
> Hi,
>
> On 10/12/20 8:31 PM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski 
> >
> > Since commit c40c1018 ("iommu/vt-d: Gracefully handle DMAR units
> > with no supported address widths") dmar.c needs struct iommu_device to
> > be defined. We need to unconditionally select IOMMU_API when DMAR_TABLE
> > is selected. This fixes the following build error when IOMMU_API is not
> > selected:
> >
> > drivers/iommu/intel/dmar.c: In function ‘free_iommu’:
> > drivers/iommu/intel/dmar.c:1139:41: error: ‘struct iommu_device’ has no 
> > member named ‘ops’
> >   1139 |  if (intel_iommu_enabled && iommu->iommu.ops) {
>
> Thanks!
>
> How about making it symmetric with the registration code?
>
> if (intel_iommu_enabled && !iommu->drhd->ignored)
>
> Best regards,
> baolu
>

Sounds good, just sent out a patch.

Bartosz
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH next] iommu: intel: don't dereference iommu_device if IOMMU_API is not built

2020-10-13 Thread Bartosz Golaszewski
From: Bartosz Golaszewski 

Since commit c40c1018 ("iommu/vt-d: Gracefully handle DMAR units
with no supported address widths") dmar.c needs struct iommu_device to
be selected. We can drop this dependency by not dereferencing struct
iommu_device if IOMMU_API is not selected and by reusing the information
stored in iommu->drhd->ignored instead.

This fixes the following build error when IOMMU_API is not selected:

drivers/iommu/intel/dmar.c: In function ‘free_iommu’:
drivers/iommu/intel/dmar.c:1139:41: error: ‘struct iommu_device’ has no member 
named ‘ops’
 1139 |  if (intel_iommu_enabled && iommu->iommu.ops) {
^

Fixes: c40c1018 ("iommu/vt-d: Gracefully handle DMAR units with no 
supported address widths")
Signed-off-by: Bartosz Golaszewski 
---
 drivers/iommu/intel/dmar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 2d70d56d8e0d..404b40af31cb 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1136,7 +1136,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
 
 static void free_iommu(struct intel_iommu *iommu)
 {
-   if (intel_iommu_enabled && iommu->iommu.ops) {
+   if (intel_iommu_enabled && !iommu->drhd->ignored) {
iommu_device_unregister(>iommu);
iommu_device_sysfs_remove(>iommu);
}
-- 
2.28.0

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: (proposal) RE: [PATCH v7 00/16] vfio: expose virtual Shared Virtual Addressing to VMs

2020-10-13 Thread Jason Wang


On 2020/10/12 下午4:38, Tian, Kevin wrote:

From: Jason Wang 
Sent: Monday, September 14, 2020 12:20 PM


[...]
  > If it's possible, I would suggest a generic uAPI instead of a VFIO

specific one.

Jason suggest something like /dev/sva. There will be a lot of other
subsystems that could benefit from this (e.g vDPA).

Have you ever considered this approach?


Hi, Jason,

We did some study on this approach and below is the output. It's a
long writing but I didn't find a way to further abstract w/o losing
necessary context. Sorry about that.

Overall the real purpose of this series is to enable IOMMU nested
translation capability with vSVA as one major usage, through
below new uAPIs:
1) Report/enable IOMMU nested translation capability;
2) Allocate/free PASID;
3) Bind/unbind guest page table;
4) Invalidate IOMMU cache;
5) Handle IOMMU page request/response (not in this series);
1/3/4) is the minimal set for using IOMMU nested translation, with
the other two optional. For example, the guest may enable vSVA on
a device without using PASID. Or, it may bind its gIOVA page table
which doesn't require page fault support. Finally, all operations can
be applied to either physical device or subdevice.

Then we evaluated each uAPI whether generalizing it is a good thing
both in concept and regarding to complexity.

First, unlike other uAPIs which are all backed by iommu_ops, PASID
allocation/free is through the IOASID sub-system.



A question here, is IOASID expected to be the single management 
interface for PASID?


(I'm asking since there're already vendor specific IDA based PASID 
allocator e.g amdgpu_pasid_alloc())




  From this angle
we feel generalizing PASID management does make some sense.
First, PASID is just a number and not related to any device before
it's bound to a page table and IOMMU domain. Second, PASID is a
global resource (at least on Intel VT-d),



I think we need a definition of "global" here. It looks to me for vt-d 
the PASID table is per device.


Another question, is this possible to have two DMAR hardware unit(at 
least I can see two even in my laptop). In this case, is PASID still a 
global resource?




  while having separate VFIO/
VDPA allocation interfaces may easily cause confusion in userspace,
e.g. which interface to be used if both VFIO/VDPA devices exist.
Moreover, an unified interface allows centralized control over how
many PASIDs are allowed per process.



Yes.




One unclear part with this generalization is about the permission.
Do we open this interface to any process or only to those which
have assigned devices? If the latter, what would be the mechanism
to coordinate between this new interface and specific passthrough
frameworks?



I'm not sure, but if you just want a permission, you probably can 
introduce new capability (CAP_XXX) for this.




  A more tricky case, vSVA support on ARM (Eric/Jean
please correct me) plans to do per-device PASID namespace which
is built on a bind_pasid_table iommu callback to allow guest fully
manage its PASIDs on a given passthrough device.



I see, so I think the answer is to prepare for the namespace support 
from the start. (btw, I don't see how namespace is handled in current 
IOASID module?)




  I'm not sure
how such requirement can be unified w/o involving passthrough
frameworks, or whether ARM could also switch to global PASID
style...

Second, IOMMU nested translation is a per IOMMU domain
capability. Since IOMMU domains are managed by VFIO/VDPA
  (alloc/free domain, attach/detach device, set/get domain attribute,
etc.), reporting/enabling the nesting capability is an natural
extension to the domain uAPI of existing passthrough frameworks.
Actually, VFIO already includes a nesting enable interface even
before this series. So it doesn't make sense to generalize this uAPI
out.



So my understanding is that VFIO already:

1) use multiple fds
2) separate IOMMU ops to a dedicated container fd (type1 iommu)
3) provides API to associated devices/group with a container

And all the proposal in this series is to reuse the container fd. It 
should be possible to replace e.g type1 IOMMU with a unified module.





Then the tricky part comes with the remaining operations (3/4/5),
which are all backed by iommu_ops thus effective only within an
IOMMU domain. To generalize them, the first thing is to find a way
to associate the sva_FD (opened through generic /dev/sva) with an
IOMMU domain that is created by VFIO/VDPA. The second thing is
to replicate {domain<->device/subdevice} association in /dev/sva
path because some operations (e.g. page fault) is triggered/handled
per device/subdevice.



Is there any reason that the #PF can not be handled via SVA fd?



  Therefore, /dev/sva must provide both per-
domain and per-device uAPIs similar to what VFIO/VDPA already
does. Moreover, mapping page fault to subdevice requires pre-
registering subdevice fault data to IOMMU layer when binding
guest page