qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Minwoo Im <minwoo.im@samsung.com>
To: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	"zhenzhong.duan@intel.com" <zhenzhong.duan@intel.com>,
	"kevin.tian@intel.com" <kevin.tian@intel.com>,
	"yi.l.liu@intel.com" <yi.l.liu@intel.com>,
	"joao.m.martins@oracle.com" <joao.m.martins@oracle.com>,
	"peterx@redhat.com" <peterx@redhat.com>,
	"mst@redhat.com" <mst@redhat.com>,
	minwoo.im@samsung.com
Subject: Re: [PATCH ats_vtd v5 20/22] pci: add a pci-level API for ATS
Date: Wed, 10 Jul 2024 06:17:30 +0900	[thread overview]
Message-ID: <Zo2o6kkqpSgmj4yN@localhost> (raw)
In-Reply-To: <929fc1df-0a19-4da0-8399-5d792dd25684@eviden.com>

[-- Attachment #1: Type: text/plain, Size: 3805 bytes --]

On 24-07-09 11:58:53, CLEMENT MATHIEU--DRIF wrote:
> 
> 
> On 09/07/2024 12:15, Minwoo Im wrote:
> > Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
> >
> >
> > On 24-07-02 05:52:45, CLEMENT MATHIEU--DRIF wrote:
> >> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> >>
> >> Devices implementing ATS can send translation requests using
> >> pci_ats_request_translation_pasid.
> >>
> >> The invalidation events are sent back to the device using the iommu
> >> notifier managed with pci_register_iommu_tlb_event_notifier and
> >> pci_unregister_iommu_tlb_event_notifier
> >>
> >> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> >> ---
> >>   hw/pci/pci.c         | 44 +++++++++++++++++++++++++++++++++++++
> >>   include/hw/pci/pci.h | 52 ++++++++++++++++++++++++++++++++++++++++++++
> >>   2 files changed, 96 insertions(+)
> >>
> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >> index 7a483dd05d..93b816aff2 100644
> >> --- a/hw/pci/pci.c
> >> +++ b/hw/pci/pci.c
> >> @@ -2833,6 +2833,50 @@ void pci_device_unset_iommu_device(PCIDevice *dev)
> >>       }
> >>   }
> >>
> >> +ssize_t pci_ats_request_translation_pasid(PCIDevice *dev, uint32_t pasid,
> >> +                                          bool priv_req, bool exec_req,
> >> +                                          hwaddr addr, size_t length,
> >> +                                          bool no_write, IOMMUTLBEntry *result,
> >> +                                          size_t result_length,
> >> +                                          uint32_t *err_count)
> >> +{
> >> +    assert(result_length);
> >> +    IOMMUMemoryRegion *iommu_mr = pci_device_iommu_memory_region_pasid(dev,
> >> +                                                                        pasid);
> >> +    if (!iommu_mr || !pcie_ats_enabled(dev)) {
> >> +        return -EPERM;
> >> +    }
> >> +    return memory_region_iommu_ats_request_translation(iommu_mr, priv_req,
> >> +                                                       exec_req, addr, length,
> >> +                                                       no_write, result,
> >> +                                                       result_length,
> >> +                                                       err_count);
> >> +}
> > Can we use this function not from the endpoint PCI device, but inside of the pci
> > subsystem (hw/pci/pci.c) to make transparent abstraction for ATS request from
> > PCI endpoint device POV?  I guess it would be better to have PCI subsystem to
> > issue ATS request if pcie_ats_enabled(dev) rather than calling from the endpoint
> > side.
> Hi,
> 
> This series aims to bring support for SVM (we are trying to integrate 
> the patches bit by bit).
>  From a spec point of view, I don't know if it would make sense to 
> implement the SVM logic at the PCI level
> as it's supposed to be implemented by endpoint devices.

Understood that this series is targeting the SVM usage.  But ATS feature is
something general to PCI devices, not only just for SVM, so I guess it would be
better to have caller to `pci_ats_request_translation_pasid()` in pci subsystem
like pci_dma_rw() to avoid duplicated implementation in the future for the
other PCI enpoint devices.

> However, we could consider providing a reference/reusable/encapsulated 
> implementation of SVM with a simplified API
> that would call the pci_* functions under the hood.

I would prefer that PCI devices which want to request ATS translation has no
additional implementation for ATS, but only pcie_ats_init().

> 
> Do you have a specific use case in mind?

ATS/PRI is the actual use case, and it's not that different what you are
targeting for :)

