[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 0/9] hw/mos6522: VIA timer emulation fixes and improvement
Re: [PATCH v1 0/9] hw/mos6522: VIA timer emulation fixes and improvements
Sat, 20 Nov 2021 21:53:48 +0000
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0
On 19/11/2021 08:39, Finn Thain wrote:
On Thu, 18 Nov 2021, Mark Cave-Ayland wrote:
I've not forgotten about this series - we're now in 6.2 freeze, but it's
on my TODO list to revisit in the next development cycle this along with
the ESP stress-ng changes which I've also been looking at. As mentioned
in my previous reviews the patch will need some further analysis:
particularly the logic in mos6522_read() that can generate a spurious
interrupt on a register read needs to be removed,
If mos6522 fails to raise its IRQ then the counter value observed by the
guest will not make sense. This relationship was explained in the
description of patch 8. If you have a problem with that patch, please
reply there so that your misapprehension can be placed in context.
It is the existing code in mos6522_read() which is doing the wrong thing here, which
I mentioned in https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg02883.html.
and also testing is required to ensure that these changes don't affect
the CUDA clock warping which allows OS X to boot under qemu-system-ppc.
I'm not expecting any issues. What is required in order to confirm this?
Would it be sufficient to boot a Mac OS X 10.4 installer DVD?
Possibly: I've only been testing various images since after the timing workaround was
added so I'm not sure exactly what the symptoms are, but the links sent earlier in
the thread are still valid.
I'm confident that qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) is monotonic,
As I mentioned, it is monotonic here.
since if it were not then there would be huge numbers of complaints from
QEMU users. It appears that Linux can potentially alter the ticks in
which suggests the issue is related to timer wraparound. I'd like to
confirm exactly which part of your series fixes the specific problem of
the clock jumping backwards before merging these changes.
You really only need a good datasheet to review these patches. You don't
need a deep understanding of any particular guest, and you shouldn't be
targetting any particular guest.
One of the purposes of this patch series is to allow the guest to change
to better exploit actual, physical hardware. Since QEMU regression testing
is part of the kernel development process, regressions need to be avoided.
That means QEMU's shortcomings hinder Linux development.
Therefore, QEMU should not target the via timer driver in Linux v2.6 or
the one in v5.15 or the one in NetBSD etc. QEMU should target correctness
-- especially when that can be had for cheap. Wouldn't you agree?
QEMU deviates in numerous ways from the behaviour of real mos6522 timer.
This patch series does not address all of these deviations (see patch 8).
Instead, this patch series addresses only the most aggregious ones, and
they do impact existing guests.
That is true, but as per the link above there is an existing bug in the mos6522
device, and the patchset builds on this in its current form, including adding a state
field which shouldn't be required.
From looking at mac_read_clk() presumably the problem here is that the timer IRQ
fires on 0 rather than on 0xffff when it overflows? If so, this should be a very
small and simple patch. Once these 2 fixes are in place, it will be much easier to
test the remaining changes.
I realized that I could easily cross-compile a 5.14 kernel to test this
theory with the test root image and .config you supplied at
https://gitlab.com/qemu-project/qemu/-/issues/611 using the QEMU
docker-m68k-cross image to avoid having to build a complete toolchain by
hand. The kernel builds successfully using this method, but during boot
it hangs sending the first SCSI CDB to the ESP device, failing to send
the last byte using PDMA.
Are there known issues cross-compiling an m68k kernel on an x86 host?
Not that I'm aware of.
Or are your current kernels being built from a separate branch outside
of mainline Linux git?
I use mainline Linux when testing QEMU. I provided a mainline build,
attached to the same bug report, so you don't have to build it.
The problem here is that I have no way to reproduce your results and test any patches
other than to try and build a kernel with your extra warning and run it. Even then I
don't know how long to wait for the clock to jump, how much it jumps by, or if there
is anything else that needs to be done to trigger the warning.
Any help with being able to build a working cross-m68k kernel to test this would be
gratefully received, otherwise I don't feel I have enough knowledge of the m68k
kernel to be able to validate the fixes and review the changes in order to merge them.