Re: [Resend RFC PATCH V2 11/12] HV/Netvsc: Add Isolation VM support for netvsc driver

[Tianyu Lan]

On 4/14/2021 11:50 PM, Christoph Hellwig wrote:
> > +struct dma_range {
> > +	dma_addr_t dma;
> > +	u32 mapping_size;
> > +};
>
> That's a rather generic name that is bound to create a conflict sooner
> or later.

Good point. Will update.

> >   #include "hyperv_net.h"
> >   #include "netvsc_trace.h"
> > +#include "../../hv/hyperv_vmbus.h"
>
> Please move public interfaces out of the private header rather than doing
> this.

OK. Will update.

> > +	if (hv_isolation_type_snp()) {
> > +		area = get_vm_area(buf_size, VM_IOREMAP);
>
> Err, no.  get_vm_area is private a for a reason.
>
> > +		if (!area)
> > +			goto cleanup;
> > +
> > +		vaddr = (unsigned long)area->addr;
> > +		for (i = 0; i < buf_size / HV_HYP_PAGE_SIZE; i++) {
> > +			extra_phys = (virt_to_hvpfn(net_device->recv_buf + i * HV_HYP_PAGE_SIZE)
> > +				<< HV_HYP_PAGE_SHIFT) + ms_hyperv.shared_gpa_boundary;
> > +			ret |= ioremap_page_range(vaddr + i * HV_HYP_PAGE_SIZE,
> > +					   vaddr + (i + 1) * HV_HYP_PAGE_SIZE,
> > +					   extra_phys, PAGE_KERNEL_IO);
> > +		}
> > +
> > +		if (ret)
> > +			goto cleanup;
>
> And this is not something a driver should ever do.  I think you are badly
> reimplementing functionality that should be in the dma coherent allocator
> here.
OK. I will try hiding these in the Hyper-V dma ops callback. Thanks.