Re: Re: [PATCH 1/3] hw/arm: Add EHCI/OHCI controllers to Allwinner R40 and Bananapi board

[Bernhard Beschow]

Am 16. Januar 2024 10:13:37 UTC schrieb Gerd Hoffmann <kraxel@redhat.com>:
>On Mon, Jan 15, 2024 at 08:12:29AM -0800, Guenter Roeck wrote:
>> On 1/15/24 03:02, Philippe Mathieu-Daudé wrote:
>> > On 13/1/24 20:16, Guenter Roeck wrote:
>> > > If machine USB support is not enabled, create unimplemented devices
>> > > for the USB memory ranges to avoid crashes when booting Linux.
>> > 
>> > I never really understood the reason for machine_usb() and had on my
>> > TODO to do some archeology research to figure it out since quite some
>> > time. Having to map an UnimpDevice due to CLI options seems like an
>> > anti-pattern when the device is indeed implemented in the repository.
>> 
>> Me not either. I copied the code from aw_a10_init(), trying to use the
>> same pattern. I am perfectly fine with making it unconditional, but then
>> I would argue that it should be unconditional for Allwinner A10 as well
>> (not that I really care much, just for consistency).
>> 
>> The "-usb" option says "enable on-board USB host controller (if not
>> enabled by default)". Unfortunately, that doesn't tell me much,
>> and most specifically it doesn't tell me how to enable it by default.
>> One option I can think of would be to enable it on the machine level,
>> i.e., from bananapi_m2u.c, but then, again, I don't see if/how
>> that is done for other boards. Any suggestions ?
>
>The -usb switch is there as long as I remember (which goes back to qemu
>0.1x days).  I suspect it was introduced in the early usb emulation
>days, where usb emulation was more fragile and turning it on by default
>didn't look like a good plan.
>
>usb emulation in modern qemu should be stable enough that enabling it by
>default should not be much of a problem.  ohci/uhci/ehci emulation is
>somewhat expensive due to polling being wired into the hardware design,
>but as long as there are no usb devices connected this should not be
>much of a concern either as the guest drivers usually put the usb host
>adapter to sleep in that case (which saves power on physical hardware
>and stops the polling timer in qemu).
>
>So, turning on usb support by default makes sense to me when emulating
>boards, where guests expect the usb controllers being present at
>specific MMIO addresses, so mapping UnimpDevice is not needed.

Good idea, this might make upstreaming my VIA-south-bridges-in-the-pc-machine 
work simpler.

>
>In case guests detect hardware via generated device tree / generated
>ACPI tables / pci bus scan (i.e. arm virt + microvm + pc + q35) it IMHO
>makes sense to keep current behavior.

I remember similar discussions with Phil during PIIX consolidation where the 
motivation was to make the code simpler by removing options. That is, if a 
piece of silicon contains multiple functions, the QEMU model should always 
instantiate all (available) functions rather than providing options for 
cherry-picking.

Especially in highly integrated devices (such as south bridges) there might be 
interconnections between functions that are hard to handle when some are 
optional. For example, the ACPI function in the VIA south bridges detect 
activity from all other functions. In both PIIX and VIA, the USB function can 
be (de)activated by software from the ISA function (not implemented yet).

The latter would allow the current behavior of the pc machine(s) to be 
implemented while avoiding the cherry picking problem: The BIOS could switch 
the USB function on or off depending on fw_cfg (please correct me if I'm 
wrong). I think this would't even need compat machinery. Of course, support 
would need to be added in SeaBIOS and -- I guess -- Tianocore. I'd like to look 
into that but, as usual, no guarantees when this will arrive.

Best regards,
Bernhard

>
>take care,
>  Gerd
>
>