diff mbox series

[3/8] ACPI/IORT: Support CANWBS memory access flag

Message ID 3-v1-54e734311a7f+14f72-smmuv3_nesting_jgg@nvidia.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series Initial support for SMMUv3 nested translation | expand

Commit Message

Jason Gunthorpe Aug. 6, 2024, 11:41 p.m. UTC 
From: Nicolin Chen <nicolinc@nvidia.com>

The IORT spec, Issue E.f (April 2024), adds a new CANWBS bit to the Memory
Access Flag field in the Memory Access Properties table, mainly for a PCI
Root Complex.

This CANWBS defines the coherency of memory accesses to be not marked IOWB
cacheable/shareable. Its value further implies the coherency impact from a
pair of mismatched memory attributes (e.g. in a nested translation case):
  0x0: Use of mismatched memory attributes for accesses made by this
       device may lead to a loss of coherency.
  0x1: Coherency of accesses made by this device to locations in
       Conventional memory are ensured as follows, even if the memory
       attributes for the accesses presented by the device or provided by
       the SMMU are different from Inner and Outer Write-back cacheable,
       Shareable.

Note that the loss of coherency on a CANWBS-unsupported HW typically could
occur to an SMMU that doesn't implement the S2FWB feature where additional
cache flush operations would be required to prevent that from happening.

Add a new ACPI_IORT_MF_CANWBS flag and set IOMMU_FWSPEC_PCI_RC_CANWBS upon
the presence of this new flag.

CANWBS and S2FWB are similar features, in that they both guarantee the VM
can not violate coherency, however S2FWB can be bypassed by PCI No Snoop
TLPs, while CANWBS cannot. Thus CANWBS meets the requirements to set
IOMMU_CAP_ENFORCE_CACHE_COHERENCY.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/acpi/arm64/iort.c | 13 +++++++++++++
 include/acpi/actbl2.h     |  1 +
 include/linux/iommu.h     |  2 ++
 3 files changed, 16 insertions(+)

Comments

