Re: [PATCH 00/11] pl011 emulation support in Xen

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

> The following changes were done:

.. snip..

Thank you for this great writeup. I took a stab at it and stopped at patch
#2 b/c Julien said he would look in it deeper. But based on a brief
look I would say:
 - Please do remove most of the comments. They really do not add
   much context besides describing the code - and we all can
   read the code. The idea behind the comments is to describe some
   semantics of it, or something that is not obvious at first or
   such. Not the code.

 - Comments. One line comments are:
  /* Comment. */
   And please do use proper case and a period.

 - Be careful about compiler optimizations and jump tables.
   Specifically see https://xenbits.xen.org/xsa/advisory-155.html
   The way to make sure you don't introduce an security problem
   is to 1) use local variables 2) read once from the ring and
   make sure you use a compiler barrier.

 - There is also some unrelated changes. Like extra newlines. One
   way to avoid this is to send all your patches _just_ to yourself
   and review them - but review them in reverse order and from the
   bottom of the emails to the top. That way you can catch some of this.

 - Think in terms of how one would break this. For example the guest
   could change the HVM parameters (or maybe not?) - or find the
   console ring and muck with the ring indexes. You need to shield
   the code from such changes.

Thanks!

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel