Re: [PATCH v2 18/21] xen/arm: Allow vpl011 to be used by DomU

[Julien Grall <julien.grall@arm.com>]

Hi Stefano,

On 27/07/18 01:10, Stefano Stabellini wrote:
> On Tue, 24 Jul 2018, Julien Grall wrote:
>> On 07/07/18 00:12, Stefano Stabellini wrote:
>>> +    VPL011_UNLOCK(d, flags);
>>> +}
>>> +
>>> +static void vpl011_write_data_noring(struct domain *d, uint8_t data)
>>> +{
>>> +    unsigned long flags;
>>> +    struct vpl011 *vpl011 = &d->arch.vpl011;
>>> +
>>> +    VPL011_LOCK(d, flags);
>>> +
>>> +    printk("%c", data);
>>> +    if (data == '\n')
>>> +        printk("DOM%u: ", d->domain_id);
>>
>> You want to buffer the characters here and only print on newline or when the
>> buffer is full. Otherwise characters will get mangled with the Xen console
>> output or other domains output.
> 
> I did the implementation, but then when I type characters at the domain
> prompt, I don't see them anymore one by one. Only after I press
> "enter". That makes the domain console not very usable. Should we keep
> it as?

I haven't thought about that case. However, if you don't implement the 
buffer solution, you will get all the domain consoles mangled during 
boot. This is not really nice for debugging. A potential solution is to 
buffer for all the domains but the one that is reading characters.

>>> +
>>> +    vpl011->uartris |= TXI;
>>> +    vpl011->uartfr &= ~TXFE;
>>> +    vpl011_update_interrupt_status(d);
>>> +
>>> +    VPL011_UNLOCK(d, flags);
>>> +}
>>> +
>>> +static uint8_t vpl011_read_data_inring(struct domain *d)
>>> +{
>>
>> I think you can avoid the duplication here by using a macro.
> 
> I prefer to avoid MACROS for things like this. I would rather reuse the
> existing function for both cases like in v1. Would you be OK to go back
> to that?

I would rather keep the duplication over going back to v1.

I may have another way to keep the code common, but let's look at that 
in a latter patch. For now, I will deal with the duplication.

Cheers,

-- 
Julien Grall

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