Re: [win-pv-devel] [PATCH 0/5] Add new IOCTLs to XENIFACE

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx [mailto:win-pv-devel-
> bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf Of Rafal Wojdyla
> Sent: 07 October 2015 05:48
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: [win-pv-devel] [PATCH 0/5] Add new IOCTLs to XENIFACE
> 
> This patch series implements new IOCTLs needed for libvchan. User mode
> code will come next. Some notes:
>

Cool. Thanks for posting... I'll review as soon as I can.
 
> - All new IOCTLs have their input and output data format defined as
> structs. I kept them separate for all IOCTLs even if the contents is the
> same. Not sure if that's needed but I feel it's a good idea -- if one
> IOCTL changes its data format, others won't be affected.
> 

I think that's a good decision. It's the model I tend to prefer.

> - Grant/map IOCTLs are pended forever, a separate IOCTL is used to get
> their actual result. Unmapping memory from user mode needs to be done
> below DISPATCH_LEVEL and in context of the same process, but cancel
> routines are not guaranteed to be either. Tests showed that they were
> always called at APC_LEVEL and in the correct context, but to be safe I
> queue a work item to execute at PASSIVE and use KeStackAttachProcess to
> change the address space.
> 

Can you not directly KeStackAttachProcess() in the cancellation routine?

> - Is there a particular reason for header files not being included in VS
> projects? That would help with navigating the code.
> 

No, none at all. I don't use Visual Studio for editing so if adding the header 
files into the projects would help please feel free.

> - I only updated the VS2013 project, don't have VS2012 at hand but it's
> probably easy to do by hand.

Yes, the translation is not generally hard to do, plus the vcxproj files that 
VS writes are usually so bad they need to be hand edited afterwards anyway.

  Paul

> 
> - Feel free to change names/formatting if/where needed :)
> 
> --
> RafaÅ WojdyÅa
> Qubes Tools for Windows developer
> https://www.qubes-os.org/
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel