Re: [Qemu-devel] [PATCH 1/2] hw/isa/superio: Fix inconsistent use of Chardev->be

[Philippe Mathieu-Daudé]

On 04/20/2018 09:59 AM, Peter Maydell wrote:
> On 20 April 2018 at 13:39, Philippe Mathieu-Daudé <address@hidden> wrote:
>> On 04/20/2018 05:43 AM, Peter Maydell wrote:
>>> On 19 April 2018 at 23:09, Philippe Mathieu-Daudé <address@hidden> wrote:
>>>> 4c3119a6e3e and cd9526ab7c0 introduced an incorrect and inconsistent
>>>> use of Chardev->be. Also, this CharBackend member is private and is
>>>> not supposed to be accessible.
> 
>>> You should not need to create fake null devices like this. The
>>> device using the chardev should just cope with having a NULL
>>> pointer now we have commit 12051d82f004024.
>>
>> I am trying to model a SoC with 4 uarts, and started using the current
>> pattern:
>>
>>   for (i = 0; i < MAX_SERIAL_PORTS; i+++) {
>>     if (serial_hds[i]) {
>>       serial_mm_init(..., serial_hds[i], ...);
> 
> This is wrong, because it means that the UART device will
> only be visible to the guest if the user connected it to
> something (with -serial whatever). You should always create
> the same number of devices, which correspond to what
> hard wired UARTs the board has.

Agreed!

> (The exception would I guess be if the devices were pluggable,
> so each serial device was a pluggable PCI UART or something
> weird; in that case there's an argument that -serial means
> "create and plug in one of these devices". x86 is probably
> also different for legacy reasons.)

Good example, also USB uarts.

>> Since we can now safely call serial_mm_init(..., chr=NULL, ...), I'll
>> work on simplifying and fixing this.
> 
> Correct. There are a bunch of dubious workarounds in the
> codebase which try to handle the problem at different levels,
> but we've decided that the right thing is for the NULL pointer
> to be dealt with in one place at the bottom (in the qemu_chr_fe_*
> functions).

OK, I'll happily clean that :)

> Incidentally I was wondering if we could sensibly get rid of
> the compile time MAX_SERIAL_PORTS. Right now there's no way
> to model a device with five UARTs such that they can all
> be sent to interesting places. I think there's no reason
> for this except that the PC traditionally has 4 UARTs and
> nobody's ever changed the code...

Yes, I thought the same and planned to relax this limit (and few others)
in my "remove i386/pc dependency from non-PC world" series...