Re: [PATCH v3 02/30] drivers: hv: dxgkrnl: Driver initialization and loading

[Iouri Tarassov]

On 3/1/2022 2:06 PM, Wei Liu wrote:
> I will skip things that are pointed out by Greg.
>
> On Tue, Mar 01, 2022 at 11:45:49AM -0800, Iouri Tarassov wrote:
> > - Create skeleton and add basic functionality for the
> > hyper-v compute device driver (dxgkrnl).
> > 
> > - Register for PCI and VM bus driver notifications and
> > handle initialization of VM bus channels.
> > 
> > - Connect the dxgkrnl module to the drivers/hv/ Makefile and Kconfig
> > 
> > - Create a MAINTAINERS entry
> > 
> > A VM bus channel is a communication interface between the hyper-v guest
> > and the host. The are two type of VM bus channels, used in the driver:
> >   - the global channel
> >   - per virtual compute device channel
> > 
>
> Same comment regarding the spelling of VMBus and Hyper-V. Please fix
> other instances in code and comments.
>
> > A PCI device is created for each virtual compute device, projected
> > by the host. The device vendor is PCI_VENDOR_ID_MICROSOFT and device
> > id is PCI_DEVICE_ID_VIRTUAL_RENDER. dxg_pci_probe_device handles
> > arrival of such devices. The PCI config space of the virtual compute
> > device has luid of the corresponding virtual compute device VM
> > bus channel. This is how the compute device adapter objects are
> > linked to VM bus channels.
> > 
> > VM bus interface version is exchanged by reading/writing the PCI config
> > space of the virtual compute device.
> > 
> > The IO space is used to handle CPU accessible compute device
> > allocations. Hyper-v allocates IO space for the global VM bus channel.
> > 
> > Signed-off-by: Iouri Tarassov <iourit@xxxxxxxxxxxxxxxxxxx>
> > ---
> [...]
> > +static inline void guid_to_luid(guid_t *guid, struct winluid *luid)
> > +{
> > +	*luid = *(struct winluid *)&guid->b[0];
> > +}
>
> This should be moved to the header where luid is defined -- presumably
> this is useful for other things in the future too.
>
> Also, please provide a comment on why this conversion is okay.
>
The definition of the structure is in the public header. I do not think it makes sense to move the function there. It is a detail of the internal implementation. There is no official conversion of GUID to LUID.

The comment will be added.

Thanks
Iouri