>> > +/* Memory size expected as non cached and reserved by the VM driver >> > */ +#define RESMEM_SIZE 0x40000000 >> > +#define MEMBLK_SIZE 0x20000000 >> > + >> >> Maybe use SZ_* definitions in linux/size.h > > Good suggestion. Ack. >> >> Better move this part to the place between vfio_pci_core_enable() and >> vfio_pci_core_finish_enable() like others for respecting the expected >> device initialization sequence of life cycle. >> >> It doesn't bite something right now, but think about when someone >> changes the behavior of vfio_pci_core_finish_enable() in the future, >> they have to propose a patch for this. > > Agree. Good point, will move it. >> >> Wouldn't it be better to do the map in the open path? > > AIUI the device would typically be used exclusively through the mmap of > these ranges, these mappings are only for pread/pwrite type accesses, > so I think it makes sense to map them on demand. That's right, agree with Alex to keep it on-demand. >> > + * Determine how many bytes to be actually read from the >> > device memory. >> > + * Read request beyond the actual device memory size is >> > filled with ~0, >> > + * while those beyond the actual reported size is skipped. >> > + */ >> > + if (offset >= memregion->memlength) >> > + mem_count = 0; >> >> If mem_count == 0, going through nvgrace_gpu_map_and_read() is not >> necessary. > > Harmless, other than the possibly unnecessary call through to > nvgrace_gpu_map_device_mem(). Maybe both nvgrace_gpu_map_and_read() > and nvgrace_gpu_map_and_write() could conditionally return 0 as their > first operation when !mem_count. Thanks, > >Alex IMO, this seems like adding too much code to reduce the call length for a very specific case. If there aren't any strong opinion on this, I'm planning to leave this code as it is.
