From c0d0b716ba3e52236088eb9f75ef5cbd7e15a4f3 Mon Sep 17 00:00:00 2001 From: Daniel Hoffman Date: Sat, 18 Nov 2023 15:11:29 -0800 Subject: hw/timer/hpet: Convert DPRINTF to trace events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This conversion is pretty straight-forward. Standardized some formatting so the +0 and +4 offset cases can recycle the same message. Signed-off-by: Daniel Hoffman Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20231118231129.2840388-1-dhoff749@gmail.com> [PMD: Fixed few string formats] Signed-off-by: Philippe Mathieu-Daudé --- hw/timer/hpet.c | 55 +++++++++++++++++++++------------------------------ hw/timer/trace-events | 15 ++++++++++++++ 2 files changed, 37 insertions(+), 33 deletions(-) (limited to 'hw') diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c index f2f1580..1672faa 100644 --- a/hw/timer/hpet.c +++ b/hw/timer/hpet.c @@ -39,13 +39,7 @@ #include "hw/timer/i8254.h" #include "exec/address-spaces.h" #include "qom/object.h" - -//#define HPET_DEBUG -#ifdef HPET_DEBUG -#define DPRINTF printf -#else -#define DPRINTF(...) -#endif +#include "trace.h" #define HPET_MSI_SUPPORT 0 @@ -431,7 +425,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr, HPETState *s = opaque; uint64_t cur_tick, index; - DPRINTF("qemu: Enter hpet_ram_readl at %" PRIx64 "\n", addr); + trace_hpet_ram_read(addr); index = addr; /*address range of all TN regs*/ if (index >= 0x100 && index <= 0x3ff) { @@ -439,7 +433,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr, HPETTimer *timer = &s->timer[timer_id]; if (timer_id > s->num_timers) { - DPRINTF("qemu: timer id out of range\n"); + trace_hpet_timer_id_out_of_range(timer_id); return 0; } @@ -457,7 +451,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr, case HPET_TN_ROUTE + 4: return timer->fsb >> 32; default: - DPRINTF("qemu: invalid hpet_ram_readl\n"); + trace_hpet_ram_read_invalid(); break; } } else { @@ -469,7 +463,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr, case HPET_CFG: return s->config; case HPET_CFG + 4: - DPRINTF("qemu: invalid HPET_CFG + 4 hpet_ram_readl\n"); + trace_hpet_invalid_hpet_cfg(4); return 0; case HPET_COUNTER: if (hpet_enabled(s)) { @@ -477,7 +471,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr, } else { cur_tick = s->hpet_counter; } - DPRINTF("qemu: reading counter = %" PRIx64 "\n", cur_tick); + trace_hpet_ram_read_reading_counter(0, cur_tick); return cur_tick; case HPET_COUNTER + 4: if (hpet_enabled(s)) { @@ -485,12 +479,12 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr, } else { cur_tick = s->hpet_counter; } - DPRINTF("qemu: reading counter + 4 = %" PRIx64 "\n", cur_tick); + trace_hpet_ram_read_reading_counter(4, cur_tick); return cur_tick >> 32; case HPET_STATUS: return s->isr; default: - DPRINTF("qemu: invalid hpet_ram_readl\n"); + trace_hpet_ram_read_invalid(); break; } } @@ -504,8 +498,7 @@ static void hpet_ram_write(void *opaque, hwaddr addr, HPETState *s = opaque; uint64_t old_val, new_val, val, index; - DPRINTF("qemu: Enter hpet_ram_writel at %" PRIx64 " = 0x%" PRIx64 "\n", - addr, value); + trace_hpet_ram_write(addr, value); index = addr; old_val = hpet_ram_read(opaque, addr, 4); new_val = value; @@ -515,14 +508,14 @@ static void hpet_ram_write(void *opaque, hwaddr addr, uint8_t timer_id = (addr - 0x100) / 0x20; HPETTimer *timer = &s->timer[timer_id]; - DPRINTF("qemu: hpet_ram_writel timer_id = 0x%x\n", timer_id); + trace_hpet_ram_write_timer_id(timer_id); if (timer_id > s->num_timers) { - DPRINTF("qemu: timer id out of range\n"); + trace_hpet_timer_id_out_of_range(timer_id); return; } switch ((addr - 0x100) % 0x20) { case HPET_TN_CFG: - DPRINTF("qemu: hpet_ram_writel HPET_TN_CFG\n"); + trace_hpet_ram_write_tn_cfg(); if (activating_bit(old_val, new_val, HPET_TN_FSB_ENABLE)) { update_irq(timer, 0); } @@ -540,10 +533,10 @@ static void hpet_ram_write(void *opaque, hwaddr addr, } break; case HPET_TN_CFG + 4: // Interrupt capabilities - DPRINTF("qemu: invalid HPET_TN_CFG+4 write\n"); + trace_hpet_ram_write_invalid_tn_cfg(4); break; case HPET_TN_CMP: // comparator register - DPRINTF("qemu: hpet_ram_writel HPET_TN_CMP\n"); + trace_hpet_ram_write_tn_cmp(0); if (timer->config & HPET_TN_32BIT) { new_val = (uint32_t)new_val; } @@ -566,7 +559,7 @@ static void hpet_ram_write(void *opaque, hwaddr addr, } break; case HPET_TN_CMP + 4: // comparator register high order - DPRINTF("qemu: hpet_ram_writel HPET_TN_CMP + 4\n"); + trace_hpet_ram_write_tn_cmp(4); if (!timer_is_periodic(timer) || (timer->config & HPET_TN_SETVAL)) { timer->cmp = (timer->cmp & 0xffffffffULL) | new_val << 32; @@ -591,7 +584,7 @@ static void hpet_ram_write(void *opaque, hwaddr addr, timer->fsb = (new_val << 32) | (timer->fsb & 0xffffffff); break; default: - DPRINTF("qemu: invalid hpet_ram_writel\n"); + trace_hpet_ram_write_invalid(); break; } return; @@ -631,7 +624,7 @@ static void hpet_ram_write(void *opaque, hwaddr addr, } break; case HPET_CFG + 4: - DPRINTF("qemu: invalid HPET_CFG+4 write\n"); + trace_hpet_invalid_hpet_cfg(4); break; case HPET_STATUS: val = new_val & s->isr; @@ -643,24 +636,20 @@ static void hpet_ram_write(void *opaque, hwaddr addr, break; case HPET_COUNTER: if (hpet_enabled(s)) { - DPRINTF("qemu: Writing counter while HPET enabled!\n"); + trace_hpet_ram_write_counter_write_while_enabled(); } s->hpet_counter = (s->hpet_counter & 0xffffffff00000000ULL) | value; - DPRINTF("qemu: HPET counter written. ctr = 0x%" PRIx64 " -> " - "%" PRIx64 "\n", value, s->hpet_counter); + trace_hpet_ram_write_counter_written(0, value, s->hpet_counter); break; case HPET_COUNTER + 4: - if (hpet_enabled(s)) { - DPRINTF("qemu: Writing counter while HPET enabled!\n"); - } + trace_hpet_ram_write_counter_write_while_enabled(); s->hpet_counter = (s->hpet_counter & 0xffffffffULL) | (((uint64_t)value) << 32); - DPRINTF("qemu: HPET counter + 4 written. ctr = 0x%" PRIx64 " -> " - "%" PRIx64 "\n", value, s->hpet_counter); + trace_hpet_ram_write_counter_written(4, value, s->hpet_counter); break; default: - DPRINTF("qemu: invalid hpet_ram_writel\n"); + trace_hpet_ram_write_invalid(); break; } } diff --git a/hw/timer/trace-events b/hw/timer/trace-events index 8145e18..de769f4 100644 --- a/hw/timer/trace-events +++ b/hw/timer/trace-events @@ -99,3 +99,18 @@ sifive_pwm_write(uint64_t data, uint64_t offset) "Write 0x%" PRIx64 " at address sh_timer_start_stop(int enable, int current) "%d (%d)" sh_timer_read(uint64_t offset) "tmu012_read 0x%" PRIx64 sh_timer_write(uint64_t offset, uint64_t value) "tmu012_write 0x%" PRIx64 " 0x%08" PRIx64 + +# hpet.c +hpet_timer_id_out_of_range(uint8_t timer_id) "timer id out of range: 0x%" PRIx8 +hpet_invalid_hpet_cfg(uint8_t reg_off) "invalid HPET_CFG + %u" PRIx8 +hpet_ram_read(uint64_t addr) "enter hpet_ram_readl at 0x%" PRIx64 +hpet_ram_read_reading_counter(uint8_t reg_off, uint64_t cur_tick) "reading counter + %" PRIu8 " = 0x%" PRIx64 +hpet_ram_read_invalid(void) "invalid hpet_ram_readl" +hpet_ram_write(uint64_t addr, uint64_t value) "enter hpet_ram_writel at 0x%" PRIx64 " = 0x%" PRIx64 +hpet_ram_write_timer_id(uint64_t timer_id) "hpet_ram_writel timer_id = 0x%" PRIx64 +hpet_ram_write_tn_cfg(void) "hpet_ram_writel HPET_TN_CFG" +hpet_ram_write_invalid_tn_cfg(uint8_t reg_off) "invalid HPET_TN_CFG + %" PRIu8 " write" +hpet_ram_write_tn_cmp(uint8_t reg_off) "hpet_ram_writel HPET_TN_CMP + %" PRIu8 +hpet_ram_write_invalid(void) "invalid hpet_ram_writel" +hpet_ram_write_counter_write_while_enabled(void) "Writing counter while HPET enabled!" +hpet_ram_write_counter_written(uint8_t reg_off, uint64_t value, uint64_t counter) "HPET counter + %" PRIu8 "written. crt = 0x%" PRIx64 " -> 0x%" PRIx64 -- cgit v1.1 From 322b038c9411bae0c9f518fe1cb55934ac4e1a67 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Tue, 9 Jan 2024 09:30:52 +0100 Subject: target/sh4: Deprecate the shix machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shix machine has been designed and used at Télécom Paris from 2003 to 2010. It had been added to QEMU in 2005 and has not been maintained since. Since nobody is using the physical board anymore nor interested in maintaining the QEMU port, it is time to deprecate it. Signed-off-by: Samuel Tardieu Reviewed-by: Cédric Le Goater Reviewed-by: Yoshinori Sato Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240109083053.2581588-2-sam@rfc1149.net> Signed-off-by: Philippe Mathieu-Daudé --- hw/sh4/shix.c | 1 + 1 file changed, 1 insertion(+) (limited to 'hw') diff --git a/hw/sh4/shix.c b/hw/sh4/shix.c index aa81251..eb3150b 100644 --- a/hw/sh4/shix.c +++ b/hw/sh4/shix.c @@ -80,6 +80,7 @@ static void shix_machine_init(MachineClass *mc) mc->init = shix_init; mc->is_default = true; mc->default_cpu_type = TYPE_SH7750R_CPU; + mc->deprecation_reason = "old and unmaintained"; } DEFINE_MACHINE("shix", shix_machine_init) -- cgit v1.1 From c8cdec74e6214d52f1924f9db09cab8c2c4ad150 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Tue, 9 Jan 2024 09:30:53 +0100 Subject: hw/block: Deprecate the TC58128 block device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 16MiB flash device is only used by the deprecated shix machine. Its code it old and unmaintained, and has never been adapted to the QOM architecture. It still contains debug statements and uses global variables. It is time to deprecate it. Signed-off-by: Samuel Tardieu Reviewed-by: Cédric Le Goater Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240109083053.2581588-3-sam@rfc1149.net> Signed-off-by: Philippe Mathieu-Daudé --- hw/block/tc58128.c | 1 + 1 file changed, 1 insertion(+) (limited to 'hw') diff --git a/hw/block/tc58128.c b/hw/block/tc58128.c index d350126..6944cf5 100644 --- a/hw/block/tc58128.c +++ b/hw/block/tc58128.c @@ -202,6 +202,7 @@ static sh7750_io_device tc58128 = { int tc58128_init(struct SH7750State *s, const char *zone1, const char *zone2) { + warn_report_once("The TC58128 flash device is deprecated"); init_dev(&tc58128_devs[0], zone1); init_dev(&tc58128_devs[1], zone2); return sh7750_register_io_device(s, &tc58128); -- cgit v1.1 From ebd92d6de37eacd109cf320ca8ece7a0f5a243ae Mon Sep 17 00:00:00 2001 From: Bernhard Beschow Date: Mon, 8 Jan 2024 00:16:23 +0100 Subject: hw/i386/pc_piix: Make piix_intx_routing_notifier_xen() more device independent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a follow-up on commit 89965db43cce "hw/isa/piix3: Avoid Xen-specific variant of piix3_write_config()" which introduced piix_intx_routing_notifier_xen(). This function is implemented in board code but accesses the PCI configuration space of the PIIX ISA function to determine the PCI interrupt routes. Avoid this by reusing pci_device_route_intx_to_irq() which makes piix_intx_routing_notifier_xen() more device-agnostic. One remaining improvement would be making piix_intx_routing_notifier_xen() agnostic towards the number of PCI interrupt routes and move it to xen-hvm. This might be useful for possible Q35 Xen efforts but remains a future exercise for now. Signed-off-by: Bernhard Beschow Reviewed-by: David Woodhouse Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240107231623.5282-1-shentey@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/i386/pc_piix.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'hw') diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 042c13c..abfcfe4 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -92,13 +92,10 @@ static void piix_intx_routing_notifier_xen(PCIDevice *dev) { int i; - /* Scan for updates to PCI link routes (0x60-0x63). */ + /* Scan for updates to PCI link routes. */ for (i = 0; i < PIIX_NUM_PIRQS; i++) { - uint8_t v = dev->config_read(dev, PIIX_PIRQCA + i, 1); - if (v & 0x80) { - v = 0; - } - v &= 0xf; + const PCIINTxRoute route = pci_device_route_intx_to_irq(dev, i); + const uint8_t v = route.mode == PCI_INTX_ENABLED ? route.irq : 0; xen_set_pci_link_route(i, v); } } -- cgit v1.1 From 3b14a555fdb627ac091559ef5931c887d06590d8 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 8 Jan 2024 17:08:57 +0100 Subject: hw/pflash: refactor pflash_data_write() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the offset calculation, do it once at the start of the function and let the 'p' variable point directly to the memory location which should be updated. This makes it simpler to update other buffers than pfl->storage in an upcoming patch. No functional change. Signed-off-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240108160900.104835-2-kraxel@redhat.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/block/pflash_cfi01.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) (limited to 'hw') diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 3e2dc08..67f1c97 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -403,33 +403,35 @@ static void pflash_update(PFlashCFI01 *pfl, int offset, static inline void pflash_data_write(PFlashCFI01 *pfl, hwaddr offset, uint32_t value, int width, int be) { - uint8_t *p = pfl->storage; + uint8_t *p; trace_pflash_data_write(pfl->name, offset, width, value, pfl->counter); + p = pfl->storage + offset; + switch (width) { case 1: - p[offset] = value; + p[0] = value; break; case 2: if (be) { - p[offset] = value >> 8; - p[offset + 1] = value; + p[0] = value >> 8; + p[1] = value; } else { - p[offset] = value; - p[offset + 1] = value >> 8; + p[0] = value; + p[1] = value >> 8; } break; case 4: if (be) { - p[offset] = value >> 24; - p[offset + 1] = value >> 16; - p[offset + 2] = value >> 8; - p[offset + 3] = value; + p[0] = value >> 24; + p[1] = value >> 16; + p[2] = value >> 8; + p[3] = value; } else { - p[offset] = value; - p[offset + 1] = value >> 8; - p[offset + 2] = value >> 16; - p[offset + 3] = value >> 24; + p[0] = value; + p[1] = value >> 8; + p[2] = value >> 16; + p[3] = value >> 24; } break; } -- cgit v1.1 From 5dd58358a57048e5ceabf5c91c0544f4f56afdcd Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 8 Jan 2024 17:08:58 +0100 Subject: hw/pflash: use ldn_{be,le}_p and stn_{be,le}_p MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the helper functions we have to read/write multi-byte values in correct byte order. Suggested-by: Philippe Mathieu-Daudé Signed-off-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240108160900.104835-3-kraxel@redhat.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/block/pflash_cfi01.c | 63 +++++++------------------------------------------ 1 file changed, 8 insertions(+), 55 deletions(-) (limited to 'hw') diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 67f1c97..8434a45 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -225,34 +225,10 @@ static uint32_t pflash_data_read(PFlashCFI01 *pfl, hwaddr offset, uint32_t ret; p = pfl->storage; - switch (width) { - case 1: - ret = p[offset]; - break; - case 2: - if (be) { - ret = p[offset] << 8; - ret |= p[offset + 1]; - } else { - ret = p[offset]; - ret |= p[offset + 1] << 8; - } - break; - case 4: - if (be) { - ret = p[offset] << 24; - ret |= p[offset + 1] << 16; - ret |= p[offset + 2] << 8; - ret |= p[offset + 3]; - } else { - ret = p[offset]; - ret |= p[offset + 1] << 8; - ret |= p[offset + 2] << 16; - ret |= p[offset + 3] << 24; - } - break; - default: - abort(); + if (be) { + ret = ldn_be_p(p + offset, width); + } else { + ret = ldn_le_p(p + offset, width); } trace_pflash_data_read(pfl->name, offset, width, ret); return ret; @@ -408,34 +384,11 @@ static inline void pflash_data_write(PFlashCFI01 *pfl, hwaddr offset, trace_pflash_data_write(pfl->name, offset, width, value, pfl->counter); p = pfl->storage + offset; - switch (width) { - case 1: - p[0] = value; - break; - case 2: - if (be) { - p[0] = value >> 8; - p[1] = value; - } else { - p[0] = value; - p[1] = value >> 8; - } - break; - case 4: - if (be) { - p[0] = value >> 24; - p[1] = value >> 16; - p[2] = value >> 8; - p[3] = value; - } else { - p[0] = value; - p[1] = value >> 8; - p[2] = value >> 16; - p[3] = value >> 24; - } - break; + if (be) { + stn_be_p(p, width, value); + } else { + stn_le_p(p, width, value); } - } static void pflash_write(PFlashCFI01 *pfl, hwaddr offset, -- cgit v1.1 From 284a7ee2e290e0c9b8cd3ea6164d92386933054f Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 8 Jan 2024 17:08:59 +0100 Subject: hw/pflash: implement update buffer for block writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an update buffer where all block updates are staged. Flush or discard updates properly, so we should never see half-completed block writes in pflash storage. Drop a bunch of FIXME comments ;) Signed-off-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240108160900.104835-4-kraxel@redhat.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/block/pflash_cfi01.c | 110 ++++++++++++++++++++++++++++++++++++------------ hw/block/pflash_cfi02.c | 2 +- hw/block/trace-events | 7 ++- 3 files changed, 89 insertions(+), 30 deletions(-) (limited to 'hw') diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 8434a45..f956f8b 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -80,16 +80,39 @@ struct PFlashCFI01 { uint16_t ident3; uint8_t cfi_table[0x52]; uint64_t counter; - unsigned int writeblock_size; + uint32_t writeblock_size; MemoryRegion mem; char *name; void *storage; VMChangeStateEntry *vmstate; bool old_multiple_chip_handling; + + /* block update buffer */ + unsigned char *blk_bytes; + uint32_t blk_offset; }; static int pflash_post_load(void *opaque, int version_id); +static bool pflash_blk_write_state_needed(void *opaque) +{ + PFlashCFI01 *pfl = opaque; + + return (pfl->blk_offset != -1); +} + +static const VMStateDescription vmstate_pflash_blk_write = { + .name = "pflash_cfi01_blk_write", + .version_id = 1, + .minimum_version_id = 1, + .needed = pflash_blk_write_state_needed, + .fields = (const VMStateField[]) { + VMSTATE_VBUFFER_UINT32(blk_bytes, PFlashCFI01, 0, NULL, writeblock_size), + VMSTATE_UINT32(blk_offset, PFlashCFI01), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_pflash = { .name = "pflash_cfi01", .version_id = 1, @@ -101,6 +124,10 @@ static const VMStateDescription vmstate_pflash = { VMSTATE_UINT8(status, PFlashCFI01), VMSTATE_UINT64(counter, PFlashCFI01), VMSTATE_END_OF_LIST() + }, + .subsections = (const VMStateDescription * const []) { + &vmstate_pflash_blk_write, + NULL } }; @@ -376,13 +403,55 @@ static void pflash_update(PFlashCFI01 *pfl, int offset, } } +/* copy current flash content to block update buffer */ +static void pflash_blk_write_start(PFlashCFI01 *pfl, hwaddr offset) +{ + hwaddr mask = ~(pfl->writeblock_size - 1); + + trace_pflash_write_block_start(pfl->name, pfl->counter); + pfl->blk_offset = offset & mask; + memcpy(pfl->blk_bytes, pfl->storage + pfl->blk_offset, + pfl->writeblock_size); +} + +/* commit block update buffer changes */ +static void pflash_blk_write_flush(PFlashCFI01 *pfl) +{ + g_assert(pfl->blk_offset != -1); + trace_pflash_write_block_flush(pfl->name); + memcpy(pfl->storage + pfl->blk_offset, pfl->blk_bytes, + pfl->writeblock_size); + pflash_update(pfl, pfl->blk_offset, pfl->writeblock_size); + pfl->blk_offset = -1; +} + +/* discard block update buffer changes */ +static void pflash_blk_write_abort(PFlashCFI01 *pfl) +{ + trace_pflash_write_block_abort(pfl->name); + pfl->blk_offset = -1; +} + static inline void pflash_data_write(PFlashCFI01 *pfl, hwaddr offset, uint32_t value, int width, int be) { uint8_t *p; - trace_pflash_data_write(pfl->name, offset, width, value, pfl->counter); - p = pfl->storage + offset; + if (pfl->blk_offset != -1) { + /* block write: redirect writes to block update buffer */ + if ((offset < pfl->blk_offset) || + (offset + width > pfl->blk_offset + pfl->writeblock_size)) { + pfl->status |= 0x10; /* Programming error */ + return; + } + trace_pflash_data_write_block(pfl->name, offset, width, value, + pfl->counter); + p = pfl->blk_bytes + (offset - pfl->blk_offset); + } else { + /* write directly to storage */ + trace_pflash_data_write(pfl->name, offset, width, value); + p = pfl->storage + offset; + } if (be) { stn_be_p(p, width, value); @@ -503,9 +572,9 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset, } else { value = extract32(value, 0, pfl->bank_width * 8); } - trace_pflash_write_block(pfl->name, value); pfl->counter = value; pfl->wcycle++; + pflash_blk_write_start(pfl, offset); break; case 0x60: if (cmd == 0xd0) { @@ -536,12 +605,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset, switch (pfl->cmd) { case 0xe8: /* Block write */ /* FIXME check @offset, @width */ - if (!pfl->ro) { - /* - * FIXME writing straight to memory is *wrong*. We - * should write to a buffer, and flush it to memory - * only on confirm command (see below). - */ + if (!pfl->ro && (pfl->blk_offset != -1)) { pflash_data_write(pfl, offset, value, width, be); } else { pfl->status |= 0x10; /* Programming error */ @@ -550,18 +614,8 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset, pfl->status |= 0x80; if (!pfl->counter) { - hwaddr mask = pfl->writeblock_size - 1; - mask = ~mask; - trace_pflash_write(pfl->name, "block write finished"); pfl->wcycle++; - if (!pfl->ro) { - /* Flush the entire write buffer onto backing storage. */ - /* FIXME premature! */ - pflash_update(pfl, offset & mask, pfl->writeblock_size); - } else { - pfl->status |= 0x10; /* Programming error */ - } } pfl->counter--; @@ -573,20 +627,17 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset, case 3: /* Confirm mode */ switch (pfl->cmd) { case 0xe8: /* Block write */ - if (cmd == 0xd0) { - /* FIXME this is where we should write out the buffer */ + if ((cmd == 0xd0) && !(pfl->status & 0x10)) { + pflash_blk_write_flush(pfl); pfl->wcycle = 0; pfl->status |= 0x80; } else { - qemu_log_mask(LOG_UNIMP, - "%s: Aborting write to buffer not implemented," - " the data is already written to storage!\n" - "Flash device reset into READ mode.\n", - __func__); + pflash_blk_write_abort(pfl); goto mode_read_array; } break; default: + pflash_blk_write_abort(pfl); goto error_flash; } break; @@ -820,6 +871,9 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp) pfl->cmd = 0x00; pfl->status = 0x80; /* WSM ready */ pflash_cfi01_fill_cfi_table(pfl); + + pfl->blk_bytes = g_malloc(pfl->writeblock_size); + pfl->blk_offset = -1; } static void pflash_cfi01_system_reset(DeviceState *dev) @@ -839,6 +893,8 @@ static void pflash_cfi01_system_reset(DeviceState *dev) * This model deliberately ignores this delay. */ pfl->status = 0x80; + + pfl->blk_offset = -1; } static Property pflash_cfi01_properties[] = { diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c index 2a99b28..6fa56f1 100644 --- a/hw/block/pflash_cfi02.c +++ b/hw/block/pflash_cfi02.c @@ -546,7 +546,7 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value, } goto reset_flash; } - trace_pflash_data_write(pfl->name, offset, width, value, 0); + trace_pflash_data_write(pfl->name, offset, width, value); if (!pfl->ro) { p = (uint8_t *)pfl->storage + offset; if (pfl->be) { diff --git a/hw/block/trace-events b/hw/block/trace-events index bab21d3..cc9a9f2 100644 --- a/hw/block/trace-events +++ b/hw/block/trace-events @@ -12,7 +12,8 @@ fdctrl_tc_pulse(int level) "TC pulse: %u" pflash_chip_erase_invalid(const char *name, uint64_t offset) "%s: chip erase: invalid address 0x%" PRIx64 pflash_chip_erase_start(const char *name) "%s: start chip erase" pflash_data_read(const char *name, uint64_t offset, unsigned size, uint32_t value) "%s: data offset:0x%04"PRIx64" size:%u value:0x%04x" -pflash_data_write(const char *name, uint64_t offset, unsigned size, uint32_t value, uint64_t counter) "%s: data offset:0x%04"PRIx64" size:%u value:0x%04x counter:0x%016"PRIx64 +pflash_data_write(const char *name, uint64_t offset, unsigned size, uint32_t value) "%s: data offset:0x%04"PRIx64" size:%u value:0x%04x" +pflash_data_write_block(const char *name, uint64_t offset, unsigned size, uint32_t value, uint64_t counter) "%s: data offset:0x%04"PRIx64" size:%u value:0x%04x counter:0x%016"PRIx64 pflash_device_id(const char *name, uint16_t id) "%s: read device ID: 0x%04x" pflash_device_info(const char *name, uint64_t offset) "%s: read device information offset:0x%04" PRIx64 pflash_erase_complete(const char *name) "%s: sector erase complete" @@ -32,7 +33,9 @@ pflash_unlock0_failed(const char *name, uint64_t offset, uint8_t cmd, uint16_t a pflash_unlock1_failed(const char *name, uint64_t offset, uint8_t cmd) "%s: unlock0 failed 0x%" PRIx64 " 0x%02x" pflash_unsupported_device_configuration(const char *name, uint8_t width, uint8_t max) "%s: unsupported device configuration: device_width:%d max_device_width:%d" pflash_write(const char *name, const char *str) "%s: %s" -pflash_write_block(const char *name, uint32_t value) "%s: block write: bytes:0x%x" +pflash_write_block_start(const char *name, uint32_t value) "%s: block write start: bytes:0x%x" +pflash_write_block_flush(const char *name) "%s: block write flush" +pflash_write_block_abort(const char *name) "%s: block write abort" pflash_write_block_erase(const char *name, uint64_t offset, uint64_t len) "%s: block erase offset:0x%" PRIx64 " bytes:0x%" PRIx64 pflash_write_failed(const char *name, uint64_t offset, uint8_t cmd) "%s: command failed 0x%" PRIx64 " 0x%02x" pflash_write_invalid(const char *name, uint8_t cmd) "%s: invalid write for command 0x%02x" -- cgit v1.1 From 3c756f489af07dc913d0ce247de3ca6d2b563027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 10 Jan 2024 09:45:57 +0100 Subject: hw/core/cpu: Rename cpu_class_init() to include 'common' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cpu_class_init() is common, so rename it as cpu_common_class_init() to ease navigating the code. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Daniel Henrique Barboza Message-ID: <20240111120221.35072-3-philmd@linaro.org> --- hw/core/cpu-common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c index 3ccfe88..67db077 100644 --- a/hw/core/cpu-common.c +++ b/hw/core/cpu-common.c @@ -273,7 +273,7 @@ static int64_t cpu_common_get_arch_id(CPUState *cpu) return cpu->cpu_index; } -static void cpu_class_init(ObjectClass *klass, void *data) +static void cpu_common_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); ResettableClass *rc = RESETTABLE_CLASS(klass); @@ -304,7 +304,7 @@ static const TypeInfo cpu_type_info = { .instance_finalize = cpu_common_finalize, .abstract = true, .class_size = sizeof(CPUClass), - .class_init = cpu_class_init, + .class_init = cpu_common_class_init, }; static void cpu_register_types(void) -- cgit v1.1 From 83f1ab12b13dfd284084ce8daa6c548caad6ef67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 10 Jan 2024 09:46:42 +0100 Subject: hw/s390x: Rename cpu_class_init() to include 'sclp' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cpu_class_init() is specific to s390x SCLP, so rename it as sclp_cpu_class_init() (as other names in this file) to ease navigating the code. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Reviewed-by: Eric Farman Message-ID: <20240111120221.35072-4-philmd@linaro.org> --- hw/s390x/sclpcpu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/s390x/sclpcpu.c b/hw/s390x/sclpcpu.c index f2b1a4b..fa79891 100644 --- a/hw/s390x/sclpcpu.c +++ b/hw/s390x/sclpcpu.c @@ -73,7 +73,7 @@ static int read_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr, return 1; } -static void cpu_class_init(ObjectClass *oc, void *data) +static void sclp_cpu_class_init(ObjectClass *oc, void *data) { SCLPEventClass *k = SCLP_EVENT_CLASS(oc); DeviceClass *dc = DEVICE_CLASS(oc); @@ -94,7 +94,7 @@ static const TypeInfo sclp_cpu_info = { .name = TYPE_SCLP_CPU_HOTPLUG, .parent = TYPE_SCLP_EVENT, .instance_size = sizeof(SCLPEvent), - .class_init = cpu_class_init, + .class_init = sclp_cpu_class_init, .class_size = sizeof(SCLPEventClass), }; -- cgit v1.1 From 84a6835e004c257037492167d4f266dbb54dc33e Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Fri, 12 Jan 2024 13:15:26 +0000 Subject: hw/scsi/esp-pci: use correct address register for PCI DMA transfers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current code in esp_pci_dma_memory_rw() sets the DMA address to the value of the DMA_SPA (Starting Physical Address) register which is incorrect: this means that for each callback from the SCSI layer the DMA address is set back to the starting address. In the case where only a single SCSI callback occurs (currently for transfer lengths < 128kB) this works fine, however for larger transfers the DMA address wraps back to the initial starting address, corrupting the buffer holding the data transferred to the guest. Fix esp_pci_dma_memory_rw() to use the DMA_WAC (Working Address Counter) for the DMA address which is correctly incremented across multiple SCSI layer transfers. Signed-off-by: Mark Cave-Ayland Reviewed-by: Guenter Roeck Tested-by: Guenter Roeck Message-ID: <20240112131529.515642-2-mark.cave-ayland@ilande.co.uk> Signed-off-by: Philippe Mathieu-Daudé --- hw/scsi/esp-pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw') diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c index 93b3429..7117725 100644 --- a/hw/scsi/esp-pci.c +++ b/hw/scsi/esp-pci.c @@ -275,7 +275,7 @@ static void esp_pci_dma_memory_rw(PCIESPState *pci, uint8_t *buf, int len, qemu_log_mask(LOG_UNIMP, "am53c974: MDL transfer not implemented\n"); } - addr = pci->dma_regs[DMA_SPA]; + addr = pci->dma_regs[DMA_WAC]; if (pci->dma_regs[DMA_WBC] < len) { len = pci->dma_regs[DMA_WBC]; } -- cgit v1.1 From 6b41417d934b2640b7ccf893544d656eea92a2e7 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Fri, 12 Jan 2024 13:15:27 +0000 Subject: hw/scsi/esp-pci: generate PCI interrupt from separate ESP and PCI sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The am53c974/dc390 PCI interrupt has two separate sources: the first is from the internal ESP device, and the second is from the PCI DMA transfer logic. Update the ESP interrupt handler so that it sets DMA_STAT_SCSIINT rather than driving the PCI IRQ directly, and introduce a new esp_pci_update_irq() function to generate the correct PCI IRQ level. In particular this fixes spurious interrupts being generated by setting DMA_STAT_DONE at the end of a transfer if DMA_CMD_INTE_D isn't set in the DMA_CMD register. Signed-off-by: Mark Cave-Ayland Reviewed-by: Guenter Roeck Tested-by: Guenter Roeck Message-ID: <20240112131529.515642-3-mark.cave-ayland@ilande.co.uk> Signed-off-by: Philippe Mathieu-Daudé --- hw/scsi/esp-pci.c | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) (limited to 'hw') diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c index 7117725..15dc3c0 100644 --- a/hw/scsi/esp-pci.c +++ b/hw/scsi/esp-pci.c @@ -77,6 +77,29 @@ struct PCIESPState { ESPState esp; }; +static void esp_pci_update_irq(PCIESPState *pci) +{ + int scsi_level = !!(pci->dma_regs[DMA_STAT] & DMA_STAT_SCSIINT); + int dma_level = (pci->dma_regs[DMA_CMD] & DMA_CMD_INTE_D) ? + !!(pci->dma_regs[DMA_STAT] & DMA_STAT_DONE) : 0; + int level = scsi_level || dma_level; + + pci_set_irq(PCI_DEVICE(pci), level); +} + +static void esp_irq_handler(void *opaque, int irq_num, int level) +{ + PCIESPState *pci = PCI_ESP(opaque); + + if (level) { + pci->dma_regs[DMA_STAT] |= DMA_STAT_SCSIINT; + } else { + pci->dma_regs[DMA_STAT] &= ~DMA_STAT_SCSIINT; + } + + esp_pci_update_irq(pci); +} + static void esp_pci_handle_idle(PCIESPState *pci, uint32_t val) { ESPState *s = &pci->esp; @@ -151,6 +174,7 @@ static void esp_pci_dma_write(PCIESPState *pci, uint32_t saddr, uint32_t val) /* clear some bits on write */ uint32_t mask = DMA_STAT_ERROR | DMA_STAT_ABORT | DMA_STAT_DONE; pci->dma_regs[DMA_STAT] &= ~(val & mask); + esp_pci_update_irq(pci); } break; default: @@ -161,17 +185,14 @@ static void esp_pci_dma_write(PCIESPState *pci, uint32_t saddr, uint32_t val) static uint32_t esp_pci_dma_read(PCIESPState *pci, uint32_t saddr) { - ESPState *s = &pci->esp; uint32_t val; val = pci->dma_regs[saddr]; if (saddr == DMA_STAT) { - if (s->rregs[ESP_RSTAT] & STAT_INT) { - val |= DMA_STAT_SCSIINT; - } if (!(pci->sbac & SBAC_STATUS)) { pci->dma_regs[DMA_STAT] &= ~(DMA_STAT_ERROR | DMA_STAT_ABORT | DMA_STAT_DONE); + esp_pci_update_irq(pci); } } @@ -350,6 +371,7 @@ static void esp_pci_command_complete(SCSIRequest *req, size_t resid) esp_command_complete(req, resid); pci->dma_regs[DMA_WBC] = 0; pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE; + esp_pci_update_irq(pci); } static const struct SCSIBusInfo esp_pci_scsi_info = { @@ -386,7 +408,7 @@ static void esp_pci_scsi_realize(PCIDevice *dev, Error **errp) "esp-io", 0x80); pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->io); - s->irq = pci_allocate_irq(dev); + s->irq = qemu_allocate_irq(esp_irq_handler, pci, 0); scsi_bus_init(&s->bus, sizeof(s->bus), d, &esp_pci_scsi_info); } -- cgit v1.1 From 1e8e6644e063b20ad391140fae13d00ad7750b33 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Fri, 12 Jan 2024 13:15:28 +0000 Subject: hw/scsi/esp-pci: synchronise setting of DMA_STAT_DONE with ESP completion interrupt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The setting of DMA_STAT_DONE at the end of a DMA transfer can be configured to generate an interrupt, however the Linux driver manually checks for DMA_STAT_DONE being set and if it is, considers that a DMA transfer has completed. If DMA_STAT_DONE is set but the ESP device isn't indicating an interrupt then the Linux driver considers this to be a spurious interrupt. However this can occur in QEMU as there is a delay between the end of DMA transfer where DMA_STAT_DONE is set, and the ESP device raising its completion interrupt. This appears to be an incorrect assumption in the Linux driver as the ESP and PCI DMA interrupt sources are separate (and may not be raised exactly together), however we can work around this by synchronising the setting of DMA_STAT_DONE at the end of a DMA transfer with the ESP completion interrupt. In conjunction with the previous commit Linux is now able to correctly boot from an am53c974 PCI SCSI device on the hppa C3700 machine without emitting "iget: checksum invalid" and "Spurious irq, sreg=10" errors. Signed-off-by: Mark Cave-Ayland Reviewed-by: Guenter Roeck Tested-by: Guenter Roeck Message-ID: <20240112131529.515642-4-mark.cave-ayland@ilande.co.uk> Signed-off-by: Philippe Mathieu-Daudé --- hw/scsi/esp-pci.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) (limited to 'hw') diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c index 15dc3c0..875a491 100644 --- a/hw/scsi/esp-pci.c +++ b/hw/scsi/esp-pci.c @@ -93,6 +93,18 @@ static void esp_irq_handler(void *opaque, int irq_num, int level) if (level) { pci->dma_regs[DMA_STAT] |= DMA_STAT_SCSIINT; + + /* + * If raising the ESP IRQ to indicate end of DMA transfer, set + * DMA_STAT_DONE at the same time. In theory this should be done in + * esp_pci_dma_memory_rw(), however there is a delay between setting + * DMA_STAT_DONE and the ESP IRQ arriving which is visible to the + * guest that can cause confusion e.g. Linux + */ + if ((pci->dma_regs[DMA_CMD] & DMA_CMD_MASK) == 0x3 && + pci->dma_regs[DMA_WBC] == 0) { + pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE; + } } else { pci->dma_regs[DMA_STAT] &= ~DMA_STAT_SCSIINT; } @@ -306,9 +318,6 @@ static void esp_pci_dma_memory_rw(PCIESPState *pci, uint8_t *buf, int len, /* update status registers */ pci->dma_regs[DMA_WBC] -= len; pci->dma_regs[DMA_WAC] += len; - if (pci->dma_regs[DMA_WBC] == 0) { - pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE; - } } static void esp_pci_dma_memory_read(void *opaque, uint8_t *buf, int len) @@ -363,24 +372,13 @@ static const VMStateDescription vmstate_esp_pci_scsi = { } }; -static void esp_pci_command_complete(SCSIRequest *req, size_t resid) -{ - ESPState *s = req->hba_private; - PCIESPState *pci = container_of(s, PCIESPState, esp); - - esp_command_complete(req, resid); - pci->dma_regs[DMA_WBC] = 0; - pci->dma_regs[DMA_STAT] |= DMA_STAT_DONE; - esp_pci_update_irq(pci); -} - static const struct SCSIBusInfo esp_pci_scsi_info = { .tcq = false, .max_target = ESP_MAX_DEVS, .max_lun = 7, .transfer_data = esp_transfer_data, - .complete = esp_pci_command_complete, + .complete = esp_command_complete, .cancel = esp_request_cancelled, }; -- cgit v1.1 From c2d7de557d19ec76eb83b87b6bf77c8114e2f183 Mon Sep 17 00:00:00 2001 From: Mark Cave-Ayland Date: Fri, 12 Jan 2024 13:15:29 +0000 Subject: hw/scsi/esp-pci: set DMA_STAT_BCMBLT when BLAST command issued MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even though the BLAST command isn't fully implemented in QEMU, the DMA_STAT_BCMBLT bit should be set after the command has been issued to indicate that the command has completed. This fixes an issue with the DC390 DOS driver which issues the BLAST command as part of its normal error recovery routine at startup, and otherwise sits in a tight loop waiting for DMA_STAT_BCMBLT to be set before continuing. Signed-off-by: Mark Cave-Ayland Reviewed-by: Guenter Roeck Tested-by: Guenter Roeck Message-ID: <20240112131529.515642-5-mark.cave-ayland@ilande.co.uk> Signed-off-by: Philippe Mathieu-Daudé --- hw/scsi/esp-pci.c | 1 + 1 file changed, 1 insertion(+) (limited to 'hw') diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c index 875a491..42d9d2e 100644 --- a/hw/scsi/esp-pci.c +++ b/hw/scsi/esp-pci.c @@ -124,6 +124,7 @@ static void esp_pci_handle_blast(PCIESPState *pci, uint32_t val) { trace_esp_pci_dma_blast(val); qemu_log_mask(LOG_UNIMP, "am53c974: cmd BLAST not implemented\n"); + pci->dma_regs[DMA_STAT] |= DMA_STAT_BCMBLT; } static void esp_pci_handle_abort(PCIESPState *pci, uint32_t val) -- cgit v1.1