Re: [PATCH v4 7/9] pcie_sriov: Release VFs failed to realize
On 2024/02/14 16:54, Michael S. Tsirkin wrote:
On Wed, Feb 14, 2024 at 02:13:45PM +0900, Akihiko Odaki wrote:
Release VFs failed to realize just as we do in unregister_vfs().
Fixes: 7c0fa8dff811 ("pcie: Add support for Single Root I/O Virtualization
(SR/IOV)")
Signed-off-by: Akihiko Odaki
Is this fixing an old bug or a bug introduced by a previous patch?
Old bug, present since: commit 7c0fa8dff811 ("pcie: Add support for
Single Root I/O Virtualization (SR/IOV)")
Re: [PATCH v6] util: Move dbus_display1 to util
On 2024/02/14 16:05, Marc-André Lureau wrote: Hi Akihiko On Wed, Feb 14, 2024 at 9:39 AM Akihiko Odaki wrote: Move dbus_display1 from ui to util as it's used not only by ui/dbus but also dbus-display-test. It doesn't seem like the right place either. So let's focus on what this is fixing. Ok. Then please have a look at v2, which doesn't use util: https://lore.kernel.org/all/20231215-dbus-v2-0-1e2e6aa02...@daynix.com/
Re: [PATCH v4 7/9] pcie_sriov: Release VFs failed to realize
On Wed, Feb 14, 2024 at 02:13:45PM +0900, Akihiko Odaki wrote:
> Release VFs failed to realize just as we do in unregister_vfs().
>
> Fixes: 7c0fa8dff811 ("pcie: Add support for Single Root I/O Virtualization
> (SR/IOV)")
> Signed-off-by: Akihiko Odaki
Is this fixing an old bug or a bug introduced by a previous patch?
> ---
> hw/pci/pcie_sriov.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> index 9ba34cf8f8ed..9d668b8d6c17 100644
> --- a/hw/pci/pcie_sriov.c
> +++ b/hw/pci/pcie_sriov.c
> @@ -91,6 +91,8 @@ bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
> vf->exp.sriov_vf.vf_number = i;
>
> if (!qdev_realize(&vf->qdev, bus, errp)) {
> +object_unparent(OBJECT(vf));
> +object_unref(vf);
> unrealize_vfs(dev, i);
> return false;
> }
>
> --
> 2.43.0
Re: [PATCH v4 6/9] pcie_sriov: Reuse SR-IOV VF device instances
On Wed, Feb 14, 2024 at 02:13:44PM +0900, Akihiko Odaki wrote:
> Disable SR-IOV VF devices by reusing code to power down PCI devices
> instead of removing them when the guest requests to disable VFs. This
> allows to realize devices and report VF realization errors at PF
> realization time.
>
> Signed-off-by: Akihiko Odaki
This patch does much more than what commit log says.
This really needs to be split up with each change you make
documented.
> ---
> docs/pcie_sriov.txt | 8 ++--
> include/hw/pci/pci.h| 2 +-
> include/hw/pci/pci_device.h | 2 +-
> include/hw/pci/pcie_sriov.h | 6 +--
> hw/net/igb.c| 13 --
> hw/nvme/ctrl.c | 24 +++
> hw/pci/pci.c| 18
> hw/pci/pci_host.c | 4 +-
> hw/pci/pcie.c | 4 +-
> hw/pci/pcie_sriov.c | 100
>
> 10 files changed, 97 insertions(+), 84 deletions(-)
>
> diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
> index a47aad0bfab0..ab2142807f79 100644
> --- a/docs/pcie_sriov.txt
> +++ b/docs/pcie_sriov.txt
> @@ -52,9 +52,11 @@ setting up a BAR for a VF.
>...
>
>/* Add and initialize the SR/IOV capability */
> - pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
> - vf_devid, initial_vfs, total_vfs,
> - fun_offset, stride);
> + if (!pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
> + vf_devid, initial_vfs, total_vfs,
> + fun_offset, stride, errp)) {
> + return;
> + }
>
>/* Set up individual VF BARs (parameters as for normal BARs) */
>pcie_sriov_pf_init_vf_bar( ... )
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index eaa3fc99d884..442017b4865d 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -642,6 +642,6 @@ static inline void pci_irq_pulse(PCIDevice *pci_dev)
> }
>
> MSIMessage pci_get_msi_message(PCIDevice *dev, int vector);
> -void pci_set_power(PCIDevice *pci_dev, bool state);
> +void pci_set_enabled(PCIDevice *pci_dev, bool state);
Not a good name. Does this set enabled state? Test whether
some kind of set is enabled?
>
> #endif
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index 54fa0676abf1..f5aba8ae2675 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -56,7 +56,7 @@ typedef struct PCIReqIDCache PCIReqIDCache;
> struct PCIDevice {
> DeviceState qdev;
> bool partially_hotplugged;
> -bool has_power;
> +bool is_enabled;
>
> /* PCI config space */
> uint8_t *config;
> diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
> index 095fb0c9edf9..d9a39daccac4 100644
> --- a/include/hw/pci/pcie_sriov.h
> +++ b/include/hw/pci/pcie_sriov.h
> @@ -18,7 +18,6 @@
> struct PCIESriovPF {
> uint16_t num_vfs; /* Number of virtual functions created */
> uint8_t vf_bar_type[PCI_NUM_REGIONS]; /* Store type for each VF bar */
> -const char *vfname; /* Reference to the device type used for the VFs */
> PCIDevice **vf; /* Pointer to an array of num_vfs VF devices */
> };
>
> @@ -27,10 +26,11 @@ struct PCIESriovVF {
> uint16_t vf_number; /* Logical VF number of this function */
> };
>
> -void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
> +bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
> const char *vfname, uint16_t vf_dev_id,
> uint16_t init_vfs, uint16_t total_vfs,
> -uint16_t vf_offset, uint16_t vf_stride);
> +uint16_t vf_offset, uint16_t vf_stride,
> +Error **errp);
Returning false to indicate an error is a bad idea -
most APIs we have return 0 on success, < 0 on error.
Or just use errp.
> void pcie_sriov_pf_exit(PCIDevice *dev);
>
> /* Set up a VF bar in the SR/IOV bar area */
> diff --git a/hw/net/igb.c b/hw/net/igb.c
> index 0b5c31a58bba..1079a33d4000 100644
> --- a/hw/net/igb.c
> +++ b/hw/net/igb.c
> @@ -447,9 +447,16 @@ static void igb_pci_realize(PCIDevice *pci_dev, Error
> **errp)
>
> pcie_ari_init(pci_dev, 0x150);
>
> -pcie_sriov_pf_init(pci_dev, IGB_CAP_SRIOV_OFFSET, TYPE_IGBVF,
> -IGB_82576_VF_DEV_ID, IGB_MAX_VF_FUNCTIONS, IGB_MAX_VF_FUNCTIONS,
> -IGB_VF_OFFSET, IGB_VF_STRIDE);
> +if (!pcie_sriov_pf_init(pci_dev, IGB_CAP_SRIOV_OFFSET,
> +TYPE_IGBVF, IGB_82576_VF_DEV_ID,
> +IGB_MAX_VF_FUNCTIONS, IGB_MAX_VF_FUNCTIONS,
> +IGB_VF_OFFSET, IGB_VF_STRIDE,
> +errp)) {
> +pcie_cap_exit(pci_dev);
> +igb_cleanup_msix(s);
> +msi_uninit(pci_dev);
> +return;
> +}
>
> pcie_sriov_pf_init_vf_bar(pci_dev, IGBVF_MMIO_BAR_IDX,
> P
Re: possible deprecation and removal of some old QEMU Arm machine types (pxa2xx, omap, sa1110)
On 14/2/24 08:37, Philippe Mathieu-Daudé wrote: On 12/2/24 13:32, Peter Maydell wrote: So I would like to explore whether we can deprecate-and-drop some or all of them. This would let us delete the code entirely rather than spending a long time trying to bring it up to scratch for a probably very small to nonexistent userbase. The aim of this email is to see if anybody is still using any of these and would be upset if they went away. Reports of "I tried to use this machine type and it's just broken" are also interesting as they would strongly suggest that the machine has no real users and can be removed. The machines I have in mind are: OMAP2 machines: n800 Nokia N800 tablet aka. RX-34 (OMAP2420) n810 Nokia N810 tablet aka. RX-44 (OMAP2420) For me these are broken since 2020 (commit 7998beb9c2 "arm/nseries: use memdev for RAM"), this was discussed in https://lore.kernel.org/qemu-devel/f2f276a9-a6ad-a2f8-2fbc-f1aca5423...@amsat.org/ but there was no clear consensus so I gave up testing them. Eh good news the tests work again: $ make check-avocado AVOCADO_TAGS='machine:n800 machine:n810' AVOCADO_ALLOW_UNTRUSTED_CODE=1 QEMU_TEST_FLAKY_TESTS=1 AVOCADO_SHOW=app,console AVOCADO tests/avocado JOB ID : e089badc8daeae0882eb48e452164d00e8e19c18 JOB LOG: job-2024-02-14T07.44-e089bad/job.log (1/2) tests/avocado/machine_arm_n8x0.py:N8x0Machine.test_n800: \ console: [0.00] Linux version 2.6.35~rc4-129.1-n8x0 (abuild@build08) (gcc version 4.4.2 20091027 (MeeGo 4.4.2-7) (GCC) ) #1 PREEMPT Mon Jul 12 15:04:46 UTC 2010 console: [0.00] CPU: ARMv6-compatible processor [4107b362] revision 2 (ARMv6TEJ), cr=00c5387f console: [0.00] CPU: VIPT aliasing data cache, VIPT aliasing instruction cache console: [0.00] Machine: Nokia N800 console: [0.00] Memory policy: ECC disabled, Data cache writeback console: [0.00] OMAP2420 console: [0.00] console: [0.00] SRAM: Mapped pa 0x4020 to va 0xfe40 size: 0x10 console: [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 32512 console: [0.00] Kernel command line: printk.time=0 console=ttyS1 console: PID hash table entries: 512 (order: -1, 2048 bytes) console: Dentry cache hash table entries: 16384 (order: 4, 65536 bytes) console: Inode-cache hash table entries: 8192 (order: 3, 32768 bytes) console: Memory: 128MB = 128MB total console: Memory: 124008k/124008k available, 7064k reserved, 0K highmem console: Virtual kernel memory layout: console: vector : 0x - 0x1000 ( 4 kB) console: fixmap : 0xfff0 - 0xfffe ( 896 kB) console: DMA : 0xffc0 - 0xffe0 ( 2 MB) console: vmalloc : 0xc880 - 0xf800 ( 760 MB) console: lowmem : 0xc000 - 0xc800 ( 128 MB) console: modules : 0xbf00 - 0xc000 ( 16 MB) console: .init : 0xc0008000 - 0xc0029000 ( 132 kB) console: .text : 0xc0029000 - 0xc0379000 (3392 kB) console: .data : 0xc038e000 - 0xc03ba2e0 ( 177 kB) console: SLUB: Genslabs=11, HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1 console: Hierarchical RCU implementation. console: RCU-based detection of stalled CPUs is disabled. console: Verbose stalled-CPUs detection is disabled. console: NR_IRQS:368 console: [ cut here ] console: WARNING: at arch/arm/mach-omap2/clkt_clksel.c:194 omap2_clksel_recalc+0xcc/0xe0() console: clock: Could not find fieldval 0 for clock iva1_ifck parent core_ck console: Modules linked in: console: [] (unwind_backtrace+0x0/0xf0) from [] (warn_slowpath_common+0x4c/0x64) console: [] (warn_slowpath_common+0x4c/0x64) from [] (warn_slowpath_fmt+0x2c/0x3c) console: [] (warn_slowpath_fmt+0x2c/0x3c) from [] (omap2_clksel_recalc+0xcc/0xe0) console: [] (omap2_clksel_recalc+0xcc/0xe0) from [] (propagate_rate+0x20/0x50) console: [] (propagate_rate+0x20/0x50) from [] (propagate_rate+0x2c/0x50) console: ---[ end trace 1b75b31a2719ed1c ]--- console: [ cut here ] console: WARNING: at arch/arm/mach-omap2/clkt_clksel.c:194 omap2_clksel_recalc+0xcc/0xe0() console: clock: Could not find fieldval 0 for clock dsp_fck parent core_ck console: Modules linked in: console: [] (unwind_backtrace+0x0/0xf0) from [] (warn_slowpath_common+0x4c/0x64) console: [] (warn_slowpath_common+0x4c/0x64) from [] (warn_slowpath_fmt+0x2c/0x3c) console: [] (warn_slowpath_fmt+0x2c/0x3c) from [] (omap2_clksel_recalc+0xcc/0xe0) console: [] (omap2_clksel_recalc+0xcc/0xe0) from [] (propagate_rate+0x20/0x50) console: [] (propagate_rate+0x20/0x50) from [] (propagate_rate+0x2c/0x50) console: ---[ end trace 1b75b31a2719ed1d ]--- console: [ cut here ] console: WARNING: at arch/arm/mach-omap2/clkt_clksel.c:194 omap2_clksel_recalc+0xcc/0xe0() console: clock: Could not find fieldval 0 for clock dsp_irate_ick parent dsp_fck console: Modules linked in: console: [] (unwind_backtrace+0x0/0xf0) fro
Re: [PATCH v4 4/4] qemu-options.hx: Add an entry for virtio-iommu-pci and document aw-bits
On 2/13/24 19:28, Eric Auger wrote: We are missing an entry for the virtio-iommu-pci device. Add the information on which machine it is currently supported and document the new aw-bits option. Signed-off-by: Eric Auger --- qemu-options.hx | 8 1 file changed, 8 insertions(+) diff --git a/qemu-options.hx b/qemu-options.hx index 8547254dbf..6a8c970640 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1172,6 +1172,14 @@ SRST Please also refer to the wiki page for general scenarios of VT-d emulation in QEMU: https://wiki.qemu.org/Features/VT-d. +``-device virtio-iommu-pci[,option=...]`` +This is only supported by ``-machine q35`` and ``-machine virt``. +It supports below options: + +``aw-bits=val`` (val between 32 and 64, default depends on machine) +This decides the address width of IOVA address space. With +q35 it defaults to 39 bits. On arm virt it defaults to 48 bits. Minor improvement : "It defaults to 39 bits on q35 machines and 48 bits on ARM virt machines." Anyhow, Reviewed-by: Cédric Le Goater Thanks, C.
Re: possible deprecation and removal of some old QEMU Arm machine types (pxa2xx, omap, sa1110)
On 12/2/24 13:32, Peter Maydell wrote: So I would like to explore whether we can deprecate-and-drop some or all of them. This would let us delete the code entirely rather than spending a long time trying to bring it up to scratch for a probably very small to nonexistent userbase. The aim of this email is to see if anybody is still using any of these and would be upset if they went away. Reports of "I tried to use this machine type and it's just broken" are also interesting as they would strongly suggest that the machine has no real users and can be removed. The machines I have in mind are: PXA2xx machines: connex Gumstix Connex (PXA255) verdex Gumstix Verdex Pro XL6P COMs (PXA270) I can still run U-boot on these, but Gumstix webs are slowly disappearing with the prebuilt images there were sharing. Their wiki is also dead. I'm happy to use a stable release for my pflash experiments. OMAP2 machines: n800 Nokia N800 tablet aka. RX-34 (OMAP2420) n810 Nokia N810 tablet aka. RX-44 (OMAP2420) For me these are broken since 2020 (commit 7998beb9c2 "arm/nseries: use memdev for RAM"), this was discussed in https://lore.kernel.org/qemu-devel/f2f276a9-a6ad-a2f8-2fbc-f1aca5423...@amsat.org/ but there was no clear consensus so I gave up testing them.
Re: [PATCH] target/ppc: Fix lxv/stxv MSR facility check
On 2/13/24 09:39, Nicholas Piggin wrote:
The move to decodetree flipped the inequality test for the VEC / VSX
MSR facility check.
This caused application crashes under Linux, where these facility
unavailable interrupts are used for lazy-switching of VEC/VSX register
sets. Getting the incorrect interrupt would result in wrong registers
being loaded, potentially overwriting live values and/or exposing
stale ones.
Cc: qemu-sta...@nongnu.org
Reported-by: Joel Stanley
Fixes: 70426b5bb738 ("target/ppc: moved stxvx and lxvx from legacy to
decodtree")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1769
Tested-by: Harsh Prateek Bora
Signed-off-by: Nicholas Piggin
Reviewed-by: Cédric Le Goater
Tested-by: Cédric Le Goater
with a RHEL9 image.
Thanks,
C.
---
target/ppc/translate/vsx-impl.c.inc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/ppc/translate/vsx-impl.c.inc
b/target/ppc/translate/vsx-impl.c.inc
index 6db87ab336..0266f09119 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -2268,7 +2268,7 @@ static bool do_lstxv(DisasContext *ctx, int ra, TCGv
displ,
static bool do_lstxv_D(DisasContext *ctx, arg_D *a, bool store, bool paired)
{
-if (paired || a->rt >= 32) {
+if (paired || a->rt < 32) {
REQUIRE_VSX(ctx);
} else {
REQUIRE_VECTOR(ctx);
Re: [PATCH 4/4] hw/arm/stellaris: Add missing QOM 'SoC' parent
On 2/13/24 16:36, Philippe Mathieu-Daudé wrote: On 1/2/24 17:46, Peter Maydell wrote: On Tue, 30 Jan 2024 at 19:03, Philippe Mathieu-Daudé wrote: QDev objects created with qdev_new() need to manually add their parent relationship with object_property_add_child(). Since we don't model the SoC, just use a QOM container. Signed-off-by: Philippe Mathieu-Daudé --- Ah, this is where the other qdev_new() calls are sorted. Reviewed-by: Peter Maydell I wonder if we should add a variant on qdev_new() that you can pass in the parent object to? Yes, this is what we discussed with Markus. In order to stop using the "/unattached" container from pre-QOM, qdev_new() must take a QOM parent. I tried to do it but hit some problem with some odd use in PPC or S390 (discussed with Cédric so likely PPC, I need to go back to it). Can you remind what this was about ? Thanks, C.
Re: [PATCH v4 9/9] hw/nvme: Refer to dev->exp.sriov_pf.num_vfs
On Wed, Feb 14, 2024 at 02:13:47PM +0900, Akihiko Odaki wrote:
> NumVFs may not equal to the current effective number of VFs because VF
> Enable is cleared, NumVFs is set after VF Enable is set, or NumVFs is
> greater than TotalVFs.
>
> Fixes: 11871f53ef8e ("hw/nvme: Add support for the Virtualization Management
> command")
> Signed-off-by: Akihiko Odaki
I don't get what this is saying about VF enable.
This code will not trigger on numVFs write when VF enable is set.
Generally this commit makes no sense on its own, squash it with
the pci core change pls.
> ---
> hw/nvme/ctrl.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index f8df622fe590..daedda5d326f 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -8481,7 +8481,7 @@ static void nvme_sriov_pre_write_ctrl(PCIDevice *dev,
> uint32_t address,
> NvmeSecCtrlEntry *sctrl;
> uint16_t sriov_cap = dev->exp.sriov_cap;
> uint32_t off = address - sriov_cap;
> -int i, num_vfs;
> +int i;
>
> if (!sriov_cap) {
> return;
> @@ -8489,8 +8489,7 @@ static void nvme_sriov_pre_write_ctrl(PCIDevice *dev,
> uint32_t address,
>
> if (range_covers_byte(off, len, PCI_SRIOV_CTRL)) {
> if (!(val & PCI_SRIOV_CTRL_VFE)) {
> -num_vfs = pci_get_word(dev->config + sriov_cap +
> PCI_SRIOV_NUM_VF);
> -for (i = 0; i < num_vfs; i++) {
> +for (i = 0; i < dev->exp.sriov_pf.num_vfs; i++) {
> sctrl = &n->sec_ctrl_list.sec[i];
> nvme_virt_set_state(n, le16_to_cpu(sctrl->scid), false);
> }
>
> --
> 2.43.0
Re: [PATCH v6] util: Move dbus_display1 to util
Hi Akihiko
On Wed, Feb 14, 2024 at 9:39 AM Akihiko Odaki wrote:
>
> Move dbus_display1 from ui to util as it's used not only by ui/dbus but
> also dbus-display-test.
It doesn't seem like the right place either. So let's focus on what
this is fixing.
> dbus_display1 is now added to util_ss accordingly. It ensures that the
> source will be linked with audio/dbus, and also avoids recompilation
> when linking with dbus-display-test.
- "It ensures that the source will be linked with audio/dbus"
Right, this could be achieved with: module_ss.add(when: [gio, pixman],
if_true: [files('dbusaudio.c'), dbus_display1_dep])
- "avoids recompilation when linking with dbus-display-test"
Similarly: qtests += {'dbus-display-test': [dbus_display1_dep, gio]}
>
> dbus-display.h is also added to genh to ensure it is generated before
> compiling ui/dbus, audio/dbus, and dbus-display-test.
>
meson should take care of those dependencies if they are expressed correctly.
> Both changes combined, it is no longer necessary for ui/dbus,
> audio/dbus, and dbus-display-test to explicitly state the dependency on
> dbus_display1.
This is eventually nice for things that are really common, but in this
case I don't think it's a win.
>
> Signed-off-by: Akihiko Odaki
> ---
> I found it was failing to build dbus modules when --enable-dbus so here
> are fixes.
> ---
> Changes in v6:
> - Dropped patch "audio: Do not include ui/dbus.h" (Marc-André Lureau).
> - Rebased.
> - Link to v5:
> https://lore.kernel.org/r/20231217-dbus-v5-0-8122e822a...@daynix.com
>
> Changes in v5:
> - Fixed docs/interop/dbus-display.rst.
> - Link to v4:
> https://lore.kernel.org/r/20231217-dbus-v4-0-4fd5410bf...@daynix.com
>
> Changes in v4:
> - Moved dbus_display1 to util.
> - Link to v3:
> https://lore.kernel.org/r/20231216-dbus-v3-0-b4bcbed73...@daynix.com
>
> Changes in v3:
> - Merged dbus_display1_lib into libqemuutil.
> - Added patch "audio: Do not include ui/dbus.h".
> - Link to v2:
> https://lore.kernel.org/r/20231215-dbus-v2-0-1e2e6aa02...@daynix.com
>
> Changes in v2:
> - Updated MAINTAINERS.
> - Link to v1:
> https://lore.kernel.org/r/20231215-dbus-v1-0-349e059ac...@daynix.com
> ---
> MAINTAINERS | 2 +-
> docs/interop/dbus-display.rst | 6 +++---
> ui/dbus.h | 2 +-
> audio/dbusaudio.c | 2 +-
> tests/qtest/dbus-display-test.c | 2 +-
> tests/qtest/meson.build | 2 +-
> ui/meson.build | 20 +---
> {ui => util}/dbus-display1.xml | 0
> util/meson.build| 21 +
> 9 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f80db6a96a3f..0a81159e33d1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3413,7 +3413,7 @@ S: Maintained
> F: backends/dbus-vmstate.c
> F: ui/dbus*
> F: audio/dbus*
> -F: util/dbus.c
> +F: util/dbus*
> F: include/ui/dbus*
> F: include/qemu/dbus.h
> F: docs/interop/dbus*
> diff --git a/docs/interop/dbus-display.rst b/docs/interop/dbus-display.rst
> index 8c6e8e0f5a82..efec89723a34 100644
> --- a/docs/interop/dbus-display.rst
> +++ b/docs/interop/dbus-display.rst
> @@ -18,14 +18,14 @@ QEMU also implements the standard interfaces, such as
>
> .. only:: sphinx4
>
> - .. dbus-doc:: ui/dbus-display1.xml
> + .. dbus-doc:: util/dbus-display1.xml
>
> .. only:: not sphinx4
>
> .. warning::
>Sphinx 4 is required to build D-Bus documentation.
>
> - This is the content of ``ui/dbus-display1.xml``:
> + This is the content of ``util/dbus-display1.xml``:
>
> - .. literalinclude:: ../../ui/dbus-display1.xml
> + .. literalinclude:: ../../util/dbus-display1.xml
>:language: xml
> diff --git a/ui/dbus.h b/ui/dbus.h
> index 1e8c24a48e32..a847bee98876 100644
> --- a/ui/dbus.h
> +++ b/ui/dbus.h
> @@ -31,7 +31,7 @@
> #include "ui/console.h"
> #include "ui/clipboard.h"
>
> -#include "ui/dbus-display1.h"
> +#include "util/dbus-display1.h"
>
> typedef struct DBusClipboardRequest {
> GDBusMethodInvocation *invocation;
> diff --git a/audio/dbusaudio.c b/audio/dbusaudio.c
> index 60fcf643ecf8..2aacdac6715b 100644
> --- a/audio/dbusaudio.c
> +++ b/audio/dbusaudio.c
> @@ -34,7 +34,7 @@
> #endif
>
> #include "ui/dbus.h"
> -#include "ui/dbus-display1.h"
> +#include "util/dbus-display1.h"
>
> #define AUDIO_CAP "dbus"
> #include "audio.h"
> diff --git a/tests/qtest/dbus-display-test.c b/tests/qtest/dbus-display-test.c
> index 21edaa1e321f..d4871e2fd80f 100644
> --- a/tests/qtest/dbus-display-test.c
> +++ b/tests/qtest/dbus-display-test.c
> @@ -5,7 +5,7 @@
> #include
> #include
> #include "libqtest.h"
> -#include "ui/dbus-display1.h"
> +#include "util/dbus-display1.h"
>
> static GDBusConnection*
> test_dbus_p2p_from_fd(int fd)
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 39557d5ecbb0..627ff8fbe1c7 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -344,7 +344,7 @
Re: [PATCH 4/7] hw/i386/acpi: Declare pc_madt_cpu_entry() in 'acpi-common.h'
On 13/2/24 19:57, Bernhard Beschow wrote: Am 13. Februar 2024 12:01:49 UTC schrieb "Philippe Mathieu-Daudé" : Since pc_madt_cpu_entry() is only used by: - hw/i386/acpi-build.c // single call - hw/i386/acpi-common.c // definition there is no need to expose it outside of hw/i386/. Declare it in "acpi-common.h". acpi-build.c doesn't need "hw/i386/pc.h" anymore. Signed-off-by: Philippe Mathieu-Daudé --- hw/i386/acpi-common.h | 3 +++ include/hw/i386/pc.h | 4 hw/i386/acpi-common.c | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h index b3c56ee014..e305aaac15 100644 --- a/hw/i386/acpi-common.h +++ b/hw/i386/acpi-common.h @@ -1,12 +1,15 @@ #ifndef HW_I386_ACPI_COMMON_H #define HW_I386_ACPI_COMMON_H +#include "hw/boards.h" #include "hw/acpi/bios-linker-loader.h" #include "hw/i386/x86.h" /* Default IOAPIC ID */ #define ACPI_BUILD_IOAPIC_ID 0x0 +void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids, Since the function is apparently not pc but rather x86-specific: Does it make sense to rename the function as well, e.g. to x86_madt_cpu_entry()? I don't know much about ACPI tables. Is it? Ani, can you confirm and do you mind posting a cleanup patch on top? :) Regards, Phil.
Re: [PATCH v4 8/9] pcie_sriov: Do not reset NumVFs after unregistering VFs
On Wed, Feb 14, 2024 at 02:13:46PM +0900, Akihiko Odaki wrote: > I couldn't find such a behavior specified. Is it fixing a bug or just removing unnecessary code? Is this guest visible at all? > Signed-off-by: Akihiko Odaki > --- > hw/pci/pcie_sriov.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c > index 9d668b8d6c17..410bc090fc58 100644 > --- a/hw/pci/pcie_sriov.c > +++ b/hw/pci/pcie_sriov.c > @@ -209,7 +209,6 @@ static void unregister_vfs(PCIDevice *dev) > pci_set_enabled(dev->exp.sriov_pf.vf[i], false); > } > dev->exp.sriov_pf.num_vfs = 0; > -pci_set_word(dev->config + dev->exp.sriov_cap + PCI_SRIOV_NUM_VF, 0); > } > > void pcie_sriov_config_write(PCIDevice *dev, uint32_t address, > > -- > 2.43.0
Re: [PATCH v4 5/9] pcie_sriov: Validate NumVFs
On Wed, Feb 14, 2024 at 02:13:43PM +0900, Akihiko Odaki wrote:
> The guest may write NumVFs greater than TotalVFs and that can lead
> to buffer overflow in VF implementations.
>
> Fixes: 7c0fa8dff811 ("pcie: Add support for Single Root I/O Virtualization
> (SR/IOV)")
> Signed-off-by: Akihiko Odaki
> ---
> hw/pci/pcie_sriov.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> index a1fe65f5d801..da209b7f47fd 100644
> --- a/hw/pci/pcie_sriov.c
> +++ b/hw/pci/pcie_sriov.c
> @@ -176,6 +176,9 @@ static void register_vfs(PCIDevice *dev)
>
> assert(sriov_cap > 0);
> num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
> +if (num_vfs > pci_get_word(dev->config + sriov_cap +
> PCI_SRIOV_TOTAL_VF)) {
> +return;
> +}
yes but with your change PCI_SRIOV_NUM_VF no longer reflects the
number of registered VFs and that might lead to uninitialized
data read which is not better :(.
How about just forcing the PCI_SRIOV_NUM_VF register to be
below PCI_SRIOV_TOTAL_VF at all times?
> dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
>
>
> --
> 2.43.0
Re: [PATCH v4 2/9] hw/pci: Determine if rombar is explicitly enabled
On Wed, Feb 14, 2024 at 02:13:40PM +0900, Akihiko Odaki wrote:
> vfio determines if rombar is explicitly enabled by inspecting QDict.
> Inspecting QDict is not nice because QDict is untyped and depends on the
> details on the external interface. Add an infrastructure to determine if
> rombar is explicitly enabled to hw/pci.
>
> Signed-off-by: Akihiko Odaki
I frankly don't know what the issue with using qdict is.
Alex do you want to switch?
> ---
> include/hw/pci/pci_device.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index d3dd0f64b273..54fa0676abf1 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -205,6 +205,11 @@ static inline uint16_t pci_get_bdf(PCIDevice *dev)
> return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn);
> }
>
> +static inline bool pci_rom_bar_explicitly_enabled(PCIDevice *dev)
> +{
> +return dev->rom_bar && dev->rom_bar != -1;
> +}
> +
> uint16_t pci_requester_id(PCIDevice *dev);
>
> /* DMA access functions */
>
> --
> 2.43.0
[PULL v2 1/4] chardev/parallel: Don't close stdin on inappropriate device
The __linux__ version of qemu_chr_open_pp_fd() tries to claim the
parport device with a PPCLAIM ioctl(). On success, it stores the file
descriptor in the chardev object, and returns success. On failure, it
closes the file descriptor, and returns failure.
chardev_new() then passes the Chardev to object_unref(). This duly
calls char_parallel_finalize(), which closes the file descriptor
stored in the chardev object. Since qemu_chr_open_pp_fd() didn't
store it, it's still zero, so this closes standard input. Ooopsie.
To demonstate, add a unit test. With the bug above unfixed, running
this test closes standard input. char_hotswap_test() happens to run
next. It opens a socket, duly gets file descriptor 0, and since it
tests for success with > 0 instead of >= 0, it fails.
The new unit test needs to be conditional exactly like the chardev it
tests. Since the condition is rather complicated, steal the solution
from the serial chardev: define HAVE_CHARDEV_PARALLEL in qemu/osdep.h.
This also permits simplifying chardev/meson.build a bit.
The bug fix is easy enough: store the file descriptor, and leave
closing it to char_parallel_finalize().
The next commit will fix char_hotswap_test()'s test for success.
Signed-off-by: Markus Armbruster
Message-ID: <20240203080228.2766159-2-arm...@redhat.com>
Reviewed-by: Marc-André Lureau
Reviewed-by: Eric Blake
[Test fixed up for BSDs, indentation fixed up, commit message improved]
---
include/qemu/osdep.h| 9 -
chardev/char-parallel.c | 7 +--
tests/unit/test-char.c | 27 +++
chardev/meson.build | 4 +---
4 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 7d359dabc4..c7053cdc2b 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -508,11 +508,18 @@ void qemu_anon_ram_free(void *ptr, size_t size);
#ifdef _WIN32
#define HAVE_CHARDEV_SERIAL 1
-#elif defined(__linux__) || defined(__sun__) || defined(__FreeBSD__)\
+#define HAVE_CHARDEV_PARALLEL 1
+#else
+#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
|| defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
|| defined(__GLIBC__) || defined(__APPLE__)
#define HAVE_CHARDEV_SERIAL 1
#endif
+#if defined(__linux__) || defined(__FreeBSD__) \
+|| defined(__FreeBSD_kernel__) || defined(__DragonFly__)
+#define HAVE_CHARDEV_PARALLEL 1
+#endif
+#endif
#if defined(__HAIKU__)
#define SIGIO SIGPOLL
diff --git a/chardev/char-parallel.c b/chardev/char-parallel.c
index a5164f975a..78697d7522 100644
--- a/chardev/char-parallel.c
+++ b/chardev/char-parallel.c
@@ -164,13 +164,13 @@ static void qemu_chr_open_pp_fd(Chardev *chr,
{
ParallelChardev *drv = PARALLEL_CHARDEV(chr);
+drv->fd = fd;
+
if (ioctl(fd, PPCLAIM) < 0) {
error_setg_errno(errp, errno, "not a parallel port");
-close(fd);
return;
}
-drv->fd = fd;
drv->mode = IEEE1284_MODE_COMPAT;
}
#endif /* __linux__ */
@@ -238,6 +238,7 @@ static void qemu_chr_open_pp_fd(Chardev *chr,
}
#endif
+#ifdef HAVE_CHARDEV_PARALLEL
static void qmp_chardev_open_parallel(Chardev *chr,
ChardevBackend *backend,
bool *be_opened,
@@ -306,3 +307,5 @@ static void register_types(void)
}
type_init(register_types);
+
+#endif /* HAVE_CHARDEV_PARALLEL */
diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
index 649fdf64e1..2aea49c3b6 100644
--- a/tests/unit/test-char.c
+++ b/tests/unit/test-char.c
@@ -1203,6 +1203,30 @@ static void char_serial_test(void)
}
#endif
+#if defined(HAVE_CHARDEV_PARALLEL) && !defined(WIN32)
+static void char_parallel_test(void)
+{
+QemuOpts *opts;
+Chardev *chr;
+
+opts = qemu_opts_create(qemu_find_opts("chardev"), "parallel-id",
+1, &error_abort);
+qemu_opt_set(opts, "backend", "parallel", &error_abort);
+qemu_opt_set(opts, "path", "/dev/null", &error_abort);
+
+chr = qemu_chr_new_from_opts(opts, NULL, NULL);
+#ifdef __linux__
+/* fails to PPCLAIM, see qemu_chr_open_pp_fd() */
+g_assert_null(chr);
+#else
+g_assert_nonnull(chr);
+object_unparent(OBJECT(chr));
+#endif
+
+qemu_opts_del(opts);
+}
+#endif
+
#ifndef _WIN32
static void char_file_fifo_test(void)
{
@@ -1544,6 +1568,9 @@ int main(int argc, char **argv)
g_test_add_func("/char/udp", char_udp_test);
#if defined(HAVE_CHARDEV_SERIAL) && !defined(WIN32)
g_test_add_func("/char/serial", char_serial_test);
+#endif
+#if defined(HAVE_CHARDEV_PARALLEL) && !defined(WIN32)
+g_test_add_func("/char/parallel", char_parallel_test);
#endif
g_test_add_func("/char/hotswap", char_hotswap_test);
g_test_add_func("/char/websocket", char_websock_test);
diff --git a/chardev/meson.build b/chardev/meson.build
index c80337d15f..70070a8279 100644
--- a/chardev/meson.build
+++ b/chardev/meson.build
@@ -21,11 +21,
[PULL v2 3/4] qapi/char: Make backend types properly conditional
Character backends are actually QOM types. When a backend's
compile-time conditional QOM type is not compiled in, creation fails
with "'FOO' is not a valid char driver name". Okay, except
introspecting chardev-add with query-qmp-schema doesn't work then: the
backend type is there even though the QOM type isn't.
A management application can work around this issue by using
qom-list-types instead.
Fix the issue anyway: add the conditionals to the QAPI schema.
Signed-off-by: Markus Armbruster
Message-ID: <20240203080228.2766159-4-arm...@redhat.com>
Reviewed-by: Marc-André Lureau
Reviewed-by: Eric Blake
---
qapi/char.json | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/qapi/char.json b/qapi/char.json
index 6c6ad3b10c..2d74e66746 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -472,8 +472,8 @@
##
{ 'enum': 'ChardevBackendKind',
'data': [ 'file',
-'serial',
-'parallel',
+{ 'name': 'serial', 'if': 'HAVE_CHARDEV_SERIAL' },
+{ 'name': 'parallel', 'if': 'HAVE_CHARDEV_PARALLEL' },
'pipe',
'socket',
'udp',
@@ -482,10 +482,10 @@
'mux',
'msmouse',
'wctablet',
-'braille',
+{ 'name': 'braille', 'if': 'CONFIG_BRLAPI' },
'testdev',
'stdio',
-'console',
+{ 'name': 'console', 'if': 'CONFIG_WIN32' },
{ 'name': 'spicevmc', 'if': 'CONFIG_SPICE' },
{ 'name': 'spiceport', 'if': 'CONFIG_SPICE' },
{ 'name': 'qemu-vdagent', 'if': 'CONFIG_SPICE_PROTOCOL' },
@@ -614,8 +614,10 @@
'base': { 'type': 'ChardevBackendKind' },
'discriminator': 'type',
'data': { 'file': 'ChardevFileWrapper',
-'serial': 'ChardevHostdevWrapper',
-'parallel': 'ChardevHostdevWrapper',
+'serial': { 'type': 'ChardevHostdevWrapper',
+'if': 'HAVE_CHARDEV_SERIAL' },
+'parallel': { 'type': 'ChardevHostdevWrapper',
+ 'if': 'HAVE_CHARDEV_PARALLEL' },
'pipe': 'ChardevHostdevWrapper',
'socket': 'ChardevSocketWrapper',
'udp': 'ChardevUdpWrapper',
@@ -624,10 +626,12 @@
'mux': 'ChardevMuxWrapper',
'msmouse': 'ChardevCommonWrapper',
'wctablet': 'ChardevCommonWrapper',
-'braille': 'ChardevCommonWrapper',
+'braille': { 'type': 'ChardevCommonWrapper',
+ 'if': 'CONFIG_BRLAPI' },
'testdev': 'ChardevCommonWrapper',
'stdio': 'ChardevStdioWrapper',
-'console': 'ChardevCommonWrapper',
+'console': { 'type': 'ChardevCommonWrapper',
+ 'if': 'CONFIG_WIN32' },
'spicevmc': { 'type': 'ChardevSpiceChannelWrapper',
'if': 'CONFIG_SPICE' },
'spiceport': { 'type': 'ChardevSpicePortWrapper',
--
2.43.0
[PULL v2 4/4] qapi/char: Deprecate backend type "memory"
It's an alias for "ringbuf" we kept for backward compatibility; see
commit 3a1da42eb35 (qapi: Rename ChardevBackend member "memory" to
"ringbuf"). Deprecation is long overdue.
Signed-off-by: Markus Armbruster
Message-ID: <20240203080228.2766159-5-arm...@redhat.com>
Reviewed-by: Marc-André Lureau
Reviewed-by: Ján Tomko
Reviewed-by: Eric Blake
---
docs/about/deprecated.rst | 8
qapi/char.json| 8 +---
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index c7b95e6068..7d9343676c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -380,6 +380,14 @@ Specifying the iSCSI password in plain text on the command
line using the
used instead, to refer to a ``--object secret...`` instance that provides
a password via a file, or encrypted.
+Character device options
+
+
+Backend ``memory`` (since 9.0)
+^^
+
+``memory`` is a deprecated synonym for ``ringbuf``.
+
CPU device properties
'
diff --git a/qapi/char.json b/qapi/char.json
index 2d74e66746..75a7e057f0 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -468,6 +468,10 @@
#
# @memory: Since 1.5
#
+# Features:
+#
+# @deprecated: Member @memory is deprecated. Use @ringbuf instead.
+#
# Since: 1.4
##
{ 'enum': 'ChardevBackendKind',
@@ -492,8 +496,7 @@
{ 'name': 'dbus', 'if': 'CONFIG_DBUS_DISPLAY' },
'vc',
'ringbuf',
-# next one is just for compatibility
-'memory' ] }
+{ 'name': 'memory', 'features': [ 'deprecated' ] } ] }
##
# @ChardevFileWrapper:
@@ -642,7 +645,6 @@
'if': 'CONFIG_DBUS_DISPLAY' },
'vc': 'ChardevVCWrapper',
'ringbuf': 'ChardevRingbufWrapper',
-# next one is just for compatibility
'memory': 'ChardevRingbufWrapper' } }
##
--
2.43.0
[PULL v2 0/4] Character device backend patches for 2024-02-12
I offered Marc-André to do this pull request, and he accepted. The following changes since commit 5d1fc614413b10dd94858b07a1b2e26b1aa0296c: Merge tag 'migration-staging-pull-request' of https://gitlab.com/peterx/qemu into staging (2024-02-09 11:22:20 +) are available in the Git repository at: https://repo.or.cz/qemu/armbru.git tags/pull-char-2024-02-12-v2 for you to fetch changes up to b04c12282b33e81ba29b54dd001667f5c4002252: qapi/char: Deprecate backend type "memory" (2024-02-14 07:45:08 +0100) Character device backend patches for 2024-02-12 Markus Armbruster (4): chardev/parallel: Don't close stdin on inappropriate device tests/unit/test-char: Fix qemu_socket(), make_udp_socket() check qapi/char: Make backend types properly conditional qapi/char: Deprecate backend type "memory" docs/about/deprecated.rst | 8 qapi/char.json| 28 +--- include/qemu/osdep.h | 9 - chardev/char-parallel.c | 7 +-- tests/unit/test-char.c| 31 +-- chardev/meson.build | 4 +--- 6 files changed, 68 insertions(+), 19 deletions(-) -- 2.43.0
[PULL v2 2/4] tests/unit/test-char: Fix qemu_socket(), make_udp_socket() check
qemu_socket() and make_udp_socket() return a file descriptor on
success, -1 on failure. The check misinterprets 0 as failure. Fix
that.
Signed-off-by: Markus Armbruster
Message-ID: <20240203080228.2766159-3-arm...@redhat.com>
Reviewed-by: Marc-André Lureau
Reviewed-by: Eric Blake
---
tests/unit/test-char.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
index 2aea49c3b6..f273ce5226 100644
--- a/tests/unit/test-char.c
+++ b/tests/unit/test-char.c
@@ -556,7 +556,7 @@ static int make_udp_socket(int *port)
socklen_t alen = sizeof(addr);
int ret, sock = qemu_socket(PF_INET, SOCK_DGRAM, 0);
-g_assert_cmpint(sock, >, 0);
+g_assert_cmpint(sock, >=, 0);
addr.sin_family = AF_INET ;
addr.sin_addr.s_addr = htonl(INADDR_ANY);
addr.sin_port = 0;
@@ -1407,7 +1407,7 @@ static void char_hotswap_test(void)
int port;
int sock = make_udp_socket(&port);
-g_assert_cmpint(sock, >, 0);
+g_assert_cmpint(sock, >=, 0);
chr_args = g_strdup_printf("udp:127.0.0.1:%d", port);
--
2.43.0
Re: possible deprecation and removal of some old QEMU Arm machine types (pxa2xx, omap, sa1110)
* Aaro Koskinen [240214 01:27]: > On Tue, Feb 13, 2024 at 09:11:38PM +0100, Arnd Bergmann wrote: > > I think Tony still tests these on both hardware and qemu. > > The platform side here is much more modern than any of the > > others above since it does use DT and it has enough RAM > > to be somewhat usable. > > I have also these boards (real hardware) and test them frequently with > mainline Linux. However, QEMU support I haven't used/needed. I recall it > was a bit buggy, and some changes in mainline made the kernel unbootable. > Unless Tony needs the support, I guess they are good to go. I've only used real hardware to test omap1 for at least 10 years, and I have currently no omap1 devices booting. Still hoping to add n770 and osk back at some point though for basic boot testing. I could see qemu being handy for automating boot testing for git bisect, but as far as I'm concerned no objections to dropping old device support for qemu. IMO it's best to concentrate on where there is activity and users, and try to make thing easier to maintain in the long run. Anything with users and active development happening we should try to help and encourage :) Regards, Tony
[PATCH v6] util: Move dbus_display1 to util
Move dbus_display1 from ui to util as it's used not only by ui/dbus but
also dbus-display-test.
dbus_display1 is now added to util_ss accordingly. It ensures that the
source will be linked with audio/dbus, and also avoids recompilation
when linking with dbus-display-test.
dbus-display.h is also added to genh to ensure it is generated before
compiling ui/dbus, audio/dbus, and dbus-display-test.
Both changes combined, it is no longer necessary for ui/dbus,
audio/dbus, and dbus-display-test to explicitly state the dependency on
dbus_display1.
Signed-off-by: Akihiko Odaki
---
I found it was failing to build dbus modules when --enable-dbus so here
are fixes.
---
Changes in v6:
- Dropped patch "audio: Do not include ui/dbus.h" (Marc-André Lureau).
- Rebased.
- Link to v5:
https://lore.kernel.org/r/20231217-dbus-v5-0-8122e822a...@daynix.com
Changes in v5:
- Fixed docs/interop/dbus-display.rst.
- Link to v4:
https://lore.kernel.org/r/20231217-dbus-v4-0-4fd5410bf...@daynix.com
Changes in v4:
- Moved dbus_display1 to util.
- Link to v3:
https://lore.kernel.org/r/20231216-dbus-v3-0-b4bcbed73...@daynix.com
Changes in v3:
- Merged dbus_display1_lib into libqemuutil.
- Added patch "audio: Do not include ui/dbus.h".
- Link to v2:
https://lore.kernel.org/r/20231215-dbus-v2-0-1e2e6aa02...@daynix.com
Changes in v2:
- Updated MAINTAINERS.
- Link to v1:
https://lore.kernel.org/r/20231215-dbus-v1-0-349e059ac...@daynix.com
---
MAINTAINERS | 2 +-
docs/interop/dbus-display.rst | 6 +++---
ui/dbus.h | 2 +-
audio/dbusaudio.c | 2 +-
tests/qtest/dbus-display-test.c | 2 +-
tests/qtest/meson.build | 2 +-
ui/meson.build | 20 +---
{ui => util}/dbus-display1.xml | 0
util/meson.build| 21 +
9 files changed, 30 insertions(+), 27 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index f80db6a96a3f..0a81159e33d1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3413,7 +3413,7 @@ S: Maintained
F: backends/dbus-vmstate.c
F: ui/dbus*
F: audio/dbus*
-F: util/dbus.c
+F: util/dbus*
F: include/ui/dbus*
F: include/qemu/dbus.h
F: docs/interop/dbus*
diff --git a/docs/interop/dbus-display.rst b/docs/interop/dbus-display.rst
index 8c6e8e0f5a82..efec89723a34 100644
--- a/docs/interop/dbus-display.rst
+++ b/docs/interop/dbus-display.rst
@@ -18,14 +18,14 @@ QEMU also implements the standard interfaces, such as
.. only:: sphinx4
- .. dbus-doc:: ui/dbus-display1.xml
+ .. dbus-doc:: util/dbus-display1.xml
.. only:: not sphinx4
.. warning::
Sphinx 4 is required to build D-Bus documentation.
- This is the content of ``ui/dbus-display1.xml``:
+ This is the content of ``util/dbus-display1.xml``:
- .. literalinclude:: ../../ui/dbus-display1.xml
+ .. literalinclude:: ../../util/dbus-display1.xml
:language: xml
diff --git a/ui/dbus.h b/ui/dbus.h
index 1e8c24a48e32..a847bee98876 100644
--- a/ui/dbus.h
+++ b/ui/dbus.h
@@ -31,7 +31,7 @@
#include "ui/console.h"
#include "ui/clipboard.h"
-#include "ui/dbus-display1.h"
+#include "util/dbus-display1.h"
typedef struct DBusClipboardRequest {
GDBusMethodInvocation *invocation;
diff --git a/audio/dbusaudio.c b/audio/dbusaudio.c
index 60fcf643ecf8..2aacdac6715b 100644
--- a/audio/dbusaudio.c
+++ b/audio/dbusaudio.c
@@ -34,7 +34,7 @@
#endif
#include "ui/dbus.h"
-#include "ui/dbus-display1.h"
+#include "util/dbus-display1.h"
#define AUDIO_CAP "dbus"
#include "audio.h"
diff --git a/tests/qtest/dbus-display-test.c b/tests/qtest/dbus-display-test.c
index 21edaa1e321f..d4871e2fd80f 100644
--- a/tests/qtest/dbus-display-test.c
+++ b/tests/qtest/dbus-display-test.c
@@ -5,7 +5,7 @@
#include
#include
#include "libqtest.h"
-#include "ui/dbus-display1.h"
+#include "util/dbus-display1.h"
static GDBusConnection*
test_dbus_p2p_from_fd(int fd)
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 39557d5ecbb0..627ff8fbe1c7 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -344,7 +344,7 @@ if vnc.found()
endif
if dbus_display
- qtests += {'dbus-display-test': [dbus_display1, gio]}
+ qtests += {'dbus-display-test': [gio]}
endif
qtest_executables = {}
diff --git a/ui/meson.build b/ui/meson.build
index 376e0d771ba9..74e2a79b8c1e 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -74,25 +74,7 @@ endif
if dbus_display
dbus_ss = ss.source_set()
- env = environment()
- env.set('HOST_OS', host_os)
- xml = custom_target('dbus-display preprocess',
- input: 'dbus-display1.xml',
- output: 'dbus-display1.xml',
- env: env,
- command: [xml_pp, '@INPUT@', '@OUTPUT@'])
- dbus_display1 = custom_target('dbus-display gdbus-codegen',
-output: ['dbus-display1.h', 'dbus-display1.c'],
-input: xml,
-
[PATCH v4 5/9] pcie_sriov: Validate NumVFs
The guest may write NumVFs greater than TotalVFs and that can lead
to buffer overflow in VF implementations.
Fixes: 7c0fa8dff811 ("pcie: Add support for Single Root I/O Virtualization
(SR/IOV)")
Signed-off-by: Akihiko Odaki
---
hw/pci/pcie_sriov.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index a1fe65f5d801..da209b7f47fd 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -176,6 +176,9 @@ static void register_vfs(PCIDevice *dev)
assert(sriov_cap > 0);
num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
+if (num_vfs > pci_get_word(dev->config + sriov_cap + PCI_SRIOV_TOTAL_VF)) {
+return;
+}
dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
--
2.43.0
[PATCH v4 9/9] hw/nvme: Refer to dev->exp.sriov_pf.num_vfs
NumVFs may not equal to the current effective number of VFs because VF
Enable is cleared, NumVFs is set after VF Enable is set, or NumVFs is
greater than TotalVFs.
Fixes: 11871f53ef8e ("hw/nvme: Add support for the Virtualization Management
command")
Signed-off-by: Akihiko Odaki
---
hw/nvme/ctrl.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index f8df622fe590..daedda5d326f 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8481,7 +8481,7 @@ static void nvme_sriov_pre_write_ctrl(PCIDevice *dev,
uint32_t address,
NvmeSecCtrlEntry *sctrl;
uint16_t sriov_cap = dev->exp.sriov_cap;
uint32_t off = address - sriov_cap;
-int i, num_vfs;
+int i;
if (!sriov_cap) {
return;
@@ -8489,8 +8489,7 @@ static void nvme_sriov_pre_write_ctrl(PCIDevice *dev,
uint32_t address,
if (range_covers_byte(off, len, PCI_SRIOV_CTRL)) {
if (!(val & PCI_SRIOV_CTRL_VFE)) {
-num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
-for (i = 0; i < num_vfs; i++) {
+for (i = 0; i < dev->exp.sriov_pf.num_vfs; i++) {
sctrl = &n->sec_ctrl_list.sec[i];
nvme_virt_set_state(n, le16_to_cpu(sctrl->scid), false);
}
--
2.43.0
[PATCH v4 8/9] pcie_sriov: Do not reset NumVFs after unregistering VFs
I couldn't find such a behavior specified. Signed-off-by: Akihiko Odaki --- hw/pci/pcie_sriov.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c index 9d668b8d6c17..410bc090fc58 100644 --- a/hw/pci/pcie_sriov.c +++ b/hw/pci/pcie_sriov.c @@ -209,7 +209,6 @@ static void unregister_vfs(PCIDevice *dev) pci_set_enabled(dev->exp.sriov_pf.vf[i], false); } dev->exp.sriov_pf.num_vfs = 0; -pci_set_word(dev->config + dev->exp.sriov_cap + PCI_SRIOV_NUM_VF, 0); } void pcie_sriov_config_write(PCIDevice *dev, uint32_t address, -- 2.43.0
[PATCH v4 2/9] hw/pci: Determine if rombar is explicitly enabled
vfio determines if rombar is explicitly enabled by inspecting QDict.
Inspecting QDict is not nice because QDict is untyped and depends on the
details on the external interface. Add an infrastructure to determine if
rombar is explicitly enabled to hw/pci.
Signed-off-by: Akihiko Odaki
---
include/hw/pci/pci_device.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index d3dd0f64b273..54fa0676abf1 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -205,6 +205,11 @@ static inline uint16_t pci_get_bdf(PCIDevice *dev)
return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn);
}
+static inline bool pci_rom_bar_explicitly_enabled(PCIDevice *dev)
+{
+return dev->rom_bar && dev->rom_bar != -1;
+}
+
uint16_t pci_requester_id(PCIDevice *dev);
/* DMA access functions */
--
2.43.0
[PATCH v4 4/9] hw/qdev: Remove opts member
It is no longer used.
Signed-off-by: Akihiko Odaki
Reviewed-by: Philippe Mathieu-Daudé
---
include/hw/qdev-core.h | 4
hw/core/qdev.c | 1 -
system/qdev-monitor.c | 12 +++-
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 9228e96c87e9..5954404dcbfe 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -237,10 +237,6 @@ struct DeviceState {
* @pending_deleted_expires_ms: optional timeout for deletion events
*/
int64_t pending_deleted_expires_ms;
-/**
- * @opts: QDict of options for the device
- */
-QDict *opts;
/**
* @hotplugged: was device added after PHASE_MACHINE_READY?
*/
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index c68d0f7c512f..7349c9a86be8 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -706,7 +706,6 @@ static void device_finalize(Object *obj)
dev->canonical_path = NULL;
}
-qobject_unref(dev->opts);
g_free(dev->id);
}
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index a13db763e5dd..71c00f62ee38 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -625,6 +625,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
char *id;
DeviceState *dev = NULL;
BusState *bus = NULL;
+QDict *properties;
driver = qdict_get_try_str(opts, "driver");
if (!driver) {
@@ -705,13 +706,14 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
}
/* set properties */
-dev->opts = qdict_clone_shallow(opts);
-qdict_del(dev->opts, "driver");
-qdict_del(dev->opts, "bus");
-qdict_del(dev->opts, "id");
+properties = qdict_clone_shallow(opts);
+qdict_del(properties, "driver");
+qdict_del(properties, "bus");
+qdict_del(properties, "id");
-object_set_properties_from_keyval(&dev->parent_obj, dev->opts, from_json,
+object_set_properties_from_keyval(&dev->parent_obj, properties, from_json,
errp);
+qobject_unref(properties);
if (*errp) {
goto err_del_dev;
}
--
2.43.0
[PATCH v4 0/9] hw/pci: SR-IOV related fixes and improvements
I submitted a RFC series[1] to add support for SR-IOV emulation to virtio-net-pci. During the development of the series, I fixed some trivial bugs and made improvements that I think are independently useful. This series extracts those fixes and improvements from the RFC series. Below is an explanation of the patches: Patch 1 adds a function to check if ROM BAR is explicitly enabled. It is used in the RFC series to report an error if the user requests to enable ROM BAR for SR-IOV VF. Patch 2 and 3 use it for vfio to remove hacky device option dictionary inspection. Patch 4 adds SR-IOV NumVFs validation to fix potential buffer overflow. Patch 5 changes to realize SR-IOV VFs when the PF is being realized to validate VF configuration. Patch 6 fixes memory leak that occurs if a SR-IOV VF fails to realize. [1]: https://patchew.org/QEMU/20231210-sriov-v2-0-b959e8a6d...@daynix.com/ Signed-off-by: Akihiko Odaki --- Changes in v4: - Reverted the change to pci_rom_bar_explicitly_enabled(). (Michael S. Tsirkin) - Added patch "pcie_sriov: Do not reset NumVFs after unregistering VFs". - Added patch "hw/nvme: Refer to dev->exp.sriov_pf.num_vfs". - Link to v3: https://lore.kernel.org/r/20240212-reuse-v3-0-8017b689c...@daynix.com Changes in v3: - Extracted patch "hw/pci: Use -1 as a default value for rombar" from patch "hw/pci: Determine if rombar is explicitly enabled" (Philippe Mathieu-Daudé) - Added an audit result of PCIDevice::rom_bar to the message of patch "hw/pci: Use -1 as a default value for rombar" (Philippe Mathieu-Daudé) - Link to v2: https://lore.kernel.org/r/20240210-reuse-v2-0-24ba2a502...@daynix.com Changes in v2: - Reset after enabling a function so that NVMe VF state gets updated. - Link to v1: https://lore.kernel.org/r/20240203-reuse-v1-0-5be8c5ce6...@daynix.com --- Akihiko Odaki (9): hw/pci: Use -1 as a default value for rombar hw/pci: Determine if rombar is explicitly enabled vfio: Avoid inspecting option QDict for rombar hw/qdev: Remove opts member pcie_sriov: Validate NumVFs pcie_sriov: Reuse SR-IOV VF device instances pcie_sriov: Release VFs failed to realize pcie_sriov: Do not reset NumVFs after unregistering VFs hw/nvme: Refer to dev->exp.sriov_pf.num_vfs docs/pcie_sriov.txt | 8 ++-- include/hw/pci/pci.h| 2 +- include/hw/pci/pci_device.h | 7 ++- include/hw/pci/pcie_sriov.h | 6 +-- include/hw/qdev-core.h | 4 -- hw/core/qdev.c | 1 - hw/net/igb.c| 13 -- hw/nvme/ctrl.c | 29 +++- hw/pci/pci.c| 20 + hw/pci/pci_host.c | 4 +- hw/pci/pcie.c | 4 +- hw/pci/pcie_sriov.c | 106 +--- hw/vfio/pci.c | 3 +- system/qdev-monitor.c | 12 ++--- 14 files changed, 118 insertions(+), 101 deletions(-) --- base-commit: 5005aed8a7e728d028efb40e243ecfc2b4f3df3a change-id: 20240129-reuse-faae22b11934 Best regards, -- Akihiko Odaki
[PATCH v4 1/9] hw/pci: Use -1 as a default value for rombar
Currently there is no way to distinguish the case that rombar is
explicitly specified as 1 and the case that rombar is not specified.
Set rombar -1 by default to distinguish these cases just as it is done
for addr and romsize. It was confirmed that changing the default value
to -1 will not change the behavior by looking at occurences of rom_bar.
$ git grep -w rom_bar
hw/display/qxl.c:328:QXLRom *rom = memory_region_get_ram_ptr(&d->rom_bar);
hw/display/qxl.c:431:qxl_set_dirty(&qxl->rom_bar, 0, qxl->rom_size);
hw/display/qxl.c:1048:QXLRom *rom =
memory_region_get_ram_ptr(&qxl->rom_bar);
hw/display/qxl.c:2131:memory_region_init_rom(&qxl->rom_bar, OBJECT(qxl),
"qxl.vrom",
hw/display/qxl.c:2154: PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->rom_bar);
hw/display/qxl.h:101:MemoryRegion rom_bar;
hw/pci/pci.c:74:DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
hw/pci/pci.c:2329:if (!pdev->rom_bar) {
hw/vfio/pci.c:1019:if (vdev->pdev.romfile || !vdev->pdev.rom_bar) {
hw/xen/xen_pt_load_rom.c:29:if (dev->romfile || !dev->rom_bar) {
include/hw/pci/pci_device.h:150:uint32_t rom_bar;
rom_bar refers to a different variable in qxl. It is only tested if
the value is 0 or not in the other places.
Signed-off-by: Akihiko Odaki
---
hw/pci/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 6496d027ca61..47f38375bb09 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -71,7 +71,7 @@ static Property pci_props[] = {
DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, -1),
-DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
+DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, -1),
DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
DEFINE_PROP_BIT("x-pcie-lnksta-dllla", PCIDevice, cap_present,
--
2.43.0
[PATCH v4 7/9] pcie_sriov: Release VFs failed to realize
Release VFs failed to realize just as we do in unregister_vfs().
Fixes: 7c0fa8dff811 ("pcie: Add support for Single Root I/O Virtualization
(SR/IOV)")
Signed-off-by: Akihiko Odaki
---
hw/pci/pcie_sriov.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index 9ba34cf8f8ed..9d668b8d6c17 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -91,6 +91,8 @@ bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
vf->exp.sriov_vf.vf_number = i;
if (!qdev_realize(&vf->qdev, bus, errp)) {
+object_unparent(OBJECT(vf));
+object_unref(vf);
unrealize_vfs(dev, i);
return false;
}
--
2.43.0
[PATCH v4 6/9] pcie_sriov: Reuse SR-IOV VF device instances
Disable SR-IOV VF devices by reusing code to power down PCI devices
instead of removing them when the guest requests to disable VFs. This
allows to realize devices and report VF realization errors at PF
realization time.
Signed-off-by: Akihiko Odaki
---
docs/pcie_sriov.txt | 8 ++--
include/hw/pci/pci.h| 2 +-
include/hw/pci/pci_device.h | 2 +-
include/hw/pci/pcie_sriov.h | 6 +--
hw/net/igb.c| 13 --
hw/nvme/ctrl.c | 24 +++
hw/pci/pci.c| 18
hw/pci/pci_host.c | 4 +-
hw/pci/pcie.c | 4 +-
hw/pci/pcie_sriov.c | 100
10 files changed, 97 insertions(+), 84 deletions(-)
diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
index a47aad0bfab0..ab2142807f79 100644
--- a/docs/pcie_sriov.txt
+++ b/docs/pcie_sriov.txt
@@ -52,9 +52,11 @@ setting up a BAR for a VF.
...
/* Add and initialize the SR/IOV capability */
- pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
- vf_devid, initial_vfs, total_vfs,
- fun_offset, stride);
+ if (!pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
+ vf_devid, initial_vfs, total_vfs,
+ fun_offset, stride, errp)) {
+ return;
+ }
/* Set up individual VF BARs (parameters as for normal BARs) */
pcie_sriov_pf_init_vf_bar( ... )
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index eaa3fc99d884..442017b4865d 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -642,6 +642,6 @@ static inline void pci_irq_pulse(PCIDevice *pci_dev)
}
MSIMessage pci_get_msi_message(PCIDevice *dev, int vector);
-void pci_set_power(PCIDevice *pci_dev, bool state);
+void pci_set_enabled(PCIDevice *pci_dev, bool state);
#endif
diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index 54fa0676abf1..f5aba8ae2675 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -56,7 +56,7 @@ typedef struct PCIReqIDCache PCIReqIDCache;
struct PCIDevice {
DeviceState qdev;
bool partially_hotplugged;
-bool has_power;
+bool is_enabled;
/* PCI config space */
uint8_t *config;
diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
index 095fb0c9edf9..d9a39daccac4 100644
--- a/include/hw/pci/pcie_sriov.h
+++ b/include/hw/pci/pcie_sriov.h
@@ -18,7 +18,6 @@
struct PCIESriovPF {
uint16_t num_vfs; /* Number of virtual functions created */
uint8_t vf_bar_type[PCI_NUM_REGIONS]; /* Store type for each VF bar */
-const char *vfname; /* Reference to the device type used for the VFs */
PCIDevice **vf; /* Pointer to an array of num_vfs VF devices */
};
@@ -27,10 +26,11 @@ struct PCIESriovVF {
uint16_t vf_number; /* Logical VF number of this function */
};
-void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
+bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
const char *vfname, uint16_t vf_dev_id,
uint16_t init_vfs, uint16_t total_vfs,
-uint16_t vf_offset, uint16_t vf_stride);
+uint16_t vf_offset, uint16_t vf_stride,
+Error **errp);
void pcie_sriov_pf_exit(PCIDevice *dev);
/* Set up a VF bar in the SR/IOV bar area */
diff --git a/hw/net/igb.c b/hw/net/igb.c
index 0b5c31a58bba..1079a33d4000 100644
--- a/hw/net/igb.c
+++ b/hw/net/igb.c
@@ -447,9 +447,16 @@ static void igb_pci_realize(PCIDevice *pci_dev, Error
**errp)
pcie_ari_init(pci_dev, 0x150);
-pcie_sriov_pf_init(pci_dev, IGB_CAP_SRIOV_OFFSET, TYPE_IGBVF,
-IGB_82576_VF_DEV_ID, IGB_MAX_VF_FUNCTIONS, IGB_MAX_VF_FUNCTIONS,
-IGB_VF_OFFSET, IGB_VF_STRIDE);
+if (!pcie_sriov_pf_init(pci_dev, IGB_CAP_SRIOV_OFFSET,
+TYPE_IGBVF, IGB_82576_VF_DEV_ID,
+IGB_MAX_VF_FUNCTIONS, IGB_MAX_VF_FUNCTIONS,
+IGB_VF_OFFSET, IGB_VF_STRIDE,
+errp)) {
+pcie_cap_exit(pci_dev);
+igb_cleanup_msix(s);
+msi_uninit(pci_dev);
+return;
+}
pcie_sriov_pf_init_vf_bar(pci_dev, IGBVF_MMIO_BAR_IDX,
PCI_BASE_ADDRESS_MEM_TYPE_64 | PCI_BASE_ADDRESS_MEM_PREFETCH,
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index f026245d1e9e..f8df622fe590 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8031,7 +8031,8 @@ static uint64_t nvme_bar_size(unsigned total_queues,
unsigned total_irqs,
return bar_size;
}
-static void nvme_init_sriov(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t offset)
+static bool nvme_init_sriov(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t offset,
+Error **errp)
{
uint16_t vf_dev_id = n->params.use_intel_id ?
PCI_DEVICE_ID_INTEL_NVME : PCI_DEVICE_ID_RED
[PATCH v4 3/9] vfio: Avoid inspecting option QDict for rombar
Use pci_rom_bar_explicitly_enabled() to determine if rombar is explicitly
enabled.
Signed-off-by: Akihiko Odaki
---
hw/vfio/pci.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 4fa387f0430d..647f15b2a060 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1012,7 +1012,6 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
{
uint32_t orig, size = cpu_to_le32((uint32_t)PCI_ROM_ADDRESS_MASK);
off_t offset = vdev->config_offset + PCI_ROM_ADDRESS;
-DeviceState *dev = DEVICE(vdev);
char *name;
int fd = vdev->vbasedev.fd;
@@ -1046,7 +1045,7 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
}
if (vfio_opt_rom_in_denylist(vdev)) {
-if (dev->opts && qdict_haskey(dev->opts, "rombar")) {
+if (pci_rom_bar_explicitly_enabled(&vdev->pdev)) {
warn_report("Device at %s is known to cause system instability"
" issues during option rom execution",
vdev->vbasedev.name);
--
2.43.0
[PATCH] linux-user: Remove pgb_dynamic alignment assertion
The assertion was never correct, because the alignment is a composite
of the image alignment and SHMLBA. Even if the alignment didn't match
the image an assertion would not be correct -- more appropriate would
be an error message about an ill formed image. But the image cannot
be held to SHMLBA under any circumstances.
Fixes: ee94743034b ("linux-user: completely re-write init_guest_space")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2157
Signed-off-by: Richard Henderson
---
linux-user/elfload.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index f3f1ab4f69..d92d66ca1e 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3022,8 +3022,6 @@ static void pgb_dynamic(const char *image_name, uintptr_t
guest_loaddr,
uintptr_t brk, ret;
PGBAddrs ga;
-assert(QEMU_IS_ALIGNED(guest_loaddr, align));
-
/* Try the identity map first. */
if (pgb_addr_set(&ga, guest_loaddr, guest_hiaddr, true)) {
brk = (uintptr_t)sbrk(0);
--
2.34.1
[PATCH v10 2/6] ui/cocoa: Scale with NSView instead of Core Graphics
Core Graphics is not accelerated and slow.
Signed-off-by: Akihiko Odaki
---
ui/cocoa.m | 23 +--
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index fe0eb74b0743..cb6090905999 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -496,10 +496,8 @@ - (void) drawRect:(NSRect) rect
[self getRectsBeingDrawn:&rectList count:&rectCount];
for (i = 0; i < rectCount; i++) {
-clipRect.origin.x = rectList[i].origin.x / cdx;
-clipRect.origin.y = (float)h - (rectList[i].origin.y +
rectList[i].size.height) / cdy;
-clipRect.size.width = rectList[i].size.width / cdx;
-clipRect.size.height = rectList[i].size.height / cdy;
+clipRect = rectList[i];
+clipRect.origin.y = (float)h - (clipRect.origin.y +
clipRect.size.height);
clipImageRef = CGImageCreateWithImageInRect(
imageRef,
clipRect
@@ -545,6 +543,11 @@ - (void) setContentDimensions
}
}
+- (void) updateBounds
+{
+[self setBoundsSize:NSMakeSize(screen.width, screen.height)];
+}
+
- (void) updateUIInfoLocked
{
/* Must be called with the BQL, i.e. via updateUIInfo */
@@ -1292,6 +1295,7 @@ - (void)windowDidChangeScreen:(NSNotification
*)notification
- (void)windowDidResize:(NSNotification *)notification
{
+[cocoaView updateBounds];
[cocoaView updateUIInfo];
}
@@ -1940,16 +1944,7 @@ static void cocoa_update(DisplayChangeListener *dcl,
COCOA_DEBUG("qemu_cocoa: cocoa_update\n");
dispatch_async(dispatch_get_main_queue(), ^{
-NSRect rect;
-if ([cocoaView cdx] == 1.0) {
-rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h);
-} else {
-rect = NSMakeRect(
-x * [cocoaView cdx],
-([cocoaView gscreen].height - y - h) * [cocoaView cdy],
-w * [cocoaView cdx],
-h * [cocoaView cdy]);
-}
+NSRect rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h);
[cocoaView setNeedsDisplayInRect:rect];
});
}
--
2.43.0
[PATCH v10 6/6] ui/cocoa: Remove stretch_video flag
Evaluate [normalWindow styleMask] & NSWindowStyleMaskResizable instead.
Signed-off-by: Akihiko Odaki
---
ui/cocoa.m | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index aeec3c48859c..0ed40cd97d28 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -103,7 +103,6 @@ static void cocoa_switch(DisplayChangeListener *dcl,
static int left_command_key_enabled = 1;
static bool swap_opt_cmd;
-static bool stretch_video;
static NSTextField *pauseLabel;
static bool allow_events;
@@ -533,7 +532,7 @@ - (void) resizeWindow
{
[[self window] setContentAspectRatio:NSMakeSize(screen.width,
screen.height)];
-if (!stretch_video) {
+if (!([[self window] styleMask] & NSWindowStyleMaskResizable)) {
[[self window] setContentSize:NSMakeSize(screen.width, screen.height)];
[[self window] center];
} else if ([[self window] styleMask] & NSWindowStyleMaskFullScreen) {
@@ -1295,7 +1294,7 @@ - (BOOL)windowShouldClose:(id)sender
- (NSSize) window:(NSWindow *)window
willUseFullScreenContentSize:(NSSize)proposedSize
{
-if (stretch_video) {
+if ([normalWindow styleMask] & NSWindowStyleMaskResizable) {
return [cocoaView fixZoomedFullScreenSize:proposedSize];
}
@@ -1376,8 +1375,7 @@ - (void)showQEMUDoc:(id)sender
/* Stretches video to fit host monitor size */
- (void)zoomToFit:(id) sender
{
-stretch_video = !stretch_video;
-if (stretch_video == true) {
+if (([normalWindow styleMask] & NSWindowStyleMaskResizable) == 0) {
[normalWindow setStyleMask:[normalWindow styleMask] |
NSWindowStyleMaskResizable];
[sender setState: NSControlStateValueOn];
} else {
@@ -1649,7 +1647,7 @@ static void create_initial_menus(void)
menu = [[NSMenu alloc] initWithTitle:@"View"];
[menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen"
action:@selector(doToggleFullScreen:) keyEquivalent:@"f"] autorelease]]; //
Fullscreen
menuItem = [[[NSMenuItem alloc] initWithTitle:@"Zoom To Fit"
action:@selector(zoomToFit:) keyEquivalent:@""] autorelease];
-[menuItem setState: stretch_video ? NSControlStateValueOn :
NSControlStateValueOff];
+[menuItem setState: [normalWindow styleMask] & NSWindowStyleMaskResizable
? NSControlStateValueOn : NSControlStateValueOff];
[menu addItem: menuItem];
menuItem = [[[NSMenuItem alloc] initWithTitle:@"View" action:nil
keyEquivalent:@""] autorelease];
[menuItem setSubmenu:menu];
@@ -2035,7 +2033,6 @@ static void cocoa_display_init(DisplayState *ds,
DisplayOptions *opts)
}
if (opts->u.cocoa.has_zoom_to_fit && opts->u.cocoa.zoom_to_fit) {
-stretch_video = true;
[normalWindow setStyleMask:[normalWindow styleMask] |
NSWindowStyleMaskResizable];
}
--
2.43.0
[PATCH v10 5/6] ui/cocoa: Call console_select() with the BQL
console_select() can be called anytime so explicitly take the BQL.
Signed-off-by: Akihiko Odaki
---
ui/cocoa.m | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index a1f54b3ebb9a..aeec3c48859c 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1390,7 +1390,9 @@ - (void)zoomToFit:(id) sender
/* Displays the console on the screen */
- (void)displayConsole:(id)sender
{
-console_select([sender tag]);
+with_bql(^{
+console_select([sender tag]);
+});
}
/* Pause the guest */
--
2.43.0
[PATCH v10 4/6] ui/cocoa: Make window resizable
The window will be resizable when zoom-to-fit is on.
Signed-off-by: Akihiko Odaki
---
ui/cocoa.m | 3 +++
1 file changed, 3 insertions(+)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 518fae26f6f4..a1f54b3ebb9a 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1378,8 +1378,10 @@ - (void)zoomToFit:(id) sender
{
stretch_video = !stretch_video;
if (stretch_video == true) {
+[normalWindow setStyleMask:[normalWindow styleMask] |
NSWindowStyleMaskResizable];
[sender setState: NSControlStateValueOn];
} else {
+[normalWindow setStyleMask:[normalWindow styleMask] &
~NSWindowStyleMaskResizable];
[cocoaView resizeWindow];
[sender setState: NSControlStateValueOff];
}
@@ -2032,6 +2034,7 @@ static void cocoa_display_init(DisplayState *ds,
DisplayOptions *opts)
if (opts->u.cocoa.has_zoom_to_fit && opts->u.cocoa.zoom_to_fit) {
stretch_video = true;
+[normalWindow setStyleMask:[normalWindow styleMask] |
NSWindowStyleMaskResizable];
}
create_initial_menus();
--
2.43.0
[PATCH v10 3/6] ui/cocoa: Let the platform toggle fullscreen
It allows making the window full screen by clicking full screen button
provided by the platform (the left-top green button) and save some code.
Signed-off-by: Akihiko Odaki
---
ui/cocoa.m | 423 ++---
1 file changed, 207 insertions(+), 216 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index cb6090905999..518fae26f6f4 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -303,20 +303,17 @@ static void handleAnyDeviceErrors(Error * err)
*/
@interface QemuCocoaView : NSView
{
+NSTrackingArea *trackingArea;
QEMUScreen screen;
-NSWindow *fullScreenWindow;
-float cx,cy,cw,ch,cdx,cdy;
pixman_image_t *pixman_image;
QKbdState *kbd;
BOOL isMouseGrabbed;
-BOOL isFullscreen;
BOOL isAbsoluteEnabled;
CFMachPortRef eventsTap;
}
- (void) switchSurface:(pixman_image_t *)image;
- (void) grabMouse;
- (void) ungrabMouse;
-- (void) toggleFullScreen:(id)sender;
- (void) setFullGrab:(id)sender;
- (void) handleMonitorInput:(NSEvent *)event;
- (bool) handleEvent:(NSEvent *)event;
@@ -332,8 +329,6 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
*/
- (BOOL) isMouseGrabbed;
- (BOOL) isAbsoluteEnabled;
-- (float) cdx;
-- (float) cdy;
- (QEMUScreen) gscreen;
- (void) raiseAllKeys;
@end
@@ -391,46 +386,43 @@ - (BOOL) isOpaque
return YES;
}
-- (BOOL) screenContainsPoint:(NSPoint) p
+- (void) removeTrackingRect
{
-return (p.x > -1 && p.x < screen.width && p.y > -1 && p.y < screen.height);
+if (trackingArea) {
+[self removeTrackingArea:trackingArea];
+[trackingArea release];
+trackingArea = nil;
+}
}
-/* Get location of event and convert to virtual screen coordinate */
-- (CGPoint) screenLocationOfEvent:(NSEvent *)ev
+- (void) frameUpdated
{
-NSWindow *eventWindow = [ev window];
-// XXX: Use CGRect and -convertRectFromScreen: to support macOS 10.10
-CGRect r = CGRectZero;
-r.origin = [ev locationInWindow];
-if (!eventWindow) {
-if (!isFullscreen) {
-return [[self window] convertRectFromScreen:r].origin;
-} else {
-CGPoint locationInSelfWindow = [[self window]
convertRectFromScreen:r].origin;
-CGPoint loc = [self convertPoint:locationInSelfWindow
fromView:nil];
-if (stretch_video) {
-loc.x /= cdx;
-loc.y /= cdy;
-}
-return loc;
-}
-} else if ([[self window] isEqual:eventWindow]) {
-if (!isFullscreen) {
-return r.origin;
-} else {
-CGPoint loc = [self convertPoint:r.origin fromView:nil];
-if (stretch_video) {
-loc.x /= cdx;
-loc.y /= cdy;
-}
-return loc;
-}
-} else {
-return [[self window] convertRectFromScreen:[eventWindow
convertRectToScreen:r]].origin;
+[self removeTrackingRect];
+
+if ([self window]) {
+NSTrackingAreaOptions options = NSTrackingActiveInKeyWindow |
+NSTrackingMouseEnteredAndExited |
+NSTrackingMouseMoved;
+trackingArea = [[NSTrackingArea alloc] initWithRect:[self frame]
+options:options
+ owner:self
+ userInfo:nil];
+[self addTrackingArea:trackingArea];
+[self updateUIInfo];
}
}
+- (void) viewDidMoveToWindow
+{
+[self resizeWindow];
+[self frameUpdated];
+}
+
+- (void) viewWillMoveToWindow:(NSWindow *)newWindow
+{
+[self removeTrackingRect];
+}
+
- (void) hideCursor
{
if (!cursor_hide) {
@@ -510,36 +502,43 @@ - (void) drawRect:(NSRect) rect
}
}
-- (void) setContentDimensions
+- (NSSize) fixZoomedFullScreenSize:(NSSize)proposedSize
{
-COCOA_DEBUG("QemuCocoaView: setContentDimensions\n");
+NSSize size;
-if (isFullscreen) {
-cdx = [[NSScreen mainScreen] frame].size.width / (float)screen.width;
-cdy = [[NSScreen mainScreen] frame].size.height / (float)screen.height;
+size.width = (CGFloat)screen.width * proposedSize.height;
+size.height = (CGFloat)screen.height * proposedSize.width;
-/* stretches video, but keeps same aspect ratio */
-if (stretch_video == true) {
-/* use smallest stretch value - prevents clipping on sides */
-if (MIN(cdx, cdy) == cdx) {
-cdy = cdx;
-} else {
-cdx = cdy;
-}
-} else { /* No stretching */
-cdx = cdy = 1;
-}
-cw = screen.width * cdx;
-ch = screen.height * cdy;
-cx = ([[NSScreen mainScreen] frame].size.width - cw) / 2.0;
-cy = ([[NSScreen mainScreen] frame].size.height - ch) / 2.0;
+if (size.width < size.height) {
+size.width /= screen.he
[PATCH v10 0/6] ui/cocoa: Use NSWindow's ability to resize
V5 -> V6: Rebased. Signed-off-by: Akihiko Odaki --- Changes in v10: - Removed relative mouse input scaling. - Link to v9: https://lore.kernel.org/r/20240213-cocoa-v9-0-d5a5e1bf0...@daynix.com Changes in v9: - Split patch "ui/cocoa: Use NSWindow's ability to resize" into patches "ui/cocoa: Let the platform toggle fullscreen", "ui/cocoa: Make window resizable", "ui/cocoa: Call console_select() with the BQL". - Added patch "ui/cocoa: Scale with NSView instead of Core Graphics". - Rebased. - Dropped Tested-by: from patch "ui/cocoa: Use NSWindow's ability to resize". - Link to v8: https://lore.kernel.org/r/20231218-cocoa-v8-0-d7fad80c7...@daynix.com Changes in v8: - Split into three patches. (BALATON Zoltan) - Removed negative full-screen conditions. (BALATON Zoltan) - Converted a C++-style comment to C style. - Link to v7: https://lore.kernel.org/r/20231217-cocoa-v7-1-6af21ef75...@daynix.com Changes in v7: - Fixed zoom-to-fit option. (Marek Glogowski) - Link to v6: https://lore.kernel.org/r/20231211-cocoa-v6-1-49f3be019...@daynix.com --- Akihiko Odaki (6): ui/cocoa: Release specific mouse buttons ui/cocoa: Scale with NSView instead of Core Graphics ui/cocoa: Let the platform toggle fullscreen ui/cocoa: Make window resizable ui/cocoa: Call console_select() with the BQL ui/cocoa: Remove stretch_video flag ui/cocoa.m | 532 + 1 file changed, 249 insertions(+), 283 deletions(-) --- base-commit: 5005aed8a7e728d028efb40e243ecfc2b4f3df3a change-id: 20231211-cocoa-576b8639e9bd Best regards, -- Akihiko Odaki
[PATCH v10 1/6] ui/cocoa: Release specific mouse buttons
ui/cocoa used to release all mouse buttons when it sees
NSEventTypeLeftMouseUp, NSEventTypeRightMouseUp, or
NSEventTypeOtherMouseUp, but it can instead release specific one
according to the delivered event.
Signed-off-by: Akihiko Odaki
---
ui/cocoa.m | 132 ++---
1 file changed, 55 insertions(+), 77 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index eb99064beeb4..fe0eb74b0743 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -99,7 +99,6 @@ static void cocoa_switch(DisplayChangeListener *dcl,
static DisplayChangeListener dcl = {
.ops = &dcl_ops,
};
-static int last_buttons;
static int cursor_hide = 1;
static int left_command_key_enabled = 1;
static bool swap_opt_cmd;
@@ -801,8 +800,6 @@ - (bool) handleEventLocked:(NSEvent *)event
COCOA_DEBUG("QemuCocoaView: handleEvent\n");
int buttons = 0;
int keycode = 0;
-bool mouse_event = false;
-// Location of event in virtual screen coordinates
NSPoint p = [self screenLocationOfEvent:event];
NSUInteger modifiers = [event modifierFlags];
@@ -883,25 +880,25 @@ - (bool) handleEventLocked:(NSEvent *)event
if (!!(modifiers & NSEventModifierFlagShift)) {
[self toggleKey:Q_KEY_CODE_SHIFT];
}
-break;
+return true;
case kVK_RightShift:
if (!!(modifiers & NSEventModifierFlagShift)) {
[self toggleKey:Q_KEY_CODE_SHIFT_R];
}
-break;
+return true;
case kVK_Control:
if (!!(modifiers & NSEventModifierFlagControl)) {
[self toggleKey:Q_KEY_CODE_CTRL];
}
-break;
+return true;
case kVK_RightControl:
if (!!(modifiers & NSEventModifierFlagControl)) {
[self toggleKey:Q_KEY_CODE_CTRL_R];
}
-break;
+return true;
case kVK_Option:
if (!!(modifiers & NSEventModifierFlagOption)) {
@@ -911,7 +908,7 @@ - (bool) handleEventLocked:(NSEvent *)event
[self toggleKey:Q_KEY_CODE_ALT];
}
}
-break;
+return true;
case kVK_RightOption:
if (!!(modifiers & NSEventModifierFlagOption)) {
@@ -921,7 +918,7 @@ - (bool) handleEventLocked:(NSEvent *)event
[self toggleKey:Q_KEY_CODE_ALT_R];
}
}
-break;
+return true;
/* Don't pass command key changes to guest unless mouse is
grabbed */
case kVK_Command:
@@ -934,7 +931,7 @@ - (bool) handleEventLocked:(NSEvent *)event
[self toggleKey:Q_KEY_CODE_META_L];
}
}
-break;
+return true;
case kVK_RightCommand:
if (isMouseGrabbed &&
@@ -945,9 +942,11 @@ - (bool) handleEventLocked:(NSEvent *)event
[self toggleKey:Q_KEY_CODE_META_R];
}
}
-break;
+return true;
+
+default:
+return true;
}
-break;
case NSEventTypeKeyDown:
keycode = cocoa_keycode_to_qemu([event keyCode]);
@@ -983,7 +982,7 @@ - (bool) handleEventLocked:(NSEvent *)event
} else {
[self handleMonitorInput: event];
}
-break;
+return true;
case NSEventTypeKeyUp:
keycode = cocoa_keycode_to_qemu([event keyCode]);
@@ -996,7 +995,7 @@ - (bool) handleEventLocked:(NSEvent *)event
if (qemu_console_is_graphic(NULL)) {
qkbd_state_key_event(kbd, keycode, false);
}
-break;
+return true;
case NSEventTypeMouseMoved:
if (isAbsoluteEnabled) {
// Cursor re-entered into a window might generate events bound
to screen coordinates
@@ -1012,34 +1011,18 @@ - (bool) handleEventLocked:(NSEvent *)event
}
}
}
-mouse_event = true;
-break;
+return [self handleMouseEvent:event];
case NSEventTypeLeftMouseDown:
-buttons |= MOUSE_EVENT_LBUTTON;
-mouse_event = true;
-break;
+return [self handleMouseEvent:event button:MOUSE_EVENT_LBUTTON
down:true];
case NSEventTypeRightMouseDown:
-buttons |= MOUSE_EVENT_RBUTTON;
-mouse_even
[PATCH v3 1/3] Hexagon (target/hexagon) Pass P0 explicitly to helpers that need it
Rather than reading P0 from the env, pass it explicitly Signed-off-by: Taylor Simpson Reviewed-by: Anton Johansson Tested-by: Anton Johansson --- target/hexagon/macros.h | 4 ++-- target/hexagon/hex_common.py | 12 +++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h index 1376d6ccc1..aedc863fab 100644 --- a/target/hexagon/macros.h +++ b/target/hexagon/macros.h @@ -1,5 +1,5 @@ /* - * Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights Reserved. + * Copyright(c) 2019-2024 Qualcomm Innovation Center, Inc. All Rights Reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -358,7 +358,7 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, int shift) #endif #define fREAD_PC() (PC) -#define fREAD_P0() (env->pred[0]) +#define fREAD_P0() (P0) #define fCHECK_PCALIGN(A) diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py index 195620c7ec..14dcf261b4 100755 --- a/target/hexagon/hex_common.py +++ b/target/hexagon/hex_common.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 ## -## Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights Reserved. +## Copyright(c) 2019-2024 Qualcomm Innovation Center, Inc. All Rights Reserved. ## ## This program is free software; you can redistribute it and/or modify ## it under the terms of the GNU General Public License as published by @@ -197,6 +197,10 @@ def get_tagimms(): return dict(zip(tags, list(map(compute_tag_immediates, tags +def need_p0(tag): +return "A_IMPLICIT_READS_P0" in attribdict[tag] + + def need_slot(tag): if ( "A_CVI_SCATTER" not in attribdict[tag] @@ -1118,6 +1122,12 @@ def helper_args(tag, regs, imms): "tcg_constant_tl(ctx->next_PC)", "target_ulong next_PC" )) +if need_p0(tag): +args.append(HelperArg( +"i32", +"hex_pred[0]", +"uint32_t P0" +)) if need_slot(tag): args.append(HelperArg( "i32", -- 2.34.1
[PATCH v3 0/3] Hexagon (target/hexagon) Only pass env to generated helper when needed
Currently, we pass env to every generated helper. When the semantics of the instruction only depend on the arguments, this is unnecessary and adds extra overhead to the helper call. Changes in v3 Update copyright year to 2024 Mark Reviewed-by/Tested-by: Anton Johansson Changes in v2 - Separate patches to pass P0 and SP explicitly to helpers that need it - Add the TCG_CALL_NO_RWG_SE flag to any non-HVX helpers that don't get ptr to env Taylor Simpson (3): Hexagon (target/hexagon) Pass P0 explicitly to helpers that need it Hexagon (target/hexagon) Pass SP explicitly to helpers that need it Hexagon (target/hexagon) Only pass env to generated helper when needed target/hexagon/gen_tcg.h| 5 +++- target/hexagon/macros.h | 6 ++-- target/hexagon/attribs_def.h.inc| 3 +- target/hexagon/gen_helper_protos.py | 12 ++-- target/hexagon/hex_common.py| 46 + 5 files changed, 59 insertions(+), 13 deletions(-) -- 2.34.1
[PATCH v3 2/3] Hexagon (target/hexagon) Pass SP explicitly to helpers that need it
Rather than reading SP from the env, pass it explicitly
Signed-off-by: Taylor Simpson
Reviewed-by: Anton Johansson
Tested-by: Anton Johansson
---
target/hexagon/macros.h | 2 +-
target/hexagon/attribs_def.h.inc | 3 ++-
target/hexagon/hex_common.py | 11 +++
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index aedc863fab..feb798c6c0 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -343,7 +343,7 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, int
shift)
#define fREAD_LR() (env->gpr[HEX_REG_LR])
-#define fREAD_SP() (env->gpr[HEX_REG_SP])
+#define fREAD_SP() (SP)
#define fREAD_LC0 (env->gpr[HEX_REG_LC0])
#define fREAD_LC1 (env->gpr[HEX_REG_LC1])
#define fREAD_SA0 (env->gpr[HEX_REG_SA0])
diff --git a/target/hexagon/attribs_def.h.inc b/target/hexagon/attribs_def.h.inc
index 87942d46f4..9e3a05f882 100644
--- a/target/hexagon/attribs_def.h.inc
+++ b/target/hexagon/attribs_def.h.inc
@@ -1,5 +1,5 @@
/*
- * Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights
Reserved.
+ * Copyright(c) 2019-2024 Qualcomm Innovation Center, Inc. All Rights
Reserved.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -117,6 +117,7 @@ DEF_ATTRIB(IMPLICIT_READS_P1, "Reads the P1 register", "",
"")
DEF_ATTRIB(IMPLICIT_READS_P2, "Reads the P2 register", "", "")
DEF_ATTRIB(IMPLICIT_READS_P3, "Reads the P3 register", "", "")
DEF_ATTRIB(IMPLICIT_WRITES_USR, "May write USR", "", "")
+DEF_ATTRIB(IMPLICIT_READS_SP, "Reads the SP register", "", "")
DEF_ATTRIB(COMMUTES, "The operation is communitive", "", "")
DEF_ATTRIB(DEALLOCRET, "dealloc_return", "", "")
DEF_ATTRIB(DEALLOCFRAME, "deallocframe", "", "")
diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
index 14dcf261b4..b96f67972d 100755
--- a/target/hexagon/hex_common.py
+++ b/target/hexagon/hex_common.py
@@ -101,6 +101,7 @@ def calculate_attribs():
add_qemu_macro_attrib('fLSBNEW1', 'A_IMPLICIT_READS_P1')
add_qemu_macro_attrib('fLSBNEW1NOT', 'A_IMPLICIT_READS_P1')
add_qemu_macro_attrib('fREAD_P3', 'A_IMPLICIT_READS_P3')
+add_qemu_macro_attrib('fREAD_SP', 'A_IMPLICIT_READS_SP')
# Recurse down macros, find attributes from sub-macros
macroValues = list(macros.values())
@@ -201,6 +202,10 @@ def need_p0(tag):
return "A_IMPLICIT_READS_P0" in attribdict[tag]
+def need_sp(tag):
+return "A_IMPLICIT_READS_SP" in attribdict[tag]
+
+
def need_slot(tag):
if (
"A_CVI_SCATTER" not in attribdict[tag]
@@ -1128,6 +1133,12 @@ def helper_args(tag, regs, imms):
"hex_pred[0]",
"uint32_t P0"
))
+if need_sp(tag):
+args.append(HelperArg(
+"i32",
+"hex_gpr[HEX_REG_SP]",
+"uint32_t SP"
+))
if need_slot(tag):
args.append(HelperArg(
"i32",
--
2.34.1
[PATCH v3 3/3] Hexagon (target/hexagon) Only pass env to generated helper when needed
Currently, we pass env to every generated helper. When the semantics of
the instruction only depend on the arguments, this is unnecessary and
adds extra overhead to the helper call.
We add the TCG_CALL_NO_RWG_SE flag to any non-HVX helpers that don't get
the ptr to env.
The A2_nop and SA1_setin1 instructions end up with no arguments. This
results in a "old-style function definition" error from the compiler, so
we write overrides for them.
With this change, the number of helpers with env argument is
idef-parser enabled:329 total, 23 with env
idef-parser disabled: 1543 total, 550 with env
Signed-off-by: Taylor Simpson
Reviewed-by: Anton Johansson
Tested-by: Anton Johansson
---
target/hexagon/gen_tcg.h| 5 -
target/hexagon/gen_helper_protos.py | 12 ++--
target/hexagon/hex_common.py| 23 ++-
3 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
index 1c4391b415..3fc1f4e281 100644
--- a/target/hexagon/gen_tcg.h
+++ b/target/hexagon/gen_tcg.h
@@ -1,5 +1,5 @@
/*
- * Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights
Reserved.
+ * Copyright(c) 2019-2024 Qualcomm Innovation Center, Inc. All Rights
Reserved.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -1369,3 +1369,6 @@
gen_helper_raise_exception(tcg_env, excp); \
} while (0)
#endif
+
+#define fGEN_TCG_A2_nop(SHORTCODE) do { } while (0)
+#define fGEN_TCG_SA1_setin1(SHORTCODE) tcg_gen_movi_tl(RdV, -1)
diff --git a/target/hexagon/gen_helper_protos.py
b/target/hexagon/gen_helper_protos.py
index c82b0f54e4..f8578d5033 100755
--- a/target/hexagon/gen_helper_protos.py
+++ b/target/hexagon/gen_helper_protos.py
@@ -1,7 +1,7 @@
#!/usr/bin/env python3
##
-## Copyright(c) 2019-2023 Qualcomm Innovation Center, Inc. All Rights
Reserved.
+## Copyright(c) 2019-2024 Qualcomm Innovation Center, Inc. All Rights
Reserved.
##
## This program is free software; you can redistribute it and/or modify
## it under the terms of the GNU General Public License as published by
@@ -40,7 +40,15 @@ def gen_helper_prototype(f, tag, tagregs, tagimms):
declared.append(arg.proto_arg)
arguments = ", ".join(declared)
-f.write(f"DEF_HELPER_{len(declared) - 1}({tag}, {arguments})\n")
+
+## Add the TCG_CALL_NO_RWG_SE flag to helpers that don't take the env
+## argument and aren't HVX instructions. Since HVX instructions take
+## pointers to their arguments, they will have side effects.
+if hex_common.need_env(tag) or hex_common.is_hvx_insn(tag):
+f.write(f"DEF_HELPER_{len(declared) - 1}({tag}, {arguments})\n")
+else:
+f.write(f"DEF_HELPER_FLAGS_{len(declared) - 1}({tag}, "
+f"TCG_CALL_NO_RWG_SE, {arguments})\n")
def main():
diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
index b96f67972d..d3d8560fcf 100755
--- a/target/hexagon/hex_common.py
+++ b/target/hexagon/hex_common.py
@@ -206,6 +206,18 @@ def need_sp(tag):
return "A_IMPLICIT_READS_SP" in attribdict[tag]
+def is_hvx_insn(tag):
+return "A_CVI" in attribdict[tag]
+
+
+def need_env(tag):
+return ("A_STORE" in attribdict[tag] or
+"A_LOAD" in attribdict[tag] or
+"A_CVI_GATHER" in attribdict[tag] or
+"A_CVI_SCATTER" in attribdict[tag] or
+"A_IMPLICIT_WRITES_USR" in attribdict[tag])
+
+
def need_slot(tag):
if (
"A_CVI_SCATTER" not in attribdict[tag]
@@ -1069,11 +1081,12 @@ def helper_args(tag, regs, imms):
args = []
## First argument is the CPU state
-args.append(HelperArg(
-"env",
-"tcg_env",
-"CPUHexagonState *env"
-))
+if need_env(tag):
+args.append(HelperArg(
+"env",
+"tcg_env",
+"CPUHexagonState *env"
+))
## For predicated instructions, we pass in the destination register
if is_predicated(tag):
--
2.34.1
Re: [External] [PATCH v2 05/23] migration/multifd: Drop MultiFDSendParams.normal[] array
On Fri, Feb 9, 2024 at 4:20 AM Fabiano Rosas wrote:
>
> Hao Xiang writes:
>
> > On Fri, Feb 2, 2024 at 2:30 AM wrote:
> >>
> >> From: Peter Xu
> >>
> >> This array is redundant when p->pages exists. Now we extended the life of
> >> p->pages to the whole period where pending_job is set, it should be safe to
> >> always use p->pages->offset[] rather than p->normal[]. Drop the array.
> >>
> >> Alongside, the normal_num is also redundant, which is the same to
> >> p->pages->num.
> >
> > Can we not drop p->normal and p_normal_num? It is redundant now but I
> > think it will be needed for multifd zero page checking. In multifd
> > zero page, we find out all zero pages and we sort the normal pages and
> > zero pages in two seperate arrays. p->offset is the original array of
> > pages, p->normal will contain the array of normal pages and p->zero
> > will contain the array of zero pages.
>
> We're moving send_fill_packet into send_prepare(), so you should be able
> to do whatever data transformation at send_prepare() and add any fields
> you need into p->pages.
>
> If we keep p->normal we will not be able to switch into an opaque
> payload later on. There should be no mention of pages outside of
> hooks. This is long-term work, but let's avoid blocking it if possible.
>
Got it. I will make the proper changes.
Aside from that, I would like to get opinions from you guys regarding
zero page detection interface.
Here are the options I am thinking:
1) Do zero page detection in send_prepare().
This means no dedicated hook for zero_page_detection() otherwise we
will be calling a hook from inside a hook. But we will need a new
function multifd_zero_page_check_send() similar to how we use
multifd_send_fill_packet() now. multifd_zero_page_check_send() will
need to be called by all send_prepare() implementations.
2) Do zero page detection in a new hook zero_page_detection().
zero_page_detection will be called before send_prepare(). Seems like
extra complexity but I can go with that routine if you guys think it's
a cleaner way.
I am leaning towards 1) right now.
> >>
> >> This doesn't apply to recv side, because there's no extra buffering on recv
> >> side, so p->normal[] array is still needed.
> >>
> >> Reviewed-by: Fabiano Rosas
> >> Signed-off-by: Peter Xu
> >> ---
> >> migration/multifd.h | 4
> >> migration/multifd-zlib.c | 7 ---
> >> migration/multifd-zstd.c | 7 ---
> >> migration/multifd.c | 33 +
> >> 4 files changed, 21 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/migration/multifd.h b/migration/multifd.h
> >> index 7c040cb85a..3920bdbcf1 100644
> >> --- a/migration/multifd.h
> >> +++ b/migration/multifd.h
> >> @@ -122,10 +122,6 @@ typedef struct {
> >> struct iovec *iov;
> >> /* number of iovs used */
> >> uint32_t iovs_num;
> >> -/* Pages that are not zero */
> >> -ram_addr_t *normal;
> >> -/* num of non zero pages */
> >> -uint32_t normal_num;
> >> /* used for compression methods */
> >> void *data;
> >> } MultiFDSendParams;
> >> diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
> >> index 37ce48621e..100809abc1 100644
> >> --- a/migration/multifd-zlib.c
> >> +++ b/migration/multifd-zlib.c
> >> @@ -116,17 +116,18 @@ static void zlib_send_cleanup(MultiFDSendParams *p,
> >> Error **errp)
> >> */
> >> static int zlib_send_prepare(MultiFDSendParams *p, Error **errp)
> >> {
> >> +MultiFDPages_t *pages = p->pages;
> >> struct zlib_data *z = p->data;
> >> z_stream *zs = &z->zs;
> >> uint32_t out_size = 0;
> >> int ret;
> >> uint32_t i;
> >>
> >> -for (i = 0; i < p->normal_num; i++) {
> >> +for (i = 0; i < pages->num; i++) {
> >> uint32_t available = z->zbuff_len - out_size;
> >> int flush = Z_NO_FLUSH;
> >>
> >> -if (i == p->normal_num - 1) {
> >> +if (i == pages->num - 1) {
> >> flush = Z_SYNC_FLUSH;
> >> }
> >>
> >> @@ -135,7 +136,7 @@ static int zlib_send_prepare(MultiFDSendParams *p,
> >> Error **errp)
> >> * with compression. zlib does not guarantee that this is safe,
> >> * therefore copy the page before calling deflate().
> >> */
> >> -memcpy(z->buf, p->pages->block->host + p->normal[i],
> >> p->page_size);
> >> +memcpy(z->buf, p->pages->block->host + pages->offset[i],
> >> p->page_size);
> >> zs->avail_in = p->page_size;
> >> zs->next_in = z->buf;
> >>
> >> diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
> >> index b471daadcd..2023edd8cc 100644
> >> --- a/migration/multifd-zstd.c
> >> +++ b/migration/multifd-zstd.c
> >> @@ -113,6 +113,7 @@ static void zstd_send_cleanup(MultiFDSendParams *p,
> >> Error **errp)
> >> */
> >> static int zstd_send_prepare(MultiFDSendParams *p, Error **errp)
> >> {
> >> +MultiFDPages_t *pages = p->pages;
> >> struct zstd_data *z = p->data;
> >> int ret
[PULL 0/2] tcg patch queue
Dangit, PULL. r~ On 2/13/24 15:25, Richard Henderson wrote: The following changes since commit bc2e8b18fba33f30f25b7c2d74328493c0a2231d: Merge tag 'hppa64-pull-request' of https://github.com/hdeller/qemu-hppa into staging (2024-02-13 13:56:46 +) are available in the Git repository at: https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20240213 for you to fetch changes up to e41f1825b43796c3508ef309ed0b150ef89acc44: tcg/arm: Fix goto_tb for large translation blocks (2024-02-13 07:42:45 -1000) tcg: Increase width of temp_subindex tcg/arm: Fix goto_tb for large translation blocks Richard Henderson (2): tcg: Increase width of temp_subindex tcg/arm: Fix goto_tb for large translation blocks include/tcg/tcg.h| 2 +- tcg/arm/tcg-target.c.inc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
Re: possible deprecation and removal of some old QEMU Arm machine types (pxa2xx, omap, sa1110)
Hi, On Tue, Feb 13, 2024 at 09:11:38PM +0100, Arnd Bergmann wrote: > On Tue, Feb 13, 2024, at 16:36, Guenter Roeck wrote: > >> > > OMAP1 machines: > >> > > > >> > > cheetah Palm Tungsten|E aka. Cheetah PDA (OMAP310) > >> > > sx1 Siemens SX1 (OMAP310) V2 > >> > > sx1-v1 Siemens SX1 (OMAP310) V1 > >> > > > >> > I test sx1. I don't think I ever tried cheetah, and I could not get > >> > sx1-v1 > >> > to work. > > This is similar. omap1 development is slightly more active > than pxa, but then again they have no DT support today and > are unlikely to ever get there at this point. > > Out of the five machines that are still supported in the > kernel, I think three still run on hardware (osk, ams-delta > and nokia770), while the other ones were left there only > for their qemu support. I don't mind removing them from > the kernel as well if they are gone from qemu. I'm one of the OMAP1 Linux kernel maintainers, and I have Palm TE which I have been using for testing and development (and reporting bugs, regressions) along with those other boards you mentioned. Since I have the real Palm HW, I haven't used QEMU for that particular board. But however I use QEMU SX1 support frequently as it's quickest way to check if OMAP1 is bootable, and if the basic peripherals are working. SX1 is close to Palm/AMS-Delta, and also it's ARMv4T which is rare these days. I think it's useful to keep it in QEMU as long there are hardware that people use. So my wish is to keep at least SX1 support in QEMU as long as ARMv4T supported in the Linux kernel. > >> > > OMAP2 machines: > >> > > > >> > > n800 Nokia N800 tablet aka. RX-34 (OMAP2420) > >> > > n810 Nokia N810 tablet aka. RX-44 (OMAP2420) > >> > > > >> > I never managed to get those to boot the Linux kernel. > > I think Tony still tests these on both hardware and qemu. > The platform side here is much more modern than any of the > others above since it does use DT and it has enough RAM > to be somewhat usable. I have also these boards (real hardware) and test them frequently with mainline Linux. However, QEMU support I haven't used/needed. I recall it was a bit buggy, and some changes in mainline made the kernel unbootable. Unless Tony needs the support, I guess they are good to go. (Arnd: RAM isn't everything. Some of the OMAP1 boards today are still more useful than N800/N810, even with modern bloaty Linux.) A.
[PATCH 1/2] tcg: Increase width of temp_subindex
We need values 0-3 for TCG_TYPE_I128 on 32-bit hosts.
Cc: qemu-sta...@nongnu.org
Fixes: 43eef72f4109 ("tcg: Add temp allocation for TCGv_i128")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2159
Signed-off-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Michael Tokarev
Tested-by: Michael Tokarev
---
include/tcg/tcg.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index daf2a5bf9e..451f3fec41 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -412,7 +412,7 @@ typedef struct TCGTemp {
unsigned int mem_coherent:1;
unsigned int mem_allocated:1;
unsigned int temp_allocated:1;
-unsigned int temp_subindex:1;
+unsigned int temp_subindex:2;
int64_t val;
struct TCGTemp *mem_base;
--
2.34.1
[PATCH 0/2] tcg patch queue
The following changes since commit bc2e8b18fba33f30f25b7c2d74328493c0a2231d: Merge tag 'hppa64-pull-request' of https://github.com/hdeller/qemu-hppa into staging (2024-02-13 13:56:46 +) are available in the Git repository at: https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20240213 for you to fetch changes up to e41f1825b43796c3508ef309ed0b150ef89acc44: tcg/arm: Fix goto_tb for large translation blocks (2024-02-13 07:42:45 -1000) tcg: Increase width of temp_subindex tcg/arm: Fix goto_tb for large translation blocks Richard Henderson (2): tcg: Increase width of temp_subindex tcg/arm: Fix goto_tb for large translation blocks include/tcg/tcg.h| 2 +- tcg/arm/tcg-target.c.inc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
[PATCH 2/2] tcg/arm: Fix goto_tb for large translation blocks
Correct arithmetic for separating high and low
on a large negative number.
Cc: qemu-sta...@nongnu.org
Fixes: 79ffece4447 ("tcg/arm: Implement direct branch for goto_tb")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1714
Signed-off-by: Richard Henderson
Reviewed-by: Michael Tokarev
---
tcg/arm/tcg-target.c.inc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tcg/arm/tcg-target.c.inc b/tcg/arm/tcg-target.c.inc
index ffd23ef789..6a04c73c76 100644
--- a/tcg/arm/tcg-target.c.inc
+++ b/tcg/arm/tcg-target.c.inc
@@ -1771,9 +1771,9 @@ static void tcg_out_goto_tb(TCGContext *s, int which)
* shifted immediate from pc.
*/
int h = -i_disp;
-int l = h & 0xfff;
+int l = -(h & 0xfff);
-h = encode_imm_nofail(h - l);
+h = encode_imm_nofail(h + l);
tcg_out_dat_imm(s, COND_AL, ARITH_SUB, TCG_REG_R0, TCG_REG_PC, h);
tcg_out_ld32_12(s, COND_AL, TCG_REG_PC, TCG_REG_R0, l);
}
--
2.34.1
Re: [PATCH v15 0/9] rutabaga_gfx + gfxstream
On Tue, Jan 30, 2024 at 7:10 PM Gurchetan Singh
wrote:
>
>
> On Fri, Jan 26, 2024 at 6:23 AM Alyssa Ross wrote:
>
>> Gurchetan Singh writes:
>>
>> > On Sat, Jan 20, 2024 at 4:19 AM Alyssa Ross wrote:
>> >
>> >> Gurchetan Singh writes:
>> >>
>> >> > On Fri, Jan 19, 2024 at 1:13 PM Alyssa Ross wrote:
>> >> >>
>> >> >> Hi Gurchetan,
>> >> >>
>> >> >> > Thanks for the reminder. I did make a request to create the
>> release
>> >> >> > tags, but changes were requested by Fedora packaging effort:
>> >> >> >
>> >> >> > https://bugzilla.redhat.com/show_bug.cgi?id=2242058
>> >> >> > https://bugzilla.redhat.com/show_bug.cgi?id=2241701
>> >> >> >
>> >> >> > So the request was canceled, but never re-requested. I'll fire
>> off
>> >> >> > another request, with:
>> >> >> >
>> >> >> > gfxstream: 23d05703b94035ac045df60823fb1fc4be0fdf1c ("gfxstream:
>> >> >> > manually add debug logic")
>> >> >> > AEMU: dd8b929c247ce9872c775e0e5ddc4300011d0e82 ("aemu: improve
>> >> licensing")
>> >> >> >
>> >> >> > as the commits. These match the Fedora requests, and the AEMU
>> one has
>> >> >> > been merged into Fedora already it seems.
>> >> >>
>> >> >> These revisions have the problem I mentioned in my previous message:
>> >> >>
>> >> >> >> The gfxstream ref mentioned here isn't compatible with
>> >> >> >> v0.1.2-rutabaga-release, because it no longer provides
>> >> logging_base.pc,
>> >> >>
>> >> >> rutabaga was not fixed to use the new AEMU package names until
>> after the
>> >> >> v0.1.2-rutabaga-release tag, in commit 5dfd74a06. So will there be
>> a
>> >> >> new Rutabaga release that's compatible with these release versions
>> of
>> >> >> gfxstream and AEMU?
>> >> >
>> >> > Good catch.
>> >> >
>> >> > One possible workaround is to build gfxstream as a shared library. I
>> >> > think that would avoid rutabaga looking for AEMU package config
>> files.
>> >> >
>> >> > But if another rutabaga release is desired with support for a static
>> >> > library, then we can make that happen too.
>> >>
>> >> We're exclusively building gfxstream as a shared library.
>> >>
>> >> Looking at rutabaga's build.rs, it appears to me like pkg-config is
>> >> always used for gfxstream unless overridden by GFXSTREAM_PATH.
>> >>
>> >
>> > Hmm, it seems we should be checking pkg-config --static before looking
>> for
>> > AEMU in build.rs -- oh well.
>> >
>> > Would this be a suitable commit for the 0.1.3 release of rutabaga?
>> >
>> >
>> https://chromium.googlesource.com/crosvm/crosvm/+/5dfd74a0680d317c6edf44138def886f47cb1c7c
>> >
>> > The gfxstream/AEMU commits would remain unchanged.
>>
>> That combination works for me.
>>
>
> Just FYI, still working on it. Could take 1-2 more weeks.
>
FYI:
https://android.googlesource.com/platform/hardware/google/gfxstream/+/refs/tags/v0.1.2-gfxstream-release
https://android.googlesource.com/platform/hardware/google/aemu/+/refs/tags/v0.1.2-aemu-release
https://chromium.googlesource.com/crosvm/crosvm/+/refs/tags/v0.1.3-rutabaga-release
>
>
>
Re: [PATCH v3 29/33] linux-user: Allow TARGET_PAGE_BITS_VARY
On 1/30/24 03:47, Ilya Leoshkevich wrote:
I wonder if it would make sense to add something like the following to
this patch?
--- a/page-vary-target.c
+++ b/page-vary-target.c
@@ -26,8 +26,7 @@
bool set_preferred_target_page_bits(int bits)
{
#ifdef TARGET_PAGE_BITS_VARY
-assert(bits >= TARGET_PAGE_BITS_MIN);
-return set_preferred_target_page_bits_common(bits);
+return set_preferred_target_page_bits_common(MAX(TARGET_PAGE_BITS_MIN,
bits));
#else
return true;
#endif
No, this conflicts with the system-mode usage.
If we want to bound, then we need this MAX in the user-only caller.
r~
Re: possible deprecation and removal of some old QEMU Arm machine types (pxa2xx, omap, sa1110)
On Tue, Feb 13, 2024 at 9:12 PM Arnd Bergmann wrote: > On Tue, Feb 13, 2024, at 16:36, Guenter Roeck wrote: > > On Tue, Feb 13, 2024 at 03:14:21PM +, Peter Maydell wrote: > >> On Mon, 12 Feb 2024 at 14:36, Guenter Roeck wrote: > >> > On 2/12/24 04:32, Peter Maydell wrote: > >> > > The one SA1110 machine: > >> > > > >> > > collie Sharp SL-5500 (Collie) PDA (SA-1110) > >> > > > >> > I do test collie. > > Adding Linus Walleij and Stefan Lehner to Cc, as they were > interested in modernizing sa1100 back in 2022. If they > are still interested in that, they might want to keep collie > support. I'm not personally interested in the Collie, I have a SA1100 hardware but not that one. > Surprisingly, at the time I removed unused old board files, > there was a lot more interest in sa1100 than in the newer > pxa platform, which I guess wasn't as appealing for > retrocomputing yet. Andrea Adami and Dmitry Eremin-Solenikov did the work in 2017 to modernize it a bit, and Russell helped out. I was under the impression that they only used real hardware though! The Collie is popular because it is/was easy to get hold of and easy to hack. PXA was in candybar phones (right?) which are just veritable fortresses and really hard to hack so that is why there is no interest (except for the occasional hyperfocused Harald Welte), so those are a bit like the iPhones: you *can* boot something custom on them, but it won't be easy or quick, and not as fun and rewarding. The thriving world of PostmarketOS only exist because Google was clever to realize devices should have a developer mode. Yours, Linus Walleij
Re: [PATCH 3/3] monitor: only run coroutine commands in qemu_aio_context
On Sat, 3 Feb 2024 at 06:30, Michael Tokarev wrote: > > 03.02.2024 12:01, Michael Tokarev wrote: > ... > > This change broke something in 7.2. I'm still debugging it, will > > come with a follow-up once some more details are found, I'll also > > check current master with and without this commit. > > > > The prob happens with multiple suspend-resume cycles, - with this > > change applied, guest does not work as expected after *second* > > suspend-resume. > > So, it turned out the prob here exists on master too, and manifests > itself the same way on 7.2.9 or on 8.2.1, - in all cases where we > have this change applied it works (or breaks) equally. > > A (simple) reproducer so far is a hibernate test, - it fails *only* > after suspend-to-ram, but works fine after just hibernate. > > I used just an initrd (with a drive image used for swap - > for hibernation space). > > qemu-img create s.img 256M > mkswap s.img > qemu-system-x86_64 \ >-serial stdio -vga none -display none -parallel none -net none \ >-machine q35 \ >-drive file=s.img,if=ide,format=raw \ >-m 256 \ >-monitor unix:ttyS0,server,nowait \ >-kernel /boot/vmlinuz-6.1.0-15-amd64 \ >-initrd /boot/initrd.img-6.1.0-15-amd64 \ >-append "shell=/bin/sh console=ttyS0 root=none" > > There, in the guest (it has busybox only here): > # swapon /dev/sda > # echo mem > /sys/power/state > (system_wakeup on the monitor) > # echo disk > /sys/power/state > > The system will hibernate but *not* turn off power, qemu > will continue running, while all console messages are the > same as when it works fine. qemu process is spinning up > with 100% cpu usage at this stage. > > Without the intermediate suspend-to-ram or without the > commit in question, qemu process will exit normally at > this stage. > > This is a somewhat patalogical test case, but I see it as an > indicator of something else being wrong, like we aren't saving > or restoring some state now which we should do. > > The tight loop also suggests we're not having success in there. I'm unable to reproduce this. QEMU v8.2.0, v8.1.0, and even v7.2.0 all spin with 100% CPU usage on my machine. It looks to me like this behavior is not a regression caused by this commit. Do v7.2.0, v8.1.0, and v8.2.0 terminate QEMU on your machine? Stefan
[PULL 62/88] esp.c: zero command register when TI command terminates due to phase change
This is the behaviour documented in the datasheet and allows the state machine
to correctly process multiple consecutive TI commands.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-63-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 8
1 file changed, 8 insertions(+)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 4c1ca63a57..ccb8ad4bae 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -519,6 +519,7 @@ static void esp_do_dma(ESPState *s)
/* ATN remains asserted until TC == 0 */
if (esp_get_tc(s) == 0) {
esp_set_phase(s, STAT_CD);
+s->rregs[ESP_CMD] = 0;
s->rregs[ESP_RSEQ] = SEQ_CD;
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
@@ -717,6 +718,7 @@ static void esp_do_nodma(ESPState *s)
*/
s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
esp_set_phase(s, STAT_CD);
+s->rregs[ESP_CMD] = 0;
s->rregs[ESP_RSEQ] = SEQ_CD;
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
@@ -831,6 +833,11 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
*/
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
break;
+
+case CMD_TI | CMD_DMA:
+case CMD_TI:
+s->rregs[ESP_CMD] = 0;
+break;
}
/* Raise bus service interrupt to indicate change to STATUS phase */
@@ -885,6 +892,7 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
* Bus service interrupt raised because of initial change to
* DATA phase
*/
+s->rregs[ESP_CMD] = 0;
s->rregs[ESP_RINTR] |= INTR_BS;
break;
}
--
2.39.2
[PULL 60/88] esp.c: use deferred interrupts for both DATA IN and DATA OUT phases
This brings DATA OUT transfers in line with DATA IN transfers by ensuring that
the guest visible function complete interrupt is only set once the SCSI layer
has returned.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-61-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 35 ++-
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index b6cf1b43db..d71465718c 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -248,10 +248,8 @@ static int esp_select(ESPState *s)
/*
* Note that we deliberately don't raise the IRQ here: this will be done
- * either in do_command_phase() for DATA OUT transfers or by the deferred
- * IRQ mechanism in esp_transfer_data() for DATA IN transfers
+ * either in esp_transfer_data() or esp_command_complete()
*/
-s->rregs[ESP_RINTR] |= INTR_FC;
s->rregs[ESP_RSEQ] = SEQ_CD;
return 0;
}
@@ -321,20 +319,17 @@ static void do_command_phase(ESPState *s)
datalen = scsi_req_enqueue(s->current_req);
s->ti_size = datalen;
fifo8_reset(&s->cmdfifo);
+s->data_ready = false;
if (datalen != 0) {
s->ti_cmd = 0;
+/*
+ * Switch to DATA phase but wait until initial data xfer is
+ * complete before raising the command completion interrupt
+ */
if (datalen > 0) {
-/*
- * Switch to DATA IN phase but wait until initial data xfer is
- * complete before raising the command completion interrupt
- */
-s->data_ready = false;
esp_set_phase(s, STAT_DI);
} else {
esp_set_phase(s, STAT_DO);
-s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-esp_raise_irq(s);
-esp_lower_drq(s);
}
scsi_req_continue(s->current_req);
return;
@@ -832,9 +827,9 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
case CMD_SELATN:
/*
* No data phase for sequencer command so raise deferred bus service
- * interrupt
+ * and function complete interrupt
*/
-s->rregs[ESP_RINTR] |= INTR_BS;
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
break;
}
@@ -854,14 +849,13 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
void esp_transfer_data(SCSIRequest *req, uint32_t len)
{
ESPState *s = req->hba_private;
-int to_device = (esp_get_phase(s) == STAT_DO);
uint32_t dmalen = esp_get_tc(s);
trace_esp_transfer_data(dmalen, s->ti_size);
s->async_len = len;
s->async_buf = scsi_req_get_buf(req);
-if (!to_device && !s->data_ready) {
+if (!s->data_ready) {
s->data_ready = true;
switch (s->rregs[ESP_CMD]) {
@@ -869,6 +863,13 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
case CMD_SEL:
case CMD_SELATN | CMD_DMA:
case CMD_SELATN:
+/*
+ * Initial incoming data xfer is complete for sequencer command
+ * so raise deferred bus service and function complete interrupt
+ */
+ s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
+ break;
+
case CMD_SELATNS | CMD_DMA:
case CMD_SELATNS:
/*
@@ -876,7 +877,6 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
* completion interrupt
*/
s->rregs[ESP_RINTR] |= INTR_BS;
- esp_raise_irq(s);
break;
case CMD_TI | CMD_DMA:
@@ -886,9 +886,10 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
* DATA phase
*/
s->rregs[ESP_RINTR] |= INTR_BS;
-esp_raise_irq(s);
break;
}
+
+esp_raise_irq(s);
}
/*
--
2.39.2
[PULL 73/88] esp.c: remove restriction on FIFO read access when DMA memory routines defined
The latest state machines can handle mixing DMA and non-DMA FIFO access for all
SCSI phases except DATA IN and DATA OUT. For DATA IN and DATA OUT phases, the
transfer is complete when TC == 0 and the updated logic will now handle TC
underflow correctly, which makes it just about impossible to manually manipulate
the FIFO during a DMA transfer.
Remove the restriction on FIFO read access when DMA memory routines are defined
which also allows the NeXTCube machine to pass its self-test.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-74-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index a3e18bb3d7..f9d848171f 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -1133,14 +1133,7 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
switch (saddr) {
case ESP_FIFO:
-if (s->dma_memory_read && s->dma_memory_write &&
-(s->rregs[ESP_RSTAT] & STAT_PIO_MASK) == 0) {
-/* Data out. */
-qemu_log_mask(LOG_UNIMP, "esp: PIO data read not implemented\n");
-s->rregs[ESP_FIFO] = 0;
-} else {
-s->rregs[ESP_FIFO] = esp_fifo_pop(&s->fifo);
-}
+s->rregs[ESP_FIFO] = esp_fifo_pop(&s->fifo);
val = s->rregs[ESP_FIFO];
break;
case ESP_RINTR:
--
2.39.2
[PULL 66/88] esp.c: process non-DMA FIFO writes in esp_do_nodma()
Currently any write to the ESP FIFO in the MESSAGE OUT or COMMAND phases will
manually raise the bus service interrupt. Instead of duplicating the interrupt
logic in esp_reg_write(), update esp_do_nodma() to correctly process incoming
FIFO data during the MESSAGE OUT and COMMAND phases. Part of this change is to
call esp_nodma_ti_dataout() from handle_ti() to ensure that the DATA OUT phase
FIFO transfer only occurs when executing a non-DMA TI command instead of for
each byte entering the FIFO.
One slight complication is that NextSTEP uses multiple TI commands to transfer
the CDB one byte at a time (as opposed to loading the FIFO and using a single
TI command), so it is necessary to determine the expected length of the SCSI
CDB being received. This is handled by the introduction of a new
esp_cdb_length() function which returns the expected SCSI CDB length based
upon the first command byte.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-67-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 121 +++---
1 file changed, 86 insertions(+), 35 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 97e48e9526..5bb8cc4ea7 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -420,6 +420,7 @@ static void handle_satn_stop(ESPState *s)
esp_set_phase(s, STAT_MO);
s->rregs[ESP_RSEQ] = SEQ_MO;
+s->cmdfifo_cdb_offset = 0;
if (s->dma) {
esp_do_dma(s);
@@ -454,6 +455,22 @@ static void write_response(ESPState *s)
}
}
+static int esp_cdb_length(ESPState *s)
+{
+const uint8_t *pbuf;
+int cmdlen, len;
+
+cmdlen = fifo8_num_used(&s->cmdfifo);
+if (cmdlen < s->cmdfifo_cdb_offset) {
+return 0;
+}
+
+pbuf = fifo8_peek_buf(&s->cmdfifo, cmdlen, NULL);
+len = scsi_cdb_length((uint8_t *)&pbuf[s->cmdfifo_cdb_offset]);
+
+return len;
+}
+
static void esp_dma_ti_check(ESPState *s)
{
if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
@@ -738,16 +755,40 @@ static void esp_do_nodma(ESPState *s)
fifo8_push_all(&s->cmdfifo, buf, n);
s->cmdfifo_cdb_offset += n;
-/*
- * Extra message out bytes received: update cmdfifo_cdb_offset
- * and then switch to command phase
- */
-s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
-esp_set_phase(s, STAT_CD);
-s->rregs[ESP_CMD] = 0;
-s->rregs[ESP_RSEQ] = SEQ_CD;
-s->rregs[ESP_RINTR] |= INTR_BS;
-esp_raise_irq(s);
+switch (s->rregs[ESP_CMD]) {
+case CMD_SELATN:
+if (fifo8_num_used(&s->cmdfifo) >= 1) {
+/* First byte received, switch to command phase */
+esp_set_phase(s, STAT_CD);
+s->cmdfifo_cdb_offset = 1;
+
+if (fifo8_num_used(&s->cmdfifo) > 1) {
+/* Process any additional command phase data */
+esp_do_nodma(s);
+}
+}
+break;
+
+case CMD_SELATNS:
+if (fifo8_num_used(&s->cmdfifo) == 1) {
+/* First byte received, stop in message out phase */
+s->cmdfifo_cdb_offset = 1;
+
+/* Raise command completion interrupt */
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
+esp_raise_irq(s);
+}
+break;
+
+case CMD_TI:
+/* ATN remains asserted until FIFO empty */
+s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
+esp_set_phase(s, STAT_CD);
+s->rregs[ESP_CMD] = 0;
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+break;
+}
break;
case STAT_CD:
@@ -756,21 +797,40 @@ static void esp_do_nodma(ESPState *s)
n = MIN(fifo8_num_free(&s->cmdfifo), n);
fifo8_push_all(&s->cmdfifo, buf, n);
-cmdlen = fifo8_num_used(&s->cmdfifo);
-trace_esp_handle_ti_cmd(cmdlen);
-s->ti_size = 0;
+switch (s->rregs[ESP_CMD]) {
+case CMD_TI:
+cmdlen = fifo8_num_used(&s->cmdfifo);
+trace_esp_handle_ti_cmd(cmdlen);
+
+/* CDB may be transferred in one or more TI commands */
+if (esp_cdb_length(s) && esp_cdb_length(s) ==
+fifo8_num_used(&s->cmdfifo) - s->cmdfifo_cdb_offset) {
+/* Command has been received */
+do_cmd(s);
+} else {
+/*
+ * If data was transferred from the FIFO then raise bus
+ * service interrupt to indicate transfer complete. Otherwise
+ * defer until the next FIFO write.
+ */
+if (n) {
+/* Raise interrupt to indicate transfer complete */
+s->rregs[ESP_RINTR] |= INTR_BS;
+
[PULL 52/88] esp.c: move CMD_SELATNS end of command logic to esp_do_dma() and do_dma_pdma_cb()
The special logic in satn_stop_pdma_cb() is now no longer required since
esp_do_dma() can be used as a direct replacement.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-53-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 82 +--
include/hw/scsi/esp.h | 1 -
2 files changed, 41 insertions(+), 42 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 3cf8b2b4eb..29e3869173 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -418,54 +418,31 @@ static void handle_s_without_atn(ESPState *s)
}
}
-static void satn_stop_pdma_cb(ESPState *s)
-{
-uint8_t buf[ESP_FIFO_SZ];
-int n;
-
-/* Copy FIFO into cmdfifo */
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
-
-if (!esp_get_tc(s) && !fifo8_is_empty(&s->cmdfifo)) {
-trace_esp_handle_satn_stop(fifo8_num_used(&s->cmdfifo));
-s->cmdfifo_cdb_offset = 1;
-esp_set_phase(s, STAT_CD);
-s->rregs[ESP_RSTAT] |= STAT_TC;
-s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-s->rregs[ESP_RSEQ] = SEQ_CD;
-esp_raise_irq(s);
-}
-}
-
static void handle_satn_stop(ESPState *s)
{
-int32_t cmdlen;
-
if (s->dma && !s->dma_enabled) {
s->dma_cb = handle_satn_stop;
return;
}
-esp_set_pdma_cb(s, SATN_STOP_PDMA_CB);
+esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
if (esp_select(s) < 0) {
return;
}
-cmdlen = get_cmd(s, 1);
-if (cmdlen > 0) {
-trace_esp_handle_satn_stop(fifo8_num_used(&s->cmdfifo));
-s->cmdfifo_cdb_offset = 1;
-esp_set_phase(s, STAT_MO);
-s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-s->rregs[ESP_RSEQ] = SEQ_MO;
-esp_raise_irq(s);
-} else if (cmdlen == 0) {
-if (s->dma) {
-esp_raise_drq(s);
+
+esp_set_phase(s, STAT_MO);
+s->rregs[ESP_RSEQ] = SEQ_MO;
+
+if (s->dma) {
+esp_do_dma(s);
+} else {
+if (get_cmd(s, 1)) {
+trace_esp_handle_satn_stop(fifo8_num_used(&s->cmdfifo));
+
+/* Raise command completion interrupt */
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
+s->rregs[ESP_RSEQ] = SEQ_MO;
+esp_raise_irq(s);
}
-/* Target present, switch to message out phase */
-s->rregs[ESP_RSEQ] = SEQ_MO;
-esp_set_phase(s, STAT_MO);
}
}
@@ -554,6 +531,19 @@ static void do_dma_pdma_cb(ESPState *s)
}
break;
+case CMD_SELATNS | CMD_DMA:
+if (fifo8_num_used(&s->cmdfifo) == 1) {
+/* First byte received, stop in message out phase */
+esp_set_phase(s, STAT_CD);
+s->cmdfifo_cdb_offset = 1;
+
+/* Raise command completion interrupt */
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
+s->rregs[ESP_RSEQ] = SEQ_CD;
+esp_raise_irq(s);
+}
+break;
+
case CMD_TI | CMD_DMA:
/* ATN remains asserted until TC == 0 */
if (esp_get_tc(s) == 0) {
@@ -676,6 +666,19 @@ static void esp_do_dma(ESPState *s)
}
break;
+case CMD_SELATNS | CMD_DMA:
+if (fifo8_num_used(&s->cmdfifo) == 1) {
+/* First byte received, stop in message out phase */
+esp_set_phase(s, STAT_CD);
+s->cmdfifo_cdb_offset = 1;
+
+/* Raise command completion interrupt */
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
+s->rregs[ESP_RSEQ] = SEQ_CD;
+esp_raise_irq(s);
+}
+break;
+
case CMD_TI | CMD_DMA:
/* ATN remains asserted until TC == 0 */
if (esp_get_tc(s) == 0) {
@@ -906,9 +909,6 @@ static void esp_do_nodma(ESPState *s)
static void esp_pdma_cb(ESPState *s)
{
switch (s->pdma_cb) {
-case SATN_STOP_PDMA_CB:
-satn_stop_pdma_cb(s);
-break;
case WRITE_RESPONSE_PDMA_CB:
write_response_pdma_cb(s);
break;
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 9945645837..a4b2ed115c 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -152,7 +152,6 @@ struct SysBusESPState {
/* PDMA callbacks */
enum pdma_cb {
-SATN_STOP_PDMA_CB = 2,
WRITE_RESPONSE_PDMA_CB = 3,
DO_DMA_PDMA_CB = 4
};
--
2.39.2
[PULL 87/88] esp.c: switch TypeInfo registration to use DEFINE_TYPES() macro
The use of the DEFINE_TYPES() macro will soon be recommended over the use of
calling type_init() directly.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20240112125420.514425-88-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 39 +--
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 04615d8b5f..b8762d5ee0 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -1549,14 +1549,6 @@ static void sysbus_esp_class_init(ObjectClass *klass,
void *data)
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
}
-static const TypeInfo sysbus_esp_info = {
-.name = TYPE_SYSBUS_ESP,
-.parent= TYPE_SYS_BUS_DEVICE,
-.instance_init = sysbus_esp_init,
-.instance_size = sizeof(SysBusESPState),
-.class_init= sysbus_esp_class_init,
-};
-
static void esp_finalize(Object *obj)
{
ESPState *s = ESP(obj);
@@ -1582,19 +1574,22 @@ static void esp_class_init(ObjectClass *klass, void
*data)
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
}
-static const TypeInfo esp_info = {
-.name = TYPE_ESP,
-.parent = TYPE_DEVICE,
-.instance_init = esp_init,
-.instance_finalize = esp_finalize,
-.instance_size = sizeof(ESPState),
-.class_init = esp_class_init,
+static const TypeInfo esp_info_types[] = {
+{
+.name = TYPE_SYSBUS_ESP,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_init = sysbus_esp_init,
+.instance_size = sizeof(SysBusESPState),
+.class_init= sysbus_esp_class_init,
+},
+{
+.name = TYPE_ESP,
+.parent = TYPE_DEVICE,
+.instance_init = esp_init,
+.instance_finalize = esp_finalize,
+.instance_size = sizeof(ESPState),
+.class_init = esp_class_init,
+},
};
-static void esp_register_types(void)
-{
-type_register_static(&sysbus_esp_info);
-type_register_static(&esp_info);
-}
-
-type_init(esp_register_types)
+DEFINE_TYPES(esp_info_types)
--
2.39.2
Re: [PATCH v4 11/12] hw/sparc/leon3: Initialize GPIO before realizing CPU devices
On 13/02/2024 13:03, Philippe Mathieu-Daudé wrote: Inline cpu_create() in order to call qdev_init_gpio_in_named_with_opaque() before the CPU is realized. Note that with the previous new patch included in the series, qdev_init_gpio_in_named_with_opaque() should be replaced by qdev_init_gpio_in_named() in the commit message above instead. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Reviewed-by: Mark Cave-Ayland --- hw/sparc/leon3.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c index d2be900988..d0ff4a2892 100644 --- a/hw/sparc/leon3.c +++ b/hw/sparc/leon3.c @@ -233,7 +233,9 @@ static void leon3_generic_hw_init(MachineState *machine) APBPnp *apb_pnp; /* Init CPU */ -cpu = SPARC_CPU(cpu_create(machine->cpu_type)); +cpu = SPARC_CPU(object_new(machine->cpu_type)); +qdev_init_gpio_in_named(DEVICE(cpu), leon3_set_pil_in, "pil", 1); +qdev_realize(DEVICE(cpu), NULL, &error_fatal); env = &cpu->env; cpu_sparc_set_id(env, 0); @@ -260,7 +262,6 @@ static void leon3_generic_hw_init(MachineState *machine) /* Allocate IRQ manager */ irqmpdev = qdev_new(TYPE_GRLIB_IRQMP); -qdev_init_gpio_in_named(DEVICE(cpu), leon3_set_pil_in, "pil", 1); sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), &error_fatal); sysbus_mmio_map(SYS_BUS_DEVICE(irqmpdev), 0, LEON3_IRQMP_OFFSET); qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", 0, ATB, Mark.
Re: [PATCH v4 10/12] hw/sparc/leon3: Pass DeviceState opaque argument to leon3_set_pil_in()
On 13/02/2024 13:03, Philippe Mathieu-Daudé wrote:
By passing a DeviceState context to a QDev IRQ handler,
we can simplify and use qdev_init_gpio_in_named() instead
of qdev_init_gpio_in_named_with_opaque().
Suggested-by: Mark Cave-Ayland
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sparc/leon3.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 0df5fc949d..d2be900988 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -175,9 +175,10 @@ static void leon3_irq_ack(CPUSPARCState *env, int intno)
*/
static void leon3_set_pil_in(void *opaque, int n, int level)
{
-CPUSPARCState *env = opaque;
+DeviceState *cpu = opaque;
+CPUState *cs = CPU(cpu);
+CPUSPARCState *env = cpu_env(cs);
uint32_t pil_in = level;
-CPUState *cs;
assert(env != NULL);
@@ -193,7 +194,6 @@ static void leon3_set_pil_in(void *opaque, int n, int level)
env->interrupt_index = TT_EXTINT | i;
if (old_interrupt != env->interrupt_index) {
-cs = env_cpu(env);
trace_leon3_set_irq(i);
cpu_interrupt(cs, CPU_INTERRUPT_HARD);
}
@@ -201,7 +201,6 @@ static void leon3_set_pil_in(void *opaque, int n, int level)
}
}
} else if (!env->pil_in && (env->interrupt_index & ~15) == TT_EXTINT) {
-cs = env_cpu(env);
trace_leon3_reset_irq(env->interrupt_index & 15);
env->interrupt_index = 0;
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
@@ -261,8 +260,7 @@ static void leon3_generic_hw_init(MachineState *machine)
/* Allocate IRQ manager */
irqmpdev = qdev_new(TYPE_GRLIB_IRQMP);
-qdev_init_gpio_in_named_with_opaque(DEVICE(cpu), leon3_set_pil_in,
-env, "pil", 1);
+qdev_init_gpio_in_named(DEVICE(cpu), leon3_set_pil_in, "pil", 1);
sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), &error_fatal);
sysbus_mmio_map(SYS_BUS_DEVICE(irqmpdev), 0, LEON3_IRQMP_OFFSET);
qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", 0,
Reviewed-by: Mark Cave-Ayland
ATB,
Mark.
Re: possible deprecation and removal of some old QEMU Arm machine types (pxa2xx, omap, sa1110)
On Tue, Feb 13, 2024, at 16:36, Guenter Roeck wrote: > On Tue, Feb 13, 2024 at 03:14:21PM +, Peter Maydell wrote: >> On Mon, 12 Feb 2024 at 14:36, Guenter Roeck wrote: >> > On 2/12/24 04:32, Peter Maydell wrote: >> > > The machines I have in mind are: >> > > >> > > PXA2xx machines: >> > > >> > > akitaSharp SL-C1000 (Akita) PDA (PXA270) >> > > borzoi Sharp SL-C3100 (Borzoi) PDA (PXA270) >> > > connex Gumstix Connex (PXA255) >> > > mainstoneMainstone II (PXA27x) >> > > spitzSharp SL-C3000 (Spitz) PDA (PXA270) >> > > terrier Sharp SL-C3200 (Terrier) PDA (PXA270) >> > > tosa Sharp SL-6000 (Tosa) PDA (PXA255) >> > > verdex Gumstix Verdex Pro XL6P COMs (PXA270) >> > > z2 Zipit Z2 (PXA27x) >> > > >> > I test akita, borzoi, spitz, and terrier. Upstream Linux removed support >> > for mainstone, tosa, and z2 from the Linux kernel as of version 6.0, so It was 6.3 (about one year ago). >> > I am no longer testing those. >> > >> > I never managed to boot connex or verdex. I kept specifically the pxa boards that would be sensible to port to devicetree because they had qemu support. gumstix verdex is the one with the most RAM out of those; spitz, sharpsl and their variants are the ones that some of us still have around. Robert had working devicetree support for some machines out of tree that he has not submitted, and presumably not worked on since. Unless someone starts working on converting the remaining pxa board files to DT, we can probably remove them after the next LTS kernel a year from now. I have no objection to removing them from qemu if that helps, the existing qemu releases should be sufficient for anyone trying the conversion. >> > > OMAP1 machines: >> > > >> > > cheetah Palm Tungsten|E aka. Cheetah PDA (OMAP310) >> > > sx1 Siemens SX1 (OMAP310) V2 >> > > sx1-v1 Siemens SX1 (OMAP310) V1 >> > > >> > I test sx1. I don't think I ever tried cheetah, and I could not get sx1-v1 >> > to work. This is similar. omap1 development is slightly more active than pxa, but then again they have no DT support today and are unlikely to ever get there at this point. Out of the five machines that are still supported in the kernel, I think three still run on hardware (osk, ams-delta and nokia770), while the other ones were left there only for their qemu support. I don't mind removing them from the kernel as well if they are gone from qemu. >> > > OMAP2 machines: >> > > >> > > n800 Nokia N800 tablet aka. RX-34 (OMAP2420) >> > > n810 Nokia N810 tablet aka. RX-44 (OMAP2420) >> > > >> > I never managed to get those to boot the Linux kernel. I think Tony still tests these on both hardware and qemu. The platform side here is much more modern than any of the others above since it does use DT and it has enough RAM to be somewhat usable. On the other hand, this is the one platform actually using the cursed arm1136r0 core (not counting imx31 and realview here as I'm not aware of any real users), and anything that would get us closer to dropping support for this CPU would be welcome ;-) >> > > The one SA1110 machine: >> > > >> > > collie Sharp SL-5500 (Collie) PDA (SA-1110) >> > > >> > I do test collie. Adding Linus Walleij and Stefan Lehner to Cc, as they were interested in modernizing sa1100 back in 2022. If they are still interested in that, they might want to keep collie support. Surprisingly, at the time I removed unused old board files, there was a lot more interest in sa1100 than in the newer pxa platform, which I guess wasn't as appealing for retrocomputing yet. >> > All the ones I use still boot the latest Linux kernel. >> > >> > > Obviously if we can remove all the machines that used a given >> > > SoC, that's much more effective than if we just delete one or two. >> > > >> > > I don't have any test images for the SA1110 or OMAP1 machines, >> > > so those are the ones I am most keen to be able to drop. >> > > I do have test images for a few of the pxa2xx and the OMAP2 machines. >> > > >> > I don't mind dropping them, just listing what I use for testing the >> > Linux kernel. I suspect I may be the only "user" of those boards, >> > though, both in Linux and qemu. >> >> Mmm; there's not much point in both QEMU and the kernel >> maintaining code that nobody's using. Are you considering >> dropping support for any of these SoC families from the kernel? >> > Not me personally. Arnd is the one mostly involved in dropping > support of obsolete hardware from the kernel These are all clearly among the least maintained boards we have left in the kernel after the last purge. At the time I asked for remaining users in 2022, I kept pretty much anything that had the slightest chance of still being used and I was already planning another round for the next LTS kernel in early 2023 that would be more aggr
[PULL 68/88] esp.c: move write_response() non-DMA logic to esp_do_nodma()
This moves the remaining non-DMA STATUS and MESSAGE IN phase logic from
write_response() to esp_do_nodma(). Note that we can also now drop the extra
fifo_reset() which is no longer required.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-69-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 32 +++-
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 277eb8647b..824ebe9ff0 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -392,20 +392,12 @@ static void handle_satn_stop(ESPState *s)
static void write_response(ESPState *s)
{
-uint8_t buf[2];
-
trace_esp_write_response(s->status);
if (s->dma) {
esp_do_dma(s);
} else {
-buf[0] = s->status;
-buf[1] = 0;
-
-fifo8_reset(&s->fifo);
-fifo8_push_all(&s->fifo, buf, 2);
-s->rregs[ESP_RFLAGS] = 2;
-esp_raise_irq(s);
+esp_do_nodma(s);
}
}
@@ -815,6 +807,28 @@ static void esp_do_nodma(ESPState *s)
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
break;
+
+case STAT_ST:
+switch (s->rregs[ESP_CMD]) {
+case CMD_ICCS:
+fifo8_push(&s->fifo, s->status);
+esp_set_phase(s, STAT_MI);
+
+/* Process any message in phase data */
+esp_do_nodma(s);
+break;
+}
+break;
+
+case STAT_MI:
+switch (s->rregs[ESP_CMD]) {
+case CMD_ICCS:
+fifo8_push(&s->fifo, 0);
+
+esp_raise_irq(s);
+break;
+}
+break;
}
}
--
2.39.2
[PULL 86/88] esp.c: keep track of the DRQ state during DMA
Currently the DRQ IRQ is updated every time DMA data is sent/received which
is both inefficient and causes excessive logging of the DRQ state. Add a
new drq_state bool that only updates the DRQ IRQ if its state changes.
This commit adds the new drq_state bool to the migration state: since the
version number has already been increased earlier in the series, there is
no need to repeat it again here. The DRQ IRQ is (currently) only used for
PDMA transfers which already have a migration break in this series so
there are no problems setting its value post-load.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-87-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 15 +++
include/hw/scsi/esp.h | 1 +
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index fb68606f00..04615d8b5f 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -62,14 +62,20 @@ static void esp_lower_irq(ESPState *s)
static void esp_raise_drq(ESPState *s)
{
-qemu_irq_raise(s->drq_irq);
-trace_esp_raise_drq();
+if (!(s->drq_state)) {
+qemu_irq_raise(s->drq_irq);
+trace_esp_raise_drq();
+s->drq_state = true;
+}
}
static void esp_lower_drq(ESPState *s)
{
-qemu_irq_lower(s->drq_irq);
-trace_esp_lower_drq();
+if (s->drq_state) {
+qemu_irq_lower(s->drq_irq);
+trace_esp_lower_drq();
+s->drq_state = false;
+}
}
void esp_dma_enable(ESPState *s, int irq, int level)
@@ -1358,6 +1364,7 @@ const VMStateDescription vmstate_esp = {
VMSTATE_UINT8_TEST(mig_ti_cmd, ESPState,
esp_is_between_version_5_and_6),
VMSTATE_UINT8_TEST(lun, ESPState, esp_is_version_6),
+VMSTATE_BOOL(drq_state, ESPState),
VMSTATE_END_OF_LIST()
},
};
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index c6e8b64e20..533d856aa3 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -26,6 +26,7 @@ struct ESPState {
uint8_t wregs[ESP_REGS];
qemu_irq irq;
qemu_irq drq_irq;
+bool drq_state;
uint8_t chip_id;
bool tchi_written;
int32_t ti_size;
--
2.39.2
[PULL 53/88] esp.c: replace do_dma_pdma_cb() with esp_do_dma()
Now that the DMA logic is identical between do_dma_pdma_cb() and esp_do_dma()
we can replace do_dma_pdma_cb() with esp_do_dma().
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-54-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 138 +-
1 file changed, 1 insertion(+), 137 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 29e3869173..f69b2709fc 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -491,141 +491,6 @@ static void esp_dma_ti_check(ESPState *s)
}
}
-static void do_dma_pdma_cb(ESPState *s)
-{
-uint8_t buf[ESP_CMDFIFO_SZ];
-int len;
-uint32_t n, cmdlen;
-
-len = esp_get_tc(s);
-
-switch (esp_get_phase(s)) {
-case STAT_MO:
-if (s->dma_memory_read) {
-len = MIN(len, fifo8_num_free(&s->cmdfifo));
-s->dma_memory_read(s->dma_opaque, buf, len);
-fifo8_push_all(&s->cmdfifo, buf, len);
-esp_set_tc(s, esp_get_tc(s) - len);
-s->cmdfifo_cdb_offset += len;
-} else {
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
-s->cmdfifo_cdb_offset += n;
-}
-
-esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
-esp_raise_drq(s);
-
-switch (s->rregs[ESP_CMD]) {
-case CMD_SELATN | CMD_DMA:
-if (fifo8_num_used(&s->cmdfifo) >= 1) {
-/* First byte received, switch to command phase */
-esp_set_phase(s, STAT_CD);
-s->cmdfifo_cdb_offset = 1;
-
-if (fifo8_num_used(&s->cmdfifo) > 1) {
-/* Process any additional command phase data */
-esp_do_dma(s);
-}
-}
-break;
-
-case CMD_SELATNS | CMD_DMA:
-if (fifo8_num_used(&s->cmdfifo) == 1) {
-/* First byte received, stop in message out phase */
-esp_set_phase(s, STAT_CD);
-s->cmdfifo_cdb_offset = 1;
-
-/* Raise command completion interrupt */
-s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-s->rregs[ESP_RSEQ] = SEQ_CD;
-esp_raise_irq(s);
-}
-break;
-
-case CMD_TI | CMD_DMA:
-/* ATN remains asserted until TC == 0 */
-if (esp_get_tc(s) == 0) {
-esp_set_phase(s, STAT_CD);
-s->rregs[ESP_RSEQ] = SEQ_CD;
-s->rregs[ESP_RINTR] |= INTR_BS;
-esp_raise_irq(s);
-}
-break;
-}
-break;
-
-case STAT_CD:
-cmdlen = fifo8_num_used(&s->cmdfifo);
-trace_esp_do_dma(cmdlen, len);
-if (s->dma_memory_read) {
-len = MIN(len, fifo8_num_free(&s->cmdfifo));
-s->dma_memory_read(s->dma_opaque, buf, len);
-fifo8_push_all(&s->cmdfifo, buf, len);
-esp_set_tc(s, esp_get_tc(s) - len);
-} else {
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
-
-esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
-esp_raise_drq(s);
-}
-trace_esp_handle_ti_cmd(cmdlen);
-s->ti_size = 0;
-if (esp_get_tc(s) == 0) {
-/* Command has been received */
-do_cmd(s);
-}
-break;
-
-case STAT_DO:
-if (!s->current_req) {
-return;
-}
-/* Copy FIFO data to device */
-len = MIN(s->async_len, ESP_FIFO_SZ);
-len = MIN(len, fifo8_num_used(&s->fifo));
-n = esp_fifo_pop_buf(&s->fifo, s->async_buf, len);
-s->async_buf += n;
-s->async_len -= n;
-s->ti_size += n;
-
-if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
-/* Defer until the scsi layer has completed */
-scsi_req_continue(s->current_req);
-return;
-}
-
-esp_dma_ti_check(s);
-break;
-
-case STAT_DI:
-if (!s->current_req) {
-return;
-}
-/* Copy device data to FIFO */
-len = MIN(s->async_len, esp_get_tc(s));
-len = MIN(len, fifo8_num_free(&s->fifo));
-fifo8_push_all(&s->fifo, s->async_buf, len);
-s->async_buf += len;
-s->async_len -= len;
-s->ti_size -= len;
-esp_set_tc(s, esp_get_tc(s) - len);
-
-if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
-/* Defer until the scsi layer has completed */
-scsi_req_continue(s->current_req);
-s->data_in_ready = false;
-return;
-}
-
-esp_dma_ti_check(s);
-break;
-}
-}
-
stati
[PULL 80/88] esp.c: consolidate DMA and PDMA logic in MESSAGE OUT phase
This allows the removal of duplicate logic shared between the two
implementations.
Note that we restrict esp_raise_drq() to PDMA to help reduce the log verbosity
for normal DMA.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-81-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 49fc059eaa..ae65c2ef37 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -436,17 +436,15 @@ static void esp_do_dma(ESPState *s)
if (s->dma_memory_read) {
len = MIN(len, fifo8_num_free(&s->cmdfifo));
s->dma_memory_read(s->dma_opaque, buf, len);
-fifo8_push_all(&s->cmdfifo, buf, len);
esp_set_tc(s, esp_get_tc(s) - len);
-s->cmdfifo_cdb_offset += len;
} else {
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
-s->cmdfifo_cdb_offset += n;
+len = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+len = MIN(fifo8_num_free(&s->cmdfifo), len);
+esp_raise_drq(s);
}
-esp_raise_drq(s);
+fifo8_push_all(&s->cmdfifo, buf, len);
+s->cmdfifo_cdb_offset += len;
switch (s->rregs[ESP_CMD]) {
case CMD_SELATN | CMD_DMA:
--
2.39.2
[PULL 58/88] esp.c: separate logic based upon ESP command in esp_command_complete()
The handling of the INTR_FC and INTR_BS bits is different depending upon the
last command executed by the ESP. Note that currently INTR_FC is managed
elsewhere, but that will change soon.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-59-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 30 --
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 73c723afcc..75538f5859 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -823,25 +823,27 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
* Switch to status phase. For non-DMA transfers from the target the last
* byte is still in the FIFO
*/
-esp_set_phase(s, STAT_ST);
-if (s->ti_size == 0) {
-/*
- * Transfer complete: force TC to zero just in case a TI command was
- * requested for more data than the command returns (Solaris 8 does
- * this)
- */
-esp_set_tc(s, 0);
-esp_dma_ti_check(s);
-} else {
+s->ti_size = 0;
+
+switch (s->rregs[ESP_CMD]) {
+case CMD_SEL | CMD_DMA:
+case CMD_SEL:
+case CMD_SELATN | CMD_DMA:
+case CMD_SELATN:
/*
- * Transfer truncated: raise INTR_BS to indicate early change of
- * phase
+ * No data phase for sequencer command so raise deferred bus service
+ * interrupt
*/
s->rregs[ESP_RINTR] |= INTR_BS;
-esp_raise_irq(s);
-s->ti_size = 0;
+break;
}
+/* Raise bus service interrupt to indicate change to STATUS phase */
+esp_set_phase(s, STAT_ST);
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+esp_lower_drq(s);
+
if (s->current_req) {
scsi_req_unref(s->current_req);
s->current_req = NULL;
--
2.39.2
[PULL 75/88] esp.c: improve ESP_RSEQ logic consolidation
The ESP_RSEQ logic is scattered in a few places throughout the ESP state machine
which is mainly because the ESP_RSEQ register isn't always reset when executing
an ESP select command. Once this is done, the ESP_RSEQ register only needs to be
updated at the point where the sequencer command completes.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-76-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index aa7dec71e3..ca26415d5f 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -230,6 +230,7 @@ static int esp_select(ESPState *s)
target = s->wregs[ESP_WBUSID] & BUSID_DID;
s->ti_size = 0;
+s->rregs[ESP_RSEQ] = SEQ_0;
if (s->current_req) {
/* Started a new command before the old one finished. Cancel it. */
@@ -241,7 +242,6 @@ static int esp_select(ESPState *s)
/* No such drive */
s->rregs[ESP_RSTAT] = 0;
s->rregs[ESP_RINTR] = INTR_DC;
-s->rregs[ESP_RSEQ] = SEQ_0;
esp_raise_irq(s);
return -1;
}
@@ -250,7 +250,6 @@ static int esp_select(ESPState *s)
* Note that we deliberately don't raise the IRQ here: this will be done
* either in esp_transfer_data() or esp_command_complete()
*/
-s->rregs[ESP_RSEQ] = SEQ_CD;
return 0;
}
@@ -358,7 +357,6 @@ static void handle_s_without_atn(ESPState *s)
}
esp_set_phase(s, STAT_CD);
-s->rregs[ESP_RSEQ] = SEQ_CD;
s->cmdfifo_cdb_offset = 0;
if (s->dma) {
@@ -380,7 +378,6 @@ static void handle_satn_stop(ESPState *s)
}
esp_set_phase(s, STAT_MO);
-s->rregs[ESP_RSEQ] = SEQ_MO;
s->cmdfifo_cdb_offset = 0;
if (s->dma) {
@@ -456,6 +453,7 @@ static void esp_do_dma(ESPState *s)
if (fifo8_num_used(&s->cmdfifo) >= 1) {
/* First byte received, switch to command phase */
esp_set_phase(s, STAT_CD);
+s->rregs[ESP_RSEQ] = SEQ_CD;
s->cmdfifo_cdb_offset = 1;
if (fifo8_num_used(&s->cmdfifo) > 1) {
@@ -468,11 +466,11 @@ static void esp_do_dma(ESPState *s)
case CMD_SELATNS | CMD_DMA:
if (fifo8_num_used(&s->cmdfifo) == 1) {
/* First byte received, stop in message out phase */
+s->rregs[ESP_RSEQ] = SEQ_MO;
s->cmdfifo_cdb_offset = 1;
/* Raise command completion interrupt */
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-s->rregs[ESP_RSEQ] = SEQ_CD;
esp_raise_irq(s);
}
break;
@@ -482,7 +480,6 @@ static void esp_do_dma(ESPState *s)
if (esp_get_tc(s) == 0) {
esp_set_phase(s, STAT_CD);
s->rregs[ESP_CMD] = 0;
-s->rregs[ESP_RSEQ] = SEQ_CD;
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
}
@@ -726,6 +723,7 @@ static void esp_do_nodma(ESPState *s)
if (fifo8_num_used(&s->cmdfifo) >= 1) {
/* First byte received, switch to command phase */
esp_set_phase(s, STAT_CD);
+s->rregs[ESP_RSEQ] = SEQ_CD;
s->cmdfifo_cdb_offset = 1;
if (fifo8_num_used(&s->cmdfifo) > 1) {
@@ -738,6 +736,7 @@ static void esp_do_nodma(ESPState *s)
case CMD_SELATNS:
if (fifo8_num_used(&s->cmdfifo) >= 1) {
/* First byte received, stop in message out phase */
+s->rregs[ESP_RSEQ] = SEQ_MO;
s->cmdfifo_cdb_offset = 1;
/* Raise command completion interrupt */
@@ -903,6 +902,7 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
* and function complete interrupt
*/
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
+s->rregs[ESP_RSEQ] = SEQ_CD;
break;
case CMD_TI | CMD_DMA:
@@ -948,6 +948,7 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
* so raise deferred bus service and function complete interrupt
*/
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
+ s->rregs[ESP_RSEQ] = SEQ_CD;
break;
case CMD_SELATNS | CMD_DMA:
@@ -957,6 +958,7 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
* completion interrupt
*/
s->rregs[ESP_RINTR] |= INTR_BS;
+ s->rregs[ESP_RSEQ] = SEQ_MO;
break;
case CMD_TI | CMD_DMA:
--
2.39.2
[PULL 56/88] esp.c: remove unused PDMA callback implementation
Note that this is a migration break for the q800 machine because the extra PDMA
information is no longer included.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-57-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 55 ---
include/hw/scsi/esp.h | 6 -
2 files changed, 5 insertions(+), 56 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index bdbdb209f7..5061c9d5a1 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -223,11 +223,6 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
esp_set_tc(s, dmalen);
}
-static void esp_set_pdma_cb(ESPState *s, enum pdma_cb cb)
-{
-s->pdma_cb = cb;
-}
-
static int esp_select(ESPState *s)
{
int target;
@@ -377,7 +372,7 @@ static void handle_satn(ESPState *s)
s->dma_cb = handle_satn;
return;
}
-esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
+
if (esp_select(s) < 0) {
return;
}
@@ -400,7 +395,7 @@ static void handle_s_without_atn(ESPState *s)
s->dma_cb = handle_s_without_atn;
return;
}
-esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
+
if (esp_select(s) < 0) {
return;
}
@@ -424,7 +419,7 @@ static void handle_satn_stop(ESPState *s)
s->dma_cb = handle_satn_stop;
return;
}
-esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
+
if (esp_select(s) < 0) {
return;
}
@@ -497,7 +492,6 @@ static void esp_do_dma(ESPState *s)
s->cmdfifo_cdb_offset += n;
}
-esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
switch (s->rregs[ESP_CMD]) {
@@ -551,7 +545,6 @@ static void esp_do_dma(ESPState *s)
n = MIN(fifo8_num_free(&s->cmdfifo), n);
fifo8_push_all(&s->cmdfifo, buf, n);
-esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
}
trace_esp_handle_ti_cmd(cmdlen);
@@ -597,7 +590,6 @@ static void esp_do_dma(ESPState *s)
s->async_len -= n;
s->ti_size += n;
-esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
@@ -644,7 +636,6 @@ static void esp_do_dma(ESPState *s)
s->async_len -= len;
s->ti_size -= len;
esp_set_tc(s, esp_get_tc(s) - len);
-esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
@@ -805,11 +796,6 @@ static void esp_do_nodma(ESPState *s)
}
}
-static void esp_pdma_cb(ESPState *s)
-{
-esp_do_dma(s);
-}
-
void esp_command_complete(SCSIRequest *req, size_t resid)
{
ESPState *s = req->hba_private;
@@ -1229,33 +1215,6 @@ static int esp_post_load(void *opaque, int version_id)
return 0;
}
-/*
- * PDMA (or pseudo-DMA) is only used on the Macintosh and requires the
- * guest CPU to perform the transfers between the SCSI bus and memory
- * itself. This is indicated by the dma_memory_read and dma_memory_write
- * functions being NULL (in contrast to the ESP PCI device) whilst
- * dma_enabled is still set.
- */
-
-static bool esp_pdma_needed(void *opaque)
-{
-ESPState *s = ESP(opaque);
-
-return s->dma_memory_read == NULL && s->dma_memory_write == NULL &&
- s->dma_enabled;
-}
-
-static const VMStateDescription vmstate_esp_pdma = {
-.name = "esp/pdma",
-.version_id = 0,
-.minimum_version_id = 0,
-.needed = esp_pdma_needed,
-.fields = (const VMStateField[]) {
-VMSTATE_UINT8(pdma_cb, ESPState),
-VMSTATE_END_OF_LIST()
-}
-};
-
const VMStateDescription vmstate_esp = {
.name = "esp",
.version_id = 6,
@@ -1290,10 +1249,6 @@ const VMStateDescription vmstate_esp = {
VMSTATE_UINT8_TEST(lun, ESPState, esp_is_version_6),
VMSTATE_END_OF_LIST()
},
-.subsections = (const VMStateDescription * const []) {
-&vmstate_esp_pdma,
-NULL
-}
};
static void sysbus_esp_mem_write(void *opaque, hwaddr addr,
@@ -1342,7 +1297,7 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr
addr,
esp_pdma_write(s, val);
break;
}
-esp_pdma_cb(s);
+esp_do_dma(s);
}
static uint64_t sysbus_esp_pdma_read(void *opaque, hwaddr addr,
@@ -1363,7 +1318,7 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, hwaddr
addr,
val = (val << 8) | esp_pdma_read(s);
break;
}
-esp_pdma_cb(s);
+esp_do_dma(s);
return val;
}
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 0207fdd7a6..6f942864a6 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -51,7 +51,6 @@ struct ESPState {
ESPDMAMemoryReadWriteFunc dma_memory_write;
void *dma_opaque;
void (*dma_cb)(ESPState *s);
-uint8_t pdma_cb;
uint8_t mig_version_id;
@@ -150,11
[PULL 85/88] esp.c: rename irq_data IRQ to drq_irq
The IRQ represented by irq_data is actually the DRQ (DMA request) line so rename
it accordingly.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-86-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 8
include/hw/scsi/esp.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 5583b3eb56..fb68606f00 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -62,13 +62,13 @@ static void esp_lower_irq(ESPState *s)
static void esp_raise_drq(ESPState *s)
{
-qemu_irq_raise(s->irq_data);
+qemu_irq_raise(s->drq_irq);
trace_esp_raise_drq();
}
static void esp_lower_drq(ESPState *s)
{
-qemu_irq_lower(s->irq_data);
+qemu_irq_lower(s->drq_irq);
trace_esp_lower_drq();
}
@@ -1062,7 +1062,7 @@ void esp_hard_reset(ESPState *s)
static void esp_soft_reset(ESPState *s)
{
qemu_irq_lower(s->irq);
-qemu_irq_lower(s->irq_data);
+qemu_irq_lower(s->drq_irq);
esp_hard_reset(s);
}
@@ -1489,7 +1489,7 @@ static void sysbus_esp_realize(DeviceState *dev, Error
**errp)
}
sysbus_init_irq(sbd, &s->irq);
-sysbus_init_irq(sbd, &s->irq_data);
+sysbus_init_irq(sbd, &s->drq_irq);
assert(sysbus->it_shift != -1);
s->chip_id = TCHI_FAS100A;
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 39b416f538..c6e8b64e20 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -25,7 +25,7 @@ struct ESPState {
uint8_t rregs[ESP_REGS];
uint8_t wregs[ESP_REGS];
qemu_irq irq;
-qemu_irq irq_data;
+qemu_irq drq_irq;
uint8_t chip_id;
bool tchi_written;
int32_t ti_size;
--
2.39.2
[PULL 65/88] esp.c: move non-DMA TI logic to separate esp_nodma_ti_dataout() function
This is to allow the logic to be moved during the next commit.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-66-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 51 +--
1 file changed, 29 insertions(+), 22 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index dd6bf6f033..97e48e9526 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -697,11 +697,38 @@ static void esp_do_dma(ESPState *s)
}
}
+static void esp_nodma_ti_dataout(ESPState *s)
+{
+int len;
+
+if (!s->current_req) {
+return;
+}
+if (s->async_len == 0) {
+/* Defer until data is available. */
+return;
+}
+len = MIN(s->async_len, ESP_FIFO_SZ);
+len = MIN(len, fifo8_num_used(&s->fifo));
+esp_fifo_pop_buf(&s->fifo, s->async_buf, len);
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size += len;
+
+if (s->async_len == 0) {
+scsi_req_continue(s->current_req);
+return;
+}
+
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+}
+
static void esp_do_nodma(ESPState *s)
{
uint8_t buf[ESP_FIFO_SZ];
uint32_t cmdlen;
-int len, n;
+int n;
switch (esp_get_phase(s)) {
case STAT_MO:
@@ -743,27 +770,7 @@ static void esp_do_nodma(ESPState *s)
break;
case STAT_DO:
-if (!s->current_req) {
-return;
-}
-if (s->async_len == 0) {
-/* Defer until data is available. */
-return;
-}
-len = MIN(s->async_len, ESP_FIFO_SZ);
-len = MIN(len, fifo8_num_used(&s->fifo));
-esp_fifo_pop_buf(&s->fifo, s->async_buf, len);
-s->async_buf += len;
-s->async_len -= len;
-s->ti_size += len;
-
-if (s->async_len == 0) {
-scsi_req_continue(s->current_req);
-return;
-}
-
-s->rregs[ESP_RINTR] |= INTR_BS;
-esp_raise_irq(s);
+esp_nodma_ti_dataout(s);
break;
case STAT_DI:
--
2.39.2
[PULL 72/88] esp.c: handle TC underflow for DMA SCSI requests
Detect the case where the guest underflows TC by requesting a DMA transfer which
is larger than the available data. If this case is detected, immediately
complete the SCSI request and handle any remaining FIFO accesses in the STATUS
phase by raising INTR_BS once the FIFO is below the threshold.
Note that handling the premature SCSI bus phase change in the case of TC
underflow fixes booting EMILE on m68k once again.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-73-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 25 -
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 8ea100ee9c..a3e18bb3d7 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -579,6 +579,12 @@ static void esp_do_dma(ESPState *s)
s->async_len -= len;
s->ti_size -= len;
+if (s->async_len == 0 && s->ti_size == 0 && esp_get_tc(s)) {
+/* If the guest underflows TC then terminate SCSI request */
+scsi_req_continue(s->current_req);
+return;
+}
+
if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
/* Defer until the scsi layer has completed */
scsi_req_continue(s->current_req);
@@ -596,6 +602,12 @@ static void esp_do_dma(ESPState *s)
esp_set_tc(s, esp_get_tc(s) - len);
esp_raise_drq(s);
+if (s->async_len == 0 && s->ti_size == 0 && esp_get_tc(s)) {
+/* If the guest underflows TC then terminate SCSI request */
+scsi_req_continue(s->current_req);
+return;
+}
+
if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
/* Defer until the scsi layer has completed */
scsi_req_continue(s->current_req);
@@ -630,6 +642,15 @@ static void esp_do_dma(ESPState *s)
}
}
break;
+
+default:
+/* Consume remaining data if the guest underflows TC */
+if (fifo8_num_used(&s->fifo) < 2) {
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+esp_lower_drq(s);
+}
+break;
}
break;
@@ -884,7 +905,9 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
esp_set_phase(s, STAT_ST);
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
-esp_lower_drq(s);
+
+/* Ensure DRQ is set correctly for TC underflow or normal completion */
+esp_dma_ti_check(s);
if (s->current_req) {
scsi_req_unref(s->current_req);
--
2.39.2
[PULL 76/88] esp.c: only transfer non-DMA COMMAND phase data for specific commands
The contents of the FIFO should only be copied to cmdfifo for ESP commands that
are sending data to the SCSI bus, which are the SEL_* commands and the TI
command. Otherwise any incoming data should be held in the FIFO as normal.
This fixes booting of really old 32-bit SPARC Linux kernels such as Aurelien's
debian_etch_sparc_small.qcow2 test image.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-77-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index ca26415d5f..17e2db442c 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -757,13 +757,13 @@ static void esp_do_nodma(ESPState *s)
break;
case STAT_CD:
-/* Copy FIFO into cmdfifo */
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
-
switch (s->rregs[ESP_CMD]) {
case CMD_TI:
+/* Copy FIFO into cmdfifo */
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
cmdlen = fifo8_num_used(&s->cmdfifo);
trace_esp_handle_ti_cmd(cmdlen);
@@ -788,6 +788,11 @@ static void esp_do_nodma(ESPState *s)
case CMD_SEL | CMD_DMA:
case CMD_SELATN | CMD_DMA:
+/* Copy FIFO into cmdfifo */
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
/* Handle when DMA transfer is terminated by non-DMA FIFO write */
if (esp_cdb_length(s) && esp_cdb_length(s) ==
fifo8_num_used(&s->cmdfifo) - s->cmdfifo_cdb_offset) {
@@ -798,7 +803,11 @@ static void esp_do_nodma(ESPState *s)
case CMD_SEL:
case CMD_SELATN:
-/* FIFO already contain entire CDB */
+/* FIFO already contain entire CDB: copy to cmdfifo and execute */
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
do_cmd(s);
break;
}
--
2.39.2
[PULL 74/88] esp.c: handle non-DMA FIFO writes used to terminate DMA commands
Certain versions of MacOS send the first 5 bytes of the CDB using DMA and then
send the last byte of the CDB by writing to the FIFO. Update the non-DMA state
machine to detect the end of the CDB and execute the SCSI command using similar
logic as that which already exists for transferring the remainder of the CDB
using the ESP TI command.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-75-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f9d848171f..aa7dec71e3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -787,6 +787,16 @@ static void esp_do_nodma(ESPState *s)
}
break;
+case CMD_SEL | CMD_DMA:
+case CMD_SELATN | CMD_DMA:
+/* Handle when DMA transfer is terminated by non-DMA FIFO write */
+if (esp_cdb_length(s) && esp_cdb_length(s) ==
+fifo8_num_used(&s->cmdfifo) - s->cmdfifo_cdb_offset) {
+/* Command has been received */
+do_cmd(s);
+}
+break;
+
case CMD_SEL:
case CMD_SELATN:
/* FIFO already contain entire CDB */
--
2.39.2
[PULL 61/88] esp.c: remove DATA IN phase logic when reading from FIFO
Whilst the FIFO is used a storage buffer for both DMA and non-DMA requests, the
loading and unloading is managed directly issuing commands to the ESP. As a
result there is no need to manually invoke the non-DMA command handler.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-62-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index d71465718c..4c1ca63a57 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -1067,17 +1067,6 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
qemu_log_mask(LOG_UNIMP, "esp: PIO data read not implemented\n");
s->rregs[ESP_FIFO] = 0;
} else {
-if (esp_get_phase(s) == STAT_DI) {
-if (s->ti_size) {
-esp_do_nodma(s);
-} else {
-/*
- * The last byte of a non-DMA transfer has been read out
- * of the FIFO so switch to status phase
- */
-esp_set_phase(s, STAT_ST);
-}
-}
s->rregs[ESP_FIFO] = esp_fifo_pop(&s->fifo);
}
val = s->rregs[ESP_FIFO];
--
2.39.2
[PULL 59/88] esp.c: separate logic based upon ESP command in esp_transfer_data()
The handling of the INTR_FC and INTR_BS bits is different depending upon the
last command executed by the ESP. Note that currently INTR_FC is managed
elsewhere, but that will change soon.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-60-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 32 ++--
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 75538f5859..b6cf1b43db 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -862,13 +862,33 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
s->async_buf = scsi_req_get_buf(req);
if (!to_device && !s->data_ready) {
-/*
- * Initial incoming data xfer is complete so raise command
- * completion interrupt
- */
s->data_ready = true;
-s->rregs[ESP_RINTR] |= INTR_BS;
-esp_raise_irq(s);
+
+switch (s->rregs[ESP_CMD]) {
+case CMD_SEL | CMD_DMA:
+case CMD_SEL:
+case CMD_SELATN | CMD_DMA:
+case CMD_SELATN:
+case CMD_SELATNS | CMD_DMA:
+case CMD_SELATNS:
+/*
+ * Initial incoming data xfer is complete so raise command
+ * completion interrupt
+ */
+ s->rregs[ESP_RINTR] |= INTR_BS;
+ esp_raise_irq(s);
+ break;
+
+case CMD_TI | CMD_DMA:
+case CMD_TI:
+/*
+ * Bus service interrupt raised because of initial change to
+ * DATA phase
+ */
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+break;
+}
}
/*
--
2.39.2
[PULL 50/88] esp.c: move CMD_TI end of message phase detection to esp_do_dma() and do_dma_pdma_cb()
The existing check for TC == 0 is only valid during a TI command.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-51-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 32 ++--
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f8c20d0584..9f787d12a8 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -555,14 +555,16 @@ static void do_dma_pdma_cb(ESPState *s)
}
}
break;
-}
-/* ATN remains asserted until TC == 0 */
-if (esp_get_tc(s) == 0) {
-esp_set_phase(s, STAT_CD);
-s->rregs[ESP_RSEQ] = SEQ_CD;
-s->rregs[ESP_RINTR] |= INTR_BS;
-esp_raise_irq(s);
+case CMD_TI | CMD_DMA:
+/* ATN remains asserted until TC == 0 */
+if (esp_get_tc(s) == 0) {
+esp_set_phase(s, STAT_CD);
+s->rregs[ESP_RSEQ] = SEQ_CD;
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+}
+break;
}
break;
@@ -675,14 +677,16 @@ static void esp_do_dma(ESPState *s)
}
}
break;
-}
-/* ATN remains asserted until TC == 0 */
-if (esp_get_tc(s) == 0) {
-esp_set_phase(s, STAT_CD);
-s->rregs[ESP_RSEQ] = SEQ_CD;
-s->rregs[ESP_RINTR] |= INTR_BS;
-esp_raise_irq(s);
+case CMD_TI | CMD_DMA:
+/* ATN remains asserted until TC == 0 */
+if (esp_get_tc(s) == 0) {
+esp_set_phase(s, STAT_CD);
+s->rregs[ESP_RSEQ] = SEQ_CD;
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+}
+break;
}
break;
--
2.39.2
[PULL 77/88] esp.c: only transfer non-DMA MESSAGE OUT phase data for specific commands
The contents of the FIFO should only be copied to cmdfifo for ESP commands that
are sending data to the SCSI bus, which are the SEL_* commands and the TI
command. Otherwise any incoming data should be held in the FIFO as normal.
This fixes booting of NetBSD m68k under the Q800 machine once again.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-78-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 17e2db442c..d63039af89 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -712,14 +712,13 @@ static void esp_do_nodma(ESPState *s)
switch (esp_get_phase(s)) {
case STAT_MO:
-/* Copy FIFO into cmdfifo */
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
-s->cmdfifo_cdb_offset += n;
-
switch (s->rregs[ESP_CMD]) {
case CMD_SELATN:
+/* Copy FIFO into cmdfifo */
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
if (fifo8_num_used(&s->cmdfifo) >= 1) {
/* First byte received, switch to command phase */
esp_set_phase(s, STAT_CD);
@@ -734,6 +733,11 @@ static void esp_do_nodma(ESPState *s)
break;
case CMD_SELATNS:
+/* Copy one byte from FIFO into cmdfifo */
+n = esp_fifo_pop_buf(&s->fifo, buf, 1);
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
if (fifo8_num_used(&s->cmdfifo) >= 1) {
/* First byte received, stop in message out phase */
s->rregs[ESP_RSEQ] = SEQ_MO;
@@ -746,6 +750,11 @@ static void esp_do_nodma(ESPState *s)
break;
case CMD_TI:
+/* Copy FIFO into cmdfifo */
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
/* ATN remains asserted until FIFO empty */
s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
esp_set_phase(s, STAT_CD);
--
2.39.2
[PULL 55/88] esp.c: always use esp_do_dma() in pdma_cb()
There is now only a single implementation contained within esp_do_dma() so
call it directly.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-56-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index c6e5ddd537..bdbdb209f7 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -807,13 +807,7 @@ static void esp_do_nodma(ESPState *s)
static void esp_pdma_cb(ESPState *s)
{
-switch (s->pdma_cb) {
-case DO_DMA_PDMA_CB:
-esp_do_dma(s);
-break;
-default:
-g_assert_not_reached();
-}
+esp_do_dma(s);
}
void esp_command_complete(SCSIRequest *req, size_t resid)
--
2.39.2
[PULL 84/88] esp.c: implement DMA Transfer Pad command for DATA phases
The Transfer Pad command is used to either drop incoming FIFO data during the
DATA IN phase or generate a series of zero bytes in the FIFO during the DATA
OUT phase.
Implement the DMA Transfer Pad command for the DATA phases which is used by
the NeXTCube firmware in the DATA IN phase to ignore part of the incoming SCSI
data as it is copied into memory.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-85-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 97 ---
1 file changed, 69 insertions(+), 28 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 73379a3c65..5583b3eb56 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -387,6 +387,15 @@ static void handle_satn_stop(ESPState *s)
}
}
+static void handle_pad(ESPState *s)
+{
+if (s->dma) {
+esp_do_dma(s);
+} else {
+esp_do_nodma(s);
+}
+}
+
static void write_response(ESPState *s)
{
trace_esp_write_response(s->status);
@@ -518,20 +527,38 @@ static void esp_do_dma(ESPState *s)
len = s->async_len;
}
-if (s->dma_memory_read) {
-s->dma_memory_read(s->dma_opaque, s->async_buf, len);
-esp_set_tc(s, esp_get_tc(s) - len);
-} else {
-/* Copy FIFO data to device */
-len = MIN(s->async_len, ESP_FIFO_SZ);
-len = MIN(len, fifo8_num_used(&s->fifo));
-len = esp_fifo_pop_buf(&s->fifo, s->async_buf, len);
-esp_raise_drq(s);
-}
+switch (s->rregs[ESP_CMD]) {
+case CMD_TI | CMD_DMA:
+if (s->dma_memory_read) {
+s->dma_memory_read(s->dma_opaque, s->async_buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
+} else {
+/* Copy FIFO data to device */
+len = MIN(s->async_len, ESP_FIFO_SZ);
+len = MIN(len, fifo8_num_used(&s->fifo));
+len = esp_fifo_pop_buf(&s->fifo, s->async_buf, len);
+esp_raise_drq(s);
+}
-s->async_buf += len;
-s->async_len -= len;
-s->ti_size += len;
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size += len;
+break;
+
+case CMD_PAD | CMD_DMA:
+/* Copy TC zero bytes into the incoming stream */
+if (!s->dma_memory_read) {
+len = MIN(s->async_len, ESP_FIFO_SZ);
+len = MIN(len, fifo8_num_free(&s->fifo));
+}
+
+memset(s->async_buf, 0, len);
+
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size += len;
+break;
+}
if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
/* Defer until the scsi layer has completed */
@@ -554,19 +581,35 @@ static void esp_do_dma(ESPState *s)
len = s->async_len;
}
-if (s->dma_memory_write) {
-s->dma_memory_write(s->dma_opaque, s->async_buf, len);
-} else {
-/* Copy device data to FIFO */
-len = MIN(len, fifo8_num_free(&s->fifo));
-fifo8_push_all(&s->fifo, s->async_buf, len);
-esp_raise_drq(s);
-}
+switch (s->rregs[ESP_CMD]) {
+case CMD_TI | CMD_DMA:
+if (s->dma_memory_write) {
+s->dma_memory_write(s->dma_opaque, s->async_buf, len);
+} else {
+/* Copy device data to FIFO */
+len = MIN(len, fifo8_num_free(&s->fifo));
+fifo8_push_all(&s->fifo, s->async_buf, len);
+esp_raise_drq(s);
+}
+
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size -= len;
+esp_set_tc(s, esp_get_tc(s) - len);
+break;
+
+case CMD_PAD | CMD_DMA:
+/* Drop TC bytes from the incoming stream */
+if (!s->dma_memory_write) {
+len = MIN(len, fifo8_num_free(&s->fifo));
+}
-s->async_buf += len;
-s->async_len -= len;
-s->ti_size -= len;
-esp_set_tc(s, esp_get_tc(s) - len);
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size -= len;
+esp_set_tc(s, esp_get_tc(s) - len);
+break;
+}
if (s->async_len == 0 && s->ti_size == 0 && esp_get_tc(s)) {
/* If the guest underflows TC then terminate SCSI request */
@@ -1087,9 +1130,7 @@ static void esp_run_cmd(ESPState *s)
break;
case CMD_PAD:
trace_esp_mem_writeb_cmd_pad(cmd);
-s->rregs[ESP_RSTAT] = STAT_TC;
-s->rregs[ESP_RINTR] |= INTR_FC;
-s->rregs[ESP_RSEQ] = 0;
+handle_pad(s);
break;
case CMD_SATN:
trace_esp_mem_writeb_cmd_satn(cmd);
--
2.39.2
[PULL 82/88] esp.c: consolidate DMA and PDMA logic in STATUS and MESSAGE IN phases
This allows the removal of duplicate logic shared between the two
implementations.
Note that we restrict esp_raise_drq() to PDMA to help reduce the log verbosity
for normal DMA.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-83-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 8ef6d203e0..879e311bc4 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -593,12 +593,11 @@ static void esp_do_dma(ESPState *s)
if (s->dma_memory_write) {
s->dma_memory_write(s->dma_opaque, buf, len);
-esp_set_tc(s, esp_get_tc(s) - len);
} else {
fifo8_push_all(&s->fifo, buf, len);
-esp_set_tc(s, esp_get_tc(s) - len);
}
+esp_set_tc(s, esp_get_tc(s) - len);
esp_set_phase(s, STAT_MI);
if (esp_get_tc(s) > 0) {
@@ -629,12 +628,12 @@ static void esp_do_dma(ESPState *s)
if (s->dma_memory_write) {
s->dma_memory_write(s->dma_opaque, buf, len);
-esp_set_tc(s, esp_get_tc(s) - len);
} else {
fifo8_push_all(&s->fifo, buf, len);
-esp_set_tc(s, esp_get_tc(s) - len);
}
+esp_set_tc(s, esp_get_tc(s) - len);
+
/* Raise end of command interrupt */
s->rregs[ESP_RINTR] |= INTR_FC;
esp_raise_irq(s);
--
2.39.2
[PULL 71/88] esp.c: don't clear the SCSI phase when reading ESP_RINTR
According to the documentation ESP_RSTAT is cleared (except the STAT_TC bit) when ESP_RINTR is read. This should not include the SCSI bus phase bits which are currently live from the SCSI bus, otherwise the current SCSI phase is lost when clearing an end-of-transfer interrupt. Signed-off-by: Mark Cave-Ayland Tested-by: Helge Deller Tested-by: Thomas Huth Message-Id: <20240112125420.514425-72-mark.cave-ayl...@ilande.co.uk> Signed-off-by: Mark Cave-Ayland --- hw/scsi/esp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index 604fb9235d..8ea100ee9c 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -1128,7 +1128,7 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr) val = s->rregs[ESP_RINTR]; s->rregs[ESP_RINTR] = 0; esp_lower_irq(s); -s->rregs[ESP_RSTAT] &= ~STAT_TC; +s->rregs[ESP_RSTAT] &= STAT_TC | 7; /* * According to the datasheet ESP_RSEQ should be cleared, but as the * emulation currently defers information transfers to the next TI -- 2.39.2
[PULL 83/88] esp.c: replace n variable with len in esp_do_nodma()
This brings esp_do_nodma() in line with esp_do_dma().
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-84-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 879e311bc4..73379a3c65 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -675,16 +675,16 @@ static void esp_do_nodma(ESPState *s)
{
uint8_t buf[ESP_FIFO_SZ];
uint32_t cmdlen;
-int n;
+int len;
switch (esp_get_phase(s)) {
case STAT_MO:
switch (s->rregs[ESP_CMD]) {
case CMD_SELATN:
/* Copy FIFO into cmdfifo */
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
+len = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+len = MIN(fifo8_num_free(&s->cmdfifo), len);
+fifo8_push_all(&s->cmdfifo, buf, len);
if (fifo8_num_used(&s->cmdfifo) >= 1) {
/* First byte received, switch to command phase */
@@ -701,9 +701,9 @@ static void esp_do_nodma(ESPState *s)
case CMD_SELATNS:
/* Copy one byte from FIFO into cmdfifo */
-n = esp_fifo_pop_buf(&s->fifo, buf, 1);
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
+len = esp_fifo_pop_buf(&s->fifo, buf, 1);
+len = MIN(fifo8_num_free(&s->cmdfifo), len);
+fifo8_push_all(&s->cmdfifo, buf, len);
if (fifo8_num_used(&s->cmdfifo) >= 1) {
/* First byte received, stop in message out phase */
@@ -718,9 +718,9 @@ static void esp_do_nodma(ESPState *s)
case CMD_TI:
/* Copy FIFO into cmdfifo */
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
+len = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+len = MIN(fifo8_num_free(&s->cmdfifo), len);
+fifo8_push_all(&s->cmdfifo, buf, len);
/* ATN remains asserted until FIFO empty */
s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
@@ -736,9 +736,9 @@ static void esp_do_nodma(ESPState *s)
switch (s->rregs[ESP_CMD]) {
case CMD_TI:
/* Copy FIFO into cmdfifo */
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
+len = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+len = MIN(fifo8_num_free(&s->cmdfifo), len);
+fifo8_push_all(&s->cmdfifo, buf, len);
cmdlen = fifo8_num_used(&s->cmdfifo);
trace_esp_handle_ti_cmd(cmdlen);
@@ -754,7 +754,7 @@ static void esp_do_nodma(ESPState *s)
* service interrupt to indicate transfer complete. Otherwise
* defer until the next FIFO write.
*/
-if (n) {
+if (len) {
/* Raise interrupt to indicate transfer complete */
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
@@ -765,9 +765,9 @@ static void esp_do_nodma(ESPState *s)
case CMD_SEL | CMD_DMA:
case CMD_SELATN | CMD_DMA:
/* Copy FIFO into cmdfifo */
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
+len = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+len = MIN(fifo8_num_free(&s->cmdfifo), len);
+fifo8_push_all(&s->cmdfifo, buf, len);
/* Handle when DMA transfer is terminated by non-DMA FIFO write */
if (esp_cdb_length(s) && esp_cdb_length(s) ==
@@ -780,9 +780,9 @@ static void esp_do_nodma(ESPState *s)
case CMD_SEL:
case CMD_SELATN:
/* FIFO already contain entire CDB: copy to cmdfifo and execute */
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
+len = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+len = MIN(fifo8_num_free(&s->cmdfifo), len);
+fifo8_push_all(&s->cmdfifo, buf, len);
do_cmd(s);
break;
--
2.39.2
[PULL 67/88] esp.c: replace get_cmd() with esp_do_nodma()
Now that the esp_do_nodma() state machine correctly handles incoming FIFO
data, all remaining users of get_cmd() can be replaced with esp_do_nodma()
and the get_cmd() function removed completely.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-68-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 54 ---
1 file changed, 4 insertions(+), 50 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 5bb8cc4ea7..277eb8647b 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -257,40 +257,6 @@ static int esp_select(ESPState *s)
static void esp_do_dma(ESPState *s);
static void esp_do_nodma(ESPState *s);
-static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
-{
-uint8_t buf[ESP_CMDFIFO_SZ];
-uint32_t dmalen, n;
-int target;
-
-target = s->wregs[ESP_WBUSID] & BUSID_DID;
-if (s->dma) {
-dmalen = MIN(esp_get_tc(s), maxlen);
-if (dmalen == 0) {
-return 0;
-}
-if (s->dma_memory_read) {
-s->dma_memory_read(s->dma_opaque, buf, dmalen);
-dmalen = MIN(fifo8_num_free(&s->cmdfifo), dmalen);
-fifo8_push_all(&s->cmdfifo, buf, dmalen);
-esp_set_tc(s, esp_get_tc(s) - dmalen);
-} else {
-return 0;
-}
-} else {
-dmalen = MIN(fifo8_num_used(&s->fifo), maxlen);
-if (dmalen == 0) {
-return 0;
-}
-n = esp_fifo_pop_buf(&s->fifo, buf, dmalen);
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
-}
-trace_esp_get_cmd(dmalen, target);
-
-return dmalen;
-}
-
static void do_command_phase(ESPState *s)
{
uint32_t cmdlen;
@@ -376,10 +342,7 @@ static void handle_satn(ESPState *s)
if (s->dma) {
esp_do_dma(s);
} else {
-if (get_cmd(s, ESP_CMDFIFO_SZ)) {
-s->cmdfifo_cdb_offset = 1;
-do_cmd(s);
-}
+esp_do_nodma(s);
}
}
@@ -401,9 +364,7 @@ static void handle_s_without_atn(ESPState *s)
if (s->dma) {
esp_do_dma(s);
} else {
-if (get_cmd(s, ESP_CMDFIFO_SZ)) {
-do_cmd(s);
-}
+esp_do_nodma(s);
}
}
@@ -425,14 +386,7 @@ static void handle_satn_stop(ESPState *s)
if (s->dma) {
esp_do_dma(s);
} else {
-if (get_cmd(s, 1)) {
-trace_esp_handle_satn_stop(fifo8_num_used(&s->cmdfifo));
-
-/* Raise command completion interrupt */
-s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-s->rregs[ESP_RSEQ] = SEQ_MO;
-esp_raise_irq(s);
-}
+esp_do_nodma(s);
}
}
@@ -770,7 +724,7 @@ static void esp_do_nodma(ESPState *s)
break;
case CMD_SELATNS:
-if (fifo8_num_used(&s->cmdfifo) == 1) {
+if (fifo8_num_used(&s->cmdfifo) >= 1) {
/* First byte received, stop in message out phase */
s->cmdfifo_cdb_offset = 1;
--
2.39.2
[PULL 88/88] esp.c: add my copyright to the file
This series has involved rewriting and/or updating a considerable part of the ESP emulation so update the copyright in esp.c to reflect this. Signed-off-by: Mark Cave-Ayland Tested-by: Helge Deller Tested-by: Thomas Huth Message-Id: <20240112125420.514425-89-mark.cave-ayl...@ilande.co.uk> Signed-off-by: Mark Cave-Ayland --- hw/scsi/esp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index b8762d5ee0..590ff99744 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -3,6 +3,7 @@ * * Copyright (c) 2005-2006 Fabrice Bellard * Copyright (c) 2012 Herve Poussineau + * Copyright (c) 2023 Mark Cave-Ayland * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal -- 2.39.2
[PULL 57/88] esp.c: rename data_in_ready to to data_ready
This field is currently used to handle deferred interrupts for the DATA IN phase
but the code will soon be updated to do the same for the DATA OUT phase.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-58-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 8
include/hw/scsi/esp.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 5061c9d5a1..73c723afcc 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -328,7 +328,7 @@ static void do_command_phase(ESPState *s)
* Switch to DATA IN phase but wait until initial data xfer is
* complete before raising the command completion interrupt
*/
-s->data_in_ready = false;
+s->data_ready = false;
esp_set_phase(s, STAT_DI);
} else {
esp_set_phase(s, STAT_DO);
@@ -859,12 +859,12 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
s->async_len = len;
s->async_buf = scsi_req_get_buf(req);
-if (!to_device && !s->data_in_ready) {
+if (!to_device && !s->data_ready) {
/*
* Initial incoming data xfer is complete so raise command
* completion interrupt
*/
-s->data_in_ready = true;
+s->data_ready = true;
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
}
@@ -1241,7 +1241,7 @@ const VMStateDescription vmstate_esp = {
VMSTATE_UINT32_TEST(mig_cmdlen, ESPState, esp_is_before_version_5),
VMSTATE_UINT32(do_cmd, ESPState),
VMSTATE_UINT32_TEST(mig_dma_left, ESPState, esp_is_before_version_5),
-VMSTATE_BOOL_TEST(data_in_ready, ESPState, esp_is_version_5),
+VMSTATE_BOOL_TEST(data_ready, ESPState, esp_is_version_5),
VMSTATE_UINT8_TEST(cmdfifo_cdb_offset, ESPState, esp_is_version_5),
VMSTATE_FIFO8_TEST(fifo, ESPState, esp_is_version_5),
VMSTATE_FIFO8_TEST(cmdfifo, ESPState, esp_is_version_5),
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 6f942864a6..1036606943 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -40,7 +40,7 @@ struct ESPState {
uint8_t lun;
uint32_t do_cmd;
-bool data_in_ready;
+bool data_ready;
uint8_t ti_cmd;
int dma_enabled;
--
2.39.2
[PULL 51/88] esp.c: don't use get_cmd() for CMD_SEL DMA commands
This can now be done using the existing logic in esp_do_dma() and
do_dma_pdma_cb().
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-52-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 9f787d12a8..3cf8b2b4eb 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -396,8 +396,6 @@ static void handle_satn(ESPState *s)
static void handle_s_without_atn(ESPState *s)
{
-int32_t cmdlen;
-
if (s->dma && !s->dma_enabled) {
s->dma_cb = handle_s_without_atn;
return;
@@ -406,17 +404,17 @@ static void handle_s_without_atn(ESPState *s)
if (esp_select(s) < 0) {
return;
}
-cmdlen = get_cmd(s, ESP_CMDFIFO_SZ);
-if (cmdlen > 0) {
-s->cmdfifo_cdb_offset = 0;
-do_cmd(s);
-} else if (cmdlen == 0) {
-if (s->dma) {
-esp_raise_drq(s);
+
+esp_set_phase(s, STAT_CD);
+s->rregs[ESP_RSEQ] = SEQ_CD;
+s->cmdfifo_cdb_offset = 0;
+
+if (s->dma) {
+esp_do_dma(s);
+} else {
+if (get_cmd(s, ESP_CMDFIFO_SZ)) {
+do_cmd(s);
}
-/* Target present, but no cmd yet - switch to command phase */
-s->rregs[ESP_RSEQ] = SEQ_CD;
-esp_set_phase(s, STAT_CD);
}
}
--
2.39.2
[PULL 63/88] esp.c: remove unneeded ti_cmd field
According to the datasheet the previous ESP command remains in the ESP_CMD
register, which caused a problem when consecutive TI commands were issued as
it becomes impossible for the state machine to know when the first TI
command finishes.
This was the original reason for introducing the ti_cmd field which kept
track of the last written command for this purpose. However closer reading
of the datasheet shows that a TI command that terminates due to a change of
SCSI target phase resets the ESP_CMD register to zero which solves this
problem.
Now that this has been fixed in the previous commit, remove the unneeded
ti_cmd field and access the ESP_CMD register directly instead. Bump the
vmstate_esp version to indicate that the ti_cmd field is no longer included.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-64-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 19 +--
include/hw/scsi/esp.h | 3 ++-
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index ccb8ad4bae..bcebd00831 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -321,7 +321,6 @@ static void do_command_phase(ESPState *s)
fifo8_reset(&s->cmdfifo);
s->data_ready = false;
if (datalen != 0) {
-s->ti_cmd = 0;
/*
* Switch to DATA phase but wait until initial data xfer is
* complete before raising the command completion interrupt
@@ -908,12 +907,12 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
* async data transfer is delayed then s->dma is set incorrectly.
*/
-if (s->ti_cmd == (CMD_TI | CMD_DMA)) {
+if (s->rregs[ESP_CMD] == (CMD_TI | CMD_DMA)) {
/* When the SCSI layer returns more data, raise deferred INTR_BS */
esp_dma_ti_check(s);
esp_do_dma(s);
-} else if (s->ti_cmd == CMD_TI) {
+} else if (s->rregs[ESP_CMD] == CMD_TI) {
esp_do_nodma(s);
}
}
@@ -927,7 +926,6 @@ static void handle_ti(ESPState *s)
return;
}
-s->ti_cmd = s->rregs[ESP_CMD];
if (s->dma) {
dmalen = esp_get_tc(s);
trace_esp_handle_ti(dmalen);
@@ -1200,6 +1198,14 @@ static bool esp_is_version_6(void *opaque, int
version_id)
return version_id >= 6;
}
+static bool esp_is_between_version_5_and_6(void *opaque, int version_id)
+{
+ESPState *s = ESP(opaque);
+
+version_id = MIN(version_id, s->mig_version_id);
+return version_id >= 5 && version_id <= 6;
+}
+
int esp_pre_save(void *opaque)
{
ESPState *s = ESP(object_resolve_path_component(
@@ -1237,7 +1243,7 @@ static int esp_post_load(void *opaque, int version_id)
const VMStateDescription vmstate_esp = {
.name = "esp",
-.version_id = 6,
+.version_id = 7,
.minimum_version_id = 3,
.post_load = esp_post_load,
.fields = (const VMStateField[]) {
@@ -1265,7 +1271,8 @@ const VMStateDescription vmstate_esp = {
VMSTATE_UINT8_TEST(cmdfifo_cdb_offset, ESPState, esp_is_version_5),
VMSTATE_FIFO8_TEST(fifo, ESPState, esp_is_version_5),
VMSTATE_FIFO8_TEST(cmdfifo, ESPState, esp_is_version_5),
-VMSTATE_UINT8_TEST(ti_cmd, ESPState, esp_is_version_5),
+VMSTATE_UINT8_TEST(mig_ti_cmd, ESPState,
+ esp_is_between_version_5_and_6),
VMSTATE_UINT8_TEST(lun, ESPState, esp_is_version_6),
VMSTATE_END_OF_LIST()
},
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 1036606943..39b416f538 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -41,7 +41,6 @@ struct ESPState {
uint32_t do_cmd;
bool data_ready;
-uint8_t ti_cmd;
int dma_enabled;
uint32_t async_len;
@@ -62,6 +61,8 @@ struct ESPState {
uint8_t mig_ti_buf[ESP_FIFO_SZ];
uint8_t mig_cmdbuf[ESP_CMDFIFO_SZ];
uint32_t mig_cmdlen;
+
+uint8_t mig_ti_cmd;
};
#define TYPE_SYSBUS_ESP "sysbus-esp"
--
2.39.2
[PULL 81/88] esp.c: remove redundant n variable in PDMA COMMAND phase
This variable can be replaced by the existing len variable.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-82-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index ae65c2ef37..8ef6d203e0 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -427,7 +427,6 @@ static void esp_do_dma(ESPState *s)
{
uint32_t len, cmdlen;
uint8_t buf[ESP_CMDFIFO_SZ];
-int n;
len = esp_get_tc(s);
@@ -494,10 +493,9 @@ static void esp_do_dma(ESPState *s)
fifo8_push_all(&s->cmdfifo, buf, len);
esp_set_tc(s, esp_get_tc(s) - len);
} else {
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
-
+len = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+len = MIN(fifo8_num_free(&s->cmdfifo), len);
+fifo8_push_all(&s->cmdfifo, buf, len);
esp_raise_drq(s);
}
trace_esp_handle_ti_cmd(cmdlen);
--
2.39.2
[PULL 79/88] esp.c: consolidate DMA and PDMA logic in DATA IN phase
This allows the removal of duplicate logic shared between the two
implementations.
Note that we restrict esp_raise_drq() to PDMA to help reduce the log verbosity
for normal DMA.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-80-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 51 +--
1 file changed, 17 insertions(+), 34 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 394774c379..49fc059eaa 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -557,51 +557,34 @@ static void esp_do_dma(ESPState *s)
if (len > s->async_len) {
len = s->async_len;
}
+
if (s->dma_memory_write) {
s->dma_memory_write(s->dma_opaque, s->async_buf, len);
-
-esp_set_tc(s, esp_get_tc(s) - len);
-s->async_buf += len;
-s->async_len -= len;
-s->ti_size -= len;
-
-if (s->async_len == 0 && s->ti_size == 0 && esp_get_tc(s)) {
-/* If the guest underflows TC then terminate SCSI request */
-scsi_req_continue(s->current_req);
-return;
-}
-
-if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
-/* Defer until the scsi layer has completed */
-scsi_req_continue(s->current_req);
-return;
-}
-
-esp_dma_ti_check(s);
} else {
/* Copy device data to FIFO */
len = MIN(len, fifo8_num_free(&s->fifo));
fifo8_push_all(&s->fifo, s->async_buf, len);
-s->async_buf += len;
-s->async_len -= len;
-s->ti_size -= len;
-esp_set_tc(s, esp_get_tc(s) - len);
esp_raise_drq(s);
+}
-if (s->async_len == 0 && s->ti_size == 0 && esp_get_tc(s)) {
-/* If the guest underflows TC then terminate SCSI request */
-scsi_req_continue(s->current_req);
-return;
-}
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size -= len;
+esp_set_tc(s, esp_get_tc(s) - len);
-if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
-/* Defer until the scsi layer has completed */
-scsi_req_continue(s->current_req);
-return;
-}
+if (s->async_len == 0 && s->ti_size == 0 && esp_get_tc(s)) {
+/* If the guest underflows TC then terminate SCSI request */
+scsi_req_continue(s->current_req);
+return;
+}
-esp_dma_ti_check(s);
+if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
+/* Defer until the scsi layer has completed */
+scsi_req_continue(s->current_req);
+return;
}
+
+esp_dma_ti_check(s);
break;
case STAT_ST:
--
2.39.2
[PULL 54/88] esp.c: move CMD_ICCS command logic to esp_do_dma()
The special logic in write_response_pdma_cb() is now no longer required since
esp_do_dma() can be used as a direct replacement.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-55-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 82 ++-
include/hw/scsi/esp.h | 1 -
2 files changed, 57 insertions(+), 26 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f69b2709fc..c6e5ddd537 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -446,40 +446,23 @@ static void handle_satn_stop(ESPState *s)
}
}
-static void write_response_pdma_cb(ESPState *s)
-{
-esp_set_phase(s, STAT_ST);
-s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-s->rregs[ESP_RSEQ] = SEQ_CD;
-esp_raise_irq(s);
-}
-
static void write_response(ESPState *s)
{
uint8_t buf[2];
trace_esp_write_response(s->status);
-buf[0] = s->status;
-buf[1] = 0;
-
if (s->dma) {
-if (s->dma_memory_write) {
-s->dma_memory_write(s->dma_opaque, buf, 2);
-esp_set_phase(s, STAT_ST);
-s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-s->rregs[ESP_RSEQ] = SEQ_CD;
-} else {
-esp_set_pdma_cb(s, WRITE_RESPONSE_PDMA_CB);
-esp_raise_drq(s);
-return;
-}
+esp_do_dma(s);
} else {
+buf[0] = s->status;
+buf[1] = 0;
+
fifo8_reset(&s->fifo);
fifo8_push_all(&s->fifo, buf, 2);
s->rregs[ESP_RFLAGS] = 2;
+esp_raise_irq(s);
}
-esp_raise_irq(s);
}
static void esp_dma_ti_check(ESPState *s)
@@ -673,6 +656,58 @@ static void esp_do_dma(ESPState *s)
esp_dma_ti_check(s);
}
break;
+
+case STAT_ST:
+switch (s->rregs[ESP_CMD]) {
+case CMD_ICCS | CMD_DMA:
+len = MIN(len, 1);
+
+if (len) {
+buf[0] = s->status;
+
+if (s->dma_memory_write) {
+s->dma_memory_write(s->dma_opaque, buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
+} else {
+fifo8_push_all(&s->fifo, buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
+}
+
+esp_set_phase(s, STAT_MI);
+
+if (esp_get_tc(s) > 0) {
+/* Process any message in phase data */
+esp_do_dma(s);
+}
+}
+break;
+}
+break;
+
+case STAT_MI:
+switch (s->rregs[ESP_CMD]) {
+case CMD_ICCS | CMD_DMA:
+len = MIN(len, 1);
+
+if (len) {
+buf[0] = 0;
+
+if (s->dma_memory_write) {
+s->dma_memory_write(s->dma_opaque, buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
+} else {
+fifo8_push_all(&s->fifo, buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
+}
+
+/* Raise end of command interrupt */
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
+s->rregs[ESP_RSEQ] = SEQ_CD;
+esp_raise_irq(s);
+}
+break;
+}
+break;
}
}
@@ -773,9 +808,6 @@ static void esp_do_nodma(ESPState *s)
static void esp_pdma_cb(ESPState *s)
{
switch (s->pdma_cb) {
-case WRITE_RESPONSE_PDMA_CB:
-write_response_pdma_cb(s);
-break;
case DO_DMA_PDMA_CB:
esp_do_dma(s);
break;
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index a4b2ed115c..0207fdd7a6 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -152,7 +152,6 @@ struct SysBusESPState {
/* PDMA callbacks */
enum pdma_cb {
-WRITE_RESPONSE_PDMA_CB = 3,
DO_DMA_PDMA_CB = 4
};
--
2.39.2
[PULL 64/88] esp.c: don't raise INTR_BS interrupt in DATA IN phase until TI command issued
In the case where a SCSI command with a DATA IN phase has been issued, the host
may preload the FIFO with unaligned bytes before issuing the main DMA transfer.
When accumulating data in the FIFO don't raise the INTR_BS interrupt until the
TI command is issued, otherwise the unexpected interrupt can confuse the host.
In particular this is needed to prevent the MacOS Disk Utility from failing
when switching non-DMA transfers to use esp_do_nodma().
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-65-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index bcebd00831..dd6bf6f033 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -786,6 +786,11 @@ static void esp_do_nodma(ESPState *s)
return;
}
+/* If preloading the FIFO, defer until TI command issued */
+if (s->rregs[ESP_CMD] != CMD_TI) {
+return;
+}
+
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
break;
--
2.39.2
[PULL 70/88] esp.c: ensure that STAT_INT is cleared when reading ESP_RINTR
Both esp_raise_irq() and esp_lower_irq() check the STAT_INT bit in ESP_RSTAT to ensure that the IRQ is raised or lowered if its state changes. When reading ESP_RINTR, esp_lower_irq() was being called *after* ESP_RSTAT had been cleared meaning that STAT_INT was already clear, and so if STAT_INT was asserted beforehand then the esp_lower_irq() would have no effect. Signed-off-by: Mark Cave-Ayland Tested-by: Helge Deller Tested-by: Thomas Huth Message-Id: <20240112125420.514425-71-mark.cave-ayl...@ilande.co.uk> Signed-off-by: Mark Cave-Ayland --- hw/scsi/esp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index 6c62417985..604fb9235d 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -1127,6 +1127,7 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr) */ val = s->rregs[ESP_RINTR]; s->rregs[ESP_RINTR] = 0; +esp_lower_irq(s); s->rregs[ESP_RSTAT] &= ~STAT_TC; /* * According to the datasheet ESP_RSEQ should be cleared, but as the @@ -1137,7 +1138,6 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr) * * s->rregs[ESP_RSEQ] = SEQ_0; */ -esp_lower_irq(s); break; case ESP_TCHI: /* Return the unique id if the value has never been written */ -- 2.39.2
[PULL 69/88] esp.c: consolidate end of command sequence after ICCS command
The end of command sequences for the ICCS command are currently different between the DMA and non-DMA versions, and also different from the description in the datasheet. Update the sequence so that only INTR_FC is asserted in both cases, and keep all the logic in esp_do_dma() and esp_do_nodma() rather than having some of it within esp_run_cmd(). Signed-off-by: Mark Cave-Ayland Tested-by: Helge Deller Tested-by: Thomas Huth Message-Id: <20240112125420.514425-70-mark.cave-ayl...@ilande.co.uk> Signed-off-by: Mark Cave-Ayland --- hw/scsi/esp.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index 824ebe9ff0..6c62417985 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -650,8 +650,7 @@ static void esp_do_dma(ESPState *s) } /* Raise end of command interrupt */ -s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC; -s->rregs[ESP_RSEQ] = SEQ_CD; +s->rregs[ESP_RINTR] |= INTR_FC; esp_raise_irq(s); } break; @@ -825,6 +824,8 @@ static void esp_do_nodma(ESPState *s) case CMD_ICCS: fifo8_push(&s->fifo, 0); +/* Raise end of command interrupt */ +s->rregs[ESP_RINTR] |= INTR_FC; esp_raise_irq(s); break; } @@ -1056,8 +1057,6 @@ static void esp_run_cmd(ESPState *s) case CMD_ICCS: trace_esp_mem_writeb_cmd_iccs(cmd); write_response(s); -s->rregs[ESP_RINTR] |= INTR_FC; -esp_set_phase(s, STAT_MI); break; case CMD_MSGACC: trace_esp_mem_writeb_cmd_msgacc(cmd); -- 2.39.2
[PULL 78/88] esp.c: consolidate DMA and PDMA logic in DATA OUT phase
This allows the removal of duplicate logic shared between the two
implementations.
Note that we restrict esp_raise_drq() to PDMA to help reduce the log verbosity
for normal DMA.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-79-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 35 ---
1 file changed, 12 insertions(+), 23 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index d63039af89..394774c379 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -521,40 +521,29 @@ static void esp_do_dma(ESPState *s)
if (len > s->async_len) {
len = s->async_len;
}
+
if (s->dma_memory_read) {
s->dma_memory_read(s->dma_opaque, s->async_buf, len);
-
esp_set_tc(s, esp_get_tc(s) - len);
-s->async_buf += len;
-s->async_len -= len;
-s->ti_size += len;
-
-if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
-/* Defer until the scsi layer has completed */
-scsi_req_continue(s->current_req);
-return;
-}
-
-esp_dma_ti_check(s);
} else {
/* Copy FIFO data to device */
len = MIN(s->async_len, ESP_FIFO_SZ);
len = MIN(len, fifo8_num_used(&s->fifo));
-n = esp_fifo_pop_buf(&s->fifo, s->async_buf, len);
-s->async_buf += n;
-s->async_len -= n;
-s->ti_size += n;
-
+len = esp_fifo_pop_buf(&s->fifo, s->async_buf, len);
esp_raise_drq(s);
+}
-if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
-/* Defer until the scsi layer has completed */
-scsi_req_continue(s->current_req);
-return;
-}
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size += len;
-esp_dma_ti_check(s);
+if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
+/* Defer until the scsi layer has completed */
+scsi_req_continue(s->current_req);
+return;
}
+
+esp_dma_ti_check(s);
break;
case STAT_DI:
--
2.39.2
Re: [PATCH v3 22/33] linux-user: Split out mmap_h_lt_g
On 1/29/24 05:26, Ilya Leoshkevich wrote:
On Tue, Jan 02, 2024 at 12:57:57PM +1100, Richard Henderson wrote:
Work much harder to get alignment and mapping beyond the end
of the file correct. Both of which are excercised by our
test-mmap for alpha (8k pages) on any 4k page host.
Signed-off-by: Richard Henderson
---
linux-user/mmap.c | 156 +-
1 file changed, 125 insertions(+), 31 deletions(-)
[...]
+if (fileend_adj) {
+void *t = mmap(p, len - fileend_adj, host_prot,
+ (flags & ~MAP_FIXED_NOREPLACE) | MAP_FIXED,
+ fd, offset);
+assert(t != MAP_FAILED);
Is it possible to recover here? Of course, we are remapping the memory
we've mapped a few lines earlier, but asserting the syscall result
looks a bit odd.
This first one we cannot recover from, because we've already (potentially) overwritten the
previous memory mapping.
+if (!(flags & MAP_ANONYMOUS)) {
+void *t = mmap(p, len - fileend_adj, host_prot,
+ flags | MAP_FIXED, fd, offset);
+assert(t != MAP_FAILED);
Same here.
This one we could, because the memory was previously unmapped.
r~
Re: [PATCH v3 22/33] linux-user: Split out mmap_h_lt_g
On 1/29/24 05:26, Ilya Leoshkevich wrote:
On Tue, Jan 02, 2024 at 12:57:57PM +1100, Richard Henderson wrote:
Work much harder to get alignment and mapping beyond the end
of the file correct. Both of which are excercised by our
test-mmap for alpha (8k pages) on any 4k page host.
Signed-off-by: Richard Henderson
---
linux-user/mmap.c | 156 +-
1 file changed, 125 insertions(+), 31 deletions(-)
[...]
+if (fileend_adj) {
+void *t = mmap(p, len - fileend_adj, host_prot,
+ (flags & ~MAP_FIXED_NOREPLACE) | MAP_FIXED,
+ fd, offset);
+assert(t != MAP_FAILED);
Is it possible to recover here? Of course, we are remapping the memory
we've mapped a few lines earlier, but asserting the syscall result
looks a bit odd.
Can you think of a failure mode? I couldn't.
That's why I added the assert.
I suppose there's the always present threat of running out of vmas...
r~
Re: [PATCH v3 21/33] linux-user: Split out mmap_h_eq_g
On 1/29/24 05:12, Ilya Leoshkevich wrote:
On Tue, Jan 02, 2024 at 12:57:56PM +1100, Richard Henderson wrote:
Move the MAX_FIXED_NOREPLACE check for reserved_va earlier.
Move the computation of host_prot earlier.
Signed-off-by: Richard Henderson
---
linux-user/mmap.c | 66 +--
1 file changed, 53 insertions(+), 13 deletions(-)
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 42eb3eb2b4..3b8329 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -527,6 +527,31 @@ static abi_long mmap_end(abi_ulong start, abi_ulong last,
return start;
}
+/*
+ * Special case host page size == target page size,
+ * where there are no edge conditions.
+ */
+static abi_long mmap_h_eq_g(abi_ulong start, abi_ulong len,
+int host_prot, int flags, int page_flags,
+int fd, off_t offset)
+{
+void *p, *want_p = g2h_untagged(start);
+abi_ulong last;
+
+p = mmap(want_p, len, host_prot, flags, fd, offset);
+if (p == MAP_FAILED) {
+return -1;
+}
+if ((flags & MAP_FIXED_NOREPLACE) && p != want_p) {
Should we munmap() here?
I've seen this situation in some of the previous patches as well, but
there we were about to exit, and here the program may continue
running.
Yes, when the host kernel does not support MAP_FIXED_NOREPLACE.
Which is rare these days, admittedly. I'll comment as well.
r~
[PULL 49/88] esp.c: move CMD_SELATN end of message phase detection to esp_do_dma() and do_dma_pdma_cb()
The special logic in satn_pdma_cb() is now no longer required since esp_do_dma()
can be used as a direct replacement.
Signed-off-by: Mark Cave-Ayland
Tested-by: Helge Deller
Tested-by: Thomas Huth
Message-Id: <20240112125420.514425-50-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 75 +--
include/hw/scsi/esp.h | 1 -
2 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 81144ace83..f8c20d0584 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -261,6 +261,9 @@ static int esp_select(ESPState *s)
return 0;
}
+static void esp_do_dma(ESPState *s);
+static void esp_do_nodma(ESPState *s);
+
static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
{
uint8_t buf[ESP_CMDFIFO_SZ];
@@ -368,45 +371,26 @@ static void do_cmd(ESPState *s)
do_command_phase(s);
}
-static void satn_pdma_cb(ESPState *s)
-{
-uint8_t buf[ESP_FIFO_SZ];
-int n;
-
-/* Copy FIFO into cmdfifo */
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
-
-if (!esp_get_tc(s) && !fifo8_is_empty(&s->cmdfifo)) {
-s->cmdfifo_cdb_offset = 1;
-do_cmd(s);
-}
-}
-
static void handle_satn(ESPState *s)
{
-int32_t cmdlen;
-
if (s->dma && !s->dma_enabled) {
s->dma_cb = handle_satn;
return;
}
-esp_set_pdma_cb(s, SATN_PDMA_CB);
+esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
if (esp_select(s) < 0) {
return;
}
-cmdlen = get_cmd(s, ESP_CMDFIFO_SZ);
-if (cmdlen > 0) {
-s->cmdfifo_cdb_offset = 1;
-do_cmd(s);
-} else if (cmdlen == 0) {
-if (s->dma) {
-esp_raise_drq(s);
+
+esp_set_phase(s, STAT_MO);
+
+if (s->dma) {
+esp_do_dma(s);
+} else {
+if (get_cmd(s, ESP_CMDFIFO_SZ)) {
+s->cmdfifo_cdb_offset = 1;
+do_cmd(s);
}
-/* Target present, but no cmd yet - switch to command phase */
-s->rregs[ESP_RSEQ] = SEQ_CD;
-esp_set_phase(s, STAT_CD);
}
}
@@ -558,6 +542,21 @@ static void do_dma_pdma_cb(ESPState *s)
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
+switch (s->rregs[ESP_CMD]) {
+case CMD_SELATN | CMD_DMA:
+if (fifo8_num_used(&s->cmdfifo) >= 1) {
+/* First byte received, switch to command phase */
+esp_set_phase(s, STAT_CD);
+s->cmdfifo_cdb_offset = 1;
+
+if (fifo8_num_used(&s->cmdfifo) > 1) {
+/* Process any additional command phase data */
+esp_do_dma(s);
+}
+}
+break;
+}
+
/* ATN remains asserted until TC == 0 */
if (esp_get_tc(s) == 0) {
esp_set_phase(s, STAT_CD);
@@ -663,6 +662,21 @@ static void esp_do_dma(ESPState *s)
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
+switch (s->rregs[ESP_CMD]) {
+case CMD_SELATN | CMD_DMA:
+if (fifo8_num_used(&s->cmdfifo) >= 1) {
+/* First byte received, switch to command phase */
+esp_set_phase(s, STAT_CD);
+s->cmdfifo_cdb_offset = 1;
+
+if (fifo8_num_used(&s->cmdfifo) > 1) {
+/* Process any additional command phase data */
+esp_do_dma(s);
+}
+}
+break;
+}
+
/* ATN remains asserted until TC == 0 */
if (esp_get_tc(s) == 0) {
esp_set_phase(s, STAT_CD);
@@ -890,9 +904,6 @@ static void esp_do_nodma(ESPState *s)
static void esp_pdma_cb(ESPState *s)
{
switch (s->pdma_cb) {
-case SATN_PDMA_CB:
-satn_pdma_cb(s);
-break;
case SATN_STOP_PDMA_CB:
satn_stop_pdma_cb(s);
break;
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index b727181da9..9945645837 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -152,7 +152,6 @@ struct SysBusESPState {
/* PDMA callbacks */
enum pdma_cb {
-SATN_PDMA_CB = 0,
SATN_STOP_PDMA_CB = 2,
WRITE_RESPONSE_PDMA_CB = 3,
DO_DMA_PDMA_CB = 4
--
2.39.2
