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

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?

quoted

quoted

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.

quoted

quoted

quoted

+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