Re: [PATCH 1/4] drivers: hv: dxgkrnl: core code

From: Iouri Tarassov
Date: Thu Sep 03 2020 - 14:55:22 EST


Hi Greg,

I appreciate your comments and working to address them.

On 8/27/2020 11:12 PM, Greg KH wrote:
As for "matching names", why does that matter? Who sees both names at
the same time?

> > > > endian issues?
> > > > If not, why are these bit fields?
> This matches the definition on the Windows side. Windows only works on
> little endian platforms.

But Linux works on both, so you need to properly document/handle this somehow.
This driver works only in a Linux container in conjunction with the Windows host. The structure definitions are  the same on the host and the container. The driver will not be enabled or work on platforms, where Windows does not run.

> > > > > +struct d3dkmt_destroydevice {
> > > + struct d3dkmthandle device;
> > > +};
> > > > Again, single entity structures?
> > > > Are you trying to pass around "handles" and cast them backwards?
> > > > If so, great, but then use the real kernel structures for that like
> > 'struct device' if these are actually devices.
> > > Again. The structure matches the definition on the Windows side to avoid
> confusion.

Who is confused here? We accept naming conventions that do not match
the normal Linux style when they are referring to external sources of
the data. Examples of this are USB device field names, and other
hardware specifications that are public. You aren't sharing code with a
Windows system, so please follow the Linux coding style rules, as you
want Linux developers to be helping you maintain this code, not
developers who have ever read code from other operating systems.

So please follow the rule of, "unless these fields and structures are
publically defined somewhere, use Linux naming rules", like all of the
rest of us do.

The d3dkmt* structures, like d3dkmt_destroydevice are publicly documented on MSDN (https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/d3dkmthk/ns-d3dkmthk-_d3dkmt_destroydevice). I am using the same definitions in the driver, so it is easy to find the corresponding definition and description of the structure. I have no problem to change the names, but I think using the same public definition will help the driver maintainers.

Thanks

Iouri