Re: [Xen-devel] [PATCH 01/11] xen/arm: vpl011: Add pl011 uart emulation in Xen

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel