In GPI mode, the I2C GENI driver programs an extra TX DMA transfer
descriptor (TRE) on the TX channel when handling a single read message.
This results in an unintended write phase being issued on the I2C bus,
even though a read transaction does not require any TX data.
For a single-byte read, the correct hardware sequence consists of the
CONFIG and GO commands followed by a single RX DMA TRE. Programming an
additional TX DMA TRE is redundant, causes unnecessary DMA buffer
mapping on the TX channel, and may lead to incorrect bus behavior.
Update the transfer logic to avoid programming a TX DMA TRE for single
read messages in GPI mode.
Co-developed-by: Maramaina Naresh <naresh.maramaina(a)oss.qualcomm.com>
Signed-off-by: Maramaina Naresh <naresh.maramaina(a)oss.qualcomm.com>
Signed-off-by: Aniket Randive <aniket.randive(a)oss.qualcomm.com>
---
Changes in v4:
- Added some more description in comment and changed the label name.
Changes in v3:
- Added comment in the driver for better readability and changed the
position of 'skip_dma' label to allow dma engine configuration.
Changes in v2:
- Updated the commit message.
drivers/i2c/busses/i2c-qcom-geni.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index a4acb78fafb6..a482a4c60744 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -625,8 +625,8 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
{
struct gpi_i2c_config *peripheral;
unsigned int flags;
- void *dma_buf;
- dma_addr_t addr;
+ void *dma_buf = NULL;
+ dma_addr_t addr = 0;
enum dma_data_direction map_dirn;
enum dma_transfer_direction dma_dirn;
struct dma_async_tx_descriptor *desc;
@@ -639,6 +639,16 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
gi2c_gpi_xfer = &gi2c->i2c_multi_desc_config;
msg_idx = gi2c_gpi_xfer->msg_idx_cnt;
+ /*
+ * Skip TX DMA mapping for a read message (I2C_M_RD) to avoid
+ * programming an extra TX DMA TRE that would cause an unintended
+ * write cycle on the I2C bus before the actual read operation.
+ */
+ if (op == I2C_WRITE && msgs[msg_idx].flags & I2C_M_RD) {
+ peripheral->multi_msg = true;
+ goto skip_tx_dma_map;
+ }
+
dma_buf = i2c_get_dma_safe_msg_buf(&msgs[msg_idx], 1);
if (!dma_buf) {
ret = -ENOMEM;
@@ -658,6 +668,7 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
goto out;
}
+skip_tx_dma_map:
if (gi2c->is_tx_multi_desc_xfer) {
flags = DMA_CTRL_ACK;
@@ -740,9 +751,12 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
return 0;
err_config:
- dma_unmap_single(gi2c->se.dev->parent, addr,
- msgs[msg_idx].len, map_dirn);
- i2c_put_dma_safe_msg_buf(dma_buf, &msgs[msg_idx], false);
+ /* Avoid DMA unmap as the write operation skipped DMA mapping */
+ if (dma_buf) {
+ dma_unmap_single(gi2c->se.dev->parent, addr,
+ msgs[msg_idx].len, map_dirn);
+ i2c_put_dma_safe_msg_buf(dma_buf, &msgs[msg_idx], false);
+ }
out:
gi2c->err = ret;
--
2.34.1
From: Jiri Pirko <jiri(a)nvidia.com>
Document the system_cc_shared dma-buf heap that was introduced
recently. Describe its purpose, availability conditions and
relation to confidential computing VMs.
Signed-off-by: Jiri Pirko <jiri(a)nvidia.com>
Reviewed-by: T.J.Mercier <tjmercier(a)google.com>
---
Documentation/userspace-api/dma-buf-heaps.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
index 05445c83b79a..f56b743cdb36 100644
--- a/Documentation/userspace-api/dma-buf-heaps.rst
+++ b/Documentation/userspace-api/dma-buf-heaps.rst
@@ -16,6 +16,13 @@ following heaps:
- The ``system`` heap allocates virtually contiguous, cacheable, buffers.
+ - The ``system_cc_shared`` heap allocates virtually contiguous, cacheable,
+ buffers using shared (decrypted) memory. It is only present on
+ confidential computing (CoCo) VMs where memory encryption is active
+ (e.g., AMD SEV, Intel TDX). The allocated pages have the encryption
+ bit cleared, making them accessible for device DMA without TDISP
+ support. On non-CoCo VM configurations, this heap is not registered.
+
- The ``default_cma_region`` heap allocates physically contiguous,
cacheable, buffers. Only present if a CMA region is present. Such a
region is usually created either through the kernel commandline
--
2.51.1
In GPI mode, the I2C GENI driver programs an extra TX DMA transfer
descriptor (TRE) on the TX channel when handling a single read message.
This results in an unintended write phase being issued on the I2C bus,
even though a read transaction does not require any TX data.
For a single-byte read, the correct hardware sequence consists of the
CONFIG and GO commands followed by a single RX DMA TRE. Programming an
additional TX DMA TRE is redundant, causes unnecessary DMA buffer
mapping on the TX channel, and may lead to incorrect bus behavior.
Update the transfer logic to avoid programming a TX DMA TRE for single
read messages in GPI mode.
Co-developed-by: Maramaina Naresh <naresh.maramaina(a)oss.qualcomm.com>
Signed-off-by: Maramaina Naresh <naresh.maramaina(a)oss.qualcomm.com>
Signed-off-by: Aniket Randive <aniket.randive(a)oss.qualcomm.com>
---
Changes in v3:
- Added comment in the driver for better readability and changed the
position of 'skip_dma' label to allow dma engine configuration.
Changes in v2:
- Updated the commit message.
drivers/i2c/busses/i2c-qcom-geni.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index a4acb78fafb6..78b92db7c7fd 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -625,8 +625,8 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
{
struct gpi_i2c_config *peripheral;
unsigned int flags;
- void *dma_buf;
- dma_addr_t addr;
+ void *dma_buf = NULL;
+ dma_addr_t addr = 0;
enum dma_data_direction map_dirn;
enum dma_transfer_direction dma_dirn;
struct dma_async_tx_descriptor *desc;
@@ -639,6 +639,12 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
gi2c_gpi_xfer = &gi2c->i2c_multi_desc_config;
msg_idx = gi2c_gpi_xfer->msg_idx_cnt;
+ /* Skip TX DMA map for I2C_WRITE operation to avoid unintended write cycle */
+ if (op == I2C_WRITE && msgs[msg_idx].flags & I2C_M_RD) {
+ peripheral->multi_msg = true;
+ goto skip_dma;
+ }
+
dma_buf = i2c_get_dma_safe_msg_buf(&msgs[msg_idx], 1);
if (!dma_buf) {
ret = -ENOMEM;
@@ -658,6 +664,7 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
goto out;
}
+skip_dma:
if (gi2c->is_tx_multi_desc_xfer) {
flags = DMA_CTRL_ACK;
@@ -740,9 +747,12 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
return 0;
err_config:
- dma_unmap_single(gi2c->se.dev->parent, addr,
- msgs[msg_idx].len, map_dirn);
- i2c_put_dma_safe_msg_buf(dma_buf, &msgs[msg_idx], false);
+ /* Avoid DMA unmap as the write operation skipped DMA mapping */
+ if (dma_buf) {
+ dma_unmap_single(gi2c->se.dev->parent, addr,
+ msgs[msg_idx].len, map_dirn);
+ i2c_put_dma_safe_msg_buf(dma_buf, &msgs[msg_idx], false);
+ }
out:
gi2c->err = ret;
--
2.34.1
dma_buf_put() may drop the final file reference via fput(), which
can free the dma-buf. The new tracepoint invocation was added
after fput(), and DMA_BUF_TRACE() dereferences dmabuf and takes
dmabuf->name_lock.
This leads to a use-after-free on the final put, visible for
example as a spinlock bad magic fault on a poisoned 0x6b6b6b...
lock.
Move the dma_buf_put tracepoint before fput().
Reported-by: Janusz Krzysztofik <janusz.krzysztofik(a)linux.intel.com>
Fixes: 281a22631423 ("dma-buf: add some tracepoints to debug.")
Signed-off-by: Andi Shyti <andi.shyti(a)linux.intel.com>
Cc: Xiang Gao <gaoxiang17(a)xiaomi.com>
Cc: Christian König <christian.koenig(a)amd.com>
---
Hi,
I believe this patch fixes the issue reported by Janusz, I haven't
tested it. Perhaps we can add some more checks in DMA_BUF_TRACE.
Thanks,
Andi
drivers/dma-buf/dma-buf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index a202a308c079..b72932c57cb9 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -845,9 +845,8 @@ void dma_buf_put(struct dma_buf *dmabuf)
if (WARN_ON(!dmabuf || !dmabuf->file))
return;
- fput(dmabuf->file);
-
DMA_BUF_TRACE(trace_dma_buf_put, dmabuf);
+ fput(dmabuf->file);
}
EXPORT_SYMBOL_NS_GPL(dma_buf_put, "DMA_BUF");
--
2.53.0
From: Jiri Pirko <jiri(a)nvidia.com>
Document the system_cc_shared dma-buf heap that was introduced
recently. Describe its purpose, availability conditions and
relation to confidential computing VMs.
Signed-off-by: Jiri Pirko <jiri(a)nvidia.com>
---
Documentation/userspace-api/dma-buf-heaps.rst | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
index 05445c83b79a..591732393e7d 100644
--- a/Documentation/userspace-api/dma-buf-heaps.rst
+++ b/Documentation/userspace-api/dma-buf-heaps.rst
@@ -16,6 +16,14 @@ following heaps:
- The ``system`` heap allocates virtually contiguous, cacheable, buffers.
+ - The ``system_cc_shared`` heap allocates virtually contiguous, cacheable,
+ buffers using shared (decrypted) memory. It is only present on
+ confidential computing (CoCo) VMs where memory encryption is active
+ (e.g., AMD SEV, Intel TDX). The allocated pages have the encryption
+ bit cleared, making them accessible for device DMA without TDISP
+ support. On non-CoCo VMs configurations, this heap is
+ not registered.
+
- The ``default_cma_region`` heap allocates physically contiguous,
cacheable, buffers. Only present if a CMA region is present. Such a
region is usually created either through the kernel commandline
--
2.51.1