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

[Michael Kelley <mikelley@xxxxxxxxxxxxx>]

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