Shameerali Kolothum Thodi Aug. 9, 2024, 2:36 p.m. UTC | #1 
> -----Original Message-----
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, August 7, 2024 12:41 AM
> To: acpica-devel@lists.linux.dev; Alex Williamson
> <alex.williamson@redhat.com>; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; iommu@lists.linux.dev; Joerg Roedel
> <joro@8bytes.org>; Kevin Tian <kevin.tian@intel.com>; kvm@vger.kernel.org;
> Len Brown <lenb@kernel.org>; linux-acpi@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; Lorenzo Pieralisi <lpieralisi@kernel.org>; Rafael J.
> Wysocki <rafael@kernel.org>; Robert Moore <robert.moore@intel.com>; Robin
> Murphy <robin.murphy@arm.com>; Sudeep Holla <sudeep.holla@arm.com>;
> Will Deacon <will@kernel.org>
> Cc: Eric Auger <eric.auger@redhat.com>; Jean-Philippe Brucker <jean-
> philippe@linaro.org>; Moritz Fischer <mdf@kernel.org>; Michael Shavit
> <mshavit@google.com>; Nicolin Chen <nicolinc@nvidia.com>;
> patches@lists.linux.dev; Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>
> Subject: [PATCH 3/8] ACPI/IORT: Support CANWBS memory access flag
> 
> From: Nicolin Chen <nicolinc@nvidia.com>
> 
> The IORT spec, Issue E.f (April 2024), adds a new CANWBS bit to the Memory
> Access Flag field in the Memory Access Properties table, mainly for a PCI
> Root Complex.
...
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index e27958ef82642f..56ce7fc35312c8 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -524,6 +524,7 @@ struct acpi_iort_memory_access {
> 
>  #define ACPI_IORT_MF_COHERENCY          (1)
>  #define ACPI_IORT_MF_ATTRIBUTES         (1<<1)
> +#define ACPI_IORT_MF_CANWBS             (1<<2)

I think we need to update Document number to E.f in IORT section in 
this file. Also isn't it this file normally gets updated through ACPICA pull ?

Thanks,
Shameer
Jason Gunthorpe Aug. 9, 2024, 2:59 p.m. UTC | #2 
On Fri, Aug 09, 2024 at 02:36:31PM +0000, Shameerali Kolothum Thodi wrote:
> > @@ -524,6 +524,7 @@ struct acpi_iort_memory_access {
> > 
> >  #define ACPI_IORT_MF_COHERENCY          (1)
> >  #define ACPI_IORT_MF_ATTRIBUTES         (1<<1)
> > +#define ACPI_IORT_MF_CANWBS             (1<<2)
> 
> I think we need to update Document number to E.f in IORT section in 
> this file. Also isn't it this file normally gets updated through ACPICA pull ?

I don't know anything about the ACPI process..

Can someone say for sure what to do here?

Jason
Shameerali Kolothum Thodi Aug. 9, 2024, 3:15 p.m. UTC | #3 
> -----Original Message-----
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Friday, August 9, 2024 3:59 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: acpica-devel@lists.linux.dev; Alex Williamson
> <alex.williamson@redhat.com>; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; iommu@lists.linux.dev; Joerg Roedel
> <joro@8bytes.org>; Kevin Tian <kevin.tian@intel.com>; kvm@vger.kernel.org;
> Len Brown <lenb@kernel.org>; linux-acpi@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; Lorenzo Pieralisi <lpieralisi@kernel.org>; Rafael J.
> Wysocki <rafael@kernel.org>; Robert Moore <robert.moore@intel.com>; Robin
> Murphy <robin.murphy@arm.com>; Sudeep Holla <sudeep.holla@arm.com>;
> Will Deacon <will@kernel.org>; Eric Auger <eric.auger@redhat.com>; Jean-
> Philippe Brucker <jean-philippe@linaro.org>; Moritz Fischer <mdf@kernel.org>;
> Michael Shavit <mshavit@google.com>; Nicolin Chen <nicolinc@nvidia.com>;
> patches@lists.linux.dev
> Subject: Re: [PATCH 3/8] ACPI/IORT: Support CANWBS memory access flag
> 
> On Fri, Aug 09, 2024 at 02:36:31PM +0000, Shameerali Kolothum Thodi wrote:
> > > @@ -524,6 +524,7 @@ struct acpi_iort_memory_access {
> > >
> > >  #define ACPI_IORT_MF_COHERENCY          (1)
> > >  #define ACPI_IORT_MF_ATTRIBUTES         (1<<1)
> > > +#define ACPI_IORT_MF_CANWBS             (1<<2)
> >
> > I think we need to update Document number to E.f in IORT section in
> > this file. Also isn't it this file normally gets updated through ACPICA pull ?
> 
> I don't know anything about the ACPI process..
> 
> Can someone say for sure what to do here?

From past experience, it is normally sending a PULL request to here,
https://github.com/acpica/acpica/pulls

And I think it then gets merged by Robert Moore and then send to LKML.

Thanks,
Shameer
Nicolin Chen Aug. 9, 2024, 8:14 p.m. UTC | #4 
On Fri, Aug 09, 2024 at 03:15:20PM +0000, Shameerali Kolothum Thodi wrote:
> > -----Original Message-----
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > On Fri, Aug 09, 2024 at 02:36:31PM +0000, Shameerali Kolothum Thodi wrote:
> > > > @@ -524,6 +524,7 @@ struct acpi_iort_memory_access {
> > > >
> > > >  #define ACPI_IORT_MF_COHERENCY          (1)
> > > >  #define ACPI_IORT_MF_ATTRIBUTES         (1<<1)
> > > > +#define ACPI_IORT_MF_CANWBS             (1<<2)
> > >
> > > I think we need to update Document number to E.f in IORT section in
> > > this file. Also isn't it this file normally gets updated through ACPICA pull ?
> >
> > I don't know anything about the ACPI process..
> >
> > Can someone say for sure what to do here?
> 
> From past experience, it is normally sending a PULL request to here,
> https://github.com/acpica/acpica/pulls
> 
> And I think it then gets merged by Robert Moore and then send to LKML.

Shameer, thanks for the info!

I created a pull request: https://github.com/acpica/acpica/pull/962

By looking at one of the merged pulls, it seems this is going to
take a while. So, I think we might want to split all the CANWBS
pieces out of this series, into a followup series.

Thanks!
Nicolin
diff mbox series

Patch

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 1b39e9ae7ac178..52f5836fa888db 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1218,6 +1218,17 @@  static bool iort_pci_rc_supports_ats(struct acpi_iort_node *node)
 	return pci_rc->ats_attribute & ACPI_IORT_ATS_SUPPORTED;
 }
 
+static bool iort_pci_rc_supports_canwbs(struct acpi_iort_node *node)
+{
+	struct acpi_iort_memory_access *memory_access;
+	struct acpi_iort_root_complex *pci_rc;
+
+	pci_rc = (struct acpi_iort_root_complex *)node->node_data;
+	memory_access =
+		(struct acpi_iort_memory_access *)&pci_rc->memory_properties;
+	return memory_access->memory_flags & ACPI_IORT_MF_CANWBS;
+}
+
 static int iort_iommu_xlate(struct device *dev, struct acpi_iort_node *node,
 			    u32 streamid)
 {
@@ -1335,6 +1346,8 @@  int iort_iommu_configure_id(struct device *dev, const u32 *id_in)
 		fwspec = dev_iommu_fwspec_get(dev);
 		if (fwspec && iort_pci_rc_supports_ats(node))
 			fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
+		if (fwspec && iort_pci_rc_supports_canwbs(node))
+			fwspec->flags |= IOMMU_FWSPEC_PCI_RC_CANWBS;
 	} else {
 		node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
 				      iort_match_node_callback, dev);
diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index e27958ef82642f..56ce7fc35312c8 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -524,6 +524,7 @@  struct acpi_iort_memory_access {
 
 #define ACPI_IORT_MF_COHERENCY          (1)
 #define ACPI_IORT_MF_ATTRIBUTES         (1<<1)
+#define ACPI_IORT_MF_CANWBS             (1<<2)
 
 /*
  * IORT node specific subtables
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 15d7657509f662..d1660ec23f263b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -993,6 +993,8 @@  struct iommu_fwspec {
 
 /* ATS is supported */
 #define IOMMU_FWSPEC_PCI_RC_ATS			(1 << 0)
+/* CANWBS is supported */
+#define IOMMU_FWSPEC_PCI_RC_CANWBS		(1 << 1)
 
 /*
  * An iommu attach handle represents a relationship between an iommu domain