Messages in this thread | | | Date | Thu, 19 Oct 2023 16:25:44 -0700 | Subject | Re: [PATCH] swiotlb: Rewrite comment explaining why the source is preserved on DMA_FROM_DEVICE | From | Sean Christopherson <> |
| |
On Wed, Oct 18, 2023, Robin Murphy wrote: > On 2023-10-18 18:34, Sean Christopherson wrote: > > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c > > index 01637677736f..e071415a75dc 100644 > > --- a/kernel/dma/swiotlb.c > > +++ b/kernel/dma/swiotlb.c > > @@ -1296,11 +1296,13 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, > > pool->slots[index + i].orig_addr = slot_addr(orig_addr, i); > > tlb_addr = slot_addr(pool->start, index) + offset; > > /* > > - * When dir == DMA_FROM_DEVICE we could omit the copy from the orig > > - * to the tlb buffer, if we knew for sure the device will > > - * overwrite the entire current content. But we don't. Thus > > - * unconditional bounce may prevent leaking swiotlb content (i.e. > > - * kernel memory) to user-space. > > + * When the device is writing memory, i.e. dir == DMA_FROM_DEVICE, copy > > + * the original buffer to the TLB buffer before initiating DMA in order > > + * to preserve the original's data if the device does a partial write, > > + * i.e. if the device doesn't overwrite the entire buffer. Preserving > > + * the original data, even if it's garbage, is necessary to match > > + * hardware behavior (use of swiotlb is supposed to be transparent) and > > Super-nit: I think that last "and" is superfluous (i.e. unwritten memory not > magically corrupting itself *is* the aforementioned hardware behaviour).
Ah yeah, agreed. How about this?
/* * When the device is writing memory, i.e. dir == DMA_FROM_DEVICE, copy * the original buffer to the TLB buffer before initiating DMA in order * to preserve the original's data if the device does a partial write, * i.e. if the device doesn't overwrite the entire buffer. Preserving * the original data, even if it's garbage, is necessary to match * hardware behavior. Use of swiotlb is supposed to be transparent, * i.e. swiotlb must not corrupt memory by clobbering unwritten bytes. */
| |