> 
>  >cmd
> 
> >

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



  reply	other threads:[~2024-07-09 21:31 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-02  5:52 [PATCH ats_vtd v5 00/22] ATS support for VT-d CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 01/22] intel_iommu: fix FRCD construction macro CLEMENT MATHIEU--DRIF
2024-07-02 13:01   ` Yi Liu
2024-07-02 15:10     ` CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 02/22] intel_iommu: make types match CLEMENT MATHIEU--DRIF
2024-07-02 13:20   ` Yi Liu
2024-07-02  5:52 ` [PATCH ats_vtd v5 03/22] intel_iommu: return page walk level even when the translation fails CLEMENT MATHIEU--DRIF
2024-07-03 11:59   ` Yi Liu
2024-07-04  4:23     ` CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 04/22] intel_iommu: do not consider wait_desc as an invalid descriptor CLEMENT MATHIEU--DRIF
2024-07-02 13:33   ` Yi Liu
2024-07-02 15:29     ` CLEMENT MATHIEU--DRIF
2024-07-02 15:40       ` cmd
2024-07-03  7:29       ` Yi Liu
2024-07-03  8:28         ` cmd
2024-07-04  4:23         ` CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 05/22] memory: add permissions in IOMMUAccessFlags CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 06/22] pcie: add helper to declare PASID capability for a pcie device CLEMENT MATHIEU--DRIF
2024-07-03 12:04   ` Yi Liu
2024-07-04  4:25     ` CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 07/22] pcie: helper functions to check if PASID and ATS are enabled CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 08/22] intel_iommu: declare supported PASID size CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 09/22] pci: cache the bus mastering status in the device CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 11/22] memory: store user data pointer in the IOMMU notifiers CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 10/22] pci: add IOMMU operations to get address spaces and memory regions with PASID CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 12/22] pci: add a pci-level initialization function for iommu notifiers CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 13/22] intel_iommu: implement the get_address_space_pasid iommu operation CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 15/22] memory: Allow to store the PASID in IOMMUTLBEntry CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 14/22] intel_iommu: implement the get_memory_region_pasid iommu operation CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 16/22] intel_iommu: fill the PASID field when creating an instance of IOMMUTLBEntry CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 17/22] atc: generic ATC that can be used by PCIe devices that support SVM CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 18/22] atc: add unit tests CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 19/22] memory: add an API for ATS support CLEMENT MATHIEU--DRIF
2024-07-03 12:14   ` Yi Liu
2024-07-04  4:30     ` CLEMENT MATHIEU--DRIF
2024-07-04 12:52       ` Yi Liu
2024-07-02  5:52 ` [PATCH ats_vtd v5 20/22] pci: add a pci-level API for ATS CLEMENT MATHIEU--DRIF
2024-07-09 10:15   ` Minwoo Im
2024-07-09 11:58     ` CLEMENT MATHIEU--DRIF
2024-07-09 21:17       ` Minwoo Im [this message]
2024-07-10  5:17         ` CLEMENT MATHIEU--DRIF
2024-07-11  8:04           ` Minwoo Im
2024-07-11 19:00             ` CLEMENT MATHIEU--DRIF
2024-07-17 23:44               ` Minwoo Im
2024-07-18  7:46                 ` CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 22/22] intel_iommu: add support " CLEMENT MATHIEU--DRIF
2024-07-02  5:52 ` [PATCH ats_vtd v5 21/22] intel_iommu: set the address mask even when a translation fails CLEMENT MATHIEU--DRIF
2024-07-02 12:16 ` [PATCH ats_vtd v5 00/22] ATS support for VT-d Michael S. Tsirkin
2024-07-02 15:09   ` CLEMENT MATHIEU--DRIF
2024-07-02 13:44 ` Yi Liu
2024-07-02 15:12   ` CLEMENT MATHIEU--DRIF
2024-07-03 12:32 ` Yi Liu
2024-07-04  4:36   ` CLEMENT MATHIEU--DRIF
2024-07-04  8:14     ` Yi Liu
  -- strict thread matches above, loose matches on Subject: below --
2024-06-03  5:59 CLEMENT MATHIEU--DRIF
2024-06-03  5:59 ` [PATCH ats_vtd v5 20/22] pci: add a pci-level API for ATS CLEMENT MATHIEU--DRIF

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zo2o6kkqpSgmj4yN@localhost \
    --to=minwoo.im@samsung.com \
    --cc=clement.mathieu--drif@eviden.com \
    --cc=jasowang@redhat.com \
    --cc=joao.m.martins@oracle.com \
    --cc=kevin.tian@intel.com \
    --cc=mst@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yi.l.liu@intel.com \
    --cc=zhenzhong.duan@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).