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

[Peter Maydell]

On 20 April 2018 at 13:39, Philippe Mathieu-Daudé <address@hidden> wrote:
> Hi Peter,
>
> 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.

(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.)

> 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).

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...

thanks
-- PMM