On 01/19/2018 08:07 AM, Corey Minyard wrote:
On 01/18/2018 09:17 PM, Michael S. Tsirkin wrote:
On Thu, Jan 18, 2018 at 07:55:41PM -0600, address@hidden wrote:
From: Corey Minyard <address@hidden>
This reverts commit 880b1ffe6ec2f0ae25cc4175716227ad275e8b8a.
The commit being reverted says:
PIIX4 errata says that "immediate polling of the Host Status Register BUSY
bit may indicate that the SMBus is NOT busy."
Due to this, some code does the following steps:
(a) set parameters
(b) start command
(c) check for smbus busy bit set (to know that command started)
(d) check for smbus busy bit not set (to know that command finished)
Let (c) happen, by immediately setting the busy bit, and really executing
the command when status register has been read once.
This fixes a problem with AMIBIOS, which can now properly initialize the
PIIX4.
Emulating bad hardware so badly written software will work doesn't sound
like a good idea to me. I have patches that add interrupt capability
to pm_smbus, but this change breaks that because the Linux driver
starts the transaction then waits for interrupts before reading the
status register. That obviously won't work with these changes.
The right way to fix this in AMIBIOS is to ignore the host busy bit
and use the other bits in the host status register to tell if the
transaction has completed. Using host busy is racy, anyway, if you
get interrupted or something while processing, you may miss step (c)
in your algorithm and fail.
Cc: Hervé Poussineau <address@hidden>
Cc: Philippe Mathieu-Daudé <address@hidden>
Signed-off-by: Corey Minyard <address@hidden>
Would it be possible to limit the change to when guest uses
interrupts?
I did think about that, but it seems rather frail. What if another piece of
software
does this but has the interrupt enable bit set? And AMIBIOS is still broken
doing
that algorithm on real hardware. If you get a bus collision, for instance,
that will
be almost instantaneous and the firmware is likely to miss it.
The 82801 documentation is pretty clear that you should use the INTR and error
bits in the status register to know if a transaction is complete.
If you really want to emulate real hardware, I guess the right way to do this
would be to add a delay between the start bit being set and the transaction
being done. I'm not sure how timers work with vmstate, I'd have to look at
that.
I realized that the timer is not going to be able to correctly work around the
AMIBIOS. It would probably work most of the time, but if qemu got switched
out, then switched back and the timer went off before the guest was allowed
to run, then you would have the same issue.
Also, looking at a more complete implementation of the pm_smbus device,
using the host busy bit to know when to start the transaction won't work,
that bit also does other things when doing byte at a time block transfers.
So a separate bool is needed to know when to do this.
0001-smbus-replace-transaction-execution-at-first-registe.patch