qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH 09/10] sm501: Add some more missi


From: Peter Maydell
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH 09/10] sm501: Add some more missing registers
Date: Sat, 25 Feb 2017 16:31:53 +0000

On 24 February 2017 at 21:49, BALATON Zoltan <address@hidden> wrote:
> On Fri, 24 Feb 2017, BALATON Zoltan wrote:
>>
>> On Fri, 24 Feb 2017, Peter Maydell wrote:
>>>
>>> On 19 February 2017 at 16:35, BALATON Zoltan <address@hidden> wrote:
>>>>
>>>> Write only to allow clients to initialise these without failing
>>>>
>>>> Signed-off-by: BALATON Zoltan <address@hidden>
>>>
>>>
>>> What's the point in write-only register values?

>>> If the registers are writes-ignored, there's no need to store
>>> the data written into the state struct; if the registers are
>>> reads-as-written then implement them that way.
>
>
> Still not sure what do you mean by read-as-written because I think that's
> exactly what is done here, value stored and read back as is, except for
> video_control where there are reserved bits that are always 0.

There are several ways we can make a register behave:

"Read only" -> the guest can only read, it cannot write
"Writes ignored" -> the guest can write but it has no effect
"Reads as written" -> the guest can write, and we store the
data so that when the guest reads it gets back the
value it wrote, but it doesn't have any effect on device behaviour
"Reads as zero" -> the guest can read but it always gets zero
"Write only" -> the guest can write values and we store them
but don't let the guest read them back. This is pointless.

For cases where we don't actually implement some bit of hardware
behaviour yet, it can be a reasonable choice to do either
"read-as-zero/writes-ignored", or "reads as written",
depending on what the guest is expecting. (writes-ignored
is the easiest behaviour to implement, but sometimes guests
won't work if you do that.)

If you're implementing "reads as written" then the commit
message shouldn't say "write only"... But the patch seems
to only add code to the register-write function, not to
the register-read function. That means we're storing
values the guest writes but never doing anything with them.
Either throw away the values immediately ("writes ignored")
or let the guest read them back ("reads as written").

(Optionally, we can also log the access via LOG_UNIMP
to let the user know the guest is trying to use some
bit of the device that doesn't work yet.)

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]