RE: [PATCH V4 12/13] hv_netvsc: Add Isolation VM support for netvsc driver

[Michael Kelley]

From: Michael Kelley <mikelley@xxxxxxxxxxxxx> Sent: Wednesday, September 1, 2021 7:34 PM

[snip]

> > +int netvsc_dma_map(struct hv_device *hv_dev,
> > +		   struct hv_netvsc_packet *packet,
> > +		   struct hv_page_buffer *pb)
> > +{
> > +	u32 page_count =  packet->cp_partial ?
> > +		packet->page_buf_cnt - packet->rmsg_pgcnt :
> > +		packet->page_buf_cnt;
> > +	dma_addr_t dma;
> > +	int i;
> > +
> > +	if (!hv_is_isolation_supported())
> > +		return 0;
> > +
> > +	packet->dma_range = kcalloc(page_count,
> > +				    sizeof(*packet->dma_range),
> > +				    GFP_KERNEL);
> > +	if (!packet->dma_range)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i < page_count; i++) {
> > +		char *src = phys_to_virt((pb[i].pfn << HV_HYP_PAGE_SHIFT)
> > +					 + pb[i].offset);
> > +		u32 len = pb[i].len;
> > +
> > +		dma = dma_map_single(&hv_dev->device, src, len,
> > +				     DMA_TO_DEVICE);
> > +		if (dma_mapping_error(&hv_dev->device, dma)) {
> > +			kfree(packet->dma_range);
> > +			return -ENOMEM;
> > +		}
> > +
> > +		packet->dma_range[i].dma = dma;
> > +		packet->dma_range[i].mapping_size = len;
> > +		pb[i].pfn = dma >> HV_HYP_PAGE_SHIFT;
> > +		pb[i].offset = offset_in_hvpage(dma);
> > +		pb[i].len = len;
> > +	}
> 
> Just to confirm, this driver does *not* set the DMA min_align_mask
> like storvsc does.  So after the call to dma_map_single(), the offset
> in the page could be different.  That's why you are updating
> the pb[i].offset value.  Alternatively, you could set the DMA
> min_align_mask, which would ensure the offset is unchanged.
> I'm OK with either approach, though perhaps a comment is
> warranted to explain, as this is a subtle issue.
> 

On second thought, I don't think either approach is OK.  The default
alignment in the swiotlb is 2K, and if the length of the data in the
buffer was 3K, the data could cross a page boundary in the bounce
buffer when it originally did not.  This would break the above code
which can only deal with one page at a time.  So I think the netvsc
driver also must set the DMA min_align_mask to 4K, which will
preserve the offset.

Michael