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

Iouri Tarassov
2020-08-27T23:45:59Z

On 8/14/2020 5:57 AM, Greg KH wrote:
> On Fri, Aug 14, 2020 at 08:38:53AM -0400, Sasha Levin wrote:
> > Add support for a Hyper-V based vGPU implementation that exposes the
> > DirectX API to Linux userspace.
>
> Api questions:
>
> > +struct d3dkmthandle {
> > + union {
> > + struct {
> > + u32 instance : 6;
> > + u32 index : 24;
> > + u32 unique : 2;
>
> What is the endian of this?
>
> > + };
> > + u32 v;
> > + };
> > +};
> > +
> > +extern const struct d3dkmthandle zerohandle;
> > +
The definition is the same as on the Windows side. The driver
communicates with a Windows host, so I do not expect any issues with
endiannes. Windows currently runs only on the little endian platforms.
User mode applications see this as an opaque 32 bit value
(D3DKMTHANDLE). I prefer not to use the u32 definition to avoid mistakes
when another integer or a 64-bit handle is assigned to the handle or
the handle is assigned to a 64 or 32 bit integer variable. There are
many handles in the driver model (shared NT handle, d3dkmthandle, etc).
Using a specific type allows to avoid assigning one handle to another.
> > +struct ntstatus {
> > + union {
> > + struct {
> > + int code : 16;
> > + int facility : 13;
> > + int customer : 1;
> > + int severity : 2;
>
> Same here.
>
> Are these things that cross the user/kernel boundry?
>
> And why int on one and u32 on the other?
>
> > + };
> > + int v;
> > + };
> > +};
> > +
"struct ntstatus" follows the definition for NTSTATUS on the Windows
side. NTSTATUS is an integer where the negative values indicate errors.
It is success otherwise. NTSTATUS is returned by the VM bus messages
from host. IOCTLs from the driver return Linux negative error code or
NTSTATUS positive success codes. DxCore applications expect certain
positive success codes. DxCore is a shared library, which translates the
D3DKMT* Windows interface to Linux ioctls. Applications link with DxCore
to use a paravirtualized GPU.
D3DKMTHANDLE is a 32-bit unsigned value (bitfield), not an integer.
> > +struct winluid {
> > + uint a;
> > + uint b;
>
> And now uint? Come on, be consistent please :)
Sorry about this. This came from the Windows size where we use UINT a
lot. All uints will be replaced by u32 in the next patch set.
>
> thanks,
>
> greg k-h
>
Thank you
Iouri