Goldfish TTY enhancement

Goldfish TTY enhancement

[Jason Thorpe]

Having recently written a driver for the Goldfish TTY for NetBSD, I found it a bit odd (and a little annoying) that the device has a PUT_CHAR register but not a GET_CHAR register, something particularly useful for early-console or in-kernel debugger use.  As it stands, to get a single character from the device, you have to first poke a DMA address into 1 (or 2) registers and then poke the command register.  Depending on the operating system environment, this might involve a lot of kernel machinery.  My current driver works around this, but makes an explicit assumption that a physical address is a valid DMA address, which is the true for the m68k virt platform, but is the sort of assumption one shouldn’t be making these days.

Would there be an objection to adding a single-register-read get-char operation to the Goldfish TTY?  The change I have in mind is to bump the version to 2 (so that driver software can identity the capability) and then allow reads of the PUT_CHAR register to return the first pending byte or -1 if the FIFO is empty.  Happy to create an additional register for the purpose if that’s preferred, but it seemed better to keep the footprint of the device the same.

If the consensus is that this is a worthwhile enhancement, I’ll post a patch.

Cheers.

-- thorpej

Re: Goldfish TTY enhancement

[Philippe Mathieu-Daudé]

Hi Jason,

On 10/1/24 16:24, Jason Thorpe wrote:
> Having recently written a driver for the Goldfish TTY for NetBSD, I found it a bit odd (and a little annoying) that the device has a PUT_CHAR register but not a GET_CHAR register, something particularly useful for early-console or in-kernel debugger use.  As it stands, to get a single character from the device, you have to first poke a DMA address into 1 (or 2) registers and then poke the command register.  Depending on the operating system environment, this might involve a lot of kernel machinery.  My current driver works around this, but makes an explicit assumption that a physical address is a valid DMA address, which is the true for the m68k virt platform, but is the sort of assumption one shouldn’t be making these days.
> 
> Would there be an objection to adding a single-register-read get-char operation to the Goldfish TTY?  The change I have in mind is to bump the version to 2 (so that driver software can identity the capability) and then allow reads of the PUT_CHAR register to return the first pending byte or -1 if the FIFO is empty.  Happy to create an additional register for the purpose if that’s preferred, but it seemed better to keep the footprint of the device the same.

IIUC Goldfish virtual HW is maintained externally by Google
https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT

I suppose the spec needs to be updated before the change can be
accepted in mainstream QEMU, but since I'm not sure I Cc'ed Alex,
David and Laurent.

Regards,

Phil.

> 
> If the consensus is that this is a worthwhile enhancement, I’ll post a patch.
> 
> Cheers.
> 
> -- thorpej
> 
> 

Re: Goldfish TTY enhancement

[Jason Thorpe]

> On Jan 10, 2024, at 8:01 AM, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
> IIUC Goldfish virtual HW is maintained externally by Google
> https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
> 
> I suppose the spec needs to be updated before the change can be
> accepted in mainstream QEMU, but since I'm not sure I Cc'ed Alex,
> David and Laurent.

Hey Philippe,

I have seen that document didn’t realize that it was the source of truth for the Goldfish devices in Qemu, as Qemu already has Goldfish devices that deviate in behavior from that document.  In particular:

1. There is no distinction between “rtc” and “timer” in Qemu.

2. The Goldfish “pic” device does not behave as that document describes.  In particular, the “NUMBER” register is described in that document as returning the lowest pending interrupt index or 0 for none (i.e. a number in the range 0..32).  But Qemu returns a bitmask of pending interrupts when that register is read.  And despite the name “DISABLE_ALL” that document claims that writing to it merely clears the pending interrupts without disabling them (which would be quite the trick with level-triggered interrupt sources) whereas in Qemu, it does both clear and disable.

(I am not, in any way, advocating for a behavior change in Qemu, BTW… I just thought that referenced docuemnt was no longer relevant.)

-- thorpej

Re: Goldfish TTY enhancement

[Laurent Vivier]

Hi Jason,

Le 11/01/2024 à 19:58, Jason Thorpe a écrit :
> 
>> On Jan 10, 2024, at 8:01 AM, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> IIUC Goldfish virtual HW is maintained externally by Google
>> https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
>>
>> I suppose the spec needs to be updated before the change can be
>> accepted in mainstream QEMU, but since I'm not sure I Cc'ed Alex,
>> David and Laurent.
> 
> Hey Philippe,
> 
> I have seen that document didn’t realize that it was the source of truth for the Goldfish devices in Qemu, as Qemu already has Goldfish devices that deviate in behavior from that document.  In particular:
> 
> 1. There is no distinction between “rtc” and “timer” in Qemu.
> 
> 2. The Goldfish “pic” device does not behave as that document describes.  In particular, the “NUMBER” register is described in that document as returning the lowest pending interrupt index or 0 for none (i.e. a number in the range 0..32).  But Qemu returns a bitmask of pending interrupts when that register is read.  And despite the name “DISABLE_ALL” that document claims that writing to it merely clears the pending interrupts without disabling them (which would be quite the trick with level-triggered interrupt sources) whereas in Qemu, it does both clear and disable.
> 
> (I am not, in any way, advocating for a behavior change in Qemu, BTW… I just thought that referenced docuemnt was no longer relevant.)

In fact the source of truth is the kernel. The idea of using Goldfish in the virt m68k machine is to 
only have to code the QEMU part and to use the kernel part as-is. And the kernel part has diverged 
from the documentation...

Thanks,
Laurent