Re: [PATCH 0/5] iommu/virtio: Add identity domains
On Mon, Oct 18, 2021 at 11:35 PM Michael S. Tsirkin wrote:
>
> On Mon, Oct 18, 2021 at 04:23:41PM +0100, Jean-Philippe Brucker wrote:
> > On Thu, Oct 14, 2021 at 03:00:38AM +, Tian, Kevin wrote:
> > > > From: Jean-Philippe Brucker
> > > > Sent: Wednesday, October 13, 2021 8:11 PM
> > > >
> > > > Support identity domains, allowing to only enable IOMMU protection for a
> > > > subset of endpoints (those assigned to userspace, for example). Users
> > > > may enable identity domains at compile time
> > > > (CONFIG_IOMMU_DEFAULT_PASSTHROUGH), boot time
> > > > (iommu.passthrough=1) or
> > > > runtime (/sys/kernel/iommu_groups/*/type = identity).
> > >
> > > Do we want to use consistent terms between spec (bypass domain)
> > > and code (identity domain)?
> >
> > I don't think we have to. Linux uses "identity" domains and "passthrough"
> > IOMMU. The old virtio-iommu feature was "bypass" so we should keep that
> > for the new one, to be consistent. And then I've used "bypass" for domains
> > as well, in the spec, because it would look strange to use a different
> > term for the same concept. I find that it sort of falls into place: Linux'
> > identity domains can be implemented either with bypass or identity-mapped
> > virtio-iommu domains.
> >
> > > >
> > > > Patches 1-2 support identity domains using the optional
> > > > VIRTIO_IOMMU_F_BYPASS_CONFIG feature. The feature bit is not yet in the
> > > > spec, see [1] for the latest proposal.
> > > >
> > > > Patches 3-5 add a fallback to identity mappings, when the feature is not
> > > > supported.
> > > >
> > > > Note that this series doesn't touch the global bypass bit added by
> > > > VIRTIO_IOMMU_F_BYPASS_CONFIG. All endpoints managed by the IOMMU
> > > > should
> > > > be attached to a domain, so global bypass isn't in use after endpoints
> > >
> > > I saw a concept of deferred attach in iommu core. See iommu_is_
> > > attach_deferred(). Currently this is vendor specific and I haven't
> > > looked into the exact reason why some vendor sets it now. Just
> > > be curious whether the same reason might be applied to virtio-iommu.
> > >
> > > > are probed. Before that, the global bypass policy is decided by the
> > > > hypervisor and firmware. So I don't think Linux needs to touch the
> > >
> > > This reminds me one thing. The spec says that the global bypass
> > > bit is sticky and not affected by reset.
> >
> > The spec talks about *device* reset, triggered by software writing 0 to
> > the status register, but it doesn't mention system reset. Would be good to
> > clarify that. Something like:
> >
> > If the device offers the VIRTIO_IOMMU_F_BYPASS_CONFIG feature, it MAY
> > initialize the \field{bypass} field to 1. Field \field{bypass} SHOULD
> > NOT change on device reset, but SHOULD be restored to its initial
> > value on system reset.
> >
> > > This implies that in the case
> > > of rebooting the VM into a different OS, the previous OS actually
> > > has the right to override this setting for the next OS. Is it a right
> > > design? Even the firmware itself is unable to identify the original
> > > setting enforced by the hypervisor after reboot. I feel the hypervisor
> > > setting should be recovered after reset since it reflects the
> > > security measure enforced by the virtual platform?
> >
> > So I think clarifying system reset should address your questions.
> > I believe we should leave bypass sticky across device reset, so a FW->OS
> > transition, where the OS resets the device, does not open a vulnerability
> > (if bypass was enabled at boot and then left disabled by FW.)
> >
> > It's still a good idea for the OS to restore on shutdown the bypass value
> > it was booted with. So it can kexec into an OS that doesn't support
> > virtio-iommu, for example.
> >
> > Thanks,
> > Jean
>
> Is this stickiness really important? Can't this be addressed just by
> hypervisor disabling bypass at boot?
And I'm not sure if sticky can survive transport reset.
Thanks
>
> --
> MST
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/9] [PULL REQUEST] Intel IOMMU Updates for Linux v5.16
On Thu, Oct 14, 2021 at 01:38:30PM +0800, Lu Baolu wrote: > Fenghua Yu (1): > iommu/vt-d: Clean up unused PASID updating functions > > Kyung Min Park (1): > iommu/vt-d: Dump DMAR translation structure when DMA fault occurs > > Longpeng(Mike) (2): > iommu/vt-d: Convert the return type of first_pte_in_page to bool > iommu/vt-d: Avoid duplicate removing in __domain_mapping() > > Lu Baolu (4): > iommu/vt-d: Remove duplicate identity domain flag > iommu/vt-d: Check FL and SL capability sanity in scalable mode > iommu/vt-d: Use second level for GPA->HPA translation > iommu/vt-d: Delete dev_has_feat callback > > Tvrtko Ursulin (1): > iommu/vt-d: Do not falsely log intel_iommu is unsupported kernel > option > > include/linux/dmar.h| 8 ++ > include/linux/intel-iommu.h | 13 +- > arch/x86/include/asm/fpu/api.h | 2 - > drivers/iommu/intel/cap_audit.h | 1 + > drivers/iommu/intel/cap_audit.c | 13 ++ > drivers/iommu/intel/dmar.c | 10 +- > drivers/iommu/intel/iommu.c | 213 ++-- > drivers/iommu/intel/svm.c | 24 +--- > drivers/iommu/intel/Kconfig | 4 + > 9 files changed, 188 insertions(+), 100 deletions(-) Applied, thanks Baolu. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC 11/20] iommu/iommufd: Add IOMMU_IOASID_ALLOC/FREE
On Mon, Oct 18, 2021 at 02:50:54PM +1100, David Gibson wrote: > Hrm... which makes me think... if we allow this for the common > kernel-managed case, do we even need to have capcity in the high-level > interface for reporting IO holes? If the kernel can choose a non-zero > base, it could just choose on x86 to place it's advertised window > above the IO hole. If the high level interface is like dma_map() then, no it doesn't need the ability to report holes. Kernel would find and return the IOVA from dma_map not accept it in. Since dma_map is a well proven model I'm inclined to model the simplied interface after it.. That said, if we have some ioctl 'query iova ranges' I would expect it to work on an IOAS created by the simplified interface too. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC 13/20] iommu: Extend iommu_at[de]tach_device() for multiple devices group
On Mon, Oct 18, 2021 at 02:57:12PM +1100, David Gibson wrote: > The first user might read this. Subsequent users are likely to just > copy paste examples from earlier things without fully understanding > them. In general documenting restrictions somewhere is never as > effective as making those restrictions part of the interface signature > itself. I'd think this argument would hold more water if you could point to someplace in existing userspace that cares about the VFIO grouping. >From what I see the applications do what the admin tells them to do - and if the admin says to use a certain VFIO device then that is excatly what they do. I don't know of any applications that ask the admin to tell them group information. What I see is aligning what the kernel provides to the APIs the applications have already built. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/5] iova: Move fast alloc size roundup into alloc_iova_fast()
On Fri, Sep 24, 2021 at 06:01:53PM +0800, John Garry wrote: > It really is a property of the IOVA rcache code that we need to alloc a > power-of-2 size, so relocate the functionality to resize into > alloc_iova_fast(), rather than the callsites. > > Signed-off-by: John Garry for vdpa code: Acked-by: Michael S. Tsirkin > --- > drivers/iommu/dma-iommu.c| 8 > drivers/iommu/iova.c | 9 + > drivers/vdpa/vdpa_user/iova_domain.c | 8 > 3 files changed, 9 insertions(+), 16 deletions(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 896bea04c347..a99b3445fef8 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -444,14 +444,6 @@ static dma_addr_t iommu_dma_alloc_iova(struct > iommu_domain *domain, > > shift = iova_shift(iovad); > iova_len = size >> shift; > - /* > - * Freeing non-power-of-two-sized allocations back into the IOVA caches > - * will come back to bite us badly, so we have to waste a bit of space > - * rounding up anything cacheable to make sure that can't happen. The > - * order of the unadjusted size will still match upon freeing. > - */ > - if (iova_len < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1))) > - iova_len = roundup_pow_of_two(iova_len); > > dma_limit = min_not_zero(dma_limit, dev->bus_dma_limit); > > diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c > index 9e8bc802ac05..ff567cbc42f7 100644 > --- a/drivers/iommu/iova.c > +++ b/drivers/iommu/iova.c > @@ -497,6 +497,15 @@ alloc_iova_fast(struct iova_domain *iovad, unsigned long > size, > unsigned long iova_pfn; > struct iova *new_iova; > > + /* > + * Freeing non-power-of-two-sized allocations back into the IOVA caches > + * will come back to bite us badly, so we have to waste a bit of space > + * rounding up anything cacheable to make sure that can't happen. The > + * order of the unadjusted size will still match upon freeing. > + */ > + if (size < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1))) > + size = roundup_pow_of_two(size); > + > iova_pfn = iova_rcache_get(iovad, size, limit_pfn + 1); > if (iova_pfn) > return iova_pfn; > diff --git a/drivers/vdpa/vdpa_user/iova_domain.c > b/drivers/vdpa/vdpa_user/iova_domain.c > index 1daae2608860..2b1143f11d8f 100644 > --- a/drivers/vdpa/vdpa_user/iova_domain.c > +++ b/drivers/vdpa/vdpa_user/iova_domain.c > @@ -292,14 +292,6 @@ vduse_domain_alloc_iova(struct iova_domain *iovad, > unsigned long iova_len = iova_align(iovad, size) >> shift; > unsigned long iova_pfn; > > - /* > - * Freeing non-power-of-two-sized allocations back into the IOVA caches > - * will come back to bite us badly, so we have to waste a bit of space > - * rounding up anything cacheable to make sure that can't happen. The > - * order of the unadjusted size will still match upon freeing. > - */ > - if (iova_len < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1))) > - iova_len = roundup_pow_of_two(iova_len); > iova_pfn = alloc_iova_fast(iovad, iova_len, limit >> shift, true); > > return iova_pfn << shift; > -- > 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/5] iommu/virtio: Add identity domains
On Mon, Oct 18, 2021 at 04:23:41PM +0100, Jean-Philippe Brucker wrote:
> On Thu, Oct 14, 2021 at 03:00:38AM +, Tian, Kevin wrote:
> > > From: Jean-Philippe Brucker
> > > Sent: Wednesday, October 13, 2021 8:11 PM
> > >
> > > Support identity domains, allowing to only enable IOMMU protection for a
> > > subset of endpoints (those assigned to userspace, for example). Users
> > > may enable identity domains at compile time
> > > (CONFIG_IOMMU_DEFAULT_PASSTHROUGH), boot time
> > > (iommu.passthrough=1) or
> > > runtime (/sys/kernel/iommu_groups/*/type = identity).
> >
> > Do we want to use consistent terms between spec (bypass domain)
> > and code (identity domain)?
>
> I don't think we have to. Linux uses "identity" domains and "passthrough"
> IOMMU. The old virtio-iommu feature was "bypass" so we should keep that
> for the new one, to be consistent. And then I've used "bypass" for domains
> as well, in the spec, because it would look strange to use a different
> term for the same concept. I find that it sort of falls into place: Linux'
> identity domains can be implemented either with bypass or identity-mapped
> virtio-iommu domains.
>
> > >
> > > Patches 1-2 support identity domains using the optional
> > > VIRTIO_IOMMU_F_BYPASS_CONFIG feature. The feature bit is not yet in the
> > > spec, see [1] for the latest proposal.
> > >
> > > Patches 3-5 add a fallback to identity mappings, when the feature is not
> > > supported.
> > >
> > > Note that this series doesn't touch the global bypass bit added by
> > > VIRTIO_IOMMU_F_BYPASS_CONFIG. All endpoints managed by the IOMMU
> > > should
> > > be attached to a domain, so global bypass isn't in use after endpoints
> >
> > I saw a concept of deferred attach in iommu core. See iommu_is_
> > attach_deferred(). Currently this is vendor specific and I haven't
> > looked into the exact reason why some vendor sets it now. Just
> > be curious whether the same reason might be applied to virtio-iommu.
> >
> > > are probed. Before that, the global bypass policy is decided by the
> > > hypervisor and firmware. So I don't think Linux needs to touch the
> >
> > This reminds me one thing. The spec says that the global bypass
> > bit is sticky and not affected by reset.
>
> The spec talks about *device* reset, triggered by software writing 0 to
> the status register, but it doesn't mention system reset. Would be good to
> clarify that. Something like:
>
> If the device offers the VIRTIO_IOMMU_F_BYPASS_CONFIG feature, it MAY
> initialize the \field{bypass} field to 1. Field \field{bypass} SHOULD
> NOT change on device reset, but SHOULD be restored to its initial
> value on system reset.
>
> > This implies that in the case
> > of rebooting the VM into a different OS, the previous OS actually
> > has the right to override this setting for the next OS. Is it a right
> > design? Even the firmware itself is unable to identify the original
> > setting enforced by the hypervisor after reboot. I feel the hypervisor
> > setting should be recovered after reset since it reflects the
> > security measure enforced by the virtual platform?
>
> So I think clarifying system reset should address your questions.
> I believe we should leave bypass sticky across device reset, so a FW->OS
> transition, where the OS resets the device, does not open a vulnerability
> (if bypass was enabled at boot and then left disabled by FW.)
>
> It's still a good idea for the OS to restore on shutdown the bypass value
> it was booted with. So it can kexec into an OS that doesn't support
> virtio-iommu, for example.
>
> Thanks,
> Jean
Is this stickiness really important? Can't this be addressed just by
hypervisor disabling bypass at boot?
--
MST
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/5] iommu/virtio: Add identity domains
On Thu, Oct 14, 2021 at 03:00:38AM +, Tian, Kevin wrote:
> > From: Jean-Philippe Brucker
> > Sent: Wednesday, October 13, 2021 8:11 PM
> >
> > Support identity domains, allowing to only enable IOMMU protection for a
> > subset of endpoints (those assigned to userspace, for example). Users
> > may enable identity domains at compile time
> > (CONFIG_IOMMU_DEFAULT_PASSTHROUGH), boot time
> > (iommu.passthrough=1) or
> > runtime (/sys/kernel/iommu_groups/*/type = identity).
>
> Do we want to use consistent terms between spec (bypass domain)
> and code (identity domain)?
I don't think we have to. Linux uses "identity" domains and "passthrough"
IOMMU. The old virtio-iommu feature was "bypass" so we should keep that
for the new one, to be consistent. And then I've used "bypass" for domains
as well, in the spec, because it would look strange to use a different
term for the same concept. I find that it sort of falls into place: Linux'
identity domains can be implemented either with bypass or identity-mapped
virtio-iommu domains.
> >
> > Patches 1-2 support identity domains using the optional
> > VIRTIO_IOMMU_F_BYPASS_CONFIG feature. The feature bit is not yet in the
> > spec, see [1] for the latest proposal.
> >
> > Patches 3-5 add a fallback to identity mappings, when the feature is not
> > supported.
> >
> > Note that this series doesn't touch the global bypass bit added by
> > VIRTIO_IOMMU_F_BYPASS_CONFIG. All endpoints managed by the IOMMU
> > should
> > be attached to a domain, so global bypass isn't in use after endpoints
>
> I saw a concept of deferred attach in iommu core. See iommu_is_
> attach_deferred(). Currently this is vendor specific and I haven't
> looked into the exact reason why some vendor sets it now. Just
> be curious whether the same reason might be applied to virtio-iommu.
>
> > are probed. Before that, the global bypass policy is decided by the
> > hypervisor and firmware. So I don't think Linux needs to touch the
>
> This reminds me one thing. The spec says that the global bypass
> bit is sticky and not affected by reset.
The spec talks about *device* reset, triggered by software writing 0 to
the status register, but it doesn't mention system reset. Would be good to
clarify that. Something like:
If the device offers the VIRTIO_IOMMU_F_BYPASS_CONFIG feature, it MAY
initialize the \field{bypass} field to 1. Field \field{bypass} SHOULD
NOT change on device reset, but SHOULD be restored to its initial
value on system reset.
> This implies that in the case
> of rebooting the VM into a different OS, the previous OS actually
> has the right to override this setting for the next OS. Is it a right
> design? Even the firmware itself is unable to identify the original
> setting enforced by the hypervisor after reboot. I feel the hypervisor
> setting should be recovered after reset since it reflects the
> security measure enforced by the virtual platform?
So I think clarifying system reset should address your questions.
I believe we should leave bypass sticky across device reset, so a FW->OS
transition, where the OS resets the device, does not open a vulnerability
(if bypass was enabled at boot and then left disabled by FW.)
It's still a good idea for the OS to restore on shutdown the bypass value
it was booted with. So it can kexec into an OS that doesn't support
virtio-iommu, for example.
Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/5] [RFC]iommu: Add a IOMMU_DEVONLY protection flag
Hello Joerg, On Mon, 18 Oct 2021 12:25:38 +0200 Joerg Roedel wrote: > On Fri, Oct 01, 2021 at 04:34:23PM +0200, Boris Brezillon wrote: > > +/* > > + * Mapping is only accessed by the device behind the iommu. That means > > other > > + * devices or CPUs are not expected to access this physical memory region, > > + * and the MMU driver can safely restrict the shareability domain to the > > + * device itself. > > + */ > > +#define IOMMU_DEVONLY (1 << 6) > > I am not entirely happy with the name, how about > > IOMMU_DEV_PRIVATE? Works for me. > > PRIV would conflict with IOMMU_PRIV (which should probably also be > IOMMU_PRIVILEGED, but thats another problem). Yeah, IOMMU_PRIV is confusing. I thought I could use that flag before realizing PRIV was for privileged not private, but I'll leave that to someone else :-). Regards, Boris ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Use devm_bitmap_zalloc when applicable
On Sun, Sep 26, 2021 at 03:07:18PM +0200, Christophe JAILLET wrote: > 'smmu->asids' is a bitmap. So use 'devm_kzalloc()' to simplify code, > improve the semantic of the code and avoid some open-coded arithmetic in > allocator arguments. > > Signed-off-by: Christophe JAILLET > --- > drivers/iommu/tegra-smmu.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/5] iommu/virtio: Add identity domains
On Thu, Oct 14, 2021 at 03:00:38AM +, Tian, Kevin wrote: > I saw a concept of deferred attach in iommu core. See iommu_is_ > attach_deferred(). Currently this is vendor specific and I haven't > looked into the exact reason why some vendor sets it now. Just > be curious whether the same reason might be applied to virtio-iommu. The reason for attach_deferred is kdump support, where the IOMMU driver needs to keep the mappings from the old kernel until the device driver of the new kernel takes over. Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dart: use kmemdup instead of kzalloc and memcpy
On Wed, Oct 13, 2021 at 02:34:41AM -0400, Wan Jiabing wrote: > Fix following coccicheck warning: > drivers/iommu/apple-dart.c:704:20-27: WARNING opportunity for kmemdup > > Signed-off-by: Wan Jiabing > --- > drivers/iommu/apple-dart.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-debug: teach add_dma_entry() about DMA_ATTR_SKIP_CPU_SYNC
Thanks, applied. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/5] [RFC]iommu: Add a IOMMU_DEVONLY protection flag
On Fri, Oct 01, 2021 at 04:34:23PM +0200, Boris Brezillon wrote: > +/* > + * Mapping is only accessed by the device behind the iommu. That means other > + * devices or CPUs are not expected to access this physical memory region, > + * and the MMU driver can safely restrict the shareability domain to the > + * device itself. > + */ > +#define IOMMU_DEVONLY(1 << 6) I am not entirely happy with the name, how about IOMMU_DEV_PRIVATE? PRIV would conflict with IOMMU_PRIV (which should probably also be IOMMU_PRIVILEGED, but thats another problem). Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC] virtio: wrap config->reset calls
On Wed, Oct 13, 2021 at 06:55:31AM -0400, Michael S. Tsirkin wrote: This will enable cleanups down the road. The idea is to disable cbs, then add "flush_queued_cbs" callback as a parameter, this way drivers can flush any work queued after callbacks have been disabled. Signed-off-by: Michael S. Tsirkin --- arch/um/drivers/virt-pci.c | 2 +- drivers/block/virtio_blk.c | 4 ++-- drivers/bluetooth/virtio_bt.c | 2 +- drivers/char/hw_random/virtio-rng.c| 2 +- drivers/char/virtio_console.c | 4 ++-- drivers/crypto/virtio/virtio_crypto_core.c | 8 drivers/firmware/arm_scmi/virtio.c | 2 +- drivers/gpio/gpio-virtio.c | 2 +- drivers/gpu/drm/virtio/virtgpu_kms.c | 2 +- drivers/i2c/busses/i2c-virtio.c| 2 +- drivers/iommu/virtio-iommu.c | 2 +- drivers/net/caif/caif_virtio.c | 2 +- drivers/net/virtio_net.c | 4 ++-- drivers/net/wireless/mac80211_hwsim.c | 2 +- drivers/nvdimm/virtio_pmem.c | 2 +- drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- drivers/scsi/virtio_scsi.c | 2 +- drivers/virtio/virtio.c| 5 + drivers/virtio/virtio_balloon.c| 2 +- drivers/virtio/virtio_input.c | 2 +- drivers/virtio/virtio_mem.c| 2 +- fs/fuse/virtio_fs.c| 4 ++-- include/linux/virtio.h | 1 + net/9p/trans_virtio.c | 2 +- net/vmw_vsock/virtio_transport.c | 4 ++-- Reviewed-by: Stefano Garzarella ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC] virtio: wrap config->reset calls
On Wed, Oct 13, 2021 at 06:55:31AM -0400, Michael S. Tsirkin wrote: > This will enable cleanups down the road. > The idea is to disable cbs, then add "flush_queued_cbs" callback > as a parameter, this way drivers can flush any work > queued after callbacks have been disabled. > > Signed-off-by: Michael S. Tsirkin > --- > arch/um/drivers/virt-pci.c | 2 +- > drivers/block/virtio_blk.c | 4 ++-- > drivers/bluetooth/virtio_bt.c | 2 +- > drivers/char/hw_random/virtio-rng.c| 2 +- > drivers/char/virtio_console.c | 4 ++-- > drivers/crypto/virtio/virtio_crypto_core.c | 8 > drivers/firmware/arm_scmi/virtio.c | 2 +- > drivers/gpio/gpio-virtio.c | 2 +- > drivers/gpu/drm/virtio/virtgpu_kms.c | 2 +- > drivers/i2c/busses/i2c-virtio.c| 2 +- > drivers/iommu/virtio-iommu.c | 2 +- > drivers/net/caif/caif_virtio.c | 2 +- > drivers/net/virtio_net.c | 4 ++-- > drivers/net/wireless/mac80211_hwsim.c | 2 +- > drivers/nvdimm/virtio_pmem.c | 2 +- > drivers/rpmsg/virtio_rpmsg_bus.c | 2 +- > drivers/scsi/virtio_scsi.c | 2 +- > drivers/virtio/virtio.c| 5 + > drivers/virtio/virtio_balloon.c| 2 +- > drivers/virtio/virtio_input.c | 2 +- > drivers/virtio/virtio_mem.c| 2 +- > fs/fuse/virtio_fs.c| 4 ++-- > include/linux/virtio.h | 1 + > net/9p/trans_virtio.c | 2 +- > net/vmw_vsock/virtio_transport.c | 4 ++-- > sound/virtio/virtio_card.c | 4 ++-- > 26 files changed, 39 insertions(+), 33 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 04/12] iommu/mediatek: Add device_link between the consumer and the larb devices
On 16.10.21 04:23, Yong Wu wrote:
On Mon, 2021-10-11 at 14:36 +0200, Dafna Hirschfeld wrote:
On 29.09.21 03:37, Yong Wu wrote:
MediaTek IOMMU-SMI diagram is like below. all the consumer connect
with
smi-larb, then connect with smi-common.
M4U
|
smi-common
|
-
| |...
| |
larb1 larb2
| |
vdec venc
When the consumer works, it should enable the smi-larb's power
which
also need enable the smi-common's power firstly.
Thus, First of all, use the device link connect the consumer and
the
smi-larbs. then add device link between the smi-larb and smi-
common.
This patch adds device_link between the consumer and the larbs.
When device_link_add, I add the flag DL_FLAG_STATELESS to avoid
calling
pm_runtime_xx to keep the original status of clocks. It can avoid
two
issues:
1) Display HW show fastlogo abnormally reported in [1]. At the
beggining,
all the clocks are enabled before entering kernel, but the clocks
for
display HW(always in larb0) will be gated after clk_enable and
clk_disable
called from device_link_add(->pm_runtime_resume) and rpm_idle. The
clock
operation happened before display driver probe. At that time, the
display
HW will be abnormal.
2) A deadlock issue reported in [2]. Use DL_FLAG_STATELESS to skip
pm_runtime_xx to avoid the deadlock.
Corresponding, DL_FLAG_AUTOREMOVE_CONSUMER can't be added, then
device_link_removed should be added explicitly.
[1]
https://lore.kernel.org/linux-mediatek/1564213888.22908.4.camel@mhfsdcap03/
[2] https://lore.kernel.org/patchwork/patch/1086569/
Suggested-by: Tomasz Figa
Signed-off-by: Yong Wu
Tested-by: Frank Wunderlich # BPI-
R2/MT7623
---
drivers/iommu/mtk_iommu.c| 22 ++
drivers/iommu/mtk_iommu_v1.c | 20 +++-
2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index d5848f78a677..a2fa55899434 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -560,22 +560,44 @@ static struct iommu_device
*mtk_iommu_probe_device(struct device *dev)
{
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
struct mtk_iommu_data *data;
+ struct device_link *link;
+ struct device *larbdev;
+ unsigned int larbid;
if (!fwspec || fwspec->ops != _iommu_ops)
return ERR_PTR(-ENODEV); /* Not a iommu client device
*/
data = dev_iommu_priv_get(dev);
+ /*
+* Link the consumer device with the smi-larb device(supplier)
+* The device in each a larb is a independent HW. thus only
link
+* one larb here.
+*/
+ larbid = MTK_M4U_TO_LARB(fwspec->ids[0]);
so larbid is always the same for all the ids of a device?
Yes. For me, each a dtsi node should represent a HW unit which can only
connect one larb.
If so maybe it worth testing it and return error if this is not the
case.
Thanks for the suggestion. This is very helpful. I did see someone put
the different larbs in one node. I will check this, and add return
I am working on bugs found on media drivers, could you please point me to
that wrong node?
Will you send a fix to that node in the dtsi?
Thanks,
Dafna
EINVAL for this case.
Thanks,
Dafna
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
