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

Iouri Tarassov
2020-09-03T18:55:21Z

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