Hi Bhupinder,
On 26/02/17 21:37, Julien Grall wrote:
On 02/21/2017 11:25 AM, Bhupinder Thakur wrote:
[...]
+ {
+ case VPL011_UARTCR_OFFSET:
Coding style: the case should be aligned to {. E.g
{
case ...
Also, I would prefer if you don't include _OFFSET in all the name.
Furthermore, can you please order the case by offset in the MMIO region.
This would help to find if a register has been emulated or not.
Lastly, the user may have requested to read 8-bit, 16-bit, 32-bit. But
you always return a 32-bit. Is a guest allowed to access using any size
and in the middle of a register? Give a look on what is done for
vgic-v{2,3}.c to check the size and read register. You may want to pull
some code out to re-use here.
Answering to myself here. Looking at the SBSA UART spec, register could
be accessed with different size. SO please handle that in the next version.
[...]
+ case VPL011_UARTDMACR_OFFSET:
+ *r = 0; /* uart DMA is not supported. Here it always
returns 0 */
My understanding of the spec is DMA is not optional. So what would
happen if the guest tries to enable it?
Answering to myself here. As discussed on Wednesday, we will emulate a
subset of PL011 registers to be compliant with SBSA UART as exposed in
the guest DT (see patch #6). This would avoid for us to handle
unnecessary register (such as DMA, line control register...).
But I would keep the file name vpl011.c so someone can extend to support
a full PL011 if necessary. You would also need to explain in the commit
message what we are emulating and give a link to the SBSA UART spec.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
