HPPA patches for qemu-v11
A few late fixes for the HPPA architecture for QEMU v11:
- graphics support was broken for 64-bit machines. This series adds
support for VGA graphics for Linux guests
- the various memory ranges were not correctly implemented
- TOC/NMI was not working on 64-bit machines
- minor 64-bit HP-UX boot fixes (but HP-UX 64-bit still crashes)
# -----BEGIN PGP SIGNATURE-----
#
# iHUEABYKAB0WIQS86RI+GtKfB8BJu973ErUQojoPXwUCacwXxAAKCRD3ErUQojoP
# X7NxAQCBszDUKsNX5KiB+cxW1AfT1Gyzo4q9T0NNULO5v2Fn7gD/YVzgtZ6F+crK
# 1eG1R0aVekPmx+NClsCLvy/dX1YmTww=
# =L+6i
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue Mar 31 19:51:48 2026 BST
# gpg: using EDDSA key BCE9123E1AD29F07C049BBDEF712B510A23A0F5F
# gpg: Good signature from "Helge Deller <deller@gmx.de>" [unknown]
# gpg: aka "Helge Deller <deller@kernel.org>" [unknown]
# gpg: aka "Helge Deller <deller@debian.org>" [unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg: There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 4544 8228 2CD9 10DB EF3D 25F8 3E5F 3D04 A7A2 4603
# Subkey fingerprint: BCE9 123E 1AD2 9F07 C049 BBDE F712 B510 A23A 0F5F
* tag 'hppa-more-v11-fixes-pull-request' of https://github.com/hdeller/qemu-hppa:
target/hppa: Update SeaBIOS-hppa to version 24
hw/hppa: Implement memory ranges
target/hppa: Fix TOC handler for 64-bit CPUs
hw/pci-host/astro: Add GMMIO mapping
hw/pci-host/astro: Fix LMMIO DIRECT mappings
hw/pci-host/astro: Implement LMMIO registers
hw/pci-host/astro: Fix initial addresses in IOC
hw/pci-host/astro: Make astro address arrays accessible for other users
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
All 64-bit PA-RISC machines split the memory into (up to 3) different
memory ranges, which are mapped at specific addresses. This patch
mimics the mapping as it's done on physical machines, which includes the
3.75 GB split for C3700, and 1 GB split for newer 64-bit PAT machines
like the A400.
SeaBIOS-hppa needs to know how the memory split is done, so add a new
memsplit_addr variable which stores the specific split address and hand
this over to SeaBIOS-hppa via fwcfg.
Signed-off-by: Helge Deller <deller@gmx.de>
Fix the existing code which has the mask wrong.
Implement the direct mapping via overlapping subregion with priority 3
to make sure the direct mapping gets precedence over the LMMIO region.
Signed-off-by: Helge Deller <deller@gmx.de>
Add code to adjust the memory mapping windows according to the LMMIO registers
in Astro. This allows SeaBIOS-hppa to configure Astro depending on existing
PCI cards, and especially makes it possible to enable a VGA PCI card.
Signed-off-by: Helge Deller <deller@gmx.de>
The original Artist graphics used the GSC bus, was often installed in old
32-bit machines (e.g. 715) and can not be used on 64-bit machines.
This is why this patch makes the artist driver dependend on the Lasi chip,
which was never used in a 64-bit machine.
Note that there exists a variant of Artist for the PCI-bus (Visualize-EG PCI).
It has quite some differences in the registers, and would require that we write
a PCI ROM for it, so that Linux and HP-UX would be able to use it.
Instead, for now, users can simply use a standard VGA or ATI PCI graphics card
on Linux. This can be enabled on the command line with "-device ati-vga" or
"-device VGA". If the "-nographic" option is omitted, a PCI OCHI controller
with USB keyboard and USB mouse will be added automatically.
This fixes graphics support on 64-bit hppa machines and allows us to boot up a
64-bit Linux installation with VGA graphics.
Signed-off-by: Helge Deller <deller@gmx.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20260330211859.19317-3-deller@kernel.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Update the link to the documentation, which was still pointing to the
Wiki page. In the meantime the PA-RISC wiki was converted to a
read-the-docs website.
Signed-off-by: Helge Deller <deller@gmx.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20260330211859.19317-2-deller@kernel.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
ide_sector_read() as well as its callers neglect to call ide_set_retry()
before starting I/O. If the I/O fails, this means that the retry
information is stale. In particular, ide_handle_rw_error() has an
assertion that s->bus->retry_unit == s->unit, which can fail if either
there was no previous request or it came from another device on the bus.
If the assertion weren't there, a wrong request would be retried after
resuming the VM.
Fix this by adding a ide_set_retry() call to ide_sector_read().
This affects only reads because ide_transfer_start() does call
ide_set_retry(). For writes, the data transfer comes first and the I/O
is only started when the data has been read into s->io_buffer, so by
that time, ide_set_retry() has been called. For reads, however, the I/O
comes first and only then the data is transferred to the guest, so the
call in ide_transfer_start() is too late.
Buglink: https://redhat.atlassian.net/browse/RHEL-153537
Reported-by: Tingting Mao <timao@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20260326165124.138593-1-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
In zynq_set_boot_mode() where we parse the string the user has set
the boot-mode option to, we use strncasecmp(str, "qspi", 4) and so
on. This is wrong, because it means that we will ignore any trailing
junk on the end of the option string, and handle
-machine boot-mode=sdXYZZY
the same as
-machine boot-mode=sd
In the documentation we say:
Supported values are ``jtag``, ``sd``, ``qspi`` and ``nor``.
and that's obviously what we meant to implement.
The correct tool for this job is a simple strcasecmp operation.
Switch to that.
We use the g_ascii_strcasecmp() rather than plain strcasecmp()
because we're comparing ASCII strings here and don't want the
potentially locale-specific behaviour that strcasecmp() implies (and
we're trying to standardize on the glib function for this kind of
string comparison).
Fixes: 7df3747c92 ("hw/arm/xilinx_zynq: Add boot-mode property")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20260327145012.907264-1-peter.maydell@linaro.org
In of_dpa_cmd_add_l2_flood(), we allocate memory for the
group->l2_flood.group_ids array, freeing any previous array.
However, in the error-exit path we free the group_ids memory but do
not clear the pointer to NULL. This means that if the guest causes
us to take the error-exit path and then later call the function
again, we will try again to free the memory we already freed.
Fix this by clearing the group_ids pointer in the error exit
path, so we maintain the invariant of "either it points at
allocated memory, or it is NULL" (both being valid to g_free()).
Cc: qemu-stable@nongnu.org
Fixes: dc488f8880 ("rocker: add new rocker switch device")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3253
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20260324193530.375628-1-peter.maydell@linaro.org
Commit 7aa563630b ("pc: Start with modern CPU hotplug interface
by default") removed the .needed callback (vmstate_test_use_cpuhp)
from vmstate_cpuhp_state in both piix4.c and ich9.c.
However, PIIX4 is also used by non-PC boards - MIPS Malta, which does
not select CONFIG_ACPI_CPU_HOTPLUG. For MIPS Malta, the linker resolves
vmstate_cpu_hotplug to the stub one in acpi-cpu-hotplug-stub.c, which is
a zero-initialized VMStateDescription with .fields == NULL.
Before commit 7aa563630b, .needed() of PIIX4's vmstate_cpuhp_state
returned false for MIPS Malta since PIIX4PMState always initialized the
field cpu_hotplug_legacy as true. Malta implicitly relies on this
initial value to bypass vmstate_cpuhp_state. However, this is unstable
because Malta itself does not support CPU hotplugging, whether via the
legacy way or the modern way.
Commit 7aa563630b removed .needed() check for vmstate_cpuhp_state,
this broke the existing dependency that Malta had relied on, forcing
Malta to save and load vmstate_cpuhp_state during the save/load process,
which in turn caused a segmentation fault due to NULL fields in the
stub-compiled code.
Fix this by bringing back the .needed = cpuhp_needed callback for
vmstate_cpuhp_state of PIIX4, that checks
MachineClass::has_hotpluggable_cpus. Boards that do not support CPU
hotplug (only MIPS Malta) will skip this subsection entirely, which
is both correct and consistent with the previous behavior.
At the same time, add a similar .needed() check to ICH9. Although no
boards with ICH9 are affected by this issue, this helps avoid potential
issues in the future.
Reproducer (MIPS Malta):
$ qemu-img create -f qcow2 dummy.qcow2 32M
$ qemu-system-mipsel -nographic \
-drive if=none,format=qcow2,file=dummy.qcow2
[Type "C-a c" to get the "(qemu)" monitor prompt)]
(qemu) savevm foo # segfault
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: 7aa563630b ("pc: Start with modern CPU hotplug interface by default")
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3360
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Link: https://lore.kernel.org/r/20260330053008.2721532-1-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reentrant MMIO can cause the SCSIRequest to be completed, at which
point lsi_request_orphan would drop the last reference. Anything
that happens afterwards would access freed data. Keep a reference
to the SCSIRequest and, through req->hba_private, to the lsi_request*
for as long as DMA runs.
Reported-by: Jihe Wang <wangjihe.mail@gmail.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
To protect against using the lsi_request after SCSIRequest has been freed,
keep the HBA-private data alive until the last reference to the SCSIRequest
is gone. Because req->hba_private was used (even if just for an assertion)
to check that the request was still either current or queued, add a boolean
field that is set when the SCSIRequest is cancelled or completed, which
is when the lsi_request would have been unqueued.
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Protect against changes from reentrant device MMIO during DMA, by always
operating on the same request.
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
If the device is reset, anything that is done before will not really
be visible. So do the reset and exit immediately if that is one
of the requests in the value written to ISTAT0.
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Fix an off-by-one issue in QEMU's HPET read and write MMIO handlers.
Both handlers check timer_id > s->num_timers instead of timer_id >=
s->num_timers, allowing a guest to access one timer beyond the valid
range.
The affected slot is initialized properly in hpet_realize, which goes
through all HPET_MAX_TIMERS elements of the array, so even though
it is not reset in hpet_reset() the bug does not cause any use of
uninitialized host memory. Because of this, and also because (even
though HPET_MAX_TIMERS is 32) the HPET only has room for 24 timers in
its MMIO region, the bug has no security implications.
Commit 869b0afa4f ("rust/hpet: Drop BqlCell wrapper for num_timers",
2025-06-06) silently fixed the same bug in rust/hw/timer/hpet/src/device.rs.
Reported-by: Yuma Kurogome, Ricerca Security, Inc. <yumak@ricsec.co.jp>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Commit 7e82393ed0 ("hw/i2c/aspeed: fix lost interrupts on
back-to-back commands") introduced pending_intr_sts to preserve
interrupt bits that collide with already pending status bits.
That deferred interrupt state is consumed later when the guest clears
INTR_STS, but it is not reset in aspeed_i2c_bus_reset() and it is not
part of the bus migration state. A reset can therefore leave stale
deferred bits behind, and migration can silently drop them.
Clear pending_intr_sts on reset and include it in VMState while keeping
compatibility with older migration streams.
Fixes: 7e82393ed0 ("hw/i2c/aspeed: fix lost interrupts on back-to-back commands")
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
Link: https://lore.kernel.org/qemu-devel/20260325085450.126595-1-zhaoguohan@kylinos.cn
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Jithu Joseph <jithu.joseph@oss.qualcomm.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
In the long run, we would like to get rid of the code that allows to
register migration state globally, so set global_vmstate to false when
using the isa-cirrus-vga device with new machines, and only enable it
for older machines to avoid breaking the migration there.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20260326154850.301609-1-thuth@redhat.com>
QEMU currently crashes when migrating a guest that uses the
isa-vga device as display. This happens because vga_isa_class_initfn()
registers a vmsd for vmstate_vga_common that operates on VGACommonState.
But the isa-vga device is derived from ISADevice, not from VGACommonState,
so the migration code tries to fill in the data for VGACommonState to
the memory that is a ISADevice instead, which is of cause causing trouble.
We need an indirection here as it's also e.g. done in vga-pci.c, so
that the migration data gets filled into the right location.
While we're at it, also drop the "global_vmstate = true" here. Since
migration was broken for this device during the last 15 years (!) anyway,
we don't have to worry about maintaining backward compatibility with this
switch for older versions of QEMU anymore.
Fixes: 7435b791ca ("vga-isa: convert to qdev")
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20260326113457.159065-1-thuth@redhat.com>
In nvme_abort(), the submission queue pointer is dereferenced from the
guest-controlled sqid before validating it with nvme_check_sqid():
NvmeSQueue *sq = n->sq[sqid];
Since sqid is a 16-bit value (range 0-65535) taken directly from CDW10,
and n->sq[] is typically only max_ioqpairs+1 (65) entries, a malicious
guest can trigger an out-of-bounds heap read by sending an Abort command
with a large sqid.
ASan reports this as heap-buffer-overflow in nvme_abort.
Fix this by moving the array dereference to after the nvme_check_sqid()
bounds validation.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3348
Fixes: 75209c071a ("hw/nvme: actually implement abort")
Cc: qemu-stable@nongnu.org
Signed-off-by: Kaixuan Li <kaixuanli@ntu.edu.sg>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
dlfeat was changed from 0x9 to 0x1 when PI support was added.
It was removed because we can't rely on unmap and have to physically
clear it to get the checksums right but that doesnt mean that we do not
support the bit.
The spec says that if wzds is enabled, then the controller supports
deallocate (DEAC) on write zeroes. But DEAC bit in write zeroes command
is only a hint, the controller might choose to physically write zeroes in
those areas.
As we are sending write zeroes command with BDRV_REQ_MAY_UNMAP to the
underlying block device anyway (if the unmap operation is supported),
change the dlfeat value back to 0x9.
A new flag FALLOC_FL_WRITE_ZEROES has been introduced in linux for
fallocate which will use the wzds bit in dlfeat to quickly zeroout extents
using unmap operation whenever possible[1].
[1] https://lore.kernel.org/linux-fsdevel/20250619111806.3546162-1-yi.zhang@huaweicloud.com/
Fixes: 146f720c55 ("hw/block/nvme: end-to-end data protection")
Suggested-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
The version is never set on 2.5+ machine types, so qemu_hw_version() and
qemu_set_hw_version() are not needed anymore.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Change accel SMMUv3 OAS property from uint8_t to OasMode. The
'auto' value is not implemented, as this commit is meant to
set the property to the correct type and avoid breaking JSON/QMP
when the auto mode is introduced. A future patch will implement
resolution of 'auto' value to match the host SMMUv3 OAS value.
The conversion of the "oas" property type to OnOffAuto is an
incompatible change for JSON/QMP when a uint8_t value is expected for
"oas", but this property is new in 11.0 and this patch is
submitted as a fix to the property type.
Fixes: a015ac990f ("hw/arm/smmuv3-accel: Add property to specify OAS bits")
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Shameer Kolothum <skolothumtho@nvidia.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Nathan Chen <nathanc@nvidia.com>
Message-id: 20260323182454.1416110-8-nathanc@nvidia.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Change accel SMMUv3 SSIDSIZE property from uint8_t to SsidSizeMode.
The 'auto' value is not implemented, as this commit is meant to set the
property to the correct type and avoid breaking JSON/QMP when the auto
mode is introduced. A future patch will implement resolution of 'auto'
value to match the host SMMUv3 SSIDSIZE value.
The conversion of the "ssidsize" property type to OnOffAuto is an
incompatible change for JSON/QMP when a uint8_t value is expected for
"ssidsize", but this property is new in 11.0 and this patch is
submitted as a fix to the property type.
Fixes: b8c6f8a69d ("hw/arm/smmuv3-accel: Make SubstreamID support configurable")
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Shameer Kolothum <skolothumtho@nvidia.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Nathan Chen <nathanc@nvidia.com>
Message-id: 20260323182454.1416110-6-nathanc@nvidia.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Change accel SMMUv3 RIL property from bool to OnOffAuto. The 'auto'
value is not implemented, as this commit is meant to set the property
to the correct type and avoid breaking JSON/QMP when the auto mode is
introduced. A future patch will implement resolution of the 'auto'
value to match the host SMMUv3 RIL support.
The conversion of the RIL property type to OnOffAuto is an
incompatible change for JSON/QMP when a bool value is expected for
"ril", but the "ril" property is new in 11.0 and this patch is
submitted as a fix to the property type.
Fixes: bd715ff5bd ("hw/arm/smmuv3-accel: Add a property to specify RIL support")
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
Tested-by: Shameer Kolothum <skolothumtho@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Nathan Chen <nathanc@nvidia.com>
Message-id: 20260323182454.1416110-4-nathanc@nvidia.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Change accel SMMUv3 ATS property from bool to OnOffAuto. The 'auto'
value is not implemented, as this commit is meant to set the property
to the correct type and avoid breaking JSON/QMP when the auto mode is
introduced. A future patch will implement resolution of the 'auto'
value to match the host SMMUv3 ATS support.
The conversion of the ATS property type to OnOffAuto is an
incompatible change for JSON/QMP when a bool value is expected for
"ats", but the "ats" property is new in 11.0 and this patch is
submitted as a fix to the property type.
Fixes: f7f5013a55 ("hw/arm/smmuv3-accel: Add support for ATS")
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
Tested-by: Shameer Kolothum <skolothumtho@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Nathan Chen <nathanc@nvidia.com>
Message-id: 20260323182454.1416110-3-nathanc@nvidia.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
The logic in the PL080 for transferring data has multiple bugs:
* The TransferSize field in the channel control register counts
in units of the source width; because our loop may do multiple
source loads if the destination width is greater than the
source width, we need to decrement it by (xsize / swidth),
not by 1, each loop
* It is documented in the TRM that it is a software error to program
the source and destination width such that SWidth < DWidth and
TransferSize * SWidth is not a multiple of DWidth. (This would
mean that there isn't enough data to do a full final destination
write.) We weren't doing anything sensible with this case.
The TRM doesn't document what the hardware actually does (though
it drops some hints that suggest that it probably over-reads
from the source).
* In the loop to write to the destination, each loop adds swidth
to ch->dest for each loop and also uses (ch->dest + n) as the
destination address. This moves the destination address on
further than we should each time round the loop, and also
is incrementing ch->dest by swidth when it should be dwidth.
This patch fixes these problems:
* decrement TransferSize by the correct amount
* log and ignore the transfer size mismatch case
* correct the loop logic for the destination writes
A repro case which exercises some of this is as follows. It
configures swidth to 1 byte, dwidth to 4 bytes, and transfer size 4,
for a transfer from 0x00000000 to 0x000010000. Examining the
destination memory in the QEMU monitor should show that the
source data 0x44332211 has all been copied, but before this
fix it is not:
./qemu-system-arm -M versatilepb -m 128M -nographic -S \
-device loader,addr=0x00000000,data=0x44332211,data-len=4 \
-device loader,addr=0x00001000,data=0x00000000,data-len=4 \
-device loader,addr=0x10130030,data=0x00000001,data-len=4 \
-device loader,addr=0x10130100,data=0x00000000,data-len=4 \
-device loader,addr=0x10130104,data=0x00001000,data-len=4 \
-device loader,addr=0x10130108,data=0x00000000,data-len=4 \
-device loader,addr=0x1013010C,data=0x9e47f004,data-len=4 \
-device loader,addr=0x10130110,data=0x0000c001,data-len=4
Without this patch the QEMU monitor shows:
(qemu) xp /1wx 0x00001000
00001000: 0x00002211
Correct result:
(qemu) xp /1wx 0x00001000
00001000: 0x44332211
Cc: qemu-stable@nongnu.org
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Tao Ding <dingtao0430@163.com>
[PMM: Wrote up what we are fixing in the commit message]
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
According to the Aspeed datasheet, the RX_BUF_LEN_W1T and
TX_BUF_LEN_W1T bits of the A_I2CS_DMA_LEN (0x2c) register allow
firmware to program the TX and RX DMA length (TX_BUF_LEN and
RX_BUF_LEN fields of the same register) separately without the need to
read/modify/write the value. If RX_BUF_LEN_W1T and TX_BUF_LEN_W1T
bits are 0, then both TX and RX DMA length will be written.
When setting the RX_BUF_LEN field, the TX_BUF_LEN field being set is
not an invalid condition. Remove the assert.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3315
Reviewed-by: Jamin Lin <jamin_lin@aspeedtech.com>
Link: https://lore.kernel.org/qemu-devel/20260323125545.577653-4-clg@redhat.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
QEMU executes I2C commands synchronously inside the CMD register write
handler. On real hardware each command takes time on the bus, so the
ISR can clear the previous interrupt status before the next completion
arrives. In QEMU, when the guest ISR handles a TX_ACK and immediately
issues the next command by writing to CMD, that command completes
instantly — before the ISR returns to W1C-clear the first TX_ACK.
Since the bit is already set, setting it again is a no-op. The ISR
then clears it, wiping both completions at once. No interrupt fires
for the second command and the driver stalls.
This affects any multi-step I2C transaction: register reads, SMBus
word reads, and PMBus device probes all fail ("Error: Read failed"
from i2cget, -ETIMEDOUT from kernel drivers).
The issue is exposed when the guest kernel includes commit "i2c:
aspeed: Acknowledge Tx done with and without ACK irq late" [1] which
defers W1C acknowledgment of TX_ACK until after the ISR has issued
the next command. This means the old TX_ACK is still set when the
next command completes synchronously, and the subsequent W1C wipes
both completions at once.
The trace below shows `i2cget -y 15 0x50 0x00` (read EEPROM register
0x00) failing without the fix. The first START+TX sets TX_ACK. The
ISR handles it and issues a second TX to send the register address.
That TX completes synchronously while TX_ACK is still set:
aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0 # START+TX, clean
aspeed_i2c_bus_raise_interrupt intr=0x1 ack| # TX_ACK set
aspeed_i2c_bus_read 0x10: 0x1 # ISR reads TX_ACK
aspeed_i2c_bus_write 0x14: 0x2 # ISR issues TX cmd
aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x1 # TX runs, TX_ACK already set!
aspeed_i2c_bus_raise_interrupt intr=0x1 ack| # re-set is no-op
aspeed_i2c_bus_write 0x10: 0x1 # ISR W1C clears TX_ACK
aspeed_i2c_bus_read 0x10: 0x0 # LOST — both ACKs wiped
The driver sees INTR_STS=0 and never proceeds to the read phase.
Fix this by tracking interrupt bits that collide with already-pending
bits. Before calling aspeed_i2c_bus_handle_cmd(), save and clear
INTR_STS so that only freshly set bits are visible after the call.
Any overlap between the old and new bits is saved in pending_intr_sts.
When the ISR later W1C-clears the old bits, re-apply the saved
pending bits so the ISR sees them on its next loop iteration.
With the fix, the same operation completes successfully:
aspeed_i2c_bus_cmd cmd=0x3 start|tx| intr=0x0 # START+TX, clean
aspeed_i2c_bus_raise_interrupt intr=0x1 ack| # TX_ACK set
aspeed_i2c_bus_read 0x10: 0x1 # ISR reads TX_ACK
aspeed_i2c_bus_write 0x14: 0x2 # ISR issues TX cmd
aspeed_i2c_bus_cmd cmd=0x400002 tx| intr=0x0 # INTR_STS cleared first
aspeed_i2c_bus_raise_interrupt intr=0x1 ack| # TX_ACK freshly set
aspeed_i2c_bus_write 0x10: 0x1 # ISR W1C clears TX_ACK
aspeed_i2c_bus_read 0x10: 0x1 # RE-DELIVERED from pending
aspeed_i2c_bus_write 0x14: 0x1b # ISR proceeds: START+RX
aspeed_i2c_bus_cmd cmd=0x40001b start|tx|rx|last| # read phase completes
i2c_recv recv(addr:0x50) data:0x00 # data received
[1] https://lore.kernel.org/all/20231211102217.2436294-3-quan@os.amperecomputing.com/
Signed-off-by: Jithu Joseph <jithu.joseph@oss.qualcomm.com>
Fixes: 1602001195 ("i2c: add aspeed i2c controller")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260311023712.2730185-1-jithu.joseph@oss.qualcomm.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
All examples on https://github.com/Agnoctopus/libu2f-emu/ don't
simply include u2f-emu.h without any added directory.
The additional include directory does not exist when libu2f
was built with meson.
It's up to pkgconfig to make sure that u2f-emu.h is found in any case.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
