[PATCH v3 3/3] migration/ram: Optimize ram_save_host_page()

2021-03-04 Thread Kunkun Jiang
Starting from pss->page, ram_save_host_page() will check every page
and send the dirty pages up to the end of the current host page or
the boundary of used_length of the block. If the host page size is
a huge page, the step "check" will take a lot of time.

This will improve performance to use migration_bitmap_find_dirty().

Signed-off-by: Keqian Zhu 
Signed-off-by: Kunkun Jiang 
---
 migration/ram.c | 39 +++
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 9fc5b2997c..28215aefe4 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1991,6 +1991,8 @@ static int ram_save_host_page(RAMState *rs, 
PageSearchStatus *pss,
 int pages = 0;
 size_t pagesize_bits =
 qemu_ram_pagesize(pss->block) >> TARGET_PAGE_BITS;
+unsigned long hostpage_boundary =
+QEMU_ALIGN_UP(pss->page + 1, pagesize_bits);
 unsigned long start_page = pss->page;
 int res;
 
@@ -2003,30 +2005,27 @@ static int ram_save_host_page(RAMState *rs, 
PageSearchStatus *pss,
 int pages_this_iteration = 0;
 
 /* Check if the page is dirty and send it if it is */
-if (!migration_bitmap_clear_dirty(rs, pss->block, pss->page)) {
-pss->page++;
-continue;
-}
-
-pages_this_iteration = ram_save_target_page(rs, pss, last_stage);
-if (pages_this_iteration < 0) {
-return pages_this_iteration;
-}
+if (migration_bitmap_clear_dirty(rs, pss->block, pss->page)) {
+pages_this_iteration = ram_save_target_page(rs, pss, last_stage);
+if (pages_this_iteration < 0) {
+return pages_this_iteration;
+}
 
-pages += pages_this_iteration;
-pss->page++;
-/*
- * Allow rate limiting to happen in the middle of huge pages if
- * something is sent in the current iteration.
- */
-if (pagesize_bits > 1 && pages_this_iteration > 0) {
-migration_rate_limit();
+pages += pages_this_iteration;
+/*
+ * Allow rate limiting to happen in the middle of huge pages if
+ * something is sent in the current iteration.
+ */
+if (pagesize_bits > 1 && pages_this_iteration > 0) {
+migration_rate_limit();
+}
 }
-} while ((pss->page & (pagesize_bits - 1)) &&
+pss->page = migration_bitmap_find_dirty(rs, pss->block, pss->page);
+} while ((pss->page < hostpage_boundary) &&
  offset_in_ramblock(pss->block,
 ((ram_addr_t)pss->page) << TARGET_PAGE_BITS));
-/* The offset we leave with is the last one we looked at */
-pss->page--;
+/* The offset we leave with is the min boundary of host page and block */
+pss->page = MIN(pss->page, hostpage_boundary) - 1;
 
 res = ram_save_release_protection(rs, pss, start_page);
 return (res < 0 ? res : pages);
-- 
2.23.0




[PATCH v3 2/3] migration/ram: Reduce unnecessary rate limiting

2021-03-04 Thread Kunkun Jiang
When the host page is a huge page and something is sent in the
current iteration, the migration_rate_limit() should be executed.
If not, this function can be omitted to save time.

Rename tmppages to pages_this_iteration to express its meaning
more clearly.

Signed-off-by: Keqian Zhu 
Signed-off-by: Kunkun Jiang 
---
 migration/ram.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index a168da5cdd..9fc5b2997c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1988,7 +1988,7 @@ static int ram_save_target_page(RAMState *rs, 
PageSearchStatus *pss,
 static int ram_save_host_page(RAMState *rs, PageSearchStatus *pss,
   bool last_stage)
 {
-int tmppages, pages = 0;
+int pages = 0;
 size_t pagesize_bits =
 qemu_ram_pagesize(pss->block) >> TARGET_PAGE_BITS;
 unsigned long start_page = pss->page;
@@ -2000,21 +2000,28 @@ static int ram_save_host_page(RAMState *rs, 
PageSearchStatus *pss,
 }
 
 do {
+int pages_this_iteration = 0;
+
 /* Check if the page is dirty and send it if it is */
 if (!migration_bitmap_clear_dirty(rs, pss->block, pss->page)) {
 pss->page++;
 continue;
 }
 
-tmppages = ram_save_target_page(rs, pss, last_stage);
-if (tmppages < 0) {
-return tmppages;
+pages_this_iteration = ram_save_target_page(rs, pss, last_stage);
+if (pages_this_iteration < 0) {
+return pages_this_iteration;
 }
 
-pages += tmppages;
+pages += pages_this_iteration;
 pss->page++;
-/* Allow rate limiting to happen in the middle of huge pages */
-migration_rate_limit();
+/*
+ * Allow rate limiting to happen in the middle of huge pages if
+ * something is sent in the current iteration.
+ */
+if (pagesize_bits > 1 && pages_this_iteration > 0) {
+migration_rate_limit();
+}
 } while ((pss->page & (pagesize_bits - 1)) &&
  offset_in_ramblock(pss->block,
 ((ram_addr_t)pss->page) << TARGET_PAGE_BITS));
-- 
2.23.0




[PATCH v3 1/3] migration/ram: Modify the code comment of ram_save_host_page()

2021-03-04 Thread Kunkun Jiang
The ram_save_host_page() has been modified several times
since its birth. But the comment hasn't been modified as it should
be. It'd better to modify the comment to explain ram_save_host_page()
more clearly.

Signed-off-by: Kunkun Jiang 
---
 migration/ram.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 72143da0ac..a168da5cdd 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1970,15 +1970,13 @@ static int ram_save_target_page(RAMState *rs, 
PageSearchStatus *pss,
 }
 
 /**
- * ram_save_host_page: save a whole host page
+ * ram_save_host_page: save a whole host page or the rest of a RAMBlock
  *
- * Starting at *offset send pages up to the end of the current host
- * page. It's valid for the initial offset to point into the middle of
- * a host page in which case the remainder of the hostpage is sent.
- * Only dirty target pages are sent. Note that the host page size may
- * be a huge page for this block.
- * The saving stops at the boundary of the used_length of the block
- * if the RAMBlock isn't a multiple of the host page size.
+ * Send dirty pages between pss->page and either the end of that page
+ * or the used_length of the RAMBlock, whichever is smaller.
+ *
+ * Note that if the host page is a huge page, pss->page may be in the
+ * middle of that page.
  *
  * Returns the number of pages written or negative on error
  *
@@ -2002,7 +2000,7 @@ static int ram_save_host_page(RAMState *rs, 
PageSearchStatus *pss,
 }
 
 do {
-/* Check the pages is dirty and if it is send it */
+/* Check if the page is dirty and send it if it is */
 if (!migration_bitmap_clear_dirty(rs, pss->block, pss->page)) {
 pss->page++;
 continue;
-- 
2.23.0




[PATCH v3 0/3] Some modifications about ram_save_host_page()

2021-03-04 Thread Kunkun Jiang
Hi,

This series include patches as below:
Patch 1:
- modified the comment ram_save_host_page() to make them match each other

Patch 2:
- reduce unnecessary rate limitting in ram_save_host_page()

Patch 3:
- optimized ram_save_host_page() by using migration_bitmap_find_dirty() to find
dirty pages

History:

v2 -> v3:
- Reduce unnecessary rate limitting if nothing is sent in the current iteration 
[David Edmondson]
- Invert the body of the loop in ram_save_host_page() [David Edmondson]

v1 -> v2:
- Modify ram_save_host_page() comment [David Edmondson]
- Remove 'goto' [David Edmondson]


Kunkun Jiang (3):
  migration/ram: Modify the code comment of ram_save_host_page()
  migration/ram: Reduce unnecessary rate limiting
  migration/ram: Optimize ram_save_host_page()

 migration/ram.c | 54 ++---
 1 file changed, 29 insertions(+), 25 deletions(-)

-- 
2.23.0




Re: [PATCH v3 00/42] esp: consolidate PDMA transfer buffers and other fixes

2021-03-04 Thread Mark Cave-Ayland

On 04/03/2021 22:58, Philippe Mathieu-Daudé wrote:


On 3/4/21 11:10 PM, Mark Cave-Ayland wrote:

This patch series comes from an experimental branch that I've been working on
to try and boot a MacOS toolbox ROM under the QEMU q800 machine. The effort is

...

v3:
- Rebase onto master (fix up minor conflicts with Paolo's SCSI error handling 
changes)
- Add R-B tags from Philippe and Laurent
- Check for failure of qdev_realize() in patch 3
- Touch up the commit messages on patches 9 and 10
- Remove extra "& 0xff" in patch 9
- Add deferred command completion interrupt for PDMA in patch 33
- Remove ti_size assignment comment in patch 38
- Remove extra "& 0xff" in patch 39


Thanks, series LGTM!

Phil.


Thanks Phil (and Laurent) for the review. Gitlab-CI is happy at 
https://gitlab.com/mcayland/qemu/-/pipelines/265668799 so if there are no further 
comments I'll send a qemu-sparc PR for this over the next couple of days.



ATB,

Mark.



Re: [PATCH v2 1/2] hw: Replace anti-social QOM type names

2021-03-04 Thread Mark Cave-Ayland

On 04/03/2021 14:02, Markus Armbruster wrote:


Several QOM type names contain ',':

 ARM,bitband-memory
 etraxfs,pic
 etraxfs,serial
 etraxfs,timer
 fsl,imx25
 fsl,imx31
 fsl,imx6
 fsl,imx6ul
 fsl,imx7
 grlib,ahbpnp
 grlib,apbpnp
 grlib,apbuart
 grlib,gptimer
 grlib,irqmp
 qemu,register
 SUNW,bpp
 SUNW,CS4231
 SUNW,DBRI
 SUNW,DBRI.prom
 SUNW,fdtwo
 SUNW,sx
 SUNW,tcx
 xilinx,zynq_slcr
 xlnx,zynqmp
 xlnx,zynqmp-pmu-soc
 xlnx,zynq-xadc

These are all device types.  They can't be plugged with -device /
device_add, except for xlnx,zynqmp-pmu-soc, and I doubt that one
actually works.

They *can* be used with -device / device_add to request help.
Usability is poor, though: you have to double the comma, like this:

 $ qemu-system-x86_64 -device SUNW,,fdtwo,help

Trap for the unwary.  The fact that this was broken in
device-introspect-test for more than six years until commit e27bd49876
fixed it demonstrates that "the unwary" includes seasoned developers.

One QOM type name contains ' ': "ICH9 SMB".  Because having to
remember just one way to quote would be too easy.

Rename the "SUNW,FOO types to "sun-FOO".  Summarily replace ',' and '
' by '-' in the other type names.

Signed-off-by: Markus Armbruster 
---
  include/hw/arm/armv7m.h  |  2 +-
  include/hw/arm/fsl-imx25.h   |  2 +-
  include/hw/arm/fsl-imx31.h   |  2 +-
  include/hw/arm/fsl-imx6.h|  2 +-
  include/hw/arm/fsl-imx6ul.h  |  2 +-
  include/hw/arm/fsl-imx7.h|  2 +-
  include/hw/arm/xlnx-zynqmp.h |  2 +-
  include/hw/cris/etraxfs.h|  2 +-
  include/hw/i386/ich9.h   |  2 +-
  include/hw/misc/grlib_ahb_apb_pnp.h  |  4 ++--
  include/hw/misc/zynq-xadc.h  |  2 +-
  include/hw/register.h|  2 +-
  include/hw/sparc/grlib.h |  6 +++---
  hw/arm/xilinx_zynq.c |  2 +-
  hw/audio/cs4231.c|  2 +-
  hw/block/fdc.c   |  4 ++--
  hw/char/etraxfs_ser.c|  2 +-
  hw/cris/axis_dev88.c |  6 +++---
  hw/display/tcx.c |  2 +-
  hw/intc/etraxfs_pic.c|  2 +-
  hw/microblaze/xlnx-zynqmp-pmu.c  |  2 +-
  hw/misc/zynq_slcr.c  |  2 +-
  hw/sparc/sun4m.c | 12 ++--
  hw/timer/etraxfs_timer.c |  2 +-
  softmmu/vl.c |  2 +-
  tests/vmstate-static-checker-data/dump1.json |  4 ++--
  tests/vmstate-static-checker-data/dump2.json |  4 ++--
  27 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
index 0791dcb68a..189b23a8ce 100644
--- a/include/hw/arm/armv7m.h
+++ b/include/hw/arm/armv7m.h
@@ -15,7 +15,7 @@
  #include "target/arm/idau.h"
  #include "qom/object.h"
  
-#define TYPE_BITBAND "ARM,bitband-memory"

+#define TYPE_BITBAND "ARM-bitband-memory"
  OBJECT_DECLARE_SIMPLE_TYPE(BitBandState, BITBAND)
  
  struct BitBandState {

diff --git a/include/hw/arm/fsl-imx25.h b/include/hw/arm/fsl-imx25.h
index c1603b2ac2..1b1086e945 100644
--- a/include/hw/arm/fsl-imx25.h
+++ b/include/hw/arm/fsl-imx25.h
@@ -34,7 +34,7 @@
  #include "target/arm/cpu.h"
  #include "qom/object.h"
  
-#define TYPE_FSL_IMX25 "fsl,imx25"

+#define TYPE_FSL_IMX25 "fsl-imx25"
  OBJECT_DECLARE_SIMPLE_TYPE(FslIMX25State, FSL_IMX25)
  
  #define FSL_IMX25_NUM_UARTS 5

diff --git a/include/hw/arm/fsl-imx31.h b/include/hw/arm/fsl-imx31.h
index b9792d58ae..c116a73e0b 100644
--- a/include/hw/arm/fsl-imx31.h
+++ b/include/hw/arm/fsl-imx31.h
@@ -30,7 +30,7 @@
  #include "target/arm/cpu.h"
  #include "qom/object.h"
  
-#define TYPE_FSL_IMX31 "fsl,imx31"

+#define TYPE_FSL_IMX31 "fsl-imx31"
  OBJECT_DECLARE_SIMPLE_TYPE(FslIMX31State, FSL_IMX31)
  
  #define FSL_IMX31_NUM_UARTS 2

diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h
index 29cc425acc..83291457cf 100644
--- a/include/hw/arm/fsl-imx6.h
+++ b/include/hw/arm/fsl-imx6.h
@@ -36,7 +36,7 @@
  #include "cpu.h"
  #include "qom/object.h"
  
-#define TYPE_FSL_IMX6 "fsl,imx6"

+#define TYPE_FSL_IMX6 "fsl-imx6"
  OBJECT_DECLARE_SIMPLE_TYPE(FslIMX6State, FSL_IMX6)
  
  #define FSL_IMX6_NUM_CPUS 4

diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
index f8ebfba4f9..7812e516a5 100644
--- a/include/hw/arm/fsl-imx6ul.h
+++ b/include/hw/arm/fsl-imx6ul.h
@@ -40,7 +40,7 @@
  #include "cpu.h"
  #include "qom/object.h"
  
-#define TYPE_FSL_IMX6UL "fsl,imx6ul"

+#define TYPE_FSL_IMX6UL "fsl-imx6ul"
  OBJECT_DECLARE_SIMPLE_TYPE(FslIMX6ULState, FSL_IMX6UL)
  
  enum FslIMX6ULConfiguration {

diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
index 

Re: [QEMU-SECURITY] [PATCH V4 00/10] Detect reentrant RX casued by loopback

2021-03-04 Thread Jason Wang



On 2021/3/5 2:39 下午, P J P wrote:

Hello all,

Just to note:

* Let's use  list to review non-public/embargoed patch(es) only.

* If patch(es) is being reviewed publicly on  list,
   CC'ing  list does not help much.


Thank you.
---
   -P J P
http://feedmug.com



I see.

Thanks





Re: [PATCH v2 1/3] fdc: Drop deprecated floppy configuration

2021-03-04 Thread Markus Armbruster
Peter Krempa  writes:

> On Thu, Mar 04, 2021 at 17:23:05 +0100, Markus Armbruster wrote:
>> Daniel P. Berrangé  writes:
>> 
>> > On Thu, Mar 04, 2021 at 03:26:55PM +0100, Markus Armbruster wrote:
>> >> Daniel P. Berrangé  writes:
>> >> 
>> >> > On Thu, Mar 04, 2021 at 11:00:57AM +0100, Markus Armbruster wrote:
>> >> >> Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
>> >> >> configuring floppies with -global isa-fdc" (v5.1.0).
>> >> >> 
>> >> >> Signed-off-by: Markus Armbruster 
>> >> >> ---
>> >> >>  docs/system/deprecated.rst   |  26 --
>> >> >>  docs/system/removed-features.rst |  26 ++
>> >> >>  hw/block/fdc.c   |  54 +--
>> >> >>  tests/qemu-iotests/172   |  31 +-
>> >> >>  tests/qemu-iotests/172.out   | 562 +--
>> >> >>  5 files changed, 30 insertions(+), 669 deletions(-)
>
> [...]
>
>> >> 
>> >> Correct.
>> >> 
>> >> This was deprecated in commit 4a27a638e7 "fdc: Deprecate configuring
>> >> floppies with -global isa-fdc" (v5.1.0).  Since then, its use triggers a
>> >> warning:
>> >> 
>> >> $ qemu-system-x86_64 -nodefaults -M q35 -display none -drive 
>> >> if=none,id=drive-fdc0-0-0 -device 
>> >> isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1
>> >> qemu-system-x86_64: -device 
>> >> isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1: warning: warning: property 
>> >> isa-fdc.driveA is deprecated
>> >> Use -device floppy,unit=0,drive=... instead.
>> >> 
>> >> Note the -M q35.  Needed because the default machine type has an onboard
>> >> isa-fdc, which cannot be configured this way.
>> >> 
>> >> Sadly, the commit's update of docs/system/deprecated.rst neglects to
>> >> cover this use.  Looks the series overtaxed my capacity to juggle
>> >> details; my apologies.
>> >> 
>> >> Is libvirt still using these properties?
>> >
>> > Unfortunately yes, but it seems like it ought to be fairly easy to
>> > change the syntax. Just need to figure out what the right way to
>> > detect the availability of the new syntax is. Presumably just look
>> > for existance of the 'floppy' device type ?
>> 
>> Yes.  The device type was added in merge commit fd209e4a7, v2.8.0.
>> 
>> > Can you confirm that switching from -global to the new -device floppy
>> > does /not/ have any live migration impact ?
>> 
>> Yes, it must not affect migration.
>> 
>> When Kevin split the floppy device type off the floppy controller, he
>> had to add some moderately ugly hackery to keep the old qdev properties
>> working.  Think propagate property values to floppy from controller,
>> which otherwise ignores them.
>> 
>> The way you get the values into the floppy device cannot affect the
>> migration data.  Only different values can.
>> 
>> This patch removes a deprecated way.
>
> Note that when QEMU_CAPS_BLOCKDEV is asserted we format floppies as:
>
> -blockdev '{"driver":"file","filename":"/tmp/firmware.img",\
> "node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \
> -blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"raw",\
> "file":"libvirt-2-storage"}' \
> -device floppy,unit=0,drive=libvirt-2-format,id=fdc0-0-0 \
> -blockdev '{"driver":"file","filename":"/tmp/data.img",\
> "node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
> -blockdev 
> '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2",\
> "file":"libvirt-1-storage"}' \
> -device floppy,unit=1,drive=libvirt-1-format,id=fdc0-0-1 \
>
> as visible in the test file:
>
> tests/qemuxml2argvdata/disk-floppy-q35-2_11.x86_64-latest.args
>
> So libvirt should be in the clear. isa-fdc with driveA/driveB is
> formatted only when the blockdev capability isn't present.

Even better: in the clear for some time already, which means a wider
range of libvirt versions keeps working with the latest QEMU.  Nice,
because keeping the coupling reasonably lose makes upgrading easier.

Thanks!




Re: [QEMU-SECURITY] [PATCH V4 00/10] Detect reentrant RX casued by loopback

2021-03-04 Thread P J P
Hello all,

Just to note:

* Let's use  list to review non-public/embargoed patch(es) only.

* If patch(es) is being reviewed publicly on  list,
  CC'ing  list does not help much.


Thank you.
---
  -P J P
http://feedmug.com



[PATCH V4 09/10] cadence_gem: switch to use qemu_receive_packet() for loopback

2021-03-04 Thread Jason Wang
From: Alexander Bulekov 

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

This is intended to address CVE-2021-3416.

Cc: Prasad J Pandit 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Alexander Bulekov 
Signed-off-by: Jason Wang 
---
 hw/net/cadence_gem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 9a4474a084..24b3a0ff66 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1275,8 +1275,8 @@ static void gem_transmit(CadenceGEMState *s)
 /* Send the packet somewhere */
 if (s->phy_loop || (s->regs[GEM_NWCTRL] &
 GEM_NWCTRL_LOCALLOOP)) {
-gem_receive(qemu_get_queue(s->nic), s->tx_packet,
-total_bytes);
+qemu_receive_packet(qemu_get_queue(s->nic), s->tx_packet,
+total_bytes);
 } else {
 qemu_send_packet(qemu_get_queue(s->nic), s->tx_packet,
  total_bytes);
-- 
2.24.3 (Apple Git-128)




[PATCH V4 07/10] rtl8139: switch to use qemu_receive_packet() for loopback

2021-03-04 Thread Jason Wang
From: Alexander Bulekov 

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

This is intended to address CVE-2021-3416.

Cc: Prasad J Pandit 
Buglink: https://bugs.launchpad.net/qemu/+bug/1910826
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 hw/net/rtl8139.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 4675ac878e..90b4fc63ce 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -1795,7 +1795,7 @@ static void rtl8139_transfer_frame(RTL8139State *s, 
uint8_t *buf, int size,
 }
 
 DPRINTF("+++ transmit loopback mode\n");
-rtl8139_do_receive(qemu_get_queue(s->nic), buf, size, do_interrupt);
+qemu_receive_packet(qemu_get_queue(s->nic), buf, size);
 
 if (iov) {
 g_free(buf2);
-- 
2.24.3 (Apple Git-128)




[PATCH V4 10/10] lan9118: switch to use qemu_receive_packet() for loopback

2021-03-04 Thread Jason Wang
From: Alexander Bulekov 

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

This is intended to address CVE-2021-3416.

Cc: Prasad J Pandit 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 hw/net/lan9118.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index abc796285a..6aff424cbe 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -680,7 +680,7 @@ static void do_tx_packet(lan9118_state *s)
 /* FIXME: Honor TX disable, and allow queueing of packets.  */
 if (s->phy_control & 0x4000)  {
 /* This assumes the receive routine doesn't touch the VLANClient.  */
-lan9118_receive(qemu_get_queue(s->nic), s->txp->data, s->txp->len);
+qemu_receive_packet(qemu_get_queue(s->nic), s->txp->data, s->txp->len);
 } else {
 qemu_send_packet(qemu_get_queue(s->nic), s->txp->data, s->txp->len);
 }
-- 
2.24.3 (Apple Git-128)




[PATCH V4 08/10] pcnet: switch to use qemu_receive_packet() for loopback

2021-03-04 Thread Jason Wang
From: Alexander Bulekov 

This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

This is intended to address CVE-2021-3416.

Cc: Prasad J Pandit 
Buglink: https://bugs.launchpad.net/qemu/+bug/1917085
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 hw/net/pcnet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index f3f18d8598..dcd3fc4948 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1250,7 +1250,7 @@ txagain:
 if (BCR_SWSTYLE(s) == 1)
 add_crc = !GET_FIELD(tmd.status, TMDS, NOFCS);
 s->looptest = add_crc ? PCNET_LOOPTEST_CRC : PCNET_LOOPTEST_NOCRC;
-pcnet_receive(qemu_get_queue(s->nic), s->buffer, s->xmit_pos);
+qemu_receive_packet(qemu_get_queue(s->nic), s->buffer, 
s->xmit_pos);
 s->looptest = 0;
 } else {
 if (s->nic) {
-- 
2.24.3 (Apple Git-128)




[PATCH V4 06/10] tx_pkt: switch to use qemu_receive_packet_iov() for loopback

2021-03-04 Thread Jason Wang
This patch switches to use qemu_receive_receive_iov() which can detect
reentrancy and return early.

This is intended to address CVE-2021-3416.

Cc: Prasad J Pandit 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 hw/net/net_tx_pkt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
index da262edc3e..1f9aa59eca 100644
--- a/hw/net/net_tx_pkt.c
+++ b/hw/net/net_tx_pkt.c
@@ -553,7 +553,7 @@ static inline void net_tx_pkt_sendv(struct NetTxPkt *pkt,
 NetClientState *nc, const struct iovec *iov, int iov_cnt)
 {
 if (pkt->is_loopback) {
-nc->info->receive_iov(nc, iov, iov_cnt);
+qemu_receive_packet_iov(nc, iov, iov_cnt);
 } else {
 qemu_sendv_packet(nc, iov, iov_cnt);
 }
-- 
2.24.3 (Apple Git-128)




[PATCH V4 03/10] dp8393x: switch to use qemu_receive_packet() for loopback packet

2021-03-04 Thread Jason Wang
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

This is intended to address CVE-2021-3416.

Cc: Prasad J Pandit 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/net/dp8393x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 205c0decc5..533a8304d0 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -506,7 +506,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
 s->regs[SONIC_TCR] |= SONIC_TCR_CRSL;
 if (nc->info->can_receive(nc)) {
 s->loopback_packet = 1;
-nc->info->receive(nc, s->tx_buffer, tx_len);
+qemu_receive_packet(nc, s->tx_buffer, tx_len);
 }
 } else {
 /* Transmit packet */
-- 
2.24.3 (Apple Git-128)




[PATCH V4 01/10] net: introduce qemu_receive_packet()

2021-03-04 Thread Jason Wang
Some NIC supports loopback mode and this is done by calling
nc->info->receive() directly which in fact suppresses the effort of
reentrancy check that is done in qemu_net_queue_send().

Unfortunately we can't use qemu_net_queue_send() here since for
loopback there's no sender as peer, so this patch introduce a
qemu_receive_packet() which is used for implementing loopback mode
for a NIC with this check.

NIC that supports loopback mode will be converted to this helper.

This is intended to address CVE-2021-3416.

Cc: Prasad J Pandit 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 include/net/net.h   |  5 +
 include/net/queue.h |  8 
 net/net.c   | 38 +++---
 net/queue.c | 22 ++
 4 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 919facaad2..4f56cae0fa 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -144,12 +144,17 @@ void *qemu_get_nic_opaque(NetClientState *nc);
 void qemu_del_net_client(NetClientState *nc);
 typedef void (*qemu_nic_foreach)(NICState *nic, void *opaque);
 void qemu_foreach_nic(qemu_nic_foreach func, void *opaque);
+int qemu_can_receive_packet(NetClientState *nc);
 int qemu_can_send_packet(NetClientState *nc);
 ssize_t qemu_sendv_packet(NetClientState *nc, const struct iovec *iov,
   int iovcnt);
 ssize_t qemu_sendv_packet_async(NetClientState *nc, const struct iovec *iov,
 int iovcnt, NetPacketSent *sent_cb);
 ssize_t qemu_send_packet(NetClientState *nc, const uint8_t *buf, int size);
+ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size);
+ssize_t qemu_receive_packet_iov(NetClientState *nc,
+const struct iovec *iov,
+int iovcnt);
 ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size);
 ssize_t qemu_send_packet_async(NetClientState *nc, const uint8_t *buf,
int size, NetPacketSent *sent_cb);
diff --git a/include/net/queue.h b/include/net/queue.h
index c0269bb1dc..9f2f289d77 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -55,6 +55,14 @@ void qemu_net_queue_append_iov(NetQueue *queue,
 
 void qemu_del_net_queue(NetQueue *queue);
 
+ssize_t qemu_net_queue_receive(NetQueue *queue,
+   const uint8_t *data,
+   size_t size);
+
+ssize_t qemu_net_queue_receive_iov(NetQueue *queue,
+   const struct iovec *iov,
+   int iovcnt);
+
 ssize_t qemu_net_queue_send(NetQueue *queue,
 NetClientState *sender,
 unsigned flags,
diff --git a/net/net.c b/net/net.c
index da4aa313be..d889487c0d 100644
--- a/net/net.c
+++ b/net/net.c
@@ -530,6 +530,17 @@ int qemu_set_vnet_be(NetClientState *nc, bool is_be)
 #endif
 }
 
+int qemu_can_receive_packet(NetClientState *nc)
+{
+if (nc->receive_disabled) {
+return 0;
+} else if (nc->info->can_receive &&
+   !nc->info->can_receive(nc)) {
+return 0;
+}
+return 1;
+}
+
 int qemu_can_send_packet(NetClientState *sender)
 {
 int vm_running = runstate_is_running();
@@ -542,13 +553,7 @@ int qemu_can_send_packet(NetClientState *sender)
 return 1;
 }
 
-if (sender->peer->receive_disabled) {
-return 0;
-} else if (sender->peer->info->can_receive &&
-   !sender->peer->info->can_receive(sender->peer)) {
-return 0;
-}
-return 1;
+return qemu_can_receive_packet(sender->peer);
 }
 
 static ssize_t filter_receive_iov(NetClientState *nc,
@@ -681,6 +686,25 @@ ssize_t qemu_send_packet(NetClientState *nc, const uint8_t 
*buf, int size)
 return qemu_send_packet_async(nc, buf, size, NULL);
 }
 
+ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size)
+{
+if (!qemu_can_receive_packet(nc)) {
+return 0;
+}
+
+return qemu_net_queue_receive(nc->incoming_queue, buf, size);
+}
+
+ssize_t qemu_receive_packet_iov(NetClientState *nc, const struct iovec *iov,
+int iovcnt)
+{
+if (!qemu_can_receive_packet(nc)) {
+return 0;
+}
+
+return qemu_net_queue_receive_iov(nc->incoming_queue, iov, iovcnt);
+}
+
 ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size)
 {
 return qemu_send_packet_async_with_flags(nc, QEMU_NET_PACKET_FLAG_RAW,
diff --git a/net/queue.c b/net/queue.c
index 19e32c80fd..c872d51df8 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -182,6 +182,28 @@ static ssize_t qemu_net_queue_deliver_iov(NetQueue *queue,
 return ret;
 }
 
+ssize_t qemu_net_queue_receive(NetQueue *queue,
+   const uint8_t *data,
+   size_t size)
+{
+if (queue->delivering) {
+return 

[PATCH V4 05/10] sungem: switch to use qemu_receive_packet() for loopback

2021-03-04 Thread Jason Wang
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

This is intended to address CVE-2021-3416.

Cc: Prasad J Pandit 
Reviewed-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 hw/net/sungem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/sungem.c b/hw/net/sungem.c
index 33c3722df6..3684a4d733 100644
--- a/hw/net/sungem.c
+++ b/hw/net/sungem.c
@@ -306,7 +306,7 @@ static void sungem_send_packet(SunGEMState *s, const 
uint8_t *buf,
 NetClientState *nc = qemu_get_queue(s->nic);
 
 if (s->macregs[MAC_XIFCFG >> 2] & MAC_XIFCFG_LBCK) {
-nc->info->receive(nc, buf, size);
+qemu_receive_packet(nc, buf, size);
 } else {
 qemu_send_packet(nc, buf, size);
 }
-- 
2.24.3 (Apple Git-128)




[PATCH V4 04/10] msf2-mac: switch to use qemu_receive_packet() for loopback

2021-03-04 Thread Jason Wang
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

This is intended to address CVE-2021-3416.

Cc: Prasad J Pandit 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 hw/net/msf2-emac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/msf2-emac.c b/hw/net/msf2-emac.c
index 32ba9e8412..3e6206044f 100644
--- a/hw/net/msf2-emac.c
+++ b/hw/net/msf2-emac.c
@@ -158,7 +158,7 @@ static void msf2_dma_tx(MSF2EmacState *s)
  * R_CFG1 bit 0 is set.
  */
 if (s->regs[R_CFG1] & R_CFG1_LB_EN_MASK) {
-nc->info->receive(nc, buf, size);
+qemu_receive_packet(nc, buf, size);
 } else {
 qemu_send_packet(nc, buf, size);
 }
-- 
2.24.3 (Apple Git-128)




[PATCH V4 02/10] e1000: switch to use qemu_receive_packet() for loopback

2021-03-04 Thread Jason Wang
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.

This is intended to address CVE-2021-3416.

Cc: Prasad J Pandit 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Jason Wang 
---
 hw/net/e1000.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 4345d863e6..4f75b44cfc 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -546,7 +546,7 @@ e1000_send_packet(E1000State *s, const uint8_t *buf, int 
size)
 
 NetClientState *nc = qemu_get_queue(s->nic);
 if (s->phy_reg[PHY_CTRL] & MII_CR_LOOPBACK) {
-nc->info->receive(nc, buf, size);
+qemu_receive_packet(nc, buf, size);
 } else {
 qemu_send_packet(nc, buf, size);
 }
-- 
2.24.3 (Apple Git-128)




[PATCH V4 00/10] Detect reentrant RX casued by loopback

2021-03-04 Thread Jason Wang
Hi All:

Followed by commit 22dc8663d9 ("net: forbid the reentrant RX"), we
still need to fix the issues casued by loopback mode where the NIC
usually it via calling nc->info->receive() directly.

The fix is to introduce new network helper and check the
queue->delivering.

This series addresses CVE-2021-3416.

Thanks

Changes since V3:
- clarify CVE number in the commit log
- ident fix

Changes since V2:
- add more fixes from Alexander

Changes since V1:

- Fix dp8393x compiling
- Add rtl8139 fix
- Tweak the commit log
- Silent patchew warning

Alexander Bulekov (4):
  rtl8139: switch to use qemu_receive_packet() for loopback
  pcnet: switch to use qemu_receive_packet() for loopback
  cadence_gem: switch to use qemu_receive_packet() for loopback
  lan9118: switch to use qemu_receive_packet() for loopback

Jason Wang (6):
  net: introduce qemu_receive_packet()
  e1000: switch to use qemu_receive_packet() for loopback
  dp8393x: switch to use qemu_receive_packet() for loopback packet
  msf2-mac: switch to use qemu_receive_packet() for loopback
  sungem: switch to use qemu_receive_packet() for loopback
  tx_pkt: switch to use qemu_receive_packet_iov() for loopback

 hw/net/cadence_gem.c |  4 ++--
 hw/net/dp8393x.c |  2 +-
 hw/net/e1000.c   |  2 +-
 hw/net/lan9118.c |  2 +-
 hw/net/msf2-emac.c   |  2 +-
 hw/net/net_tx_pkt.c  |  2 +-
 hw/net/pcnet.c   |  2 +-
 hw/net/rtl8139.c |  2 +-
 hw/net/sungem.c  |  2 +-
 include/net/net.h|  5 +
 include/net/queue.h  |  8 
 net/net.c| 38 +++---
 net/queue.c  | 22 ++
 13 files changed, 76 insertions(+), 17 deletions(-)

-- 
2.24.3 (Apple Git-128)




Re: [PATCH 00/38] target/riscv: support packed extension v0.9.2

2021-03-04 Thread LIU Zhiwei

ping

On 2021/2/12 23:02, LIU Zhiwei wrote:

This patchset implements the packed extension for RISC-V on QEMU.

This patchset have passed all my direct Linux user mode cases(RV64) and
bare metal cases(RV32) on X86-64 Ubuntu host machine. I will later push
these test cases to my repo(https://github.com/romanheros/qemu.git
branch:packed-upstream-v1).

I have ported packed extension on RISU, but I didn't find a simulator or
hardware to compare with. If anyone have one, please let me know.

Features:
   * support specification packed extension 
v0.9.2(https://github.com/riscv/riscv-p-spec/)
   * support basic packed extension.
   * support Zp64.

LIU Zhiwei (38):
   target/riscv: implementation-defined constant parameters
   target/riscv: Hoist vector functions
   target/riscv: Fixup saturate subtract function
   target/riscv: 16-bit Addition & Subtraction Instructions
   target/riscv: 8-bit Addition & Subtraction Instruction
   target/riscv: SIMD 16-bit Shift Instructions
   target/riscv: SIMD 8-bit Shift Instructions
   target/riscv: SIMD 16-bit Compare Instructions
   target/riscv: SIMD 8-bit Compare Instructions
   target/riscv: SIMD 16-bit Multiply Instructions
   target/riscv: SIMD 8-bit Multiply Instructions
   target/riscv: SIMD 16-bit Miscellaneous Instructions
   target/riscv: SIMD 8-bit Miscellaneous Instructions
   target/riscv: 8-bit Unpacking Instructions
   target/riscv: 16-bit Packing Instructions
   target/riscv: Signed MSW 32x32 Multiply and Add Instructions
   target/riscv: Signed MSW 32x16 Multiply and Add Instructions
   target/riscv: Signed 16-bit Multiply 32-bit Add/Subtract Instructions
   target/riscv: Signed 16-bit Multiply 64-bit Add/Subtract Instructions
   target/riscv: Partial-SIMD Miscellaneous Instructions
   target/riscv: 8-bit Multiply with 32-bit Add Instructions
   target/riscv: 64-bit Add/Subtract Instructions
   target/riscv: 32-bit Multiply 64-bit Add/Subtract Instructions
   target/riscv: Signed 16-bit Multiply with 64-bit Add/Subtract
 Instructions
   target/riscv: Non-SIMD Q15 saturation ALU Instructions
   target/riscv: Non-SIMD Q31 saturation ALU Instructions
   target/riscv: 32-bit Computation Instructions
   target/riscv: Non-SIMD Miscellaneous Instructions
   target/riscv: RV64 Only SIMD 32-bit Add/Subtract Instructions
   target/riscv: RV64 Only SIMD 32-bit Shift Instructions
   target/riscv: RV64 Only SIMD 32-bit Miscellaneous Instructions
   target/riscv: RV64 Only SIMD Q15 saturating Multiply Instructions
   target/riscv: RV64 Only 32-bit Multiply Instructions
   target/riscv: RV64 Only 32-bit Multiply & Add Instructions
   target/riscv: RV64 Only 32-bit Parallel Multiply & Add Instructions
   target/riscv: RV64 Only Non-SIMD 32-bit Shift Instructions
   target/riscv: RV64 Only 32-bit Packing Instructions
   target/riscv: configure and turn on packed extension from command line

  target/riscv/cpu.c  |   32 +
  target/riscv/cpu.h  |6 +
  target/riscv/helper.h   |  332 ++
  target/riscv/insn32-64.decode   |   93 +-
  target/riscv/insn32.decode  |  285 ++
  target/riscv/insn_trans/trans_rvp.c.inc | 1224 +++
  target/riscv/internals.h|   50 +
  target/riscv/meson.build|1 +
  target/riscv/packed_helper.c| 3862 +++
  target/riscv/translate.c|3 +
  target/riscv/vector_helper.c|   90 +-
  11 files changed, 5912 insertions(+), 66 deletions(-)
  create mode 100644 target/riscv/insn_trans/trans_rvp.c.inc
  create mode 100644 target/riscv/packed_helper.c






Re: [RFC PATCH 0/5] hw/arm/virt: Introduce cpu topology support

2021-03-04 Thread Ying Fang




On 3/1/2021 5:48 PM, Andrew Jones wrote:

On Fri, Feb 26, 2021 at 04:41:45PM +0800, Ying Fang wrote:



On 2/25/2021 8:02 PM, Andrew Jones wrote:

On Thu, Feb 25, 2021 at 04:56:22PM +0800, Ying Fang wrote:

An accurate cpu topology may help improve the cpu scheduler's decision
making when dealing with multi-core system. So cpu topology description
is helpful to provide guest with the right view. Dario Faggioli's talk
in [0] also shows the virtual topology may has impact on sched performace.
Thus this patch series is posted to introduce cpu topology support for
arm platform.

Both fdt and ACPI are introduced to present the cpu topology. To describe
the cpu topology via ACPI, a PPTT table is introduced according to the
processor hierarchy node structure. This series is derived from [1], in
[1] we are trying to bring both cpu and cache topology support for arm
platform, but there is still some issues to solve to support the cache
hierarchy. So we split the cpu topology part out and send it seperately.
The patch series to support cache hierarchy will be send later since
Salil Mehta's cpu hotplug feature need the cpu topology enabled first and
he is waiting for it to be upstreamed.

This patch series was initially based on the patches posted by Andrew Jones [2].
I jumped in on it since some OS vendor cooperative partner are eager for it.
Thanks for Andrew's contribution.

After applying this patch series, launch a guest with virt-6.0 and cpu
topology configured with sockets:cores:threads = 2:4:2, you will get the
bellow messages with the lscpu command.

-
Architecture:aarch64
CPU op-mode(s):  64-bit
Byte Order:  Little Endian
CPU(s):  16
On-line CPU(s) list: 0-15
Thread(s) per core:  2


What CPU model was used? Did it actually support threads? If these were


It's tested on Huawei Kunpeng 920 CPU model and vcpu host-passthrough.
It does not support threads for now, but the next version 930 may
support it. Here we emulate a virtual cpu topology, a virtual 2 threads
is used to do the test.



KVM VCPUs, then I guess MPIDR.MT was not set on the CPUs. Apparently
that didn't confuse Linux? See [1] for how I once tried to deal with
threads.

[1] 
https://github.com/rhdrjones/qemu/commit/60218e0dd7b331031b644872d56f2aca42d0ff1e



If ACPI PPTT table is specified, the linux kernel won't check the MPIDR
register to populate cpu topology. Moreover MPIDR does not ensure a
right cpu topology. So it won't be a problem if MPIDR.MT is not set.


OK, so Linux doesn't care about MPIDR.MT with ACPI. What happens with
DT?


Behind the logical of Linux kernel, it tries to parse cpu topology in
smp_prepare_cpus (arch/arm64/kernel/topology.c). If cpu topology is
provided via DT, Linux kernel won't check MPIDR any more. This is the
same with ACPI enabled.






Core(s) per socket:  4
Socket(s):   2


Good, but what happens if you specify '-smp 16'? Do you get 16 sockets

   ^^ You didn't answer this question.


The latest qemu use smp_parse the parse -smp command line, by default if
-smp 16 is given, arm64 virt machine will get 16 sockets.




each with 1 core? Or, do you get 1 socket with 16 cores? And, which do
we want and why? If you look at [2], then you'll see I was assuming we
want to prefer cores over sockets, since without topology descriptions
that's what the Linux guest kernel would do.

[2] 
https://github.com/rhdrjones/qemu/commit/c0670b1bccb4d08c7cf7c6957cc8878a2af131dd



Thanks, I'll check the default way Linux does.


NUMA node(s):2


Why do we have two NUMA nodes in the guest? The two sockets in the
guest should not imply this.


The two NUMA nodes are emulated by Qemu since we already have guest numa
topology feature.


That's what I suspected, and I presume only a single node is present when
you don't use QEMU's NUMA feature - even when you supply a VCPU topology
with multiple sockets?


Agreed, I would like single numa node too if we do not use guest
numa feature. Here I provide the guest with two numa nodes and set the 
cpu affinity only to do a test.





Thanks,
drew


So the two sockets in the guest has nothing to do with
it. Actually even one socket may have two numa nodes in it in real cpu
model.



Thanks,
drew


Vendor ID:   HiSilicon
Model:   0
Model name:  Kunpeng-920
Stepping:0x1
BogoMIPS:200.00
NUMA node0 CPU(s):   0-7
NUMA node1 CPU(s):   8-15

[0] 
https://kvmforum2020.sched.com/event/eE1y/virtual-topology-for-virtual-machines-friend-or-foe-dario-faggioli-suse
[1] https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg02166.html
[2] 
https://patchwork.ozlabs.org/project/qemu-devel/cover/20180704124923.32483-1-drjo...@redhat.com

Ying Fang (5):

[PATCH v2] linux-user: add missing MULTICAST_IF get/setsockopt option

2021-03-04 Thread Jiaxun Yang
{IP,IPV6}_MULTICAST_IF was not supported.

Signed-off-by: Jiaxun Yang 
---
 linux-user/syscall.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 389ec09764..77343130b3 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2219,6 +2219,7 @@ static abi_long do_setsockopt(int sockfd, int level, int 
optname,
 #ifdef IP_FREEBIND
 case IP_FREEBIND:
 #endif
+case IP_MULTICAST_IF:
 case IP_MULTICAST_TTL:
 case IP_MULTICAST_LOOP:
 val = 0;
@@ -2265,6 +2266,7 @@ static abi_long do_setsockopt(int sockfd, int level, int 
optname,
 case IPV6_V6ONLY:
 case IPV6_RECVPKTINFO:
 case IPV6_UNICAST_HOPS:
+case IPV6_MULTICAST_IF:
 case IPV6_MULTICAST_HOPS:
 case IPV6_MULTICAST_LOOP:
 case IPV6_RECVERR:
@@ -2891,6 +2893,7 @@ get_timeout:
 #ifdef IP_FREEBIND
 case IP_FREEBIND:
 #endif
+case IP_MULTICAST_IF:
 case IP_MULTICAST_TTL:
 case IP_MULTICAST_LOOP:
 if (get_user_u32(len, optlen))
@@ -2926,6 +2929,7 @@ get_timeout:
 case IPV6_V6ONLY:
 case IPV6_RECVPKTINFO:
 case IPV6_UNICAST_HOPS:
+case IPV6_MULTICAST_IF:
 case IPV6_MULTICAST_HOPS:
 case IPV6_MULTICAST_LOOP:
 case IPV6_RECVERR:
-- 
2.30.1




Re: [PATCH] linux-user: add missing MULTICAST_IF get/setsockopt option

2021-03-04 Thread Jiaxun Yang

在 2021/3/5 上午11:35, Jiaxun Yang 写道:

> {IP,IPV6}_MULTICAST_IF was not supported.

>

> Reported-by: Yunqiang Su 

> Signed-off-by: Jiaxun Yang 

> ---

>   linux-user/syscall.c  | 4 +

>   linux-user/syscall.c.orig | 13305 
^ Sorry ^ ^ sorry for the noise...


I'm drunk today.



- Jiaxun



Re: [PATCH] linux-user: add missing MULTICAST_IF get/setsockopt option

2021-03-04 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/20210305033510.8600-1-jiaxun.y...@flygoat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210305033510.8600-1-jiaxun.y...@flygoat.com
Subject: [PATCH] linux-user: add missing MULTICAST_IF get/setsockopt option

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20210218175648.1636219-1-f4...@amsat.org -> 
patchew/20210218175648.1636219-1-f4...@amsat.org
 - [tag update]  patchew/20210218212453.831406-1-...@google.com -> 
patchew/20210218212453.831406-1-...@google.com
 - [tag update]  patchew/20210219144617.4782-1-peter.mayd...@linaro.org -> 
patchew/20210219144617.4782-1-peter.mayd...@linaro.org
 - [tag update]  patchew/20210304021621.579-1-yuzeng...@huawei.com -> 
patchew/20210304021621.579-1-yuzeng...@huawei.com
 - [tag update]  patchew/20210304101738.20248-1-vsement...@virtuozzo.com -> 
patchew/20210304101738.20248-1-vsement...@virtuozzo.com
 - [tag update]  patchew/20210304140229.575481-1-arm...@redhat.com -> 
patchew/20210304140229.575481-1-arm...@redhat.com
 - [tag update]  patchew/20210304203540.41614-1-nieklinnenb...@gmail.com -> 
patchew/20210304203540.41614-1-nieklinnenb...@gmail.com
 - [tag update]  patchew/20210304220104.2574112-1-laur...@vivier.eu -> 
patchew/20210304220104.2574112-1-laur...@vivier.eu
 - [tag update]  
patchew/20210304221103.6369-1-mark.cave-ayl...@ilande.co.uk -> 
patchew/20210304221103.6369-1-mark.cave-ayl...@ilande.co.uk
 * [new tag] patchew/20210305033510.8600-1-jiaxun.y...@flygoat.com -> 
patchew/20210305033510.8600-1-jiaxun.y...@flygoat.com
Switched to a new branch 'test'
fe4b962 linux-user: add missing MULTICAST_IF get/setsockopt option

=== OUTPUT BEGIN ===
Use of uninitialized value $acpi_testexpected in string eq at 
./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#54: 
new file mode 100644

ERROR: trailing whitespace
#1158: FILE: linux-user/syscall.c.orig:1100:
+$

ERROR: trailing whitespace
#1166: FILE: linux-user/syscall.c.orig:1108:
+$

ERROR: trailing whitespace
#1176: FILE: linux-user/syscall.c.orig:1118:
+$

ERROR: trailing whitespace
#1182: FILE: linux-user/syscall.c.orig:1124:
+$

ERROR: trailing whitespace
#1934: FILE: linux-user/syscall.c.orig:1876:
+$

ERROR: trailing whitespace
#1936: FILE: linux-user/syscall.c.orig:1878:
+if (msg_controllen < sizeof (struct target_cmsghdr)) $

ERROR: trailing whitespace
#2022: FILE: linux-user/syscall.c.orig:1964:
+if (msg_controllen < sizeof (struct target_cmsghdr)) $

ERROR: trailing whitespace
#6313: FILE: linux-user/syscall.c.orig:6255:
+if (ldt_info.entry_number < TARGET_GDT_ENTRY_TLS_MIN || $

ERROR: trailing whitespace
#6391: FILE: linux-user/syscall.c.orig:6333:
+$

ERROR: trailing whitespace
#6407: FILE: linux-user/syscall.c.orig:6349:
+base_addr = (entry_1 >> 16) | $

ERROR: trailing whitespace
#6408: FILE: linux-user/syscall.c.orig:6350:
+(entry_2 & 0xff00) | $

ERROR: trailing whitespace
#11511: FILE: linux-user/syscall.c.orig:11453:
+if (!(p = lock_user_string(arg2))) $

total: 12 errors, 1 warnings, 1 lines checked

Commit fe4b9625baf8 (linux-user: add missing MULTICAST_IF get/setsockopt 
option) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210305033510.8600-1-jiaxun.y...@flygoat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH v2 1/2] spapr.c: remove duplicated assert in spapr_memory_unplug_request()

2021-03-04 Thread David Gibson
On Tue, Mar 02, 2021 at 11:10:18AM -0300, Daniel Henrique Barboza wrote:
> We are asserting the existence of the first DRC LMB after sending unplug
> requests to all LMBs of the DIMM, where every DRC is being asserted
> inside the loop. This means that the first DRC is being asserted twice.
> 
> Remove the duplicated assert.
> 
> Signed-off-by: Daniel Henrique Barboza 

Applied to ppc-for-6.0.

> ---
>  hw/ppc/spapr.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index aca3ef9d58..b579830832 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3703,7 +3703,6 @@ static void spapr_memory_unplug_request(HotplugHandler 
> *hotplug_dev,
>  
>  drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
>addr_start / SPAPR_MEMORY_BLOCK_SIZE);
> -g_assert(drc);
>  spapr_hotplug_req_remove_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB,
>nr_lmbs, spapr_drc_index(drc));
>  }

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [PATCH v2 2/2] spapr.c: send QAPI event when memory hotunplug fails

2021-03-04 Thread David Gibson
On Tue, Mar 02, 2021 at 11:10:19AM -0300, Daniel Henrique Barboza wrote:
> Recent changes allowed the pSeries machine to rollback the hotunplug
> process for the DIMM when the guest kernel signals, via a
> reconfiguration of the DR connector, that it's not going to release the
> LMBs.
> 
> Let's also warn QAPI listerners about it. One place to do it would be
> right after the unplug state is cleaned up,
> spapr_clear_pending_dimm_unplug_state(). This would mean that the
> function is now doing more than cleaning up the pending dimm state
> though.
> 
> This patch does the following changes in spapr.c:
> 
> - send a QAPI event to inform that we experienced a failure in the
>   hotunplug of the DIMM;
> 
> - rename spapr_clear_pending_dimm_unplug_state() to
>   spapr_memory_unplug_rollback(). This is a better fit for what the
>   function is now doing, and it makes callers care more about what the
>   function goal is and less about spapr.c internals such as clearing
>   the pending dimm unplug state.
> 
> Reviewed-by: Greg Kurz 
> Signed-off-by: Daniel Henrique Barboza 

Applied to ppc-for-6.0, thanks.

> ---
>  hw/ppc/spapr.c | 13 +++--
>  hw/ppc/spapr_drc.c |  5 ++---
>  include/hw/ppc/spapr.h |  3 +--
>  3 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b579830832..d56418ca29 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -28,6 +28,7 @@
>  #include "qemu-common.h"
>  #include "qemu/datadir.h"
>  #include "qapi/error.h"
> +#include "qapi/qapi-events-machine.h"
>  #include "qapi/visitor.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/hostmem.h"
> @@ -3575,14 +3576,14 @@ static SpaprDimmState 
> *spapr_recover_pending_dimm_state(SpaprMachineState *ms,
>  return spapr_pending_dimm_unplugs_add(ms, avail_lmbs, dimm);
>  }
>  
> -void spapr_clear_pending_dimm_unplug_state(SpaprMachineState *spapr,
> -   DeviceState *dev)
> +void spapr_memory_unplug_rollback(SpaprMachineState *spapr, DeviceState *dev)
>  {
>  SpaprDimmState *ds;
>  PCDIMMDevice *dimm;
>  SpaprDrc *drc;
>  uint32_t nr_lmbs;
>  uint64_t size, addr_start, addr;
> +g_autofree char *qapi_error = NULL;
>  int i;
>  
>  if (!dev) {
> @@ -3616,6 +3617,14 @@ void 
> spapr_clear_pending_dimm_unplug_state(SpaprMachineState *spapr,
>  drc->unplug_requested = false;
>  addr += SPAPR_MEMORY_BLOCK_SIZE;
>  }
> +
> +/*
> + * Tell QAPI that something happened and the memory
> + * hotunplug wasn't successful.
> + */
> +qapi_error = g_strdup_printf("Memory hotunplug rejected by the guest "
> + "for device %s", dev->id);
> +qapi_event_send_mem_unplug_error(dev->id, qapi_error);
>  }
>  
>  /* Callback to be called during DRC release. */
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 98b626acf9..8a71b03800 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -1231,12 +1231,11 @@ static void rtas_ibm_configure_connector(PowerPCCPU 
> *cpu,
>  
>  /*
>   * This indicates that the kernel is reconfiguring a LMB due to
> - * a failed hotunplug. Clear the pending unplug state for the whole
> - * DIMM.
> + * a failed hotunplug. Rollback the DIMM unplug process.
>   */
>  if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB &&
>  drc->unplug_requested) {
> -spapr_clear_pending_dimm_unplug_state(spapr, drc->dev);
> +spapr_memory_unplug_rollback(spapr, drc->dev);
>  }
>  
>  if (!drc->fdt) {
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index d6edeaaaff..47cebaf3ac 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -847,8 +847,7 @@ int spapr_hpt_shift_for_ramsize(uint64_t ramsize);
>  int spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, Error **errp);
>  void spapr_clear_pending_events(SpaprMachineState *spapr);
>  void spapr_clear_pending_hotplug_events(SpaprMachineState *spapr);
> -void spapr_clear_pending_dimm_unplug_state(SpaprMachineState *spapr,
> -   DeviceState *dev);
> +void spapr_memory_unplug_rollback(SpaprMachineState *spapr, DeviceState 
> *dev);
>  int spapr_max_server_number(SpaprMachineState *spapr);
>  void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
>uint64_t pte0, uint64_t pte1);

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [RESEND][BUG FIX HELP] QEMU main thread endlessly hangs in __ppoll()

2021-03-04 Thread Like Xu

Hi John,

Thanks for your comment.

On 2021/3/5 7:53, John Snow wrote:

On 2/28/21 9:39 PM, Like Xu wrote:

Hi Genius,

I am a user of QEMU v4.2.0 and stuck in an interesting bug, which may 
still exist in the mainline.

Thanks in advance to heroes who can take a look and share understanding.



Do you have a test case that reproduces on 5.2? It'd be nice to know if it 
was still a problem in the latest source tree or not.


We narrowed down the source of the bug, which basically came from
the following qmp usage:

{'execute': 'human-monitor-command', 'arguments':{ 'command-line': 
'drive_del replication0' } }


One of the test cases is the COLO usage (docs/colo-proxy.txt).

This issue is sporadic,the probability may be 1/15 for a io-heavy guest.

I believe it's reproducible on 5.2 and the latest tree.



--js


The qemu main thread endlessly hangs in the handle of the qmp statement:
{'execute': 'human-monitor-command', 'arguments':{ 'command-line': 
'drive_del replication0' } }

and we have the call trace looks like:

#0 0x7f3c22045bf6 in __ppoll (fds=0x555611328410, nfds=1, 
timeout=, timeout@entry=0x7ffc56c66db0,

sigmask=sigmask@entry=0x0) at ../sysdeps/unix/sysv/linux/ppoll.c:44
#1 0x55561021f415 in ppoll (__ss=0x0, __timeout=0x7ffc56c66db0, 
__nfds=, __fds=)

at /usr/include/x86_64-linux-gnu/bits/poll2.h:77
#2 qemu_poll_ns (fds=, nfds=, 
timeout=) at util/qemu-timer.c:348
#3 0x555610221430 in aio_poll (ctx=ctx@entry=0x5556113010f0, 
blocking=blocking@entry=true) at util/aio-posix.c:669
#4 0x55561019268d in bdrv_do_drained_begin (poll=true, 
ignore_bds_parents=false, parent=0x0, recursive=false,

bs=0x55561138b0a0) at block/io.c:430
#5 bdrv_do_drained_begin (bs=0x55561138b0a0, recursive=, 
parent=0x0, ignore_bds_parents=,

poll=) at block/io.c:396
#6 0x55561017b60b in quorum_del_child (bs=0x55561138b0a0, 
child=0x7f36dc0ce380, errp=)

at block/quorum.c:1063
#7 0x55560ff5836b in qmp_x_blockdev_change (parent=0x555612373120 
"colo-disk0", has_child=,
child=0x5556112df3e0 "children.1", has_node=, node=0x0, 
errp=0x7ffc56c66f98) at blockdev.c:4494
#8 0x5556100f8f57 in qmp_marshal_x_blockdev_change (args=out>, ret=, errp=0x7ffc56c67018)

at qapi/qapi-commands-block-core.c:1538
#9 0x5556101d8290 in do_qmp_dispatch (errp=0x7ffc56c67010, 
allow_oob=, request=,

cmds=0x5556109c69a0 ) at qapi/qmp-dispatch.c:132
#10 qmp_dispatch (cmds=0x5556109c69a0 , request=out>, allow_oob=)

at qapi/qmp-dispatch.c:175
#11 0x5556100d4c4d in monitor_qmp_dispatch (mon=0x5556113a6f40, 
req=) at monitor/qmp.c:145
#12 0x5556100d5437 in monitor_qmp_bh_dispatcher (data=out>) at monitor/qmp.c:234
#13 0x55561021dbec in aio_bh_call (bh=0x5556112164bGrateful0) at 
util/async.c:117

#14 aio_bh_poll (ctx=ctx@entry=0x5556112151b0) at util/async.c:117
#15 0x5556102212c4 in aio_dispatch (ctx=0x5556112151b0) at 
util/aio-posix.c:459
#16 0x55561021dab2 in aio_ctx_dispatch (source=, 
callback=, user_data=)

at util/async.c:260
#17 0x7f3c22302fbd in g_main_context_dispatch () from 
/lib/x86_64-linux-gnu/libglib-2.0.so.0

#18 0x555610220358 in glib_pollfds_poll () at util/main-loop.c:219
#19 os_host_main_loop_wait (timeout=) at util/main-loop.c:242
#20 main_loop_wait (nonblocking=) at util/main-loop.c:518
#21 0x55560ff600fe in main_loop () at vl.c:1814
#22 0x55560fddbce9 in main (argc=, argv=out>, envp=) at vl.c:4503


We found that we're doing endless check in the line of 
block/io.c:bdrv_do_drained_begin():

 BDRV_POLL_WHILE(bs, bdrv_drain_poll_top_level(bs, recursive, parent));
and it turns out that the bdrv_drain_poll() always get true from:
- bdrv_parent_drained_poll(bs, ignore_parent, ignore_bds_parents)
- AND atomic_read(>in_flight)

I personally think this is a deadlock issue in the a QEMU block layer
(as we know, we have some #FIXME comments in related codes, such as block 
permisson update).

Any comments are welcome and appreciated.

---
thx,likexu








Re: [PATCH] hw/sd/sdhci: Report error when guest access protected registers

2021-03-04 Thread Bin Meng
On Fri, Feb 19, 2021 at 1:56 AM Philippe Mathieu-Daudé  wrote:
>
> The SDHC_SYSAD and SDHC_BLKSIZE can not be accessed while a
> transaction is in progress, see 'SD Host Controller Simplified
> Specification'
>
>   1.5) SD Command Generation
>
>   The Host Driver should not read the SDMA System Address, Block
>   Size and Block Count registers during a data transaction unless
>   the transfer is stopped because the value is changing and not
>   stable.
>
> Report guest intents as errors.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Based-on: <1613447214-81951-1-git-send-email-bmeng...@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/sd/sdhci.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng 



Re: [PATCH v2 4/8] simplebench/bench-backup: add target-cache argument

2021-03-04 Thread John Snow

On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote:

Allow benchmark with different kinds of target cache.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  scripts/simplebench/bench-backup.py| 33 --
  scripts/simplebench/bench_block_job.py | 10 +---
  2 files changed, 33 insertions(+), 10 deletions(-)



Admittedly just skimming a bit on this one, it looks OK.

Reviewed-by: John Snow 




Re: [PATCH v2 5/8] simplebench/bench_block_job: handle error in BLOCK_JOB_COMPLETED

2021-03-04 Thread John Snow

On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote:

We should not report success if there is an error in final event.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  scripts/simplebench/bench_block_job.py | 4 
  1 file changed, 4 insertions(+)

diff --git a/scripts/simplebench/bench_block_job.py 
b/scripts/simplebench/bench_block_job.py
index 8f8385ccce..71d2e489c8 100755
--- a/scripts/simplebench/bench_block_job.py
+++ b/scripts/simplebench/bench_block_job.py
@@ -70,6 +70,10 @@ def bench_block_job(cmd, cmd_args, qemu_args):
  vm.shutdown()
  return {'error': 'block-job failed: ' + str(e),
  'vm-log': vm.get_log()}
+if 'error' in e['data']:
+vm.shutdown()
+return {'error': 'block-job failed: ' + e['data']['error'],
+'vm-log': vm.get_log()}
  end_ms = e['timestamp']['seconds'] * 100 + \
  e['timestamp']['microseconds']
  finally:



"Yes, the block job completed -- but not how you wanted it to."

Reviewed-by: John Snow 




Re: [PATCH v2 6/8] simplebench/bench-backup: support qcow2 source files

2021-03-04 Thread John Snow

On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote:

Add support for qcow2 source. New option says to use test-source.qcow2
instead of test-source. Of course, test-source.qcow2 should be
precreated.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  scripts/simplebench/bench-backup.py| 5 +
  scripts/simplebench/bench_block_job.py | 7 ++-
  2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/scripts/simplebench/bench-backup.py 
b/scripts/simplebench/bench-backup.py
index fbc85f266f..a2120fcbf0 100755
--- a/scripts/simplebench/bench-backup.py
+++ b/scripts/simplebench/bench-backup.py
@@ -58,6 +58,8 @@ def bench(args):
  
  if src == 'nbd':

  source = nbd_drv
+elif args.qcow2_sources:
+source = drv_qcow2(drv_file(dirs[src] + '/test-source.qcow2'))
  else:
  source = drv_file(dirs[src] + '/test-source')
  
@@ -199,6 +201,9 @@ def __call__(self, parser, namespace, values, option_string=None):

  Use compressed backup. It automatically means
  automatically creating qcow2 target with
  lazy_refcounts for each test run''', action='store_true')
+p.add_argument('--qcow2-sources', help='''\
+Use test-source.qcow2 images as sources instead of
+test-source raw images''', action='store_true')
  p.add_argument('--target-cache', help='''\
  Setup cache for target nodes. Options:
 direct: default, use O_DIRECT and aio=native
diff --git a/scripts/simplebench/bench_block_job.py 
b/scripts/simplebench/bench_block_job.py
index 71d2e489c8..4f03c12169 100755
--- a/scripts/simplebench/bench_block_job.py
+++ b/scripts/simplebench/bench_block_job.py
@@ -88,6 +88,11 @@ def get_image_size(path):
  return json.loads(out)['virtual-size']
  
  
+def get_blockdev_size(obj):

+img = obj['filename'] if 'filename' in obj else obj['file']['filename']
+return get_image_size(img)
+


Well, as long as it works :)


+
  # Bench backup or mirror
  def bench_block_copy(qemu_binary, cmd, cmd_options, source, target):
  """Helper to run bench_block_job() for mirror or backup"""
@@ -101,7 +106,7 @@ def bench_block_copy(qemu_binary, cmd, cmd_options, source, 
target):
  
  subprocess.run(['qemu-img', 'create', '-f', 'qcow2',

  target['file']['filename'],
-str(get_image_size(source['filename']))],
+str(get_blockdev_size(source))],
 stdout=subprocess.DEVNULL,
 stderr=subprocess.DEVNULL, check=True)
  



Reviewed-by: John Snow 




Re: [PATCH v2 7/8] simplebench/bench-backup: add --count and --no-initial-run

2021-03-04 Thread John Snow

On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote:

Add arguments to set number of test runs per table cell and to disable
initial run that is not counted in results.

It's convenient to set --count 1 --no-initial-run to fast run test
onece, and to set --count to some large enough number for good
precision of the results.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  scripts/simplebench/bench-backup.py | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/scripts/simplebench/bench-backup.py 
b/scripts/simplebench/bench-backup.py
index a2120fcbf0..519a985a7f 100755
--- a/scripts/simplebench/bench-backup.py
+++ b/scripts/simplebench/bench-backup.py
@@ -155,7 +155,9 @@ def bench(args):
  'qemu-binary': path
  })
  
-result = simplebench.bench(bench_func, test_envs, test_cases, count=3)

+result = simplebench.bench(bench_func, test_envs, test_cases,
+   count=args.count,
+   initial_run = not args.no_initial_run)


The double negative feels odd; "initial_run = args.initial_run" would 
read better and avoid changing behavior, but maybe that's intentional.



  with open('results.json', 'w') as f:
  json.dump(result, f, indent=4)
  print(results_to_text(result))
@@ -211,4 +213,10 @@ def __call__(self, parser, namespace, values, 
option_string=None):
 both: generate two test cases for each src:dst pair''',
 default='direct', choices=('direct', 'cached', 'both'))
  
+p.add_argument('--count', type=int, default=3, help='''\

+Number of test runs per table cell''')
+
+p.add_argument('--no-initial-run', action='store_true', help='''\
+Don't do initial run of test for each cell which doesn't count''')
+
  bench(p.parse_args())






Re: [PATCH v2 8/8] simplebench/bench_block_job: drop caches before test run

2021-03-04 Thread John Snow

On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote:

It probably may improve reliability of results when testing in cached
mode.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  scripts/simplebench/bench_block_job.py | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/scripts/simplebench/bench_block_job.py 
b/scripts/simplebench/bench_block_job.py
index 4f03c12169..fa45ad2655 100755
--- a/scripts/simplebench/bench_block_job.py
+++ b/scripts/simplebench/bench_block_job.py
@@ -53,6 +53,8 @@ def bench_block_job(cmd, cmd_args, qemu_args):
  return {'error': 'qemu failed: ' + str(vm.get_log())}
  
  try:

+subprocess.run('sync; echo 3 > /proc/sys/vm/drop_caches', shell=True,
+   check=True)
  res = vm.qmp(cmd, **cmd_args)
  if res != {'return': {}}:
  vm.shutdown()



Worth adding a conditional to allow "hot" or "cold" runs? nah?




Re: [PATCH 43/44] docs/system/arm/mps2.rst: Document the new mps3-an547 board

2021-03-04 Thread Richard Henderson

On 2/19/21 6:46 AM, Peter Maydell wrote:

@@ -1,5 +1,5 @@
-Arm MPS2 and MPS3 boards (``mps2-an385``, ``mps2-an386``, ``mps2-an500``, 
``mps2-an505``, ``mps2-an511``, ``mps2-an521``, ``mps3-an524``)
-=
+Arm MPS2 and MPS3 boards (``mps2-an385``, ``mps2-an386``, ``mps2-an500``, 
``mps2-an505``, ``mps2-an511``, ``mps2-an521``, ``mps3-an524``, ``mps3-an547``)'

I think you should drop the list here, as it has gotten *way* too long.


@@ -27,6 +27,8 @@ QEMU models the following FPGA images:
Dual Cortex-M33 as documented in Arm Application Note AN521
  ``mps3-an524``
Dual Cortex-M33 on an MPS3, as documented in Arm Application Note AN524
+``mps3-an547``
+  Cortex-M55 on an MPS3, as documented in Arm Application Note AN547


The list is down here, anyway.


r~



Re: [PATCH v2 2/8] simplebench: bench_one(): support count=1

2021-03-04 Thread John Snow

On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote:

statistics.stdev raises if sequence length is less than two. Support
that case by hand.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  scripts/simplebench/simplebench.py | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/simplebench/simplebench.py 
b/scripts/simplebench/simplebench.py
index b153cae274..712e1f845b 100644
--- a/scripts/simplebench/simplebench.py
+++ b/scripts/simplebench/simplebench.py
@@ -92,7 +92,10 @@ def bench_one(test_func, test_env, test_case, count=5, 
initial_run=True,
  dim = 'seconds'
  result['dimension'] = dim
  result['average'] = statistics.mean(r[dim] for r in succeeded)
-result['stdev'] = statistics.stdev(r[dim] for r in succeeded)
+if len(succeeded) == 1:
+result['stdev'] = 0
+else:
+result['stdev'] = statistics.stdev(r[dim] for r in succeeded)
  
  if len(succeeded) < count:

  result['n-failed'] = count - len(succeeded)



Omitted from patch context is that we have "if succeeded: ..." so we 
know it's at least 1 here.


Reviewed-by: John Snow 




Re: [PATCH v2 1/8] simplebench: bench_one(): add slow_limit argument

2021-03-04 Thread John Snow

On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote:

Sometimes one of cells in a testing table runs too slow. And we really
don't want to wait so long. Limit number of runs in this case.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  scripts/simplebench/simplebench.py | 29 +
  1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/scripts/simplebench/simplebench.py 
b/scripts/simplebench/simplebench.py
index f61513af90..b153cae274 100644
--- a/scripts/simplebench/simplebench.py
+++ b/scripts/simplebench/simplebench.py
@@ -19,9 +19,11 @@
  #
  
  import statistics

+import time
  
  
-def bench_one(test_func, test_env, test_case, count=5, initial_run=True):

+def bench_one(test_func, test_env, test_case, count=5, initial_run=True,
+  slow_limit=100):
  """Benchmark one test-case
  
  test_func   -- benchmarking function with prototype

@@ -36,6 +38,8 @@ def bench_one(test_func, test_env, test_case, count=5, 
initial_run=True):
  test_case   -- test case - opaque second argument for test_func
  count   -- how many times to call test_func, to calculate average
  initial_run -- do initial run of test_func, which don't get into result
+slow_limit  -- reduce test runs to 2, if current run exceedes the limit
+   (in seconds)
  


s/exceedes/exceeds, and you need to mention that if the initial run 
exceeds the limit, it will change the behavior to count that result.


It is also possible (conceivably) that the initial run exceeds the 
limit, but subsequent runs don't, so it might be hard to predict how 
many tests it'll actually run.


If you're OK with that behavior, maybe:

"Consider a test run 'slow' once it exceeds this limit, in seconds.
 Stop early once there are two 'slow' runs, including the initial run.
 Slow initial runs will be included in the results."

Lastly, this will change existing behavior -- do we care? Should it 
default to None instead? Should we be able to pass None or 0 to disable 
this behavior?



  Returns dict with the following fields:
  'runs': list of test_func results
@@ -47,17 +51,34 @@ def bench_one(test_func, test_env, test_case, count=5, 
initial_run=True):
  'n-failed': number of failed runs (exists only if at least one run
  failed)
  """
+runs = []
+i = 0
  if initial_run:
+t = time.time()
+
  print('  #initial run:')
-print('   ', test_func(test_env, test_case))
+res = test_func(test_env, test_case)
+print('   ', res)
+
+if time.time() - t > slow_limit:
+print('- initial run is too slow, so it counts')
+runs.append(res)
+i = 1
+
+for i in range(i, count):
+t = time.time()
  
-runs = []

-for i in range(count):
  print('  #run {}'.format(i+1))
  res = test_func(test_env, test_case)
  print('   ', res)
  runs.append(res)
  
+if time.time() - t > slow_limit and len(runs) >= 2:

+print('- run is too slow, and we have enough runs, stop here')
+break
+
+count = len(runs)
+
  result = {'runs': runs}
  
  succeeded = [r for r in runs if ('seconds' in r or 'iops' in r)]







Re: [PATCH 42/44] hw/arm/mps2-tz: Add new mps3-an547 board

2021-03-04 Thread Richard Henderson

On 2/19/21 6:46 AM, Peter Maydell wrote:

Add support for the mps3-an547 board; this is an SSE-300 based
FPGA image that runs on the MPS3.

Signed-off-by: Peter Maydell
---


Reviewed-by: Richard Henderson 

r~



Re: [RFC PATCH v2 7/8] cpu: Move CPUClass::has_work() to TCGCPUOps

2021-03-04 Thread David Gibson
On Thu, Mar 04, 2021 at 11:23:22PM +0100, Philippe Mathieu-Daudé wrote:
> We can only check if a vCPU has work with TCG.
> Restrict the has_work() handler to TCG by moving it to
> the TCGCPUOps structure, and adapt all the targets.
> 
> cpu_common_has_work() is removed as being inlined in
> cpu_has_work().
> 
> Reviewed-by: Taylor Simpson 
> Signed-off-by: Philippe Mathieu-Daudé 

ppc parts
Acked-by: David Gibson 

> ---
> v2:
> - finished PPC
> - check cc->tcg_ops->has_work non-null (thuth)
> ---
>  include/hw/core/cpu.h   |  2 --
>  include/hw/core/tcg-cpu-ops.h   |  4 
>  accel/tcg/cpu-exec.c|  6 +-
>  hw/core/cpu.c   |  6 --
>  target/alpha/cpu.c  |  2 +-
>  target/arm/cpu.c|  2 +-
>  target/avr/cpu.c|  2 +-
>  target/cris/cpu.c   |  3 ++-
>  target/hexagon/cpu.c|  2 +-
>  target/hppa/cpu.c   |  2 +-
>  target/i386/cpu.c   |  7 +--
>  target/i386/tcg/tcg-cpu.c   |  6 ++
>  target/lm32/cpu.c   |  2 +-
>  target/m68k/cpu.c   |  2 +-
>  target/microblaze/cpu.c |  2 +-
>  target/mips/cpu.c   |  2 +-
>  target/moxie/cpu.c  |  2 +-
>  target/nios2/cpu.c  |  2 +-
>  target/openrisc/cpu.c   |  2 +-
>  target/riscv/cpu.c  |  2 +-
>  target/rx/cpu.c |  2 +-
>  target/s390x/cpu.c  |  2 +-
>  target/sh4/cpu.c|  2 +-
>  target/sparc/cpu.c  |  2 +-
>  target/tilegx/cpu.c |  2 +-
>  target/tricore/cpu.c|  2 +-
>  target/unicore32/cpu.c  |  2 +-
>  target/xtensa/cpu.c |  2 +-
>  target/ppc/translate_init.c.inc | 10 +-
>  29 files changed, 44 insertions(+), 42 deletions(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 66109bcca35..8efea289e7e 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -86,7 +86,6 @@ struct AccelCPUClass;
>   * instantiatable CPU type.
>   * @parse_features: Callback to parse command line arguments.
>   * @reset_dump_flags: #CPUDumpFlags to use for reset logging.
> - * @has_work: Callback for checking if there is work to do.
>   * @virtio_is_big_endian: Callback to return %true if a CPU which supports
>   * runtime configurable endianness is currently big-endian. Non-configurable
>   * CPUs can use the default implementation of this method. This method should
> @@ -149,7 +148,6 @@ struct CPUClass {
>  void (*parse_features)(const char *typename, char *str, Error **errp);
>  
>  int reset_dump_flags;
> -bool (*has_work)(CPUState *cpu);
>  bool (*virtio_is_big_endian)(CPUState *cpu);
>  int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
> uint8_t *buf, int len, bool is_write);
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index 72d791438c2..f5d44ba59f3 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -19,6 +19,10 @@ struct TCGCPUOps {
>   * Called when the first CPU is realized.
>   */
>  void (*initialize)(void);
> +/**
> + * @has_work: Callback for checking if there is work to do
> + */
> +bool (*has_work)(CPUState *cpu);
>  /**
>   * @synchronize_from_tb: Synchronize state from a TCG #TranslationBlock
>   *
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index b9ce36e59e2..4e73550f981 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -451,7 +451,11 @@ bool cpu_has_work(CPUState *cpu)
>  {
>  CPUClass *cc = CPU_GET_CLASS(cpu);
>  
> -return cc->has_work(cpu);
> +if (cc->tcg_ops->has_work) {
> +return cc->tcg_ops->has_work(cpu);
> +}
> +
> +return false;
>  }
>  
>  static inline bool cpu_handle_halt(CPUState *cpu)
> diff --git a/hw/core/cpu.c b/hw/core/cpu.c
> index 00330ba07de..3110867c3a3 100644
> --- a/hw/core/cpu.c
> +++ b/hw/core/cpu.c
> @@ -261,11 +261,6 @@ static void cpu_common_reset(DeviceState *dev)
>  }
>  }
>  
> -static bool cpu_common_has_work(CPUState *cs)
> -{
> -return false;
> -}
> -
>  ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
>  {
>  CPUClass *cc = CPU_CLASS(object_class_by_name(typename));
> @@ -397,7 +392,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
>  
>  k->parse_features = cpu_common_parse_features;
>  k->get_arch_id = cpu_common_get_arch_id;
> -k->has_work = cpu_common_has_work;
>  k->get_paging_enabled = cpu_common_get_paging_enabled;
>  k->get_memory_mapping = cpu_common_get_memory_mapping;
>  k->write_elf32_qemunote = cpu_common_write_elf32_qemunote;
> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
> index e50ae7bef06..57e88bbe7fd 100644
> --- a/target/alpha/cpu.c
> +++ b/target/alpha/cpu.c
> @@ -210,6 +210,7 @@ static void alpha_cpu_initfn(Object *obj)
>  
>  static 

Re: [PATCH v5 0/3] Add npcm7xx emc model

2021-03-04 Thread Doug Evans
Ping.

On Thu, Feb 18, 2021 at 1:24 PM Doug Evans  wrote:

> This is a 10/100 ethernet device that has several features.
> Only the ones needed by the Linux driver have been implemented.
> See npcm7xx_emc.c for a list of unimplemented features.
>
> Doug Evans (3):
>   hw/net: Add npcm7xx emc model
>   hw/arm: Add npcm7xx emc model
>   tests/qtests: Add npcm7xx emc model test
>
>  docs/system/arm/nuvoton.rst|   3 +-
>  hw/arm/npcm7xx.c   |  50 +-
>  hw/net/meson.build |   1 +
>  hw/net/npcm7xx_emc.c   | 857 
>  hw/net/trace-events|  17 +
>  include/hw/arm/npcm7xx.h   |   2 +
>  include/hw/net/npcm7xx_emc.h   | 286 +++
>  tests/qtest/meson.build|   3 +-
>  tests/qtest/npcm7xx_emc-test.c | 862 +
>  9 files changed, 2077 insertions(+), 4 deletions(-)
>  create mode 100644 hw/net/npcm7xx_emc.c
>  create mode 100644 include/hw/net/npcm7xx_emc.h
>  create mode 100644 tests/qtest/npcm7xx_emc-test.c
>
> --
> 2.30.0.617.g56c4b15f3c-goog
>
> Differences from v4:
>
> 1/3 hw/net: Add npcm7xx emc model
> - no change
>
> 2/3 hw/arm: Add npcm7xx emc model
> - no change
>
> 3/3 tests/qtests: Add npcm7xx emc model test
> - handle --disable-slirp
>
> Differences from v3:
>
> 1/3 hw/net: Add npcm7xx emc model
> - no change
>
> 2/3 hw/arm: Add npcm7xx emc model
> - no change
>
> 3/3 tests/qtests: Add npcm7xx emc model test
> - handle big endian hosts, tested on sparc64
>
> Differences from v2:
>
> 1/3 hw/net: Add npcm7xx emc model
> - move call to qemu_set_irq
> - remove use of C99 mixed decls/statements
> - add use of g_autofree
>
> 2/3 hw/arm: Add npcm7xx emc model
> - none, patch ok as is
>
> 3/3 tests/qtests: Add npcm7xx emc model test
> - remove use of C99 mixed decls/statements
>


Re: [PATCH v5 4/8] vt82c686: Introduce abstract TYPE_VIA_ISA and base vt82c686b_isa on it

2021-03-04 Thread David Gibson
On Thu, Mar 04, 2021 at 11:42:10PM +0100, Philippe Mathieu-Daudé wrote:
> On 3/4/21 9:16 PM, BALATON Zoltan wrote:
> > On Thu, 4 Mar 2021, Philippe Mathieu-Daudé wrote:
> >> On 3/2/21 10:11 PM, BALATON Zoltan wrote:
> >>> To allow reusing ISA bridge emulation for vt8231_isa move the device
> >>> state of vt82c686b_isa emulation in an abstract via_isa class.
> >>>
> >>> Signed-off-by: BALATON Zoltan 
> >>> ---
> >>>  hw/isa/vt82c686.c    | 70 ++--
> >>>  include/hw/pci/pci_ids.h |  2 +-
> >>>  2 files changed, 40 insertions(+), 32 deletions(-)
> >>>
> >>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> >>> index 72234bc4d1..5137f97f37 100644
> >>> --- a/hw/isa/vt82c686.c
> >>> +++ b/hw/isa/vt82c686.c
> >>> @@ -609,24 +609,48 @@ static const TypeInfo vt8231_superio_info = {
> >>>  };
> >>>
> >>>
> >>> -OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState, VT82C686B_ISA)
> >>> +#define TYPE_VIA_ISA "via-isa"
> >>> +OBJECT_DECLARE_SIMPLE_TYPE(ViaISAState, VIA_ISA)
> >>>
> >>> -struct VT82C686BISAState {
> >>> +struct ViaISAState {
> >>>  PCIDevice dev;
> >>>  qemu_irq cpu_intr;
> >>>  ViaSuperIOState *via_sio;
> >>>  };
> >>>
> >>> +static const VMStateDescription vmstate_via = {
> >>> +    .name = "via-isa",
> >>
> >> You changed the migration stream name, so I think we have
> >> a problem with migration... No clue how to do that properly.
> > 
> > I don't think these machines support migration or state description of
> > vt86c686b was not missing something before these patches that would make
> > it not work anyway so I did not worry about this too much. I doubt
> > anybody wants to migrate a fuloong2e machine so this should not be a
> > problem in practice but maybe you can mention it in the release notes if
> > you think that would be necessary.
> 
> Maybe just add in the description:
> 
>  This change breaks migration back compatibility, but
>  this is not an issue for the Fuloong2E machine.

Hrm.  If migration was never supported, why is there a vmstate
description there at all though?

That said, I don't think breaking compat is a problem: that's only an
issue where we actually have versioned machine types, which covers
only pc, pseries, arm virt and a very few others.  I don't think this
device was used on any of them.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [RFC PATCH v2 5/8] target/ppc: Duplicate the TCGCPUOps structure for POWER CPUs

2021-03-04 Thread David Gibson
On Thu, Mar 04, 2021 at 11:23:20PM +0100, Philippe Mathieu-Daudé wrote:
65;6203;1c> POWER CPUs have specific CPUClass::has_work() handlers.
> In preparation of moving this field to TCGCPUOps, we need
> to duplicate the current ppc_tcg_ops structure for the
> POWER cpus.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Acked-by: David Gibson 

> ---
>  target/ppc/translate_init.c.inc | 69 +
>  1 file changed, 69 insertions(+)
> 
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index 80239077e0b..fe76d0b3773 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -48,6 +48,11 @@
>  /* #define PPC_DUMP_SPR_ACCESSES */
>  /* #define USE_APPLE_GDB */
>  
> +static const struct TCGCPUOps power7_tcg_ops;
> +static const struct TCGCPUOps power8_tcg_ops;
> +static const struct TCGCPUOps power9_tcg_ops;
> +static const struct TCGCPUOps power10_tcg_ops;
> +
>  /*
>   * Generic callbacks:
>   * do nothing but store/retrieve spr value
> @@ -8685,6 +8690,9 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>  pcc->l1_dcache_size = 0x8000;
>  pcc->l1_icache_size = 0x8000;
>  pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> +#ifdef CONFIG_TCG
> +cc->tcg_ops = _tcg_ops;
> +#endif /* CONFIG_TCG */
>  }
>  
>  static void init_proc_POWER8(CPUPPCState *env)
> @@ -8863,6 +8871,9 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>  pcc->l1_dcache_size = 0x8000;
>  pcc->l1_icache_size = 0x8000;
>  pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> +#ifdef CONFIG_TCG
> +cc->tcg_ops = _tcg_ops;
> +#endif /* CONFIG_TCG */
>  }
>  
>  #ifdef CONFIG_SOFTMMU
> @@ -9081,6 +9092,9 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>  pcc->l1_dcache_size = 0x8000;
>  pcc->l1_icache_size = 0x8000;
>  pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> +#ifdef CONFIG_TCG
> +cc->tcg_ops = _tcg_ops;
> +#endif /* CONFIG_TCG */
>  }
>  
>  #ifdef CONFIG_SOFTMMU
> @@ -9292,6 +9306,9 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>  pcc->l1_dcache_size = 0x8000;
>  pcc->l1_icache_size = 0x8000;
>  pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> +#ifdef CONFIG_TCG
> +cc->tcg_ops = _tcg_ops;
> +#endif /* CONFIG_TCG */
>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
> @@ -10851,6 +10868,58 @@ static const struct TCGCPUOps ppc_tcg_ops = {
>.cpu_exec_interrupt = ppc_cpu_exec_interrupt,
>.tlb_fill = ppc_cpu_tlb_fill,
>  
> +#ifndef CONFIG_USER_ONLY
> +  .do_interrupt = ppc_cpu_do_interrupt,
> +  .cpu_exec_enter = ppc_cpu_exec_enter,
> +  .cpu_exec_exit = ppc_cpu_exec_exit,
> +  .do_unaligned_access = ppc_cpu_do_unaligned_access,
> +#endif /* !CONFIG_USER_ONLY */
> +};
> +
> +static const struct TCGCPUOps power7_tcg_ops = {
> +  .initialize = ppc_translate_init,
> +  .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
> +  .tlb_fill = ppc_cpu_tlb_fill,
> +
> +#ifndef CONFIG_USER_ONLY
> +  .do_interrupt = ppc_cpu_do_interrupt,
> +  .cpu_exec_enter = ppc_cpu_exec_enter,
> +  .cpu_exec_exit = ppc_cpu_exec_exit,
> +  .do_unaligned_access = ppc_cpu_do_unaligned_access,
> +#endif /* !CONFIG_USER_ONLY */
> +};
> +
> +static const struct TCGCPUOps power8_tcg_ops = {
> +  .initialize = ppc_translate_init,
> +  .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
> +  .tlb_fill = ppc_cpu_tlb_fill,
> +
> +#ifndef CONFIG_USER_ONLY
> +  .do_interrupt = ppc_cpu_do_interrupt,
> +  .cpu_exec_enter = ppc_cpu_exec_enter,
> +  .cpu_exec_exit = ppc_cpu_exec_exit,
> +  .do_unaligned_access = ppc_cpu_do_unaligned_access,
> +#endif /* !CONFIG_USER_ONLY */
> +};
> +
> +static const struct TCGCPUOps power9_tcg_ops = {
> +  .initialize = ppc_translate_init,
> +  .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
> +  .tlb_fill = ppc_cpu_tlb_fill,
> +
> +#ifndef CONFIG_USER_ONLY
> +  .do_interrupt = ppc_cpu_do_interrupt,
> +  .cpu_exec_enter = ppc_cpu_exec_enter,
> +  .cpu_exec_exit = ppc_cpu_exec_exit,
> +  .do_unaligned_access = ppc_cpu_do_unaligned_access,
> +#endif /* !CONFIG_USER_ONLY */
> +};
> +
> +static const struct TCGCPUOps power10_tcg_ops = {
> +  .initialize = ppc_translate_init,
> +  .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
> +  .tlb_fill = ppc_cpu_tlb_fill,
> +
>  #ifndef CONFIG_USER_ONLY
>.do_interrupt = ppc_cpu_do_interrupt,
>.cpu_exec_enter = ppc_cpu_exec_enter,

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [PATCH 41/44] hw/arm/mps2-tz: Make initsvtor0 setting board-specific

2021-03-04 Thread Richard Henderson

On 2/19/21 6:46 AM, Peter Maydell wrote:

The AN547 configures the SSE-300 with a different initsvtor0
setting from its default; make this a board-specific setting.

Signed-off-by: Peter Maydell
---


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 40/44] hw/arm/mps2-tz: Support running APB peripherals on different clock

2021-03-04 Thread Richard Henderson

On 2/19/21 6:46 AM, Peter Maydell wrote:

The AN547 runs the APB peripherals outside the SSE-300 on a different
and slightly slower clock than it runs the SSE-300 with.  Support
making the APB peripheral clock frequency board-specific.  (For our
implementation only the UARTs actually take a clock.)

Signed-off-by: Peter Maydell
---


Reviewed-by: Richard Henderson 

r~



Re: USB port claiming / set configuration problems

2021-03-04 Thread Ben Leslie
On Fri, 5 Mar 2021 at 01:31, Gerd Hoffmann  wrote:

>   Hi,
>
> > Would adding support to host-libusb to use these
> > ioctl to claim the port be beneficial?
>
> I don't feel like side-stepping libusb.  That is asking for trouble
> because usbdevfs might behave differently then and confuse libusb.
>
> So, if anything, libusb would need support that, then qemu can use it.
>
> > Based on a simple test program and
> > hardware USB traces for a device connected to a 'claimed' port the kernel
> > does indeed leave the device in an unconfigured state. (Although it still
> > performs some basic control transfers to gather descriptor, and strangely
> > seems to in this case make an explicit SET CONFIGURATION transfer, but
> sets
> > configuration to zero, rather than an actual configuration, which, at
> least
> > for the devices I was able to test with, avoided the problems of calling
> > SET CONFIGURATION (1) twice).
>
> We could try that too (set config to zero first, then set the config we
> actually want) and see if that works better.
>

This approach seems to work well, and let's me just use libusb, which is a
plus.

It seems I had actually misdiagnosed (or only partially diagnosed) the issue
with my 'problem devices'. It turned out that setting *any* valid
configuration
twice in a row causes problems for the device! So, for example, if the
current
configuration was set to 1, and then set configuration 2 was called that
would
also cause problems. I guess that drivers on other systems ensured that
such a sequence never occurred.

I reverted bfe44898848614cfcb3a269bc965afbe1f0f331c and made this change:

--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -955,6 +955,11 @@ static int usb_host_open(USBHostDevice *s,
libusb_device *dev, int hostfd)

 usb_host_detach_kernel(s);

+rc = libusb_set_configuration(s->dh, 0);
+if (rc != 0) {
+goto fail;
+}
+
 libusb_get_device_descriptor(dev, >ddesc);
 usb_host_get_port(s->dev, s->port, sizeof(s->port));

This appears to work for my use cases. (Although I still have more testing
to do).

In terms of the transaction on the wire, this is not quite as good as the
'claim port'
approach. Specifically, with the claim port after setting address and
getting some
basic descriptors the kernel will explicitly set configuration to zero and
not perform
any more transactions. Without the 'claim port' the kernel appears to
configure to
the first configuration and then read a few more descriptors. For my test
cases
at least this doesn't appear to be problematic, but I thought it was worth
calling
out the differences. Of course the great benefit of this approach is that
it uses
existing libusb functionality.

Cheers,

Ben


Re: [PATCH 39/44] hw/misc/mps2-scc: Implement changes for AN547

2021-03-04 Thread Richard Henderson

On 2/19/21 6:46 AM, Peter Maydell wrote:

Implement the minor changes required to the SCC block for AN547 images:
  * CFG2 and CFG5 exist (like AN524)
  * CFG3 is reserved (like AN524)
  * CFG0 bit 1 is CPU_WAIT; we don't implement it, but note this
in the TODO comment

Signed-off-by: Peter Maydell
---


Reviewed-by: Richard Henderson 

r~



Re: [RESEND][BUG FIX HELP] QEMU main thread endlessly hangs in __ppoll()

2021-03-04 Thread John Snow

On 2/28/21 9:39 PM, Like Xu wrote:

Hi Genius,

I am a user of QEMU v4.2.0 and stuck in an interesting bug, which may 
still exist in the mainline.

Thanks in advance to heroes who can take a look and share understanding.



Do you have a test case that reproduces on 5.2? It'd be nice to know if 
it was still a problem in the latest source tree or not.


--js


The qemu main thread endlessly hangs in the handle of the qmp statement:
{'execute': 'human-monitor-command', 'arguments':{ 'command-line': 
'drive_del replication0' } }

and we have the call trace looks like:

#0 0x7f3c22045bf6 in __ppoll (fds=0x555611328410, nfds=1, 
timeout=, timeout@entry=0x7ffc56c66db0,

sigmask=sigmask@entry=0x0) at ../sysdeps/unix/sysv/linux/ppoll.c:44
#1 0x55561021f415 in ppoll (__ss=0x0, __timeout=0x7ffc56c66db0, 
__nfds=, __fds=)

at /usr/include/x86_64-linux-gnu/bits/poll2.h:77
#2 qemu_poll_ns (fds=, nfds=, 
timeout=) at util/qemu-timer.c:348
#3 0x555610221430 in aio_poll (ctx=ctx@entry=0x5556113010f0, 
blocking=blocking@entry=true) at util/aio-posix.c:669
#4 0x55561019268d in bdrv_do_drained_begin (poll=true, 
ignore_bds_parents=false, parent=0x0, recursive=false,

bs=0x55561138b0a0) at block/io.c:430
#5 bdrv_do_drained_begin (bs=0x55561138b0a0, recursive=, 
parent=0x0, ignore_bds_parents=,

poll=) at block/io.c:396
#6 0x55561017b60b in quorum_del_child (bs=0x55561138b0a0, 
child=0x7f36dc0ce380, errp=)

at block/quorum.c:1063
#7 0x55560ff5836b in qmp_x_blockdev_change (parent=0x555612373120 
"colo-disk0", has_child=,
child=0x5556112df3e0 "children.1", has_node=, node=0x0, 
errp=0x7ffc56c66f98) at blockdev.c:4494
#8 0x5556100f8f57 in qmp_marshal_x_blockdev_change (args=out>, ret=, errp=0x7ffc56c67018)

at qapi/qapi-commands-block-core.c:1538
#9 0x5556101d8290 in do_qmp_dispatch (errp=0x7ffc56c67010, 
allow_oob=, request=,

cmds=0x5556109c69a0 ) at qapi/qmp-dispatch.c:132
#10 qmp_dispatch (cmds=0x5556109c69a0 , request=out>, allow_oob=)

at qapi/qmp-dispatch.c:175
#11 0x5556100d4c4d in monitor_qmp_dispatch (mon=0x5556113a6f40, 
req=) at monitor/qmp.c:145
#12 0x5556100d5437 in monitor_qmp_bh_dispatcher (data=out>) at monitor/qmp.c:234
#13 0x55561021dbec in aio_bh_call (bh=0x5556112164bGrateful0) at 
util/async.c:117

#14 aio_bh_poll (ctx=ctx@entry=0x5556112151b0) at util/async.c:117
#15 0x5556102212c4 in aio_dispatch (ctx=0x5556112151b0) at 
util/aio-posix.c:459
#16 0x55561021dab2 in aio_ctx_dispatch (source=, 
callback=, user_data=)

at util/async.c:260
#17 0x7f3c22302fbd in g_main_context_dispatch () from 
/lib/x86_64-linux-gnu/libglib-2.0.so.0

#18 0x555610220358 in glib_pollfds_poll () at util/main-loop.c:219
#19 os_host_main_loop_wait (timeout=) at 
util/main-loop.c:242

#20 main_loop_wait (nonblocking=) at util/main-loop.c:518
#21 0x55560ff600fe in main_loop () at vl.c:1814
#22 0x55560fddbce9 in main (argc=, argv=out>, envp=) at vl.c:4503


We found that we're doing endless check in the line of 
block/io.c:bdrv_do_drained_begin():

 BDRV_POLL_WHILE(bs, bdrv_drain_poll_top_level(bs, recursive, parent));
and it turns out that the bdrv_drain_poll() always get true from:
- bdrv_parent_drained_poll(bs, ignore_parent, ignore_bds_parents)
- AND atomic_read(>in_flight)

I personally think this is a deadlock issue in the a QEMU block layer
(as we know, we have some #FIXME comments in related codes, such as 
block permisson update).

Any comments are welcome and appreciated.

---
thx,likexu






Re: [PATCH] hw/sd/sdhci: Report error when guest access protected registers

2021-03-04 Thread John Snow

On 2/18/21 12:56 PM, Philippe Mathieu-Daudé wrote:

The SDHC_SYSAD and SDHC_BLKSIZE can not be accessed while a
transaction is in progress, see 'SD Host Controller Simplified
Specification'

   1.5) SD Command Generation

   The Host Driver should not read the SDMA System Address, Block
   Size and Block Count registers during a data transaction unless
   the transfer is stopped because the value is changing and not
   stable.



Naive question: Is this an RFC2119 "SHOULD NOT"? (i.e., does it have a 
defined behavior that you are simply encouraged to avoid?)


Is it really an error?


Report guest intents as errors.

Signed-off-by: Philippe Mathieu-Daudé 
---
Based-on: <1613447214-81951-1-git-send-email-bmeng...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/sd/sdhci.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 5b8678110b0..98928c18542 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1136,6 +1136,10 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
  sdhci_sdma_transfer_single_block(s);
  }
  }
+} else {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: Transfer already in progress,"
+  " can not update SYSAD", __func__);
  }
  break;
  case SDHC_BLKSIZE:
@@ -1163,8 +1167,11 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
  if (blksize != s->blksize) {
  s->data_count = 0;
  }
+} else {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: Transfer already in progress,"
+  " can not update BLKSIZE", __func__);
  }
-
  break;
  case SDHC_ARGUMENT:
  MASKED_WRITE(s->argument, mask, value);






Re: [PATCH v2] multi-process: Initialize variables declared with g_auto*

2021-03-04 Thread Philippe Mathieu-Daudé
On 3/4/21 3:16 AM, Zenghui Yu wrote:
> Quote docs/devel/style.rst (section "Automatic memory deallocation"):
> 
> * Variables declared with g_auto* MUST always be initialized,
>   otherwise the cleanup function will use uninitialized stack memory
> 
> Initialize @name properly to get rid of the compilation error:

Maybe worth adding "(using gcc-7.3.0 on CentOS):"

> 
> ../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
> /usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be 
> used uninitialized in this function [-Werror=maybe-uninitialized]
>g_free (*pp);
>^~~~
> ../hw/remote/proxy.c:350:30: note: 'name' was declared here
>  g_autofree char *name;
>   ^~~~
> 
> Signed-off-by: Zenghui Yu 
> Reviewed-by: Jagannathan Raman 
> ---
> * From v1:
>   - Move the suffix iteration out of the loop (Philippe)
>   - Add Jagannathan's R-b
> 
>  hw/remote/memory.c | 5 ++---
>  hw/remote/proxy.c  | 3 +--
>  2 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/remote/memory.c b/hw/remote/memory.c
> index 32085b1e05..d97947d4b8 100644
> --- a/hw/remote/memory.c
> +++ b/hw/remote/memory.c
> @@ -42,10 +42,9 @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
>  
>  remote_sysmem_reset();
>  
> -for (region = 0; region < msg->num_fds; region++) {
> -g_autofree char *name;
> +for (region = 0; region < msg->num_fds; region++, suffix++) {
> +g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix);
>  subregion = g_new(MemoryRegion, 1);
> -name = g_strdup_printf("remote-mem-%u", suffix++);
>  memory_region_init_ram_from_fd(subregion, NULL,
> name, sysmem_info->sizes[region],
> true, msg->fds[region],
> diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
> index 4fa4be079d..6dda705fc2 100644
> --- a/hw/remote/proxy.c
> +++ b/hw/remote/proxy.c
> @@ -347,13 +347,12 @@ static void probe_pci_info(PCIDevice *dev, Error **errp)
> PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY;
>  
>  if (size) {
> -g_autofree char *name;
> +g_autofree char *name = g_strdup_printf("bar-region-%d", i);
>  pdev->region[i].dev = pdev;
>  pdev->region[i].present = true;
>  if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) {
>  pdev->region[i].memory = true;
>  }
> -name = g_strdup_printf("bar-region-%d", i);
>  memory_region_init_io(>region[i].mr, OBJECT(pdev),
>_mr_ops, >region[i],
>name, size);
> 




Re: [PATCH v2] multi-process: Initialize variables declared with g_auto*

2021-03-04 Thread Philippe Mathieu-Daudé
On 3/4/21 3:16 AM, Zenghui Yu wrote:
> Quote docs/devel/style.rst (section "Automatic memory deallocation"):
> 
> * Variables declared with g_auto* MUST always be initialized,
>   otherwise the cleanup function will use uninitialized stack memory
> 
> Initialize @name properly to get rid of the compilation error:
> 
> ../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
> /usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be 
> used uninitialized in this function [-Werror=maybe-uninitialized]
>g_free (*pp);
>^~~~
> ../hw/remote/proxy.c:350:30: note: 'name' was declared here
>  g_autofree char *name;
>   ^~~~
> 
> Signed-off-by: Zenghui Yu 
> Reviewed-by: Jagannathan Raman 
> ---
> * From v1:
>   - Move the suffix iteration out of the loop (Philippe)
>   - Add Jagannathan's R-b
> 
>  hw/remote/memory.c | 5 ++---
>  hw/remote/proxy.c  | 3 +--
>  2 files changed, 3 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH v3 00/42] esp: consolidate PDMA transfer buffers and other fixes

2021-03-04 Thread Philippe Mathieu-Daudé
On 3/4/21 11:10 PM, Mark Cave-Ayland wrote:
> This patch series comes from an experimental branch that I've been working on
> to try and boot a MacOS toolbox ROM under the QEMU q800 machine. The effort is
...
> v3:
> - Rebase onto master (fix up minor conflicts with Paolo's SCSI error handling 
> changes)
> - Add R-B tags from Philippe and Laurent
> - Check for failure of qdev_realize() in patch 3
> - Touch up the commit messages on patches 9 and 10
> - Remove extra "& 0xff" in patch 9
> - Add deferred command completion interrupt for PDMA in patch 33
> - Remove ti_size assignment comment in patch 38
> - Remove extra "& 0xff" in patch 39

Thanks, series LGTM!

Phil.



Re: [PATCH v3 1/5] char: add goldfish-tty

2021-03-04 Thread Philippe Mathieu-Daudé
On 3/4/21 11:01 PM, Laurent Vivier wrote:
> Implement the goldfish tty device as defined in
> 
> https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
> 
> and based on the kernel driver code:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/goldfish.c
> 
> Signed-off-by: Laurent Vivier 
> ---
>  include/hw/char/goldfish_tty.h |  36 +
>  hw/char/goldfish_tty.c | 266 +
>  hw/char/Kconfig|   3 +
>  hw/char/meson.build|   2 +
>  hw/char/trace-events   |   9 ++
>  5 files changed, 316 insertions(+)
>  create mode 100644 include/hw/char/goldfish_tty.h
>  create mode 100644 hw/char/goldfish_tty.c
> 
> diff --git a/include/hw/char/goldfish_tty.h b/include/hw/char/goldfish_tty.h
> new file mode 100644
> index ..84d78f8cff54
> --- /dev/null
> +++ b/include/hw/char/goldfish_tty.h
> @@ -0,0 +1,36 @@
> +/*
> + * SPDX-License-Identifer: GPL-2.0-or-later
> + *
> + * Goldfish TTY
> + *
> + * (c) 2020 Laurent Vivier 
> + *
> + */
> +
> +#ifndef HW_CHAR_GOLDFISH_TTY_H
> +#define HW_CHAR_GOLDFISH_TTY_H
> +
> +#include "chardev/char-fe.h"
> +
> +#define TYPE_GOLDFISH_TTY "goldfish_tty"
> +OBJECT_DECLARE_SIMPLE_TYPE(GoldfishTTYState, GOLDFISH_TTY)
> +
> +#define GOLFISH_TTY_BUFFER_SIZE 128
> +
> +struct GoldfishTTYState {
> +SysBusDevice parent_obj;
> +
> +MemoryRegion iomem;
> +qemu_irq irq;
> +CharBackend chr;
> +
> +uint32_t data_len;
> +uint64_t data_ptr;
> +bool int_enabled;
> +
> +uint32_t data_in_count;
> +uint8_t data_in[GOLFISH_TTY_BUFFER_SIZE];
> +uint8_t data_out[GOLFISH_TTY_BUFFER_SIZE];

Could we use Fifo8 instead?

> +};



Re: [PATCH v3 4/7] hw/core: implement a guest-loader to support static hypervisor guests

2021-03-04 Thread Alistair Francis
On Wed, Mar 3, 2021 at 12:37 PM Alex Bennée  wrote:
>
> Hypervisors, especially type-1 ones, need the firmware/bootcode to put
> their initial guest somewhere in memory and pass the information to it
> via platform data. The guest-loader is modelled after the generic
> loader for exactly this sort of purpose:
>
>   $QEMU $ARGS  -kernel ~/xen.git/xen/xen \
> -append "dom0_mem=1G,max:1G loglvl=all guest_loglvl=all" \
> -device 
> guest-loader,addr=0x4200,kernel=Image,bootargs="root=/dev/sda2 ro 
> console=hvc0 earlyprintk=xen" \
> -device guest-loader,addr=0x4700,initrd=rootfs.cpio
>
> Signed-off-by: Alex Bennée 
> Message-Id: <20201105175153.30489-5-alex.ben...@linaro.org>
> Message-Id: <20210211171945.18313-5-alex.ben...@linaro.org>

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/core/guest-loader.h |  34 ++
>  hw/core/guest-loader.c | 145 +
>  MAINTAINERS|   5 ++
>  hw/core/meson.build|   2 +
>  4 files changed, 186 insertions(+)
>  create mode 100644 hw/core/guest-loader.h
>  create mode 100644 hw/core/guest-loader.c
>
> diff --git a/hw/core/guest-loader.h b/hw/core/guest-loader.h
> new file mode 100644
> index 00..07f4b4884b
> --- /dev/null
> +++ b/hw/core/guest-loader.h
> @@ -0,0 +1,34 @@
> +/*
> + * Guest Loader
> + *
> + * Copyright (C) 2020 Linaro
> + * Written by Alex Bennée 
> + * (based on the generic-loader by Li Guang )
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef GUEST_LOADER_H
> +#define GUEST_LOADER_H
> +
> +#include "hw/qdev-core.h"
> +#include "qom/object.h"
> +
> +struct GuestLoaderState {
> +/*  */
> +DeviceState parent_obj;
> +
> +/*  */
> +uint64_t addr;
> +char *kernel;
> +char *args;
> +char *initrd;
> +};
> +
> +#define TYPE_GUEST_LOADER "guest-loader"
> +OBJECT_DECLARE_SIMPLE_TYPE(GuestLoaderState, GUEST_LOADER)
> +
> +#endif
> diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
> new file mode 100644
> index 00..bde44e27b4
> --- /dev/null
> +++ b/hw/core/guest-loader.c
> @@ -0,0 +1,145 @@
> +/*
> + * Guest Loader
> + *
> + * Copyright (C) 2020 Linaro
> + * Written by Alex Bennée 
> + * (based on the generic-loader by Li Guang )
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +/*
> + * Much like the generic-loader this is treated as a special device
> + * inside QEMU. However unlike the generic-loader this device is used
> + * to load guest images for hypervisors. As part of that process the
> + * hypervisor needs to have platform information passed to it by the
> + * lower levels of the stack (e.g. firmware/bootloader). If you boot
> + * the hypervisor directly you use the guest-loader to load the Dom0
> + * or equivalent guest images in the right place in the same way a
> + * boot loader would.
> + *
> + * This is only relevant for full system emulation.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/core/cpu.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/dma.h"
> +#include "hw/loader.h"
> +#include "hw/qdev-properties.h"
> +#include "qapi/error.h"
> +#include "qemu/module.h"
> +#include "guest-loader.h"
> +#include "sysemu/device_tree.h"
> +#include "hw/boards.h"
> +
> +/*
> + * Insert some FDT nodes for the loaded blob.
> + */
> +static void loader_insert_platform_data(GuestLoaderState *s, int size,
> +Error **errp)
> +{
> +MachineState *machine = MACHINE(qdev_get_machine());
> +void *fdt = machine->fdt;
> +g_autofree char *node = g_strdup_printf("/chosen/module@0x%08" PRIx64,
> +s->addr);
> +uint64_t reg_attr[2] = {cpu_to_be64(s->addr), cpu_to_be64(size)};
> +
> +if (!fdt) {
> +error_setg(errp, "Cannot modify FDT fields if the machine has none");
> +return;
> +}
> +
> +qemu_fdt_add_subnode(fdt, node);
> +qemu_fdt_setprop(fdt, node, "reg", _attr, sizeof(reg_attr));
> +
> +if (s->kernel) {
> +const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
> +if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
> +  (char **) ,
> +  ARRAY_SIZE(compat)) < 0) {
> +error_setg(errp, "couldn't set %s/compatible", node);
> +return;
> +}
> +if (s->args) {
> +if (qemu_fdt_setprop_string(fdt, node, "bootargs", s->args) < 0) 
> {
> +error_setg(errp, "couldn't set %s/bootargs", node);
> +}
> +}
> +} else if (s->initrd) {
> +const char *compat[2] = { "multiboot,module", 

Re: [PATCH v3 2/5] intc: add goldfish-pic

2021-03-04 Thread Philippe Mathieu-Daudé



On 3/4/21 11:01 PM, Laurent Vivier wrote:
> Implement the goldfish pic device as defined in
> 
> https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
> 
> Signed-off-by: Laurent Vivier 
> ---
>  include/hw/intc/goldfish_pic.h |  33 +
>  hw/intc/goldfish_pic.c | 214 +
>  hw/intc/Kconfig|   3 +
>  hw/intc/meson.build|   1 +
>  hw/intc/trace-events   |   8 ++
>  5 files changed, 259 insertions(+)
>  create mode 100644 include/hw/intc/goldfish_pic.h
>  create mode 100644 hw/intc/goldfish_pic.c

> +static const MemoryRegionOps goldfish_pic_ops = {
> +.read = goldfish_pic_read,
> +.write = goldfish_pic_write,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +.valid.max_access_size = 4,

Missing:

   .impl.min_access_size = 4,

> +.impl.max_access_size = 4,
> +};
> +
> +static void goldfish_pic_reset(DeviceState *dev)
> +{
> +GoldfishPICState *s = GOLDFISH_PIC(dev);
> +int i;
> +
> +trace_goldfish_pic_reset(s, s->idx);
> +s->pending = 0;
> +s->enabled = 0;
> +
> +for (i = 0; i < ARRAY_SIZE(s->stats_irq_count); i++) {
> +s->stats_irq_count[i] = 0;
> +}
> +}
> +
> +static void goldfish_pic_realize(DeviceState *dev, Error **errp)
> +{
> +GoldfishPICState *s = GOLDFISH_PIC(dev);
> +static int counter;
> +
> +s->idx = counter++;

Isn't it a bit fragile? Aren't we better using DEFINE_PROP_UINT8()?

> +trace_goldfish_pic_realize(s, s->idx);
> +
> +memory_region_init_io(>iomem, OBJECT(s), _pic_ops, s,
> +  "goldfish_pic", 0x24);
> +}

Adding .impl.min_access_size:
Reviewed-by: Philippe Mathieu-Daudé 



Re: [PATCH v2 2/2] memory: Drop "qemu:" prefix from QOM memory region type names

2021-03-04 Thread Alistair Francis
On Thu, Mar 4, 2021 at 9:03 AM Markus Armbruster  wrote:
>
> Almost all QOM type names consist only of letters, digits, '-', '_',
> and '.'.  Just two contain ':': "qemu:memory-region" and
> "qemu:iommu-memory-region".  Neither can be plugged with -object.
> Rename them to "memory-region" and "iommu-memory-region".
>
> Signed-off-by: Markus Armbruster 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  include/exec/memory.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index c6fb714e49..3c95d7831a 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -33,11 +33,11 @@
>  #define MAX_PHYS_ADDR_SPACE_BITS 62
>  #define MAX_PHYS_ADDR(((hwaddr)1 << MAX_PHYS_ADDR_SPACE_BITS) - 
> 1)
>
> -#define TYPE_MEMORY_REGION "qemu:memory-region"
> +#define TYPE_MEMORY_REGION "memory-region"
>  DECLARE_INSTANCE_CHECKER(MemoryRegion, MEMORY_REGION,
>   TYPE_MEMORY_REGION)
>
> -#define TYPE_IOMMU_MEMORY_REGION "qemu:iommu-memory-region"
> +#define TYPE_IOMMU_MEMORY_REGION "iommu-memory-region"
>  typedef struct IOMMUMemoryRegionClass IOMMUMemoryRegionClass;
>  DECLARE_OBJ_CHECKERS(IOMMUMemoryRegion, IOMMUMemoryRegionClass,
>   IOMMU_MEMORY_REGION, TYPE_IOMMU_MEMORY_REGION)
> --
> 2.26.2
>
>



Re: [PATCH v3 3/5] tests/acceptance/boot_linux_console: change URL for test_arm_orangepi_bionic_20_08

2021-03-04 Thread Philippe Mathieu-Daudé
On 3/4/21 9:35 PM, Niek Linnenbank wrote:
> Update the download URL of the Armbian 20.08 Bionic image for
> test_arm_orangepi_bionic_20_08 of the orangepi-pc machine.
> 
> The archive.armbian.com URL contains more images and should keep stable
> for a longer period of time than dl.armbian.com.
> 
> Signed-off-by: Niek Linnenbank 
> ---
>  tests/acceptance/boot_linux_console.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 




Re: [PATCH v3 3/5] m68k: add an interrupt controller

2021-03-04 Thread Philippe Mathieu-Daudé
On 3/4/21 11:01 PM, Laurent Vivier wrote:
> A (generic) copy of the GLUE device we already have for q800 to use with
> the m68k-virt machine.
> The q800 one would disappear in the future as q800 uses actually the djMEMC
> controller.
> 
> Signed-off-by: Laurent Vivier 
> ---
>  include/hw/intc/m68k_irqc.h |  41 +
>  hw/intc/m68k_irqc.c | 119 
>  hw/intc/Kconfig |   3 +
>  hw/intc/meson.build |   1 +
>  4 files changed, 164 insertions(+)
>  create mode 100644 include/hw/intc/m68k_irqc.h
>  create mode 100644 hw/intc/m68k_irqc.c

Reviewed-by: Philippe Mathieu-Daudé 



[RFC PATCH v2 6/8] cpu: Declare cpu_has_work() in 'sysemu/tcg.h'

2021-03-04 Thread Philippe Mathieu-Daudé
We can only check if a vCPU has work with TCG.
Move the cpu_has_work() prototype to "sysemu/tcg.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
RFC: could another accelerator do that?
 can we rename this tcg_vcpu_has_work()?
---
 include/hw/core/cpu.h | 16 
 include/sysemu/tcg.h  | 11 +++
 accel/tcg/cpu-exec.c  |  7 +++
 softmmu/cpus.c|  1 +
 4 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 1376e496a3f..66109bcca35 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -670,22 +670,6 @@ CPUState *cpu_create(const char *typename);
  */
 const char *parse_cpu_option(const char *cpu_option);
 
-/**
- * cpu_has_work:
- * @cpu: The vCPU to check.
- *
- * Checks whether the CPU has work to do.
- *
- * Returns: %true if the CPU has work, %false otherwise.
- */
-static inline bool cpu_has_work(CPUState *cpu)
-{
-CPUClass *cc = CPU_GET_CLASS(cpu);
-
-g_assert(cc->has_work);
-return cc->has_work(cpu);
-}
-
 /**
  * qemu_cpu_is_self:
  * @cpu: The vCPU to check against.
diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
index c16c13c3c69..3d46b0a7a93 100644
--- a/include/sysemu/tcg.h
+++ b/include/sysemu/tcg.h
@@ -10,6 +10,7 @@
 
 #ifndef CONFIG_TCG
 #define tcg_enabled() 0
+#define cpu_has_work(cpu) false
 #else
 
 void tcg_exec_init(unsigned long tb_size, int splitwx);
@@ -26,6 +27,16 @@ extern bool tcg_allowed;
 extern bool mttcg_enabled;
 #define qemu_tcg_mttcg_enabled() (mttcg_enabled)
 
+/**
+ * cpu_has_work:
+ * @cpu: The vCPU to check.
+ *
+ * Checks whether the CPU has work to do.
+ *
+ * Returns: %true if the CPU has work, %false otherwise.
+ */
+bool cpu_has_work(CPUState *cpu);
+
 #endif /* CONFIG_TCG */
 
 #endif
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 7e67ade35b9..b9ce36e59e2 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -447,6 +447,13 @@ static inline TranslationBlock *tb_find(CPUState *cpu,
 return tb;
 }
 
+bool cpu_has_work(CPUState *cpu)
+{
+CPUClass *cc = CPU_GET_CLASS(cpu);
+
+return cc->has_work(cpu);
+}
+
 static inline bool cpu_handle_halt(CPUState *cpu)
 {
 if (cpu->halted) {
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index a7ee431187a..548ab9236f1 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -42,6 +42,7 @@
 #include "sysemu/runstate.h"
 #include "sysemu/cpu-timers.h"
 #include "sysemu/whpx.h"
+#include "sysemu/tcg.h"
 #include "hw/boards.h"
 #include "hw/hw.h"
 
-- 
2.26.2




[RFC PATCH v2 8/8] target/arm: Restrict arm_cpu_has_work() to TCG

2021-03-04 Thread Philippe Mathieu-Daudé
arm_cpu_has_work() is only used from TCG.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/internals.h | 2 +-
 target/arm/cpu.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 1930be08828..db81db9bf57 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -172,8 +172,8 @@ static inline int r14_bank_number(int mode)
 void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
 void arm_translate_init(void);
 
-bool arm_cpu_has_work(CPUState *cs);
 #ifdef CONFIG_TCG
+bool arm_cpu_has_work(CPUState *cs);
 void arm_cpu_synchronize_from_tb(CPUState *cs, const TranslationBlock *tb);
 #endif /* CONFIG_TCG */
 
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 7181deee84a..02db969c00f 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -74,7 +74,6 @@ void arm_cpu_synchronize_from_tb(CPUState *cs,
 env->regs[15] = tb->pc;
 }
 }
-#endif /* CONFIG_TCG */
 
 bool arm_cpu_has_work(CPUState *cs)
 {
@@ -86,6 +85,7 @@ bool arm_cpu_has_work(CPUState *cs)
  | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ
  | CPU_INTERRUPT_EXITTB);
 }
+#endif /* CONFIG_TCG */
 
 void arm_register_pre_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook,
  void *opaque)
-- 
2.26.2




[RFC PATCH v2 1/8] sysemu/tcg: Restrict tcg_exec_init() to CONFIG_TCG

2021-03-04 Thread Philippe Mathieu-Daudé
Invert the #ifdef'ry to easily restrict tcg_exec_init() declaration
to CONFIG_TCG.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/sysemu/tcg.h | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
index 00349fb18a7..fddde2b6b9a 100644
--- a/include/sysemu/tcg.h
+++ b/include/sysemu/tcg.h
@@ -8,13 +8,15 @@
 #ifndef SYSEMU_TCG_H
 #define SYSEMU_TCG_H
 
+#ifndef CONFIG_TCG
+#define tcg_enabled() 0
+#else
+
 void tcg_exec_init(unsigned long tb_size, int splitwx);
 
-#ifdef CONFIG_TCG
 extern bool tcg_allowed;
 #define tcg_enabled() (tcg_allowed)
-#else
-#define tcg_enabled() 0
-#endif
+
+#endif /* CONFIG_TCG */
 
 #endif
-- 
2.26.2




[RFC PATCH v2 7/8] cpu: Move CPUClass::has_work() to TCGCPUOps

2021-03-04 Thread Philippe Mathieu-Daudé
We can only check if a vCPU has work with TCG.
Restrict the has_work() handler to TCG by moving it to
the TCGCPUOps structure, and adapt all the targets.

cpu_common_has_work() is removed as being inlined in
cpu_has_work().

Reviewed-by: Taylor Simpson 
Signed-off-by: Philippe Mathieu-Daudé 
---
v2:
- finished PPC
- check cc->tcg_ops->has_work non-null (thuth)
---
 include/hw/core/cpu.h   |  2 --
 include/hw/core/tcg-cpu-ops.h   |  4 
 accel/tcg/cpu-exec.c|  6 +-
 hw/core/cpu.c   |  6 --
 target/alpha/cpu.c  |  2 +-
 target/arm/cpu.c|  2 +-
 target/avr/cpu.c|  2 +-
 target/cris/cpu.c   |  3 ++-
 target/hexagon/cpu.c|  2 +-
 target/hppa/cpu.c   |  2 +-
 target/i386/cpu.c   |  7 +--
 target/i386/tcg/tcg-cpu.c   |  6 ++
 target/lm32/cpu.c   |  2 +-
 target/m68k/cpu.c   |  2 +-
 target/microblaze/cpu.c |  2 +-
 target/mips/cpu.c   |  2 +-
 target/moxie/cpu.c  |  2 +-
 target/nios2/cpu.c  |  2 +-
 target/openrisc/cpu.c   |  2 +-
 target/riscv/cpu.c  |  2 +-
 target/rx/cpu.c |  2 +-
 target/s390x/cpu.c  |  2 +-
 target/sh4/cpu.c|  2 +-
 target/sparc/cpu.c  |  2 +-
 target/tilegx/cpu.c |  2 +-
 target/tricore/cpu.c|  2 +-
 target/unicore32/cpu.c  |  2 +-
 target/xtensa/cpu.c |  2 +-
 target/ppc/translate_init.c.inc | 10 +-
 29 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 66109bcca35..8efea289e7e 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -86,7 +86,6 @@ struct AccelCPUClass;
  * instantiatable CPU type.
  * @parse_features: Callback to parse command line arguments.
  * @reset_dump_flags: #CPUDumpFlags to use for reset logging.
- * @has_work: Callback for checking if there is work to do.
  * @virtio_is_big_endian: Callback to return %true if a CPU which supports
  * runtime configurable endianness is currently big-endian. Non-configurable
  * CPUs can use the default implementation of this method. This method should
@@ -149,7 +148,6 @@ struct CPUClass {
 void (*parse_features)(const char *typename, char *str, Error **errp);
 
 int reset_dump_flags;
-bool (*has_work)(CPUState *cpu);
 bool (*virtio_is_big_endian)(CPUState *cpu);
 int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
uint8_t *buf, int len, bool is_write);
diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index 72d791438c2..f5d44ba59f3 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -19,6 +19,10 @@ struct TCGCPUOps {
  * Called when the first CPU is realized.
  */
 void (*initialize)(void);
+/**
+ * @has_work: Callback for checking if there is work to do
+ */
+bool (*has_work)(CPUState *cpu);
 /**
  * @synchronize_from_tb: Synchronize state from a TCG #TranslationBlock
  *
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index b9ce36e59e2..4e73550f981 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -451,7 +451,11 @@ bool cpu_has_work(CPUState *cpu)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
-return cc->has_work(cpu);
+if (cc->tcg_ops->has_work) {
+return cc->tcg_ops->has_work(cpu);
+}
+
+return false;
 }
 
 static inline bool cpu_handle_halt(CPUState *cpu)
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 00330ba07de..3110867c3a3 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -261,11 +261,6 @@ static void cpu_common_reset(DeviceState *dev)
 }
 }
 
-static bool cpu_common_has_work(CPUState *cs)
-{
-return false;
-}
-
 ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
 {
 CPUClass *cc = CPU_CLASS(object_class_by_name(typename));
@@ -397,7 +392,6 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 
 k->parse_features = cpu_common_parse_features;
 k->get_arch_id = cpu_common_get_arch_id;
-k->has_work = cpu_common_has_work;
 k->get_paging_enabled = cpu_common_get_paging_enabled;
 k->get_memory_mapping = cpu_common_get_memory_mapping;
 k->write_elf32_qemunote = cpu_common_write_elf32_qemunote;
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index e50ae7bef06..57e88bbe7fd 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -210,6 +210,7 @@ static void alpha_cpu_initfn(Object *obj)
 
 static const struct TCGCPUOps alpha_tcg_ops = {
 .initialize = alpha_translate_init,
+.has_work = alpha_cpu_has_work,
 .cpu_exec_interrupt = alpha_cpu_exec_interrupt,
 .tlb_fill = alpha_cpu_tlb_fill,
 
@@ -230,7 +231,6 @@ static void alpha_cpu_class_init(ObjectClass *oc, void 
*data)
 >parent_realize);
 
 

[RFC PATCH v2 5/8] target/ppc: Duplicate the TCGCPUOps structure for POWER CPUs

2021-03-04 Thread Philippe Mathieu-Daudé
POWER CPUs have specific CPUClass::has_work() handlers.
In preparation of moving this field to TCGCPUOps, we need
to duplicate the current ppc_tcg_ops structure for the
POWER cpus.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/ppc/translate_init.c.inc | 69 +
 1 file changed, 69 insertions(+)

diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index 80239077e0b..fe76d0b3773 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -48,6 +48,11 @@
 /* #define PPC_DUMP_SPR_ACCESSES */
 /* #define USE_APPLE_GDB */
 
+static const struct TCGCPUOps power7_tcg_ops;
+static const struct TCGCPUOps power8_tcg_ops;
+static const struct TCGCPUOps power9_tcg_ops;
+static const struct TCGCPUOps power10_tcg_ops;
+
 /*
  * Generic callbacks:
  * do nothing but store/retrieve spr value
@@ -8685,6 +8690,9 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
 pcc->l1_dcache_size = 0x8000;
 pcc->l1_icache_size = 0x8000;
 pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
+#ifdef CONFIG_TCG
+cc->tcg_ops = _tcg_ops;
+#endif /* CONFIG_TCG */
 }
 
 static void init_proc_POWER8(CPUPPCState *env)
@@ -8863,6 +8871,9 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 pcc->l1_dcache_size = 0x8000;
 pcc->l1_icache_size = 0x8000;
 pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
+#ifdef CONFIG_TCG
+cc->tcg_ops = _tcg_ops;
+#endif /* CONFIG_TCG */
 }
 
 #ifdef CONFIG_SOFTMMU
@@ -9081,6 +9092,9 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
 pcc->l1_dcache_size = 0x8000;
 pcc->l1_icache_size = 0x8000;
 pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
+#ifdef CONFIG_TCG
+cc->tcg_ops = _tcg_ops;
+#endif /* CONFIG_TCG */
 }
 
 #ifdef CONFIG_SOFTMMU
@@ -9292,6 +9306,9 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
 pcc->l1_dcache_size = 0x8000;
 pcc->l1_icache_size = 0x8000;
 pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
+#ifdef CONFIG_TCG
+cc->tcg_ops = _tcg_ops;
+#endif /* CONFIG_TCG */
 }
 
 #if !defined(CONFIG_USER_ONLY)
@@ -10851,6 +10868,58 @@ static const struct TCGCPUOps ppc_tcg_ops = {
   .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
   .tlb_fill = ppc_cpu_tlb_fill,
 
+#ifndef CONFIG_USER_ONLY
+  .do_interrupt = ppc_cpu_do_interrupt,
+  .cpu_exec_enter = ppc_cpu_exec_enter,
+  .cpu_exec_exit = ppc_cpu_exec_exit,
+  .do_unaligned_access = ppc_cpu_do_unaligned_access,
+#endif /* !CONFIG_USER_ONLY */
+};
+
+static const struct TCGCPUOps power7_tcg_ops = {
+  .initialize = ppc_translate_init,
+  .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
+  .tlb_fill = ppc_cpu_tlb_fill,
+
+#ifndef CONFIG_USER_ONLY
+  .do_interrupt = ppc_cpu_do_interrupt,
+  .cpu_exec_enter = ppc_cpu_exec_enter,
+  .cpu_exec_exit = ppc_cpu_exec_exit,
+  .do_unaligned_access = ppc_cpu_do_unaligned_access,
+#endif /* !CONFIG_USER_ONLY */
+};
+
+static const struct TCGCPUOps power8_tcg_ops = {
+  .initialize = ppc_translate_init,
+  .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
+  .tlb_fill = ppc_cpu_tlb_fill,
+
+#ifndef CONFIG_USER_ONLY
+  .do_interrupt = ppc_cpu_do_interrupt,
+  .cpu_exec_enter = ppc_cpu_exec_enter,
+  .cpu_exec_exit = ppc_cpu_exec_exit,
+  .do_unaligned_access = ppc_cpu_do_unaligned_access,
+#endif /* !CONFIG_USER_ONLY */
+};
+
+static const struct TCGCPUOps power9_tcg_ops = {
+  .initialize = ppc_translate_init,
+  .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
+  .tlb_fill = ppc_cpu_tlb_fill,
+
+#ifndef CONFIG_USER_ONLY
+  .do_interrupt = ppc_cpu_do_interrupt,
+  .cpu_exec_enter = ppc_cpu_exec_enter,
+  .cpu_exec_exit = ppc_cpu_exec_exit,
+  .do_unaligned_access = ppc_cpu_do_unaligned_access,
+#endif /* !CONFIG_USER_ONLY */
+};
+
+static const struct TCGCPUOps power10_tcg_ops = {
+  .initialize = ppc_translate_init,
+  .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
+  .tlb_fill = ppc_cpu_tlb_fill,
+
 #ifndef CONFIG_USER_ONLY
   .do_interrupt = ppc_cpu_do_interrupt,
   .cpu_exec_enter = ppc_cpu_exec_enter,
-- 
2.26.2




[PATCH v3 37/42] esp: transition to message out phase after SATN and stop command

2021-03-04 Thread Mark Cave-Ayland
The SCSI bus should remain in the message out phase after the SATN and stop
command rather than transitioning to the command phase. A new ESPState variable
cmdbuf_cdb_offset is added which stores the offset of the CDB from the start
of cmdbuf when accumulating extended message out phase data.

Currently any extended message out data is discarded in do_cmd() before the CDB
is processed in do_busid_cmd().

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 72 ++-
 include/hw/scsi/esp.h |  2 ++
 2 files changed, 60 insertions(+), 14 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 23fcaa90c1..0d5c07e4c1 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -272,13 +272,15 @@ static void do_cmd(ESPState *s)
 uint8_t *buf = s->cmdbuf;
 uint8_t busid = buf[0];
 
-do_busid_cmd(s, [1], busid);
+/* Ignore extended messages for now */
+do_busid_cmd(s, [s->cmdbuf_cdb_offset], busid);
 }
 
 static void satn_pdma_cb(ESPState *s)
 {
 s->do_cmd = 0;
 if (s->cmdlen) {
+s->cmdbuf_cdb_offset = 1;
 do_cmd(s);
 }
 }
@@ -295,6 +297,7 @@ static void handle_satn(ESPState *s)
 cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
 if (cmdlen > 0) {
 s->cmdlen = cmdlen;
+s->cmdbuf_cdb_offset = 1;
 do_cmd(s);
 } else if (cmdlen == 0) {
 s->cmdlen = 0;
@@ -309,6 +312,7 @@ static void s_without_satn_pdma_cb(ESPState *s)
 {
 s->do_cmd = 0;
 if (s->cmdlen) {
+s->cmdbuf_cdb_offset = 0;
 do_busid_cmd(s, s->cmdbuf, 0);
 }
 }
@@ -325,6 +329,7 @@ static void handle_s_without_atn(ESPState *s)
 cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
 if (cmdlen > 0) {
 s->cmdlen = cmdlen;
+s->cmdbuf_cdb_offset = 0;
 do_busid_cmd(s, s->cmdbuf, 0);
 } else if (cmdlen == 0) {
 s->cmdlen = 0;
@@ -341,6 +346,7 @@ static void satn_stop_pdma_cb(ESPState *s)
 if (s->cmdlen) {
 trace_esp_handle_satn_stop(s->cmdlen);
 s->do_cmd = 1;
+s->cmdbuf_cdb_offset = 1;
 s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
 s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
@@ -357,21 +363,22 @@ static void handle_satn_stop(ESPState *s)
 return;
 }
 s->pdma_cb = satn_stop_pdma_cb;
-cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
+cmdlen = get_cmd(s, 1);
 if (cmdlen > 0) {
-trace_esp_handle_satn_stop(s->cmdlen);
+trace_esp_handle_satn_stop(cmdlen);
 s->cmdlen = cmdlen;
 s->do_cmd = 1;
-s->rregs[ESP_RSTAT] = STAT_CD;
+s->cmdbuf_cdb_offset = 1;
+s->rregs[ESP_RSTAT] = STAT_MO;
 s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-s->rregs[ESP_RSEQ] = SEQ_CD;
+s->rregs[ESP_RSEQ] = SEQ_MO;
 esp_raise_irq(s);
 } else if (cmdlen == 0) {
 s->cmdlen = 0;
 s->do_cmd = 1;
-/* Target present, but no cmd yet - switch to command phase */
-s->rregs[ESP_RSEQ] = SEQ_CD;
-s->rregs[ESP_RSTAT] = STAT_CD;
+/* Target present, switch to message out phase */
+s->rregs[ESP_RSEQ] = SEQ_MO;
+s->rregs[ESP_RSTAT] = STAT_MO;
 }
 }
 
@@ -505,9 +512,27 @@ static void esp_do_dma(ESPState *s)
 }
 trace_esp_handle_ti_cmd(s->cmdlen);
 s->ti_size = 0;
-s->cmdlen = 0;
-s->do_cmd = 0;
-do_cmd(s);
+if ((s->rregs[ESP_RSTAT] & 7) == STAT_CD) {
+/* No command received */
+if (s->cmdbuf_cdb_offset == s->cmdlen) {
+return;
+}
+
+/* Command has been received */
+s->cmdlen = 0;
+s->do_cmd = 0;
+do_cmd(s);
+} else {
+/*
+ * Extra message out bytes received: update cmdbuf_cdb_offset
+ * and then switch to commmand phase
+ */
+s->cmdbuf_cdb_offset = s->cmdlen;
+s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
+s->rregs[ESP_RSEQ] = SEQ_CD;
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+}
 return;
 }
 if (s->async_len == 0) {
@@ -655,9 +680,27 @@ static void handle_ti(ESPState *s)
 } else if (s->do_cmd) {
 trace_esp_handle_ti_cmd(s->cmdlen);
 s->ti_size = 0;
-s->cmdlen = 0;
-s->do_cmd = 0;
-do_cmd(s);
+if ((s->rregs[ESP_RSTAT] & 7) == STAT_CD) {
+/* No command received */
+if (s->cmdbuf_cdb_offset == s->cmdlen) {
+return;
+}
+
+/* Command has been received */
+s->cmdlen = 0;
+s->do_cmd = 0;
+do_cmd(s);
+} else {
+/*
+ * Extra message out bytes received: update cmdbuf_cdb_offset
+ * and then switch to commmand phase
+ */
+s->cmdbuf_cdb_offset = s->cmdlen;
+

Re: [PATCH v5 4/8] vt82c686: Introduce abstract TYPE_VIA_ISA and base vt82c686b_isa on it

2021-03-04 Thread Philippe Mathieu-Daudé
On 3/4/21 9:16 PM, BALATON Zoltan wrote:
> On Thu, 4 Mar 2021, Philippe Mathieu-Daudé wrote:
>> On 3/2/21 10:11 PM, BALATON Zoltan wrote:
>>> To allow reusing ISA bridge emulation for vt8231_isa move the device
>>> state of vt82c686b_isa emulation in an abstract via_isa class.
>>>
>>> Signed-off-by: BALATON Zoltan 
>>> ---
>>>  hw/isa/vt82c686.c    | 70 ++--
>>>  include/hw/pci/pci_ids.h |  2 +-
>>>  2 files changed, 40 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>> index 72234bc4d1..5137f97f37 100644
>>> --- a/hw/isa/vt82c686.c
>>> +++ b/hw/isa/vt82c686.c
>>> @@ -609,24 +609,48 @@ static const TypeInfo vt8231_superio_info = {
>>>  };
>>>
>>>
>>> -OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState, VT82C686B_ISA)
>>> +#define TYPE_VIA_ISA "via-isa"
>>> +OBJECT_DECLARE_SIMPLE_TYPE(ViaISAState, VIA_ISA)
>>>
>>> -struct VT82C686BISAState {
>>> +struct ViaISAState {
>>>  PCIDevice dev;
>>>  qemu_irq cpu_intr;
>>>  ViaSuperIOState *via_sio;
>>>  };
>>>
>>> +static const VMStateDescription vmstate_via = {
>>> +    .name = "via-isa",
>>
>> You changed the migration stream name, so I think we have
>> a problem with migration... No clue how to do that properly.
> 
> I don't think these machines support migration or state description of
> vt86c686b was not missing something before these patches that would make
> it not work anyway so I did not worry about this too much. I doubt
> anybody wants to migrate a fuloong2e machine so this should not be a
> problem in practice but maybe you can mention it in the release notes if
> you think that would be necessary.

Maybe just add in the description:

 This change breaks migration back compatibility, but
 this is not an issue for the Fuloong2E machine.

?



[RFC PATCH v2 4/8] target/s390x: Move s390_cpu_has_work to excp_helper.c

2021-03-04 Thread Philippe Mathieu-Daudé
We will restrict the s390_cpu_has_work() function to TCG.
First declare it in "internal.h" and move it to excp_helper.c.

Reviewed-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/s390x/internal.h|  1 +
 target/s390x/cpu.c | 17 -
 target/s390x/excp_helper.c | 18 ++
 3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 11515bb6173..7184e38631c 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -263,6 +263,7 @@ ObjectClass *s390_cpu_class_by_name(const char *name);
 
 
 /* excp_helper.c */
+bool s390_cpu_has_work(CPUState *cs);
 void s390x_cpu_debug_excp_handler(CPUState *cs);
 void s390_cpu_do_interrupt(CPUState *cpu);
 bool s390_cpu_exec_interrupt(CPUState *cpu, int int_req);
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index feaf2a6d08f..d57f69e7f7d 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -56,23 +56,6 @@ static void s390_cpu_set_pc(CPUState *cs, vaddr value)
 cpu->env.psw.addr = value;
 }
 
-static bool s390_cpu_has_work(CPUState *cs)
-{
-S390CPU *cpu = S390_CPU(cs);
-
-/* STOPPED cpus can never wake up */
-if (s390_cpu_get_state(cpu) != S390_CPU_STATE_LOAD &&
-s390_cpu_get_state(cpu) != S390_CPU_STATE_OPERATING) {
-return false;
-}
-
-if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
-return false;
-}
-
-return s390_cpu_has_int(cpu);
-}
-
 #if !defined(CONFIG_USER_ONLY)
 /* S390CPUClass::load_normal() */
 static void s390_cpu_load_normal(CPUState *s)
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index ce16af394b1..64923ffb83a 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -28,12 +28,30 @@
 #include "hw/s390x/ioinst.h"
 #include "exec/address-spaces.h"
 #include "tcg_s390x.h"
+#include "qapi/qapi-types-machine.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/sysemu.h"
 #include "hw/s390x/s390_flic.h"
 #include "hw/boards.h"
 #endif
 
+bool s390_cpu_has_work(CPUState *cs)
+{
+S390CPU *cpu = S390_CPU(cs);
+
+/* STOPPED cpus can never wake up */
+if (s390_cpu_get_state(cpu) != S390_CPU_STATE_LOAD &&
+s390_cpu_get_state(cpu) != S390_CPU_STATE_OPERATING) {
+return false;
+}
+
+if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
+return false;
+}
+
+return s390_cpu_has_int(cpu);
+}
+
 void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env,
   uint32_t code, uintptr_t ra)
 {
-- 
2.26.2




[PATCH v3 31/42] esp: implement FIFO flush command

2021-03-04 Thread Mark Cave-Ayland
At this point it is now possible to properly implement the FIFO flush command
without causing guest errors.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 2dded90be6..6aae6f91c2 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -769,6 +769,8 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
val)
 case CMD_FLUSH:
 trace_esp_mem_writeb_cmd_flush(val);
 /*s->ti_size = 0;*/
+s->ti_wptr = 0;
+s->ti_rptr = 0;
 s->rregs[ESP_RINTR] = INTR_FC;
 s->rregs[ESP_RSEQ] = 0;
 s->rregs[ESP_RFLAGS] = 0;
-- 
2.20.1




[PATCH v3 40/42] esp: add trivial implementation of the ESP_RFLAGS register

2021-03-04 Thread Mark Cave-Ayland
The bottom 5 bits contain the number of bytes remaining in the FIFO which is
trivial to implement with Fifo8 (the remaining bits are unimplemented and left
as 0 for now).

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 34dc58da58..8a9b1500de 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -818,6 +818,10 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
 val = s->rregs[saddr];
 }
 break;
+ case ESP_RFLAGS:
+/* Bottom 5 bits indicate number of bytes in FIFO */
+val = fifo8_num_used(>fifo);
+break;
 default:
 val = s->rregs[saddr];
 break;
-- 
2.20.1




[RFC PATCH v2 0/8] cpu: Move CPUClass::has_work() to TCGCPUOps

2021-03-04 Thread Philippe Mathieu-Daudé
Hi,

cpu_has_work() isn't used out of TCG, so restrict it to it.
RFC because maybe it could?

Since v1:
- finished PPC
- check cc->tcg_ops->has_work non-null (thuth)

Based-on: <20210227232519.222663-1-richard.hender...@linaro.org>

Philippe Mathieu-Daudé (8):
  sysemu/tcg: Restrict tcg_exec_init() to CONFIG_TCG
  sysemu/tcg: Restrict qemu_tcg_mttcg_enabled() to TCG
  target/arm: Directly use arm_cpu_has_work instead of
CPUClass::has_work
  target/s390x: Move s390_cpu_has_work to excp_helper.c
  target/ppc: Duplicate the TCGCPUOps structure for POWER CPUs
  cpu: Declare cpu_has_work() in 'sysemu/tcg.h'
  cpu: Move CPUClass::has_work() to TCGCPUOps
  target/arm: Restrict arm_cpu_has_work() to TCG

 include/hw/core/cpu.h   | 27 ---
 include/hw/core/tcg-cpu-ops.h   |  4 ++
 include/sysemu/tcg.h| 30 +++--
 target/arm/internals.h  |  1 +
 target/s390x/internal.h |  1 +
 accel/tcg/cpu-exec.c| 12 +
 hw/core/cpu.c   |  6 ---
 softmmu/cpus.c  |  1 +
 target/alpha/cpu.c  |  2 +-
 target/arm/cpu.c|  6 +--
 target/arm/op_helper.c  |  2 +-
 target/avr/cpu.c|  2 +-
 target/cris/cpu.c   |  3 +-
 target/hexagon/cpu.c|  2 +-
 target/hppa/cpu.c   |  2 +-
 target/i386/cpu.c   |  7 +--
 target/i386/tcg/tcg-cpu.c   |  6 +++
 target/lm32/cpu.c   |  2 +-
 target/m68k/cpu.c   |  2 +-
 target/microblaze/cpu.c |  2 +-
 target/mips/cpu.c   |  2 +-
 target/moxie/cpu.c  |  2 +-
 target/nios2/cpu.c  |  2 +-
 target/openrisc/cpu.c   |  2 +-
 target/riscv/cpu.c  |  2 +-
 target/rx/cpu.c |  2 +-
 target/s390x/cpu.c  | 19 +---
 target/s390x/excp_helper.c  | 18 
 target/sh4/cpu.c|  2 +-
 target/sparc/cpu.c  |  2 +-
 target/tilegx/cpu.c |  2 +-
 target/tricore/cpu.c|  2 +-
 target/unicore32/cpu.c  |  2 +-
 target/xtensa/cpu.c |  2 +-
 tcg/tcg.c   |  1 +
 target/ppc/translate_init.c.inc | 79 ++---
 36 files changed, 171 insertions(+), 90 deletions(-)

-- 
2.26.2




[PATCH v3 27/42] esp: fix PDMA target selection

2021-03-04 Thread Mark Cave-Ayland
Currently the target selection for PDMA is done after the SCSI command has been
delivered which is not correct. Perform target selection as part of the initial
get_cmd() call when the command is submitted: if no target is present, don't
raise DRQ.

If the target is present then switch to the command phase since the MacOS 
toolbox
ROM checks for this before attempting to submit the SCSI command.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 53 +--
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index d8d7ede00a..10e63d1f62 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -243,6 +243,9 @@ static uint32_t get_cmd(ESPState *s)
 s->dma_memory_read(s->dma_opaque, buf, dmalen);
 } else {
 set_pdma(s, TI);
+if (esp_select(s) < 0) {
+return -1;
+}
 esp_raise_drq(s);
 return 0;
 }
@@ -257,7 +260,7 @@ static uint32_t get_cmd(ESPState *s)
 trace_esp_get_cmd(dmalen, target);
 
 if (esp_select(s) < 0) {
-return 0;
+return -1;
 }
 return dmalen;
 }
@@ -299,9 +302,6 @@ static void do_cmd(ESPState *s)
 
 static void satn_pdma_cb(ESPState *s)
 {
-if (esp_select(s) < 0) {
-return;
-}
 s->do_cmd = 0;
 if (s->cmdlen) {
 do_cmd(s);
@@ -310,24 +310,28 @@ static void satn_pdma_cb(ESPState *s)
 
 static void handle_satn(ESPState *s)
 {
+int32_t cmdlen;
+
 if (s->dma && !s->dma_enabled) {
 s->dma_cb = handle_satn;
 return;
 }
 s->pdma_cb = satn_pdma_cb;
-s->cmdlen = get_cmd(s);
-if (s->cmdlen) {
+cmdlen = get_cmd(s);
+if (cmdlen > 0) {
+s->cmdlen = cmdlen;
 do_cmd(s);
-} else {
+} else if (cmdlen == 0) {
+s->cmdlen = 0;
 s->do_cmd = 1;
+/* Target present, but no cmd yet - switch to command phase */
+s->rregs[ESP_RSEQ] = SEQ_CD;
+s->rregs[ESP_RSTAT] = STAT_CD;
 }
 }
 
 static void s_without_satn_pdma_cb(ESPState *s)
 {
-if (esp_select(s) < 0) {
-return;
-}
 s->do_cmd = 0;
 if (s->cmdlen) {
 do_busid_cmd(s, s->cmdbuf, 0);
@@ -336,24 +340,28 @@ static void s_without_satn_pdma_cb(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;
 }
 s->pdma_cb = s_without_satn_pdma_cb;
-s->cmdlen = get_cmd(s);
-if (s->cmdlen) {
+cmdlen = get_cmd(s);
+if (cmdlen > 0) {
+s->cmdlen = cmdlen;
 do_busid_cmd(s, s->cmdbuf, 0);
-} else {
+} else if (cmdlen == 0) {
+s->cmdlen = 0;
 s->do_cmd = 1;
+/* Target present, but no cmd yet - switch to command phase */
+s->rregs[ESP_RSEQ] = SEQ_CD;
+s->rregs[ESP_RSTAT] = STAT_CD;
 }
 }
 
 static void satn_stop_pdma_cb(ESPState *s)
 {
-if (esp_select(s) < 0) {
-return;
-}
 s->do_cmd = 0;
 if (s->cmdlen) {
 trace_esp_handle_satn_stop(s->cmdlen);
@@ -367,21 +375,28 @@ static void satn_stop_pdma_cb(ESPState *s)
 
 static void handle_satn_stop(ESPState *s)
 {
+int32_t cmdlen;
+
 if (s->dma && !s->dma_enabled) {
 s->dma_cb = handle_satn_stop;
 return;
 }
 s->pdma_cb = satn_stop_pdma_cb;
-s->cmdlen = get_cmd(s);
-if (s->cmdlen) {
+cmdlen = get_cmd(s);
+if (cmdlen > 0) {
 trace_esp_handle_satn_stop(s->cmdlen);
+s->cmdlen = cmdlen;
 s->do_cmd = 1;
 s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
 s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
 esp_raise_irq(s);
-} else {
+} else if (cmdlen == 0) {
+s->cmdlen = 0;
 s->do_cmd = 1;
+/* Target present, but no cmd yet - switch to command phase */
+s->rregs[ESP_RSEQ] = SEQ_CD;
+s->rregs[ESP_RSTAT] = STAT_CD;
 }
 }
 
-- 
2.20.1




[PATCH v3 35/42] esp: raise interrupt after every non-DMA byte transferred to the FIFO

2021-03-04 Thread Mark Cave-Ayland
This matches the description in the datasheet and is required as support for
non-DMA transfers is added.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index eb6681ca66..4ac299651f 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -760,6 +760,12 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
val)
 s->ti_size++;
 s->ti_buf[s->ti_wptr++] = val & 0xff;
 }
+
+/* Non-DMA transfers raise an interrupt after every byte */
+if (s->rregs[ESP_CMD] == CMD_TI) {
+s->rregs[ESP_RINTR] |= INTR_FC | INTR_BS;
+esp_raise_irq(s);
+}
 break;
 case ESP_CMD:
 s->rregs[saddr] = val;
-- 
2.20.1




[PATCH v3 39/42] esp: convert cmdbuf from array to Fifo8

2021-03-04 Thread Mark Cave-Ayland
Rename ESP_CMDBUF_SZ to ESP_CMDFIFO_SZ and cmdbuf_cdb_offset to 
cmdfifo_cdb_offset
to indicate that the command buffer type has changed from an array to a Fifo8.

This also enables us to remove the ESPState field cmdlen since the command 
length
is now simply the number of elements used in cmdfifo.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 151 +++---
 include/hw/scsi/esp.h |   9 +--
 2 files changed, 101 insertions(+), 59 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 44e70aa789..34dc58da58 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -117,6 +117,25 @@ static uint8_t esp_fifo_pop(ESPState *s)
 return fifo8_pop(>fifo);
 }
 
+static void esp_cmdfifo_push(ESPState *s, uint8_t val)
+{
+if (fifo8_num_used(>cmdfifo) == ESP_CMDFIFO_SZ) {
+trace_esp_error_fifo_overrun();
+return;
+}
+
+fifo8_push(>cmdfifo, val);
+}
+
+static uint8_t esp_cmdfifo_pop(ESPState *s)
+{
+if (fifo8_is_empty(>cmdfifo)) {
+return 0;
+}
+
+return fifo8_pop(>cmdfifo);
+}
+
 static uint32_t esp_get_tc(ESPState *s)
 {
 uint32_t dmalen;
@@ -151,7 +170,7 @@ static uint8_t esp_pdma_read(ESPState *s)
 uint8_t val;
 
 if (s->do_cmd) {
-val = s->cmdbuf[s->cmdlen++];
+val = esp_cmdfifo_pop(s);
 } else {
 val = esp_fifo_pop(s);
 }
@@ -168,7 +187,7 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 }
 
 if (s->do_cmd) {
-s->cmdbuf[s->cmdlen++] = val;
+esp_cmdfifo_push(s, val);
 } else {
 esp_fifo_push(s, val);
 }
@@ -214,7 +233,7 @@ static int esp_select(ESPState *s)
 
 static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
 {
-uint8_t *buf = s->cmdbuf;
+uint8_t buf[ESP_CMDFIFO_SZ];
 uint32_t dmalen, n;
 int target;
 
@@ -226,15 +245,18 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
 }
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, buf, dmalen);
+fifo8_push_all(>cmdfifo, buf, dmalen);
 } else {
 if (esp_select(s) < 0) {
+fifo8_reset(>cmdfifo);
 return -1;
 }
 esp_raise_drq(s);
+fifo8_reset(>cmdfifo);
 return 0;
 }
 } else {
-dmalen = MIN(s->ti_size, maxlen);
+dmalen = MIN(fifo8_num_used(>fifo), maxlen);
 if (dmalen == 0) {
 return 0;
 }
@@ -242,27 +264,35 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
 if (dmalen >= 3) {
 buf[0] = buf[2] >> 5;
 }
+fifo8_push_all(>cmdfifo, buf, dmalen);
 }
 trace_esp_get_cmd(dmalen, target);
 
 if (esp_select(s) < 0) {
+fifo8_reset(>cmdfifo);
 return -1;
 }
 return dmalen;
 }
 
-static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
+static void do_busid_cmd(ESPState *s, uint8_t busid)
 {
+uint32_t n, cmdlen;
 int32_t datalen;
 int lun;
 SCSIDevice *current_lun;
+uint8_t *buf;
 
 trace_esp_do_busid_cmd(busid);
 lun = busid & 7;
+cmdlen = fifo8_num_used(>cmdfifo);
+buf = (uint8_t *)fifo8_pop_buf(>cmdfifo, cmdlen, );
+
 current_lun = scsi_device_find(>bus, 0, s->current_dev->id, lun);
 s->current_req = scsi_req_new(current_lun, 0, lun, buf, s);
 datalen = scsi_req_enqueue(s->current_req);
 s->ti_size = datalen;
+fifo8_reset(>cmdfifo);
 if (datalen != 0) {
 s->rregs[ESP_RSTAT] = STAT_TC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
@@ -287,18 +317,25 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, 
uint8_t busid)
 
 static void do_cmd(ESPState *s)
 {
-uint8_t *buf = s->cmdbuf;
-uint8_t busid = buf[0];
+uint8_t busid = fifo8_pop(>cmdfifo);
+uint32_t n;
+
+s->cmdfifo_cdb_offset--;
 
 /* Ignore extended messages for now */
-do_busid_cmd(s, [s->cmdbuf_cdb_offset], busid);
+if (s->cmdfifo_cdb_offset) {
+fifo8_pop_buf(>cmdfifo, s->cmdfifo_cdb_offset, );
+s->cmdfifo_cdb_offset = 0;
+}
+
+do_busid_cmd(s, busid);
 }
 
 static void satn_pdma_cb(ESPState *s)
 {
 s->do_cmd = 0;
-if (s->cmdlen) {
-s->cmdbuf_cdb_offset = 1;
+if (!fifo8_is_empty(>cmdfifo)) {
+s->cmdfifo_cdb_offset = 1;
 do_cmd(s);
 }
 }
@@ -312,13 +349,11 @@ static void handle_satn(ESPState *s)
 return;
 }
 s->pdma_cb = satn_pdma_cb;
-cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
+cmdlen = get_cmd(s, ESP_CMDFIFO_SZ);
 if (cmdlen > 0) {
-s->cmdlen = cmdlen;
-s->cmdbuf_cdb_offset = 1;
+s->cmdfifo_cdb_offset = 1;
 do_cmd(s);
 } else if (cmdlen == 0) {
-s->cmdlen = 0;
 s->do_cmd = 1;
 /* Target present, but no cmd yet - switch to command phase */
 s->rregs[ESP_RSEQ] = SEQ_CD;
@@ -328,10 +363,13 @@ static void handle_satn(ESPState *s)
 
 static void 

[RFC PATCH v2 3/8] target/arm: Directly use arm_cpu_has_work instead of CPUClass::has_work

2021-03-04 Thread Philippe Mathieu-Daudé
There is only one CPUClass::has_work() ARM handler: arm_cpu_has_work().

Avoid a dereference by declaring it in "internals.h" and call it
directly  in the WFI helper.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/arm/internals.h | 1 +
 target/arm/cpu.c   | 2 +-
 target/arm/op_helper.c | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 05cebc8597c..1930be08828 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -172,6 +172,7 @@ static inline int r14_bank_number(int mode)
 void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
 void arm_translate_init(void);
 
+bool arm_cpu_has_work(CPUState *cs);
 #ifdef CONFIG_TCG
 void arm_cpu_synchronize_from_tb(CPUState *cs, const TranslationBlock *tb);
 #endif /* CONFIG_TCG */
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5e018b2a732..6d2d9f2100f 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -76,7 +76,7 @@ void arm_cpu_synchronize_from_tb(CPUState *cs,
 }
 #endif /* CONFIG_TCG */
 
-static bool arm_cpu_has_work(CPUState *cs)
+bool arm_cpu_has_work(CPUState *cs)
 {
 ARMCPU *cpu = ARM_CPU(cs);
 
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 65cb37d088f..a4da6f4fde8 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -289,7 +289,7 @@ void HELPER(wfi)(CPUARMState *env, uint32_t insn_len)
 CPUState *cs = env_cpu(env);
 int target_el = check_wfx_trap(env, false);
 
-if (cpu_has_work(cs)) {
+if (arm_cpu_has_work(cs)) {
 /* Don't bother to go into our "low power state" if
  * we would just wake up immediately.
  */
-- 
2.26.2




[PATCH v3 25/42] esp: remove CMD pdma_origin

2021-03-04 Thread Mark Cave-Ayland
The cmdbuf is really just a copy of FIFO data (including extra message phase
bytes) so its pdma_origin is effectively TI. Fortunately we already know when
we are receiving a SCSI command since do_cmd == 1 which enables us to
distinguish between the two cases in esp_pdma_read()/esp_pdma_write().

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 22 --
 include/hw/scsi/esp.h |  1 -
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index c80dc606e8..d5c03f9697 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -143,10 +143,11 @@ static uint8_t esp_pdma_read(ESPState *s)
 
 switch (s->pdma_origin) {
 case TI:
-val = s->ti_buf[s->ti_rptr++];
-break;
-case CMD:
-val = s->cmdbuf[s->cmdlen++];
+if (s->do_cmd) {
+val = s->cmdbuf[s->cmdlen++];
+} else {
+val = s->ti_buf[s->ti_rptr++];
+}
 break;
 case ASYNC:
 val = s->async_buf[0];
@@ -176,10 +177,11 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 
 switch (s->pdma_origin) {
 case TI:
-s->ti_buf[s->ti_wptr++] = val;
-break;
-case CMD:
-s->cmdbuf[s->cmdlen++] = val;
+if (s->do_cmd) {
+s->cmdbuf[s->cmdlen++] = val;
+} else {
+s->ti_buf[s->ti_wptr++] = val;
+}
 break;
 case ASYNC:
 s->async_buf[0] = val;
@@ -240,7 +242,7 @@ static uint32_t get_cmd(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, buf, dmalen);
 } else {
-set_pdma(s, CMD);
+set_pdma(s, TI);
 esp_raise_drq(s);
 return 0;
 }
@@ -471,7 +473,7 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, >cmdbuf[s->cmdlen], len);
 } else {
-set_pdma(s, CMD);
+set_pdma(s, TI);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
 return;
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 1be4586aff..d3fc52 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -17,7 +17,6 @@ typedef struct ESPState ESPState;
 
 enum pdma_origin_id {
 TI,
-CMD,
 ASYNC,
 };
 
-- 
2.20.1




[PATCH v3 33/42] esp: defer command completion interrupt on incoming data transfers

2021-03-04 Thread Mark Cave-Ayland
The MacOS toolbox ROM issues a command to the ESP controller as part of its
"FAST" SCSI routines and then proceeds to read the incoming data soon after
receiving the command completion interrupt.

Unfortunately due to SCSI block transfers being asynchronous the incoming data
may not yet be present causing an underflow error. Resolve this by waiting for
the SCSI subsystem transfer_data callback before raising the command completion
interrupt.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 66 ++-
 include/hw/scsi/esp.h |  1 +
 2 files changed, 54 insertions(+), 13 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 54d008c609..0eecc1d05c 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -183,6 +183,14 @@ static int esp_select(ESPState *s)
 esp_raise_irq(s);
 return -1;
 }
+
+/*
+ * Note that we deliberately don't raise the IRQ here: this will be done
+ * either in do_busid_cmd() for DATA OUT transfers or by the deferred
+ * IRQ mechanism in esp_transfer_data() for DATA IN transfers
+ */
+s->rregs[ESP_RINTR] |= INTR_FC;
+s->rregs[ESP_RSEQ] = SEQ_CD;
 return 0;
 }
 
@@ -237,18 +245,24 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, 
uint8_t busid)
 s->ti_size = datalen;
 if (datalen != 0) {
 s->rregs[ESP_RSTAT] = STAT_TC;
+s->rregs[ESP_RSEQ] = SEQ_CD;
 esp_set_tc(s, 0);
 if (datalen > 0) {
+/*
+ * 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->rregs[ESP_RSTAT] |= STAT_DI;
 } else {
 s->rregs[ESP_RSTAT] |= 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;
 }
-s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-s->rregs[ESP_RSEQ] = SEQ_CD;
-esp_raise_irq(s);
-esp_lower_drq(s);
 }
 
 static void do_cmd(ESPState *s)
@@ -439,17 +453,11 @@ static void do_dma_pdma_cb(ESPState *s)
 } else {
 if (s->async_len == 0) {
 if (s->current_req) {
+/* Defer until the scsi layer has completed */
 scsi_req_continue(s->current_req);
+s->data_in_ready = false;
 }
-
-/*
- * If there is still data to be read from the device then
- * complete the DMA operation immediately.  Otherwise defer
- * until the scsi layer has completed.
- */
-if (esp_get_tc(s) != 0 || s->ti_size == 0) {
-return;
-}
+return;
 }
 
 if (esp_get_tc(s) != 0) {
@@ -602,12 +610,35 @@ 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 = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
 uint32_t dmalen = esp_get_tc(s);
 
 assert(!s->do_cmd);
 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_in_ready) {
+/*
+ * Initial incoming data xfer is complete so raise command
+ * completion interrupt
+ */
+s->data_in_ready = true;
+s->rregs[ESP_RSTAT] |= STAT_TC;
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+
+/*
+ * If data is ready to transfer and the TI command has already
+ * been executed, start DMA immediately. Otherwise DMA will start
+ * when host sends the TI command
+ */
+if (s->ti_size && (s->rregs[ESP_CMD] == (CMD_TI | CMD_DMA))) {
+esp_do_dma(s);
+}
+return;
+}
+
 if (dmalen) {
 esp_do_dma(s);
 } else if (s->ti_size <= 0) {
@@ -869,6 +900,14 @@ static bool esp_is_before_version_5(void *opaque, int 
version_id)
 return version_id < 5;
 }
 
+static bool esp_is_version_5(void *opaque, int version_id)
+{
+ESPState *s = ESP(opaque);
+
+version_id = MIN(version_id, s->mig_version_id);
+return version_id == 5;
+}
+
 static int esp_pre_save(void *opaque)
 {
 ESPState *s = ESP(opaque);
@@ -913,6 +952,7 @@ const VMStateDescription vmstate_esp = {
 VMSTATE_UINT32(cmdlen, ESPState),
 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_END_OF_LIST()
 },
 };
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 91f8ffd6c8..61bc317a4c 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -41,6 +41,7 @@ struct ESPState {
 uint32_t 

[PATCH v3 38/42] esp: convert ti_buf from array to Fifo8

2021-03-04 Thread Mark Cave-Ayland
Rename TI_BUFSZ to ESP_FIFO_SZ since this constant is really describing the size
of the FIFO and is not directly related to the TI size.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 118 ++
 include/hw/scsi/esp.h |   8 +--
 2 files changed, 79 insertions(+), 47 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 0d5c07e4c1..44e70aa789 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -98,6 +98,25 @@ void esp_request_cancelled(SCSIRequest *req)
 }
 }
 
+static void esp_fifo_push(ESPState *s, uint8_t val)
+{
+if (fifo8_num_used(>fifo) == ESP_FIFO_SZ) {
+trace_esp_error_fifo_overrun();
+return;
+}
+
+fifo8_push(>fifo, val);
+}
+
+static uint8_t esp_fifo_pop(ESPState *s)
+{
+if (fifo8_is_empty(>fifo)) {
+return 0;
+}
+
+return fifo8_pop(>fifo);
+}
+
 static uint32_t esp_get_tc(ESPState *s)
 {
 uint32_t dmalen;
@@ -134,7 +153,7 @@ static uint8_t esp_pdma_read(ESPState *s)
 if (s->do_cmd) {
 val = s->cmdbuf[s->cmdlen++];
 } else {
-val = s->ti_buf[s->ti_rptr++];
+val = esp_fifo_pop(s);
 }
 
 return val;
@@ -151,7 +170,7 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 if (s->do_cmd) {
 s->cmdbuf[s->cmdlen++] = val;
 } else {
-s->ti_buf[s->ti_wptr++] = val;
+esp_fifo_push(s, val);
 }
 
 dmalen--;
@@ -165,8 +184,7 @@ static int esp_select(ESPState *s)
 target = s->wregs[ESP_WBUSID] & BUSID_DID;
 
 s->ti_size = 0;
-s->ti_rptr = 0;
-s->ti_wptr = 0;
+fifo8_reset(>fifo);
 
 if (s->current_req) {
 /* Started a new command before the old one finished.  Cancel it.  */
@@ -197,7 +215,7 @@ static int esp_select(ESPState *s)
 static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
 {
 uint8_t *buf = s->cmdbuf;
-uint32_t dmalen;
+uint32_t dmalen, n;
 int target;
 
 target = s->wregs[ESP_WBUSID] & BUSID_DID;
@@ -220,7 +238,7 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
 if (dmalen == 0) {
 return 0;
 }
-memcpy(buf, s->ti_buf, dmalen);
+memcpy(buf, fifo8_pop_buf(>fifo, dmalen, ), dmalen);
 if (dmalen >= 3) {
 buf[0] = buf[2] >> 5;
 }
@@ -392,12 +410,18 @@ static void write_response_pdma_cb(ESPState *s)
 
 static void write_response(ESPState *s)
 {
+uint32_t n;
+
 trace_esp_write_response(s->status);
-s->ti_buf[0] = s->status;
-s->ti_buf[1] = 0;
+
+fifo8_reset(>fifo);
+esp_fifo_push(s, s->status);
+esp_fifo_push(s, 0);
+
 if (s->dma) {
 if (s->dma_memory_write) {
-s->dma_memory_write(s->dma_opaque, s->ti_buf, 2);
+s->dma_memory_write(s->dma_opaque,
+(uint8_t *)fifo8_pop_buf(>fifo, 2, ), 2);
 s->rregs[ESP_RSTAT] = STAT_TC | STAT_ST;
 s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
@@ -408,8 +432,6 @@ static void write_response(ESPState *s)
 }
 } else {
 s->ti_size = 2;
-s->ti_rptr = 0;
-s->ti_wptr = 2;
 s->rregs[ESP_RFLAGS] = 2;
 }
 esp_raise_irq(s);
@@ -429,6 +451,7 @@ static void do_dma_pdma_cb(ESPState *s)
 {
 int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
 int len;
+uint32_t n;
 
 if (s->do_cmd) {
 s->ti_size = 0;
@@ -441,10 +464,8 @@ static void do_dma_pdma_cb(ESPState *s)
 
 if (to_device) {
 /* Copy FIFO data to device */
-len = MIN(s->ti_wptr, TI_BUFSZ);
-memcpy(s->async_buf, s->ti_buf, len);
-s->ti_wptr = 0;
-s->ti_rptr = 0;
+len = MIN(fifo8_num_used(>fifo), ESP_FIFO_SZ);
+memcpy(s->async_buf, fifo8_pop_buf(>fifo, len, ), len);
 s->async_buf += len;
 s->async_len -= len;
 s->ti_size += len;
@@ -471,11 +492,8 @@ static void do_dma_pdma_cb(ESPState *s)
 
 if (esp_get_tc(s) != 0) {
 /* Copy device data to FIFO */
-s->ti_wptr = 0;
-s->ti_rptr = 0;
-len = MIN(s->async_len, TI_BUFSZ);
-memcpy(s->ti_buf, s->async_buf, len);
-s->ti_wptr += len;
+len = MIN(s->async_len, fifo8_num_free(>fifo));
+fifo8_push_all(>fifo, s->async_buf, len);
 s->async_buf += len;
 s->async_len -= len;
 s->ti_size -= len;
@@ -555,9 +573,8 @@ static void esp_do_dma(ESPState *s)
 s->dma_memory_write(s->dma_opaque, s->async_buf, len);
 } else {
 /* Copy device data to FIFO */
-len = MIN(len, TI_BUFSZ - s->ti_wptr);
-memcpy(>ti_buf[s->ti_wptr], s->async_buf, len);
-s->ti_wptr += len;
+len = MIN(len, fifo8_num_free(>fifo));
+fifo8_push_all(>fifo, s->async_buf, len);
 s->async_buf += len;
   

[RFC PATCH v2 2/8] sysemu/tcg: Restrict qemu_tcg_mttcg_enabled() to TCG

2021-03-04 Thread Philippe Mathieu-Daudé
qemu_tcg_mttcg_enabled() shouldn't not be used outside of TCG,
restrict its declaration.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/core/cpu.h | 9 -
 include/sysemu/tcg.h  | 9 +
 accel/tcg/cpu-exec.c  | 1 +
 tcg/tcg.c | 1 +
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index e3648338dfe..1376e496a3f 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -454,15 +454,6 @@ static inline void cpu_tb_jmp_cache_clear(CPUState *cpu)
 }
 }
 
-/**
- * qemu_tcg_mttcg_enabled:
- * Check whether we are running MultiThread TCG or not.
- *
- * Returns: %true if we are in MTTCG mode %false otherwise.
- */
-extern bool mttcg_enabled;
-#define qemu_tcg_mttcg_enabled() (mttcg_enabled)
-
 /**
  * cpu_paging_enabled:
  * @cpu: The CPU whose state is to be inspected.
diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
index fddde2b6b9a..c16c13c3c69 100644
--- a/include/sysemu/tcg.h
+++ b/include/sysemu/tcg.h
@@ -17,6 +17,15 @@ void tcg_exec_init(unsigned long tb_size, int splitwx);
 extern bool tcg_allowed;
 #define tcg_enabled() (tcg_allowed)
 
+/**
+ * qemu_tcg_mttcg_enabled:
+ * Check whether we are running MultiThread TCG or not.
+ *
+ * Returns: %true if we are in MTTCG mode %false otherwise.
+ */
+extern bool mttcg_enabled;
+#define qemu_tcg_mttcg_enabled() (mttcg_enabled)
+
 #endif /* CONFIG_TCG */
 
 #endif
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 16e4fe3ccd8..7e67ade35b9 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -39,6 +39,7 @@
 #include "hw/i386/apic.h"
 #endif
 #include "sysemu/cpus.h"
+#include "sysemu/tcg.h"
 #include "exec/cpu-all.h"
 #include "sysemu/cpu-timers.h"
 #include "sysemu/replay.h"
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 63a12b197bf..4a4dac0bb3e 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -65,6 +65,7 @@
 #include "elf.h"
 #include "exec/log.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/tcg.h"
 
 /* Forward declarations for functions declared in tcg-target.c.inc and
used here. */
-- 
2.26.2




[PATCH v3 19/42] esp: remove buf parameter from do_cmd()

2021-03-04 Thread Mark Cave-Ayland
Now that all SCSI commands are accumulated in cmdbuf, remove the buf parameter
from do_cmd() since this always points to cmdbuf.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 66caa95815..8ebf5e8d4b 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -297,8 +297,9 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t 
busid)
 esp_raise_irq(s);
 }
 
-static void do_cmd(ESPState *s, uint8_t *buf)
+static void do_cmd(ESPState *s)
 {
+uint8_t *buf = s->cmdbuf;
 uint8_t busid = buf[0];
 
 do_busid_cmd(s, [1], busid);
@@ -311,7 +312,7 @@ static void satn_pdma_cb(ESPState *s)
 }
 s->do_cmd = 0;
 if (s->cmdlen) {
-do_cmd(s, s->cmdbuf);
+do_cmd(s);
 }
 }
 
@@ -324,7 +325,7 @@ static void handle_satn(ESPState *s)
 s->pdma_cb = satn_pdma_cb;
 s->cmdlen = get_cmd(s, s->cmdbuf, sizeof(s->cmdbuf));
 if (s->cmdlen) {
-do_cmd(s, s->cmdbuf);
+do_cmd(s);
 } else {
 s->do_cmd = 1;
 }
@@ -445,7 +446,7 @@ static void do_dma_pdma_cb(ESPState *s)
 s->ti_size = 0;
 s->cmdlen = 0;
 s->do_cmd = 0;
-do_cmd(s, s->cmdbuf);
+do_cmd(s);
 return;
 }
 s->async_buf += len;
@@ -497,7 +498,7 @@ static void esp_do_dma(ESPState *s)
 s->ti_size = 0;
 s->cmdlen = 0;
 s->do_cmd = 0;
-do_cmd(s, s->cmdbuf);
+do_cmd(s);
 return;
 }
 if (s->async_len == 0) {
@@ -627,7 +628,7 @@ static void handle_ti(ESPState *s)
 s->ti_size = 0;
 s->cmdlen = 0;
 s->do_cmd = 0;
-do_cmd(s, s->cmdbuf);
+do_cmd(s);
 }
 }
 
-- 
2.20.1




[PATCH v3 32/42] esp: latch individual bits in ESP_RINTR register

2021-03-04 Thread Mark Cave-Ayland
Currently the ESP_RINTR register is set to a specific value as required within
the ESP state machine. In order to implement the upcoming deferred interrupt
functionality it is necessary to set individual bits within ESP_RINTR so that
a deferred interrupt will not overwrite the value of any other interrupt bits.

This also requires fixing up a few locations where the ESP_RINTR and ESP_RSEQ
registers are set/reset unexpectedly.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 29 +
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 6aae6f91c2..54d008c609 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -178,7 +178,7 @@ static int esp_select(ESPState *s)
 if (!s->current_dev) {
 /* No such drive */
 s->rregs[ESP_RSTAT] = 0;
-s->rregs[ESP_RINTR] = INTR_DC;
+s->rregs[ESP_RINTR] |= INTR_DC;
 s->rregs[ESP_RSEQ] = SEQ_0;
 esp_raise_irq(s);
 return -1;
@@ -245,7 +245,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t 
busid)
 }
 scsi_req_continue(s->current_req);
 }
-s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
 esp_raise_irq(s);
 esp_lower_drq(s);
@@ -326,7 +326,7 @@ static void satn_stop_pdma_cb(ESPState *s)
 trace_esp_handle_satn_stop(s->cmdlen);
 s->do_cmd = 1;
 s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
-s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
 esp_raise_irq(s);
 }
@@ -346,8 +346,8 @@ static void handle_satn_stop(ESPState *s)
 trace_esp_handle_satn_stop(s->cmdlen);
 s->cmdlen = cmdlen;
 s->do_cmd = 1;
-s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
-s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
+s->rregs[ESP_RSTAT] = STAT_CD;
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
 esp_raise_irq(s);
 } else if (cmdlen == 0) {
@@ -362,7 +362,7 @@ static void handle_satn_stop(ESPState *s)
 static void write_response_pdma_cb(ESPState *s)
 {
 s->rregs[ESP_RSTAT] = STAT_TC | STAT_ST;
-s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
 esp_raise_irq(s);
 }
@@ -376,7 +376,7 @@ static void write_response(ESPState *s)
 if (s->dma_memory_write) {
 s->dma_memory_write(s->dma_opaque, s->ti_buf, 2);
 s->rregs[ESP_RSTAT] = STAT_TC | STAT_ST;
-s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
 } else {
 s->pdma_cb = write_response_pdma_cb;
@@ -395,7 +395,7 @@ static void write_response(ESPState *s)
 static void esp_dma_done(ESPState *s)
 {
 s->rregs[ESP_RSTAT] |= STAT_TC;
-s->rregs[ESP_RINTR] = INTR_BS;
+s->rregs[ESP_RINTR] |= INTR_BS;
 s->rregs[ESP_RSEQ] = 0;
 s->rregs[ESP_RFLAGS] = 0;
 esp_set_tc(s, 0);
@@ -700,7 +700,7 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
 val = s->rregs[ESP_RINTR];
 s->rregs[ESP_RINTR] = 0;
 s->rregs[ESP_RSTAT] &= ~STAT_TC;
-s->rregs[ESP_RSEQ] = SEQ_CD;
+s->rregs[ESP_RSEQ] = SEQ_0;
 esp_lower_irq(s);
 if (s->deferred_complete) {
 esp_report_command_complete(s, s->deferred_status);
@@ -771,9 +771,6 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
val)
 /*s->ti_size = 0;*/
 s->ti_wptr = 0;
 s->ti_rptr = 0;
-s->rregs[ESP_RINTR] = INTR_FC;
-s->rregs[ESP_RSEQ] = 0;
-s->rregs[ESP_RFLAGS] = 0;
 break;
 case CMD_RESET:
 trace_esp_mem_writeb_cmd_reset(val);
@@ -781,8 +778,8 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
val)
 break;
 case CMD_BUSRESET:
 trace_esp_mem_writeb_cmd_bus_reset(val);
-s->rregs[ESP_RINTR] = INTR_RST;
 if (!(s->wregs[ESP_CFG1] & CFG1_RESREPT)) {
+s->rregs[ESP_RINTR] |= INTR_RST;
 esp_raise_irq(s);
 }
 break;
@@ -793,12 +790,12 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
val)
 case CMD_ICCS:
 trace_esp_mem_writeb_cmd_iccs(val);
 write_response(s);
-s->rregs[ESP_RINTR] = INTR_FC;
+s->rregs[ESP_RINTR] |= INTR_FC;
 s->rregs[ESP_RSTAT] |= STAT_MI;
 break;
 case CMD_MSGACC:
 trace_esp_mem_writeb_cmd_msgacc(val);
-s->rregs[ESP_RINTR] = INTR_DC;
+s->rregs[ESP_RINTR] |= INTR_DC;
 s->rregs[ESP_RSEQ] = 0;
 s->rregs[ESP_RFLAGS] = 0;
 

[PATCH v3 36/42] esp: add maxlen parameter to get_cmd()

2021-03-04 Thread Mark Cave-Ayland
Some guests use a mixture of DMA and non-DMA transfers in combination with the
SATN and stop command to transfer message out phase and command phase bytes to
the target. Prepare for the next commit by adding a maxlen parameter to
get_cmd() to allow partial transfers.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 4ac299651f..23fcaa90c1 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -194,7 +194,7 @@ static int esp_select(ESPState *s)
 return 0;
 }
 
-static uint32_t get_cmd(ESPState *s)
+static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
 {
 uint8_t *buf = s->cmdbuf;
 uint32_t dmalen;
@@ -202,8 +202,8 @@ static uint32_t get_cmd(ESPState *s)
 
 target = s->wregs[ESP_WBUSID] & BUSID_DID;
 if (s->dma) {
-dmalen = esp_get_tc(s);
-if (dmalen > ESP_CMDBUF_SZ) {
+dmalen = MIN(esp_get_tc(s), maxlen);
+if (dmalen == 0) {
 return 0;
 }
 if (s->dma_memory_read) {
@@ -216,12 +216,14 @@ static uint32_t get_cmd(ESPState *s)
 return 0;
 }
 } else {
-dmalen = s->ti_size;
-if (dmalen > TI_BUFSZ) {
+dmalen = MIN(s->ti_size, maxlen);
+if (dmalen == 0) {
 return 0;
 }
 memcpy(buf, s->ti_buf, dmalen);
-buf[0] = buf[2] >> 5;
+if (dmalen >= 3) {
+buf[0] = buf[2] >> 5;
+}
 }
 trace_esp_get_cmd(dmalen, target);
 
@@ -290,7 +292,7 @@ static void handle_satn(ESPState *s)
 return;
 }
 s->pdma_cb = satn_pdma_cb;
-cmdlen = get_cmd(s);
+cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
 if (cmdlen > 0) {
 s->cmdlen = cmdlen;
 do_cmd(s);
@@ -320,7 +322,7 @@ static void handle_s_without_atn(ESPState *s)
 return;
 }
 s->pdma_cb = s_without_satn_pdma_cb;
-cmdlen = get_cmd(s);
+cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
 if (cmdlen > 0) {
 s->cmdlen = cmdlen;
 do_busid_cmd(s, s->cmdbuf, 0);
@@ -355,7 +357,7 @@ static void handle_satn_stop(ESPState *s)
 return;
 }
 s->pdma_cb = satn_stop_pdma_cb;
-cmdlen = get_cmd(s);
+cmdlen = get_cmd(s, ESP_CMDBUF_SZ);
 if (cmdlen > 0) {
 trace_esp_handle_satn_stop(s->cmdlen);
 s->cmdlen = cmdlen;
-- 
2.20.1




[PATCH v3 42/42] esp: add support for unaligned accesses

2021-03-04 Thread Mark Cave-Ayland
When the MacOS toolbox ROM transfers data from a target device to an unaligned
memory address, the first/last byte of a 16-bit transfer needs to be handled
separately. This means that the first byte is preloaded into the FIFO before
the transfer, or the last byte remains in the FIFO after the transfer.

The result of this is that the PDMA routines must be updated so that the FIFO
is loaded/unloaded if the last 16-bit word is used (rather than the last byte)
and any remaining byte from a FIFO wraparound is handled correctly.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 48 +---
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f828e70865..507ab363bc 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -498,11 +498,22 @@ static void do_dma_pdma_cb(ESPState *s)
 
 if (to_device) {
 /* Copy FIFO data to device */
-len = MIN(fifo8_num_used(>fifo), ESP_FIFO_SZ);
+len = MIN(s->async_len, ESP_FIFO_SZ);
+len = MIN(len, fifo8_num_used(>fifo));
 memcpy(s->async_buf, fifo8_pop_buf(>fifo, len, ), len);
-s->async_buf += len;
-s->async_len -= len;
-s->ti_size += len;
+s->async_buf += n;
+s->async_len -= n;
+s->ti_size += n;
+
+if (n < len) {
+/* Unaligned accesses can cause FIFO wraparound */
+len = len - n;
+memcpy(s->async_buf, fifo8_pop_buf(>fifo, len, ), len);
+s->async_buf += n;
+s->async_len -= n;
+s->ti_size += n;
+}
+
 if (s->async_len == 0) {
 scsi_req_continue(s->current_req);
 return;
@@ -526,12 +537,18 @@ static void do_dma_pdma_cb(ESPState *s)
 
 if (esp_get_tc(s) != 0) {
 /* Copy device data to FIFO */
-len = MIN(s->async_len, fifo8_num_free(>fifo));
+len = MIN(s->async_len, esp_get_tc(s));
+len = MIN(len, fifo8_num_free(>fifo));
 fifo8_push_all(>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 (esp_get_tc(s) == 0) {
+/* Indicate transfer to FIFO is complete */
+ s->rregs[ESP_RSTAT] |= STAT_TC;
+}
 return;
 }
 
@@ -606,12 +623,29 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_write) {
 s->dma_memory_write(s->dma_opaque, s->async_buf, len);
 } else {
+/* Adjust TC for any leftover data in the FIFO */
+if (!fifo8_is_empty(>fifo)) {
+esp_set_tc(s, esp_get_tc(s) - fifo8_num_used(>fifo));
+}
+
 /* Copy device data to FIFO */
 len = MIN(len, fifo8_num_free(>fifo));
 fifo8_push_all(>fifo, s->async_buf, len);
 s->async_buf += len;
 s->async_len -= len;
 s->ti_size -= len;
+
+/*
+ * MacOS toolbox uses a TI length of 16 bytes for all commands, so
+ * commands shorter than this must be padded accordingly
+ */
+if (len < esp_get_tc(s) && esp_get_tc(s) <= ESP_FIFO_SZ) {
+while (fifo8_num_used(>fifo) < ESP_FIFO_SZ) {
+esp_fifo_push(s, 0);
+len++;
+}
+}
+
 esp_set_tc(s, esp_get_tc(s) - len);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
@@ -1160,7 +1194,7 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr 
addr,
 break;
 }
 dmalen = esp_get_tc(s);
-if (dmalen == 0 || fifo8_is_full(>fifo)) {
+if (dmalen == 0 || fifo8_num_free(>fifo) < 2) {
 s->pdma_cb(s);
 }
 }
@@ -1183,7 +1217,7 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, hwaddr 
addr,
 val = (val << 8) | esp_pdma_read(s);
 break;
 }
-if (fifo8_is_empty(>fifo)) {
+if (fifo8_num_used(>fifo) < 2) {
 s->pdma_cb(s);
 }
 return val;
-- 
2.20.1




[PATCH v3 18/42] esp: accumulate SCSI commands for PDMA transfers in cmdbuf instead of pdma_buf

2021-03-04 Thread Mark Cave-Ayland
ESP SCSI commands are already accumulated in cmdbuf and so there is no need to
keep a separate pdma_buf buffer. Accumulate SCSI commands for PDMA transfers in
cmdbuf instead of pdma_buf so update cmdlen accordingly and change pdma_origin
for PDMA transfers to CMD which allows the PDMA origin to be removed.

This commit also removes a stray memcpy() from get_cmd() which is a no-op 
because
cmdlen is always zero at the start of a command.

Notionally the removal of pdma_buf from vmstate_esp_pdma also breaks migration
compatibility for the PDMA subsection until its complete removal by the end of
the series.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 56 +++
 include/hw/scsi/esp.h |  2 --
 2 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index b8d1ec41e9..66caa95815 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -139,8 +139,6 @@ static void set_pdma(ESPState *s, enum pdma_origin_id 
origin,
 static uint8_t *get_pdma_buf(ESPState *s)
 {
 switch (s->pdma_origin) {
-case PDMA:
-return s->pdma_buf;
 case TI:
 return s->ti_buf;
 case CMD:
@@ -161,14 +159,12 @@ static uint8_t esp_pdma_read(ESPState *s)
 }
 
 switch (s->pdma_origin) {
-case PDMA:
-val = s->pdma_buf[s->pdma_cur++];
-break;
 case TI:
 val = s->ti_buf[s->pdma_cur++];
 break;
 case CMD:
-val = s->cmdbuf[s->pdma_cur++];
+val = s->cmdbuf[s->cmdlen++];
+s->pdma_cur++;
 break;
 case ASYNC:
 val = s->async_buf[s->pdma_cur++];
@@ -193,14 +189,12 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 }
 
 switch (s->pdma_origin) {
-case PDMA:
-s->pdma_buf[s->pdma_cur++] = val;
-break;
 case TI:
 s->ti_buf[s->pdma_cur++] = val;
 break;
 case CMD:
-s->cmdbuf[s->pdma_cur++] = val;
+s->cmdbuf[s->cmdlen++] = val;
+s->pdma_cur++;
 break;
 case ASYNC:
 s->async_buf[s->pdma_cur++] = val;
@@ -256,8 +250,7 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf, uint8_t 
buflen)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, buf, dmalen);
 } else {
-memcpy(s->pdma_buf, buf, dmalen);
-set_pdma(s, PDMA, 0, dmalen);
+set_pdma(s, CMD, 0, dmalen);
 esp_raise_drq(s);
 return 0;
 }
@@ -316,24 +309,24 @@ static void satn_pdma_cb(ESPState *s)
 if (get_cmd_cb(s) < 0) {
 return;
 }
-if (s->pdma_cur != s->pdma_start) {
-do_cmd(s, get_pdma_buf(s) + s->pdma_start);
+s->do_cmd = 0;
+if (s->cmdlen) {
+do_cmd(s, s->cmdbuf);
 }
 }
 
 static void handle_satn(ESPState *s)
 {
-uint8_t buf[32];
-int len;
-
 if (s->dma && !s->dma_enabled) {
 s->dma_cb = handle_satn;
 return;
 }
 s->pdma_cb = satn_pdma_cb;
-len = get_cmd(s, buf, sizeof(buf));
-if (len) {
-do_cmd(s, buf);
+s->cmdlen = get_cmd(s, s->cmdbuf, sizeof(s->cmdbuf));
+if (s->cmdlen) {
+do_cmd(s, s->cmdbuf);
+} else {
+s->do_cmd = 1;
 }
 }
 
@@ -342,24 +335,24 @@ static void s_without_satn_pdma_cb(ESPState *s)
 if (get_cmd_cb(s) < 0) {
 return;
 }
-if (s->pdma_cur != s->pdma_start) {
+s->do_cmd = 0;
+if (s->cmdlen) {
 do_busid_cmd(s, get_pdma_buf(s) + s->pdma_start, 0);
 }
 }
 
 static void handle_s_without_atn(ESPState *s)
 {
-uint8_t buf[32];
-int len;
-
 if (s->dma && !s->dma_enabled) {
 s->dma_cb = handle_s_without_atn;
 return;
 }
 s->pdma_cb = s_without_satn_pdma_cb;
-len = get_cmd(s, buf, sizeof(buf));
-if (len) {
-do_busid_cmd(s, buf, 0);
+s->cmdlen = get_cmd(s, s->cmdbuf, sizeof(s->cmdbuf));
+if (s->cmdlen) {
+do_busid_cmd(s, s->cmdbuf, 0);
+} else {
+s->do_cmd = 1;
 }
 }
 
@@ -368,7 +361,7 @@ static void satn_stop_pdma_cb(ESPState *s)
 if (get_cmd_cb(s) < 0) {
 return;
 }
-s->cmdlen = s->pdma_cur - s->pdma_start;
+s->do_cmd = 0;
 if (s->cmdlen) {
 trace_esp_handle_satn_stop(s->cmdlen);
 s->do_cmd = 1;
@@ -394,6 +387,8 @@ static void handle_satn_stop(ESPState *s)
 s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
 esp_raise_irq(s);
+} else {
+s->do_cmd = 1;
 }
 }
 
@@ -864,11 +859,10 @@ static bool esp_pdma_needed(void *opaque)
 
 static const VMStateDescription vmstate_esp_pdma = {
 .name = "esp/pdma",
-.version_id = 1,
-.minimum_version_id = 1,
+.version_id = 2,
+.minimum_version_id = 2,
 .needed = esp_pdma_needed,
 .fields = (VMStateField[]) {
-VMSTATE_BUFFER(pdma_buf, ESPState),
 VMSTATE_INT32(pdma_origin, ESPState),
 

[PATCH v3 28/42] esp: use FIFO for PDMA transfers between initiator and device

2021-03-04 Thread Mark Cave-Ayland
PDMA as implemented on the Quadra 800 uses DREQ to load data into the FIFO
up to a maximum of 16 bytes at a time. The MacOS toolbox ROM requires this
because it mixes FIFO and PDMA transfers whilst checking the FIFO status
and counter registers to ensure success.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 109 ++
 1 file changed, 75 insertions(+), 34 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 10e63d1f62..aa897a4ebd 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -134,13 +134,8 @@ static void set_pdma(ESPState *s, enum pdma_origin_id 
origin)
 
 static uint8_t esp_pdma_read(ESPState *s)
 {
-uint32_t dmalen = esp_get_tc(s);
 uint8_t val;
 
-if (dmalen == 0) {
-return 0;
-}
-
 switch (s->pdma_origin) {
 case TI:
 if (s->do_cmd) {
@@ -160,10 +155,6 @@ static uint8_t esp_pdma_read(ESPState *s)
 g_assert_not_reached();
 }
 
-s->ti_size--;
-dmalen--;
-esp_set_tc(s, dmalen);
-
 return val;
 }
 
@@ -194,7 +185,6 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 g_assert_not_reached();
 }
 
-s->ti_size++;
 dmalen--;
 esp_set_tc(s, dmalen);
 }
@@ -290,6 +280,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t 
busid)
 s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
 esp_raise_irq(s);
+esp_lower_drq(s);
 }
 
 static void do_cmd(ESPState *s)
@@ -447,28 +438,71 @@ static void esp_dma_done(ESPState *s)
 static void do_dma_pdma_cb(ESPState *s)
 {
 int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
+int len;
 
 if (s->do_cmd) {
 s->ti_size = 0;
 s->cmdlen = 0;
 s->do_cmd = 0;
 do_cmd(s);
+esp_lower_drq(s);
 return;
 }
-if (s->async_len == 0) {
-scsi_req_continue(s->current_req);
-/*
- * If there is still data to be read from the device then
- * complete the DMA operation immediately.  Otherwise defer
- * until the scsi layer has completed.
- */
-if (to_device || esp_get_tc(s) != 0 || s->ti_size == 0) {
+
+if (to_device) {
+/* Copy FIFO data to device */
+len = MIN(s->ti_wptr, TI_BUFSZ);
+memcpy(s->async_buf, s->ti_buf, len);
+s->ti_wptr = 0;
+s->ti_rptr = 0;
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size += len;
+if (s->async_len == 0) {
+scsi_req_continue(s->current_req);
 return;
 }
-}
 
-/* Partially filled a scsi buffer. Complete immediately.  */
-esp_dma_done(s);
+if (esp_get_tc(s) == 0) {
+esp_lower_drq(s);
+esp_dma_done(s);
+}
+
+return;
+} else {
+if (s->async_len == 0) {
+if (s->current_req) {
+scsi_req_continue(s->current_req);
+}
+
+/*
+ * If there is still data to be read from the device then
+ * complete the DMA operation immediately.  Otherwise defer
+ * until the scsi layer has completed.
+ */
+if (esp_get_tc(s) != 0 || s->ti_size == 0) {
+return;
+}
+}
+
+if (esp_get_tc(s) != 0) {
+/* Copy device data to FIFO */
+s->ti_wptr = 0;
+s->ti_rptr = 0;
+len = MIN(s->async_len, TI_BUFSZ);
+memcpy(s->ti_buf, s->async_buf, len);
+s->ti_wptr += len;
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size -= len;
+esp_set_tc(s, esp_get_tc(s) - len);
+return;
+}
+
+/* Partially filled a scsi buffer. Complete immediately.  */
+esp_lower_drq(s);
+esp_dma_done(s);
+}
 }
 
 static void esp_do_dma(ESPState *s)
@@ -511,7 +545,7 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, s->async_buf, len);
 } else {
-set_pdma(s, ASYNC);
+set_pdma(s, TI);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -520,9 +554,20 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_write) {
 s->dma_memory_write(s->dma_opaque, s->async_buf, len);
 } else {
-set_pdma(s, ASYNC);
+/* Copy device data to FIFO */
+len = MIN(len, TI_BUFSZ - s->ti_wptr);
+memcpy(>ti_buf[s->ti_wptr], s->async_buf, len);
+s->ti_wptr += len;
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size -= len;
+esp_set_tc(s, esp_get_tc(s) - len);
+set_pdma(s, TI);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
+
+/* Indicate transfer to FIFO is complete */
+   

[PATCH v3 30/42] esp: add 4 byte PDMA read and write transfers

2021-03-04 Thread Mark Cave-Ayland
The MacOS toolbox ROM performs 4 byte reads/writes when transferring data to
and from the target. Since the SCSI bus is 16-bits wide, use the memory API
to split a 4 byte access into 2 x 2 byte accesses.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 79b84e31d3..2dded90be6 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -1002,7 +1002,9 @@ static const MemoryRegionOps sysbus_esp_pdma_ops = {
 .write = sysbus_esp_pdma_write,
 .endianness = DEVICE_NATIVE_ENDIAN,
 .valid.min_access_size = 1,
-.valid.max_access_size = 2,
+.valid.max_access_size = 4,
+.impl.min_access_size = 1,
+.impl.max_access_size = 2,
 };
 
 static const struct SCSIBusInfo esp_scsi_info = {
@@ -1049,7 +1051,7 @@ static void sysbus_esp_realize(DeviceState *dev, Error 
**errp)
   sysbus, "esp-regs", ESP_REGS << sysbus->it_shift);
 sysbus_init_mmio(sbd, >iomem);
 memory_region_init_io(>pdma, OBJECT(sysbus), _esp_pdma_ops,
-  sysbus, "esp-pdma", 2);
+  sysbus, "esp-pdma", 4);
 sysbus_init_mmio(sbd, >pdma);
 
 qdev_init_gpio_in(dev, sysbus_esp_gpio_demux, 2);
-- 
2.20.1




[PATCH v3 41/42] esp: implement non-DMA transfers in PDMA mode

2021-03-04 Thread Mark Cave-Ayland
The MacOS toolbox ROM uses non-DMA TI commands to handle the first/last byte
of an unaligned 16-bit transfer to memory.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 133 ++
 include/hw/scsi/esp.h |   1 +
 2 files changed, 98 insertions(+), 36 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 8a9b1500de..f828e70865 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -296,6 +296,7 @@ static void do_busid_cmd(ESPState *s, uint8_t busid)
 if (datalen != 0) {
 s->rregs[ESP_RSTAT] = STAT_TC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
+s->ti_cmd = 0;
 esp_set_tc(s, 0);
 if (datalen > 0) {
 /*
@@ -645,6 +646,71 @@ static void esp_do_dma(ESPState *s)
 esp_lower_drq(s);
 }
 
+static void esp_do_nodma(ESPState *s)
+{
+int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
+uint32_t cmdlen, n;
+int len;
+
+if (s->do_cmd) {
+cmdlen = fifo8_num_used(>cmdfifo);
+trace_esp_handle_ti_cmd(cmdlen);
+s->ti_size = 0;
+if ((s->rregs[ESP_RSTAT] & 7) == STAT_CD) {
+/* No command received */
+if (s->cmdfifo_cdb_offset == fifo8_num_used(>cmdfifo)) {
+return;
+}
+
+/* Command has been received */
+s->do_cmd = 0;
+do_cmd(s);
+} else {
+/*
+ * Extra message out bytes received: update cmdfifo_cdb_offset
+ * and then switch to commmand phase
+ */
+s->cmdfifo_cdb_offset = fifo8_num_used(>cmdfifo);
+s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
+s->rregs[ESP_RSEQ] = SEQ_CD;
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+}
+return;
+}
+
+if (s->async_len == 0) {
+/* Defer until data is available.  */
+return;
+}
+
+if (to_device) {
+len = MIN(fifo8_num_used(>fifo), ESP_FIFO_SZ);
+memcpy(s->async_buf, fifo8_pop_buf(>fifo, len, ), len);
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size += len;
+} else {
+len = MIN(s->ti_size, s->async_len);
+len = MIN(len, fifo8_num_free(>fifo));
+fifo8_push_all(>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);
+
+if (to_device || s->ti_size == 0) {
+return;
+}
+}
+
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+}
+
 void esp_command_complete(SCSIRequest *req, size_t resid)
 {
 ESPState *s = req->hba_private;
@@ -701,56 +767,51 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
 return;
 }
 
-if (dmalen) {
-esp_do_dma(s);
-} else if (s->ti_size <= 0) {
+if (s->ti_cmd == 0) {
 /*
- * If this was the last part of a DMA transfer then the
- * completion interrupt is deferred to here.
+ * Always perform the initial transfer upon reception of the next TI
+ * command to ensure the DMA/non-DMA status of the command is correct.
+ * It is not possible to use s->dma directly in the section below as
+ * some OSs send non-DMA NOP commands after a DMA transfer. Hence if 
the
+ * async data transfer is delayed then s->dma is set incorrectly.
  */
-esp_dma_done(s);
-esp_lower_drq(s);
+return;
+}
+
+if (s->ti_cmd & CMD_DMA) {
+if (dmalen) {
+esp_do_dma(s);
+} else if (s->ti_size <= 0) {
+/*
+ * If this was the last part of a DMA transfer then the
+ * completion interrupt is deferred to here.
+ */
+esp_dma_done(s);
+esp_lower_drq(s);
+}
+} else {
+esp_do_nodma(s);
 }
 }
 
 static void handle_ti(ESPState *s)
 {
-uint32_t dmalen, cmdlen;
+uint32_t dmalen;
 
 if (s->dma && !s->dma_enabled) {
 s->dma_cb = handle_ti;
 return;
 }
 
-dmalen = esp_get_tc(s);
+s->ti_cmd = s->rregs[ESP_CMD];
 if (s->dma) {
+dmalen = esp_get_tc(s);
 trace_esp_handle_ti(dmalen);
 s->rregs[ESP_RSTAT] &= ~STAT_TC;
 esp_do_dma(s);
-} else if (s->do_cmd) {
-cmdlen = fifo8_num_used(>cmdfifo);
-trace_esp_handle_ti_cmd(cmdlen);
-s->ti_size = 0;
-if ((s->rregs[ESP_RSTAT] & 7) == STAT_CD) {
-/* No command received */
-if (s->cmdfifo_cdb_offset == fifo8_num_used(>cmdfifo)) {
-return;
-}
-
-/* Command has been received */
-s->do_cmd = 0;
-do_cmd(s);
-} else {
-/*
- * Extra message out bytes received: update cmdfifo_cdb_offset
- * and then switch to commmand 

[PATCH v3 16/42] esp: use pdma_origin directly in esp_pdma_read()/esp_pdma_write()

2021-03-04 Thread Mark Cave-Ayland
This is the first step in removing get_pdma_buf() from esp.c.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 34 --
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 0fafc866a4..58be98f047 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -153,16 +153,38 @@ static uint8_t *get_pdma_buf(ESPState *s)
 
 static uint8_t esp_pdma_read(ESPState *s)
 {
-uint8_t *buf = get_pdma_buf(s);
-
-return buf[s->pdma_cur++];
+switch (s->pdma_origin) {
+case PDMA:
+return s->pdma_buf[s->pdma_cur++];
+case TI:
+return s->ti_buf[s->pdma_cur++];
+case CMD:
+return s->cmdbuf[s->pdma_cur++];
+case ASYNC:
+return s->async_buf[s->pdma_cur++];
+default:
+g_assert_not_reached();
+}
 }
 
 static void esp_pdma_write(ESPState *s, uint8_t val)
 {
-uint8_t *buf = get_pdma_buf(s);
-
-buf[s->pdma_cur++] = val;
+switch (s->pdma_origin) {
+case PDMA:
+s->pdma_buf[s->pdma_cur++] = val;
+break;
+case TI:
+s->ti_buf[s->pdma_cur++] = val;
+break;
+case CMD:
+s->cmdbuf[s->pdma_cur++] = val;
+break;
+case ASYNC:
+s->async_buf[s->pdma_cur++] = val;
+break;
+default:
+g_assert_not_reached();
+}
 }
 
 static int get_cmd_cb(ESPState *s)
-- 
2.20.1




[PATCH v3 24/42] esp: use in-built TC to determine PDMA transfer length

2021-03-04 Thread Mark Cave-Ayland
Real hardware simply counts down using the in-built TC to determine when the
the PDMA request is complete. Use the TC to determine the PDMA transfer length
which then enables us to remove the redundant pdma_len variable.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 28 +---
 include/hw/scsi/esp.h |  1 -
 2 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 7bf2ec9c94..c80dc606e8 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -127,10 +127,9 @@ static uint32_t esp_get_stc(ESPState *s)
 return dmalen;
 }
 
-static void set_pdma(ESPState *s, enum pdma_origin_id origin, uint32_t len)
+static void set_pdma(ESPState *s, enum pdma_origin_id origin)
 {
 s->pdma_origin = origin;
-s->pdma_len = len;
 }
 
 static uint8_t esp_pdma_read(ESPState *s)
@@ -138,7 +137,7 @@ static uint8_t esp_pdma_read(ESPState *s)
 uint32_t dmalen = esp_get_tc(s);
 uint8_t val;
 
-if (dmalen == 0 || s->pdma_len == 0) {
+if (dmalen == 0) {
 return 0;
 }
 
@@ -161,7 +160,6 @@ static uint8_t esp_pdma_read(ESPState *s)
 }
 
 s->ti_size--;
-s->pdma_len--;
 dmalen--;
 esp_set_tc(s, dmalen);
 
@@ -172,7 +170,7 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 {
 uint32_t dmalen = esp_get_tc(s);
 
-if (dmalen == 0 || s->pdma_len == 0) {
+if (dmalen == 0) {
 return;
 }
 
@@ -195,7 +193,6 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 }
 
 s->ti_size++;
-s->pdma_len--;
 dmalen--;
 esp_set_tc(s, dmalen);
 }
@@ -243,7 +240,7 @@ static uint32_t get_cmd(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, buf, dmalen);
 } else {
-set_pdma(s, CMD, dmalen);
+set_pdma(s, CMD);
 esp_raise_drq(s);
 return 0;
 }
@@ -406,7 +403,7 @@ static void write_response(ESPState *s)
 s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
 } else {
-set_pdma(s, TI, 2);
+set_pdma(s, TI);
 s->pdma_cb = write_response_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -474,7 +471,7 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, >cmdbuf[s->cmdlen], len);
 } else {
-set_pdma(s, CMD, len);
+set_pdma(s, CMD);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -497,7 +494,7 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, s->async_buf, len);
 } else {
-set_pdma(s, ASYNC, len);
+set_pdma(s, ASYNC);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -506,7 +503,7 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_write) {
 s->dma_memory_write(s->dma_opaque, s->async_buf, len);
 } else {
-set_pdma(s, ASYNC, len);
+set_pdma(s, ASYNC);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -850,7 +847,6 @@ static const VMStateDescription vmstate_esp_pdma = {
 .needed = esp_pdma_needed,
 .fields = (VMStateField[]) {
 VMSTATE_INT32(pdma_origin, ESPState),
-VMSTATE_UINT32(pdma_len, ESPState),
 VMSTATE_END_OF_LIST()
 }
 };
@@ -949,6 +945,7 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr addr,
 {
 SysBusESPState *sysbus = opaque;
 ESPState *s = ESP(>esp);
+uint32_t dmalen;
 
 trace_esp_pdma_write(size);
 
@@ -961,7 +958,8 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr addr,
 esp_pdma_write(s, val);
 break;
 }
-if (s->pdma_len == 0 && s->pdma_cb) {
+dmalen = esp_get_tc(s);
+if (dmalen == 0 && s->pdma_cb) {
 esp_lower_drq(s);
 s->pdma_cb(s);
 s->pdma_cb = NULL;
@@ -978,7 +976,7 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, hwaddr 
addr,
 
 trace_esp_pdma_read(size);
 
-if (dmalen == 0 || s->pdma_len == 0) {
+if (dmalen == 0) {
 return 0;
 }
 switch (size) {
@@ -991,7 +989,7 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, hwaddr 
addr,
 break;
 }
 dmalen = esp_get_tc(s);
-if (dmalen == 0 || (s->pdma_len == 0 && s->pdma_cb)) {
+if (dmalen == 0 && s->pdma_cb) {
 esp_lower_drq(s);
 s->pdma_cb(s);
 s->pdma_cb = NULL;
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 26bd015cf4..1be4586aff 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -57,7 +57,6 @@ struct ESPState {
 void *dma_opaque;
 void (*dma_cb)(ESPState *s);
 int pdma_origin;
-uint32_t pdma_len;
 void 

[PATCH v3 29/42] esp: remove pdma_origin from ESPState

2021-03-04 Thread Mark Cave-Ayland
Now that all data is transferred via the FIFO (ti_buf) there is no need to track
the source buffer being used for the data transfer. This also eliminates the
need for a separate subsection for PDMA state migration.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 74 +--
 include/hw/scsi/esp.h |  6 
 2 files changed, 8 insertions(+), 72 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index aa897a4ebd..79b84e31d3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -127,32 +127,14 @@ static uint32_t esp_get_stc(ESPState *s)
 return dmalen;
 }
 
-static void set_pdma(ESPState *s, enum pdma_origin_id origin)
-{
-s->pdma_origin = origin;
-}
-
 static uint8_t esp_pdma_read(ESPState *s)
 {
 uint8_t val;
 
-switch (s->pdma_origin) {
-case TI:
-if (s->do_cmd) {
-val = s->cmdbuf[s->cmdlen++];
-} else {
-val = s->ti_buf[s->ti_rptr++];
-}
-break;
-case ASYNC:
-val = s->async_buf[0];
-if (s->async_len > 0) {
-s->async_len--;
-s->async_buf++;
-}
-break;
-default:
-g_assert_not_reached();
+if (s->do_cmd) {
+val = s->cmdbuf[s->cmdlen++];
+} else {
+val = s->ti_buf[s->ti_rptr++];
 }
 
 return val;
@@ -166,23 +148,10 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 return;
 }
 
-switch (s->pdma_origin) {
-case TI:
-if (s->do_cmd) {
-s->cmdbuf[s->cmdlen++] = val;
-} else {
-s->ti_buf[s->ti_wptr++] = val;
-}
-break;
-case ASYNC:
-s->async_buf[0] = val;
-if (s->async_len > 0) {
-s->async_len--;
-s->async_buf++;
-}
-break;
-default:
-g_assert_not_reached();
+if (s->do_cmd) {
+s->cmdbuf[s->cmdlen++] = val;
+} else {
+s->ti_buf[s->ti_wptr++] = val;
 }
 
 dmalen--;
@@ -232,7 +201,6 @@ static uint32_t get_cmd(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, buf, dmalen);
 } else {
-set_pdma(s, TI);
 if (esp_select(s) < 0) {
 return -1;
 }
@@ -411,7 +379,6 @@ static void write_response(ESPState *s)
 s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
 } else {
-set_pdma(s, TI);
 s->pdma_cb = write_response_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -522,7 +489,6 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, >cmdbuf[s->cmdlen], len);
 } else {
-set_pdma(s, TI);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -545,7 +511,6 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, s->async_buf, len);
 } else {
-set_pdma(s, TI);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -562,7 +527,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);
-set_pdma(s, TI);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
 
@@ -898,24 +862,6 @@ static bool esp_mem_accepts(void *opaque, hwaddr addr,
 return (size == 1) || (is_write && size == 4);
 }
 
-static bool esp_pdma_needed(void *opaque)
-{
-ESPState *s = 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 = 2,
-.minimum_version_id = 2,
-.needed = esp_pdma_needed,
-.fields = (VMStateField[]) {
-VMSTATE_INT32(pdma_origin, ESPState),
-VMSTATE_END_OF_LIST()
-}
-};
-
 static bool esp_is_before_version_5(void *opaque, int version_id)
 {
 ESPState *s = ESP(opaque);
@@ -970,10 +916,6 @@ const VMStateDescription vmstate_esp = {
 VMSTATE_UINT32_TEST(mig_dma_left, ESPState, esp_is_before_version_5),
 VMSTATE_END_OF_LIST()
 },
-.subsections = (const VMStateDescription * []) {
-_esp_pdma,
-NULL
-}
 };
 
 static void sysbus_esp_mem_write(void *opaque, hwaddr addr,
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index d3fc52..91f8ffd6c8 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -15,11 +15,6 @@ typedef void (*ESPDMAMemoryReadWriteFunc)(void *opaque, 
uint8_t *buf, int len);
 
 typedef struct ESPState ESPState;
 
-enum pdma_origin_id {
-TI,
-ASYNC,
-};
-
 #define TYPE_ESP "esp"
 OBJECT_DECLARE_SIMPLE_TYPE(ESPState, ESP)
 
@@ -55,7 +50,6 @@ struct ESPState {
 

[PATCH v3 34/42] esp: remove old deferred command completion mechanism

2021-03-04 Thread Mark Cave-Ayland
Commit ea84a44250 "scsi: esp: Defer command completion until previous interrupts
have been handled" provided a mechanism to delay the command completion 
interrupt
until ESP_RINTR is read after the command has completed.

With the previous fixes for latching the ESP_RINTR bits and deferring the 
setting
of the command completion interrupt for incoming data to the SCSI callback, this
workaround is no longer required and can be removed.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 35 +--
 include/hw/scsi/esp.h |  4 ++--
 2 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 0eecc1d05c..eb6681ca66 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -568,18 +568,20 @@ static void esp_do_dma(ESPState *s)
 esp_lower_drq(s);
 }
 
-static void esp_report_command_complete(ESPState *s, uint32_t status)
+void esp_command_complete(SCSIRequest *req, size_t resid)
 {
+ESPState *s = req->hba_private;
+
 trace_esp_command_complete();
 if (s->ti_size != 0) {
 trace_esp_command_complete_unexpected();
 }
 s->ti_size = 0;
 s->async_len = 0;
-if (status) {
+if (req->status) {
 trace_esp_command_complete_fail();
 }
-s->status = status;
+s->status = req->status;
 s->rregs[ESP_RSTAT] = STAT_ST;
 esp_dma_done(s);
 esp_lower_drq(s);
@@ -590,23 +592,6 @@ static void esp_report_command_complete(ESPState *s, 
uint32_t status)
 }
 }
 
-void esp_command_complete(SCSIRequest *req, size_t resid)
-{
-ESPState *s = req->hba_private;
-
-if (s->rregs[ESP_RSTAT] & STAT_INT) {
-/*
- * Defer handling command complete until the previous
- * interrupt has been handled.
- */
-trace_esp_command_complete_deferred();
-s->deferred_status = req->status;
-s->deferred_complete = true;
-return;
-}
-esp_report_command_complete(s, req->status);
-}
-
 void esp_transfer_data(SCSIRequest *req, uint32_t len)
 {
 ESPState *s = req->hba_private;
@@ -733,10 +718,6 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
 s->rregs[ESP_RSTAT] &= ~STAT_TC;
 s->rregs[ESP_RSEQ] = SEQ_0;
 esp_lower_irq(s);
-if (s->deferred_complete) {
-esp_report_command_complete(s, s->deferred_status);
-s->deferred_complete = false;
-}
 break;
 case ESP_TCHI:
 /* Return the unique id if the value has never been written */
@@ -944,8 +925,10 @@ const VMStateDescription vmstate_esp = {
 VMSTATE_UINT32(ti_wptr, ESPState),
 VMSTATE_BUFFER(ti_buf, ESPState),
 VMSTATE_UINT32(status, ESPState),
-VMSTATE_UINT32(deferred_status, ESPState),
-VMSTATE_BOOL(deferred_complete, ESPState),
+VMSTATE_UINT32_TEST(mig_deferred_status, ESPState,
+esp_is_before_version_5),
+VMSTATE_BOOL_TEST(mig_deferred_complete, ESPState,
+  esp_is_before_version_5),
 VMSTATE_UINT32(dma, ESPState),
 VMSTATE_PARTIAL_BUFFER(cmdbuf, ESPState, 16),
 VMSTATE_BUFFER_START_MIDDLE_V(cmdbuf, ESPState, 16, 4),
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 61bc317a4c..7d88fa0f92 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -30,8 +30,6 @@ struct ESPState {
 int32_t ti_size;
 uint32_t ti_rptr, ti_wptr;
 uint32_t status;
-uint32_t deferred_status;
-bool deferred_complete;
 uint32_t dma;
 uint8_t ti_buf[TI_BUFSZ];
 SCSIBus bus;
@@ -57,6 +55,8 @@ struct ESPState {
 
 /* Legacy fields for vmstate_esp version < 5 */
 uint32_t mig_dma_left;
+uint32_t mig_deferred_status;
+bool mig_deferred_complete;
 };
 
 #define TYPE_SYSBUS_ESP "sysbus-esp"
-- 
2.20.1




[PATCH v3 15/42] esp: introduce esp_pdma_read() and esp_pdma_write() functions

2021-03-04 Thread Mark Cave-Ayland
Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 07d57cb791..0fafc866a4 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -151,6 +151,20 @@ static uint8_t *get_pdma_buf(ESPState *s)
 return NULL;
 }
 
+static uint8_t esp_pdma_read(ESPState *s)
+{
+uint8_t *buf = get_pdma_buf(s);
+
+return buf[s->pdma_cur++];
+}
+
+static void esp_pdma_write(ESPState *s, uint8_t val)
+{
+uint8_t *buf = get_pdma_buf(s);
+
+buf[s->pdma_cur++] = val;
+}
+
 static int get_cmd_cb(ESPState *s)
 {
 int target;
@@ -909,7 +923,6 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr addr,
 SysBusESPState *sysbus = opaque;
 ESPState *s = ESP(>esp);
 uint32_t dmalen = esp_get_tc(s);
-uint8_t *buf = get_pdma_buf(s);
 
 trace_esp_pdma_write(size);
 
@@ -918,13 +931,13 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr 
addr,
 }
 switch (size) {
 case 1:
-buf[s->pdma_cur++] = val;
+esp_pdma_write(s, val);
 s->pdma_len--;
 dmalen--;
 break;
 case 2:
-buf[s->pdma_cur++] = val >> 8;
-buf[s->pdma_cur++] = val;
+esp_pdma_write(s, val >> 8);
+esp_pdma_write(s, val);
 s->pdma_len -= 2;
 dmalen -= 2;
 break;
@@ -943,7 +956,6 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, hwaddr 
addr,
 SysBusESPState *sysbus = opaque;
 ESPState *s = ESP(>esp);
 uint32_t dmalen = esp_get_tc(s);
-uint8_t *buf = get_pdma_buf(s);
 uint64_t val = 0;
 
 trace_esp_pdma_read(size);
@@ -953,13 +965,13 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, hwaddr 
addr,
 }
 switch (size) {
 case 1:
-val = buf[s->pdma_cur++];
+val = esp_pdma_read(s);
 s->pdma_len--;
 dmalen--;
 break;
 case 2:
-val = buf[s->pdma_cur++];
-val = (val << 8) | buf[s->pdma_cur++];
+val = esp_pdma_read(s);
+val = (val << 8) | esp_pdma_read(s);
 s->pdma_len -= 2;
 dmalen -= 2;
 break;
-- 
2.20.1




[PATCH v3 23/42] esp: use ti_wptr/ti_rptr to manage the current FIFO position for PDMA

2021-03-04 Thread Mark Cave-Ayland
This eliminates the last user of the PDMA-specific pdma_cur variable which can
now be removed.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 23 ---
 include/hw/scsi/esp.h |  1 -
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index bb3a9cd5e3..7bf2ec9c94 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -127,11 +127,9 @@ static uint32_t esp_get_stc(ESPState *s)
 return dmalen;
 }
 
-static void set_pdma(ESPState *s, enum pdma_origin_id origin,
- uint32_t index, uint32_t len)
+static void set_pdma(ESPState *s, enum pdma_origin_id origin, uint32_t len)
 {
 s->pdma_origin = origin;
-s->pdma_cur = index;
 s->pdma_len = len;
 }
 
@@ -146,11 +144,10 @@ static uint8_t esp_pdma_read(ESPState *s)
 
 switch (s->pdma_origin) {
 case TI:
-val = s->ti_buf[s->pdma_cur++];
+val = s->ti_buf[s->ti_rptr++];
 break;
 case CMD:
 val = s->cmdbuf[s->cmdlen++];
-s->pdma_cur++;
 break;
 case ASYNC:
 val = s->async_buf[0];
@@ -158,7 +155,6 @@ static uint8_t esp_pdma_read(ESPState *s)
 s->async_len--;
 s->async_buf++;
 }
-s->pdma_cur++;
 break;
 default:
 g_assert_not_reached();
@@ -182,11 +178,10 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 
 switch (s->pdma_origin) {
 case TI:
-s->ti_buf[s->pdma_cur++] = val;
+s->ti_buf[s->ti_wptr++] = val;
 break;
 case CMD:
 s->cmdbuf[s->cmdlen++] = val;
-s->pdma_cur++;
 break;
 case ASYNC:
 s->async_buf[0] = val;
@@ -194,7 +189,6 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 s->async_len--;
 s->async_buf++;
 }
-s->pdma_cur++;
 break;
 default:
 g_assert_not_reached();
@@ -249,7 +243,7 @@ static uint32_t get_cmd(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, buf, dmalen);
 } else {
-set_pdma(s, CMD, 0, dmalen);
+set_pdma(s, CMD, dmalen);
 esp_raise_drq(s);
 return 0;
 }
@@ -412,7 +406,7 @@ static void write_response(ESPState *s)
 s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
 } else {
-set_pdma(s, TI, 0, 2);
+set_pdma(s, TI, 2);
 s->pdma_cb = write_response_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -480,7 +474,7 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, >cmdbuf[s->cmdlen], len);
 } else {
-set_pdma(s, CMD, s->cmdlen, len);
+set_pdma(s, CMD, len);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -503,7 +497,7 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_read) {
 s->dma_memory_read(s->dma_opaque, s->async_buf, len);
 } else {
-set_pdma(s, ASYNC, 0, len);
+set_pdma(s, ASYNC, len);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -512,7 +506,7 @@ static void esp_do_dma(ESPState *s)
 if (s->dma_memory_write) {
 s->dma_memory_write(s->dma_opaque, s->async_buf, len);
 } else {
-set_pdma(s, ASYNC, 0, len);
+set_pdma(s, ASYNC, len);
 s->pdma_cb = do_dma_pdma_cb;
 esp_raise_drq(s);
 return;
@@ -857,7 +851,6 @@ static const VMStateDescription vmstate_esp_pdma = {
 .fields = (VMStateField[]) {
 VMSTATE_INT32(pdma_origin, ESPState),
 VMSTATE_UINT32(pdma_len, ESPState),
-VMSTATE_UINT32(pdma_cur, ESPState),
 VMSTATE_END_OF_LIST()
 }
 };
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 55b0aee762..26bd015cf4 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -58,7 +58,6 @@ struct ESPState {
 void (*dma_cb)(ESPState *s);
 int pdma_origin;
 uint32_t pdma_len;
-uint32_t pdma_cur;
 void (*pdma_cb)(ESPState *s);
 
 uint8_t mig_version_id;
-- 
2.20.1




[PATCH v3 21/42] esp: remove redundant pdma_start from ESPState

2021-03-04 Thread Mark Cave-Ayland
Now that PDMA SCSI commands are accumulated in cmdbuf in the same way as normal
commands, the existing logic for locating the start of the SCSI command in
cmdbuf via cmdlen can be used. This enables the PDMA-specific pdma_start and
also get_pdma_buf() to be removed.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 19 ++-
 include/hw/scsi/esp.h |  1 -
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 44fddf082c..38c05e97c3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -131,24 +131,10 @@ static void set_pdma(ESPState *s, enum pdma_origin_id 
origin,
  uint32_t index, uint32_t len)
 {
 s->pdma_origin = origin;
-s->pdma_start = index;
 s->pdma_cur = index;
 s->pdma_len = len;
 }
 
-static uint8_t *get_pdma_buf(ESPState *s)
-{
-switch (s->pdma_origin) {
-case TI:
-return s->ti_buf;
-case CMD:
-return s->cmdbuf;
-case ASYNC:
-return s->async_buf;
-}
-return NULL;
-}
-
 static uint8_t esp_pdma_read(ESPState *s)
 {
 uint32_t dmalen = esp_get_tc(s);
@@ -339,7 +325,7 @@ static void s_without_satn_pdma_cb(ESPState *s)
 }
 s->do_cmd = 0;
 if (s->cmdlen) {
-do_busid_cmd(s, get_pdma_buf(s) + s->pdma_start, 0);
+do_busid_cmd(s, s->cmdbuf, 0);
 }
 }
 
@@ -441,7 +427,7 @@ static void esp_dma_done(ESPState *s)
 static void do_dma_pdma_cb(ESPState *s)
 {
 int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
-int len = s->pdma_cur - s->pdma_start;
+int len = s->pdma_cur;
 
 if (s->do_cmd) {
 s->ti_size = 0;
@@ -867,7 +853,6 @@ static const VMStateDescription vmstate_esp_pdma = {
 .fields = (VMStateField[]) {
 VMSTATE_INT32(pdma_origin, ESPState),
 VMSTATE_UINT32(pdma_len, ESPState),
-VMSTATE_UINT32(pdma_start, ESPState),
 VMSTATE_UINT32(pdma_cur, ESPState),
 VMSTATE_END_OF_LIST()
 }
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 600d0c31ab..55b0aee762 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -58,7 +58,6 @@ struct ESPState {
 void (*dma_cb)(ESPState *s);
 int pdma_origin;
 uint32_t pdma_len;
-uint32_t pdma_start;
 uint32_t pdma_cur;
 void (*pdma_cb)(ESPState *s);
 
-- 
2.20.1




[PATCH v3 26/42] esp: rename get_cmd_cb() to esp_select()

2021-03-04 Thread Mark Cave-Ayland
This better describes the purpose of the function.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index d5c03f9697..d8d7ede00a 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -199,7 +199,7 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 esp_set_tc(s, dmalen);
 }
 
-static int get_cmd_cb(ESPState *s)
+static int esp_select(ESPState *s)
 {
 int target;
 
@@ -256,7 +256,7 @@ static uint32_t get_cmd(ESPState *s)
 }
 trace_esp_get_cmd(dmalen, target);
 
-if (get_cmd_cb(s) < 0) {
+if (esp_select(s) < 0) {
 return 0;
 }
 return dmalen;
@@ -299,7 +299,7 @@ static void do_cmd(ESPState *s)
 
 static void satn_pdma_cb(ESPState *s)
 {
-if (get_cmd_cb(s) < 0) {
+if (esp_select(s) < 0) {
 return;
 }
 s->do_cmd = 0;
@@ -325,7 +325,7 @@ static void handle_satn(ESPState *s)
 
 static void s_without_satn_pdma_cb(ESPState *s)
 {
-if (get_cmd_cb(s) < 0) {
+if (esp_select(s) < 0) {
 return;
 }
 s->do_cmd = 0;
@@ -351,7 +351,7 @@ static void handle_s_without_atn(ESPState *s)
 
 static void satn_stop_pdma_cb(ESPState *s)
 {
-if (get_cmd_cb(s) < 0) {
+if (esp_select(s) < 0) {
 return;
 }
 s->do_cmd = 0;
-- 
2.20.1




[PATCH v3 14/42] esp: remove minlen restriction in handle_ti

2021-03-04 Thread Mark Cave-Ayland
The limiting of DMA transfers to the maximum size of the available data is 
already
handled by esp_do_dma() and do_dma_pdma_cb().

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 92fea6a8c4..07d57cb791 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -552,7 +552,7 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
 
 static void handle_ti(ESPState *s)
 {
-uint32_t dmalen, minlen;
+uint32_t dmalen;
 
 if (s->dma && !s->dma_enabled) {
 s->dma_cb = handle_ti;
@@ -560,16 +560,8 @@ static void handle_ti(ESPState *s)
 }
 
 dmalen = esp_get_tc(s);
-
-if (s->do_cmd) {
-minlen = (dmalen < ESP_CMDBUF_SZ) ? dmalen : ESP_CMDBUF_SZ;
-} else if (s->ti_size < 0) {
-minlen = (dmalen < -s->ti_size) ? dmalen : -s->ti_size;
-} else {
-minlen = (dmalen < s->ti_size) ? dmalen : s->ti_size;
-}
-trace_esp_handle_ti(minlen);
 if (s->dma) {
+trace_esp_handle_ti(dmalen);
 s->rregs[ESP_RSTAT] &= ~STAT_TC;
 esp_do_dma(s);
 } else if (s->do_cmd) {
-- 
2.20.1




[PATCH v3 22/42] esp: move PDMA length adjustments into esp_pdma_read()/esp_pdma_write()

2021-03-04 Thread Mark Cave-Ayland
Here the updates to async_len and ti_size are moved into the corresponding
esp_pdma_read()/esp_pdma_write() function to eliminate the reference to
pdma_cur in do_dma_pdma_cb().

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 38c05e97c3..bb3a9cd5e3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -153,12 +153,18 @@ static uint8_t esp_pdma_read(ESPState *s)
 s->pdma_cur++;
 break;
 case ASYNC:
-val = s->async_buf[s->pdma_cur++];
+val = s->async_buf[0];
+if (s->async_len > 0) {
+s->async_len--;
+s->async_buf++;
+}
+s->pdma_cur++;
 break;
 default:
 g_assert_not_reached();
 }
 
+s->ti_size--;
 s->pdma_len--;
 dmalen--;
 esp_set_tc(s, dmalen);
@@ -183,12 +189,18 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 s->pdma_cur++;
 break;
 case ASYNC:
-s->async_buf[s->pdma_cur++] = val;
+s->async_buf[0] = val;
+if (s->async_len > 0) {
+s->async_len--;
+s->async_buf++;
+}
+s->pdma_cur++;
 break;
 default:
 g_assert_not_reached();
 }
 
+s->ti_size++;
 s->pdma_len--;
 dmalen--;
 esp_set_tc(s, dmalen);
@@ -427,7 +439,6 @@ static void esp_dma_done(ESPState *s)
 static void do_dma_pdma_cb(ESPState *s)
 {
 int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
-int len = s->pdma_cur;
 
 if (s->do_cmd) {
 s->ti_size = 0;
@@ -436,13 +447,6 @@ static void do_dma_pdma_cb(ESPState *s)
 do_cmd(s);
 return;
 }
-s->async_buf += len;
-s->async_len -= len;
-if (to_device) {
-s->ti_size += len;
-} else {
-s->ti_size -= len;
-}
 if (s->async_len == 0) {
 scsi_req_continue(s->current_req);
 /*
-- 
2.20.1




[PATCH v3 17/42] esp: move pdma_len and TC logic into esp_pdma_read()/esp_pdma_write()

2021-03-04 Thread Mark Cave-Ayland
Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 50 --
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 58be98f047..b8d1ec41e9 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -153,22 +153,45 @@ static uint8_t *get_pdma_buf(ESPState *s)
 
 static uint8_t esp_pdma_read(ESPState *s)
 {
+uint32_t dmalen = esp_get_tc(s);
+uint8_t val;
+
+if (dmalen == 0 || s->pdma_len == 0) {
+return 0;
+}
+
 switch (s->pdma_origin) {
 case PDMA:
-return s->pdma_buf[s->pdma_cur++];
+val = s->pdma_buf[s->pdma_cur++];
+break;
 case TI:
-return s->ti_buf[s->pdma_cur++];
+val = s->ti_buf[s->pdma_cur++];
+break;
 case CMD:
-return s->cmdbuf[s->pdma_cur++];
+val = s->cmdbuf[s->pdma_cur++];
+break;
 case ASYNC:
-return s->async_buf[s->pdma_cur++];
+val = s->async_buf[s->pdma_cur++];
+break;
 default:
 g_assert_not_reached();
 }
+
+s->pdma_len--;
+dmalen--;
+esp_set_tc(s, dmalen);
+
+return val;
 }
 
 static void esp_pdma_write(ESPState *s, uint8_t val)
 {
+uint32_t dmalen = esp_get_tc(s);
+
+if (dmalen == 0 || s->pdma_len == 0) {
+return;
+}
+
 switch (s->pdma_origin) {
 case PDMA:
 s->pdma_buf[s->pdma_cur++] = val;
@@ -185,6 +208,10 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
 default:
 g_assert_not_reached();
 }
+
+s->pdma_len--;
+dmalen--;
+esp_set_tc(s, dmalen);
 }
 
 static int get_cmd_cb(ESPState *s)
@@ -944,27 +971,18 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr 
addr,
 {
 SysBusESPState *sysbus = opaque;
 ESPState *s = ESP(>esp);
-uint32_t dmalen = esp_get_tc(s);
 
 trace_esp_pdma_write(size);
 
-if (dmalen == 0 || s->pdma_len == 0) {
-return;
-}
 switch (size) {
 case 1:
 esp_pdma_write(s, val);
-s->pdma_len--;
-dmalen--;
 break;
 case 2:
 esp_pdma_write(s, val >> 8);
 esp_pdma_write(s, val);
-s->pdma_len -= 2;
-dmalen -= 2;
 break;
 }
-esp_set_tc(s, dmalen);
 if (s->pdma_len == 0 && s->pdma_cb) {
 esp_lower_drq(s);
 s->pdma_cb(s);
@@ -988,17 +1006,13 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, 
hwaddr addr,
 switch (size) {
 case 1:
 val = esp_pdma_read(s);
-s->pdma_len--;
-dmalen--;
 break;
 case 2:
 val = esp_pdma_read(s);
 val = (val << 8) | esp_pdma_read(s);
-s->pdma_len -= 2;
-dmalen -= 2;
 break;
 }
-esp_set_tc(s, dmalen);
+dmalen = esp_get_tc(s);
 if (dmalen == 0 || (s->pdma_len == 0 && s->pdma_cb)) {
 esp_lower_drq(s);
 s->pdma_cb(s);
-- 
2.20.1




[PATCH v3 20/42] esp: remove the buf and buflen parameters from get_cmd()

2021-03-04 Thread Mark Cave-Ayland
Now that all SCSI commands are accumulated in cmdbuf, remove the buf and buflen
parameters from get_cmd() since these always reference cmdbuf and ESP_CMDBUF_SZ
respectively.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 8ebf5e8d4b..44fddf082c 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -236,15 +236,16 @@ static int get_cmd_cb(ESPState *s)
 return 0;
 }
 
-static uint32_t get_cmd(ESPState *s, uint8_t *buf, uint8_t buflen)
+static uint32_t get_cmd(ESPState *s)
 {
+uint8_t *buf = s->cmdbuf;
 uint32_t dmalen;
 int target;
 
 target = s->wregs[ESP_WBUSID] & BUSID_DID;
 if (s->dma) {
 dmalen = esp_get_tc(s);
-if (dmalen > buflen) {
+if (dmalen > ESP_CMDBUF_SZ) {
 return 0;
 }
 if (s->dma_memory_read) {
@@ -323,7 +324,7 @@ static void handle_satn(ESPState *s)
 return;
 }
 s->pdma_cb = satn_pdma_cb;
-s->cmdlen = get_cmd(s, s->cmdbuf, sizeof(s->cmdbuf));
+s->cmdlen = get_cmd(s);
 if (s->cmdlen) {
 do_cmd(s);
 } else {
@@ -349,7 +350,7 @@ static void handle_s_without_atn(ESPState *s)
 return;
 }
 s->pdma_cb = s_without_satn_pdma_cb;
-s->cmdlen = get_cmd(s, s->cmdbuf, sizeof(s->cmdbuf));
+s->cmdlen = get_cmd(s);
 if (s->cmdlen) {
 do_busid_cmd(s, s->cmdbuf, 0);
 } else {
@@ -380,7 +381,7 @@ static void handle_satn_stop(ESPState *s)
 return;
 }
 s->pdma_cb = satn_stop_pdma_cb;
-s->cmdlen = get_cmd(s, s->cmdbuf, sizeof(s->cmdbuf));
+s->cmdlen = get_cmd(s);
 if (s->cmdlen) {
 trace_esp_handle_satn_stop(s->cmdlen);
 s->do_cmd = 1;
-- 
2.20.1




[PATCH v3 13/42] esp: remove dma_left from ESPState

2021-03-04 Thread Mark Cave-Ayland
The ESP device already keeps track of the remaining bytes left to transfer via
its TC (transfer counter) register which is decremented for each byte that
is transferred across the SCSI bus.

Switch the transfer logic to use the value of TC instead of dma_left and then
remove dma_left completely, adding logic to the vmstate_esp post_load() function
to transfer the old dma_left value to the TC register during migration from
older versions.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 47 ---
 include/hw/scsi/esp.h |  5 +++--
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 15c4c33052..92fea6a8c4 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -228,7 +228,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t 
busid)
 s->ti_size = datalen;
 if (datalen != 0) {
 s->rregs[ESP_RSTAT] = STAT_TC;
-s->dma_left = 0;
+esp_set_tc(s, 0);
 if (datalen > 0) {
 s->rregs[ESP_RSTAT] |= STAT_DI;
 } else {
@@ -382,6 +382,7 @@ static void do_dma_pdma_cb(ESPState *s)
 {
 int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
 int len = s->pdma_cur - s->pdma_start;
+
 if (s->do_cmd) {
 s->ti_size = 0;
 s->cmdlen = 0;
@@ -389,7 +390,6 @@ static void do_dma_pdma_cb(ESPState *s)
 do_cmd(s, s->cmdbuf);
 return;
 }
-s->dma_left -= len;
 s->async_buf += len;
 s->async_len -= len;
 if (to_device) {
@@ -404,7 +404,7 @@ static void do_dma_pdma_cb(ESPState *s)
  * complete the DMA operation immediately.  Otherwise defer
  * until the scsi layer has completed.
  */
-if (to_device || s->dma_left != 0 || s->ti_size == 0) {
+if (to_device || esp_get_tc(s) != 0 || s->ti_size == 0) {
 return;
 }
 }
@@ -418,7 +418,7 @@ static void esp_do_dma(ESPState *s)
 uint32_t len;
 int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
 
-len = s->dma_left;
+len = esp_get_tc(s);
 if (s->do_cmd) {
 /*
  * handle_ti_cmd() case: esp_do_dma() is called only from
@@ -468,7 +468,7 @@ static void esp_do_dma(ESPState *s)
 return;
 }
 }
-s->dma_left -= len;
+esp_set_tc(s, esp_get_tc(s) - len);
 s->async_buf += len;
 s->async_len -= len;
 if (to_device) {
@@ -483,7 +483,7 @@ static void esp_do_dma(ESPState *s)
  * complete the DMA operation immediately.  Otherwise defer
  * until the scsi layer has completed.
  */
-if (to_device || s->dma_left != 0 || s->ti_size == 0) {
+if (to_device || esp_get_tc(s) != 0 || s->ti_size == 0) {
 return;
 }
 }
@@ -499,7 +499,6 @@ static void esp_report_command_complete(ESPState *s, 
uint32_t status)
 trace_esp_command_complete_unexpected();
 }
 s->ti_size = 0;
-s->dma_left = 0;
 s->async_len = 0;
 if (status) {
 trace_esp_command_complete_fail();
@@ -534,12 +533,13 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
 void esp_transfer_data(SCSIRequest *req, uint32_t len)
 {
 ESPState *s = req->hba_private;
+uint32_t dmalen = esp_get_tc(s);
 
 assert(!s->do_cmd);
-trace_esp_transfer_data(s->dma_left, s->ti_size);
+trace_esp_transfer_data(dmalen, s->ti_size);
 s->async_len = len;
 s->async_buf = scsi_req_get_buf(req);
-if (s->dma_left) {
+if (dmalen) {
 esp_do_dma(s);
 } else if (s->ti_size <= 0) {
 /*
@@ -570,7 +570,6 @@ static void handle_ti(ESPState *s)
 }
 trace_esp_handle_ti(minlen);
 if (s->dma) {
-s->dma_left = minlen;
 s->rregs[ESP_RSTAT] &= ~STAT_TC;
 esp_do_dma(s);
 } else if (s->do_cmd) {
@@ -823,6 +822,14 @@ static const VMStateDescription vmstate_esp_pdma = {
 }
 };
 
+static bool esp_is_before_version_5(void *opaque, int version_id)
+{
+ESPState *s = ESP(opaque);
+
+version_id = MIN(version_id, s->mig_version_id);
+return version_id < 5;
+}
+
 static int esp_pre_save(void *opaque)
 {
 ESPState *s = ESP(opaque);
@@ -835,6 +842,12 @@ static int esp_post_load(void *opaque, int version_id)
 {
 ESPState *s = ESP(opaque);
 
+version_id = MIN(version_id, s->mig_version_id);
+
+if (version_id < 5) {
+esp_set_tc(s, s->mig_dma_left);
+}
+
 s->mig_version_id = vmstate_esp.version_id;
 return 0;
 }
@@ -860,7 +873,7 @@ const VMStateDescription vmstate_esp = {
 VMSTATE_BUFFER_START_MIDDLE_V(cmdbuf, ESPState, 16, 4),
 VMSTATE_UINT32(cmdlen, ESPState),
 VMSTATE_UINT32(do_cmd, ESPState),
-VMSTATE_UINT32(dma_left, ESPState),
+VMSTATE_UINT32_TEST(mig_dma_left, ESPState, esp_is_before_version_5),
 VMSTATE_END_OF_LIST()
 },
 .subsections = (const VMStateDescription * []) {
@@ -903,12 

[PATCH v3 12/42] esp: remove dma_counter from ESPState

2021-03-04 Thread Mark Cave-Ayland
The value of dma_counter is set once at the start of the transfer and remains
the same until the transfer is complete. This allows the check in 
esp_transfer_data
to be simplified since dma_left will always be non-zero until the transfer is
completed.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 4 +---
 include/hw/scsi/esp.h | 3 ---
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 349067eb6a..15c4c33052 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -229,7 +229,6 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t 
busid)
 if (datalen != 0) {
 s->rregs[ESP_RSTAT] = STAT_TC;
 s->dma_left = 0;
-s->dma_counter = 0;
 if (datalen > 0) {
 s->rregs[ESP_RSTAT] |= STAT_DI;
 } else {
@@ -542,7 +541,7 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
 s->async_buf = scsi_req_get_buf(req);
 if (s->dma_left) {
 esp_do_dma(s);
-} else if (s->dma_counter != 0 && s->ti_size <= 0) {
+} else if (s->ti_size <= 0) {
 /*
  * If this was the last part of a DMA transfer then the
  * completion interrupt is deferred to here.
@@ -561,7 +560,6 @@ static void handle_ti(ESPState *s)
 }
 
 dmalen = esp_get_tc(s);
-s->dma_counter = dmalen;
 
 if (s->do_cmd) {
 minlen = (dmalen < ESP_CMDBUF_SZ) ? dmalen : ESP_CMDBUF_SZ;
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 9d149cbc9f..ff1372947b 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -50,9 +50,6 @@ struct ESPState {
 
 /* The amount of data left in the current DMA transfer.  */
 uint32_t dma_left;
-/* The size of the current DMA transfer.  Zero if no transfer is in
-   progress.  */
-uint32_t dma_counter;
 int dma_enabled;
 
 uint32_t async_len;
-- 
2.20.1




[PATCH v3 09/42] esp: introduce esp_get_tc() and esp_set_tc()

2021-03-04 Thread Mark Cave-Ayland
These functions simplify reading and writing the TC register value without 
having to
manually shift each individual 8-bit value.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 38 +++---
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 5365523f6b..dd94f7b47b 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -98,6 +98,24 @@ void esp_request_cancelled(SCSIRequest *req)
 }
 }
 
+static uint32_t esp_get_tc(ESPState *s)
+{
+uint32_t dmalen;
+
+dmalen = s->rregs[ESP_TCLO];
+dmalen |= s->rregs[ESP_TCMID] << 8;
+dmalen |= s->rregs[ESP_TCHI] << 16;
+
+return dmalen;
+}
+
+static void esp_set_tc(ESPState *s, uint32_t dmalen)
+{
+s->rregs[ESP_TCLO] = dmalen;
+s->rregs[ESP_TCMID] = dmalen >> 8;
+s->rregs[ESP_TCHI] = dmalen >> 16;
+}
+
 static void set_pdma(ESPState *s, enum pdma_origin_id origin,
  uint32_t index, uint32_t len)
 {
@@ -157,9 +175,7 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf, uint8_t 
buflen)
 
 target = s->wregs[ESP_WBUSID] & BUSID_DID;
 if (s->dma) {
-dmalen = s->rregs[ESP_TCLO];
-dmalen |= s->rregs[ESP_TCMID] << 8;
-dmalen |= s->rregs[ESP_TCHI] << 16;
+dmalen = esp_get_tc(s);
 if (dmalen > buflen) {
 return 0;
 }
@@ -348,9 +364,7 @@ static void esp_dma_done(ESPState *s)
 s->rregs[ESP_RINTR] = INTR_BS;
 s->rregs[ESP_RSEQ] = 0;
 s->rregs[ESP_RFLAGS] = 0;
-s->rregs[ESP_TCLO] = 0;
-s->rregs[ESP_TCMID] = 0;
-s->rregs[ESP_TCHI] = 0;
+esp_set_tc(s, 0);
 esp_raise_irq(s);
 }
 
@@ -535,9 +549,7 @@ static void handle_ti(ESPState *s)
 return;
 }
 
-dmalen = s->rregs[ESP_TCLO];
-dmalen |= s->rregs[ESP_TCMID] << 8;
-dmalen |= s->rregs[ESP_TCHI] << 16;
+dmalen = esp_get_tc(s);
 if (dmalen == 0) {
 dmalen = 0x1;
 }
@@ -888,9 +900,7 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr addr,
 
 trace_esp_pdma_write(size);
 
-dmalen = s->rregs[ESP_TCLO];
-dmalen |= s->rregs[ESP_TCMID] << 8;
-dmalen |= s->rregs[ESP_TCHI] << 16;
+dmalen = esp_get_tc(s);
 if (dmalen == 0 || s->pdma_len == 0) {
 return;
 }
@@ -907,9 +917,7 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr addr,
 dmalen -= 2;
 break;
 }
-s->rregs[ESP_TCLO] = dmalen & 0xff;
-s->rregs[ESP_TCMID] = dmalen >> 8;
-s->rregs[ESP_TCHI] = dmalen >> 16;
+esp_set_tc(s, dmalen);
 if (s->pdma_len == 0 && s->pdma_cb) {
 esp_lower_drq(s);
 s->pdma_cb(s);
-- 
2.20.1




[PATCH v3 11/42] esp: apply transfer length adjustment when STC is zero at TC load time

2021-03-04 Thread Mark Cave-Ayland
Perform the length adjustment whereby a value of 0 in the STC represents
a transfer length of 0x1 at the point where the TC is loaded at the
start of a DMA command rather than just when a TI (Transfer Information)
command is executed. This better matches the description as given in the
datasheet.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Laurent Vivier 
---
 hw/scsi/esp.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 125bdea32a..349067eb6a 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -561,9 +561,6 @@ static void handle_ti(ESPState *s)
 }
 
 dmalen = esp_get_tc(s);
-if (dmalen == 0) {
-dmalen = 0x1;
-}
 s->dma_counter = dmalen;
 
 if (s->do_cmd) {
@@ -698,7 +695,11 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
val)
 if (val & CMD_DMA) {
 s->dma = 1;
 /* Reload DMA counter.  */
-esp_set_tc(s, esp_get_stc(s));
+if (esp_get_stc(s) == 0) {
+esp_set_tc(s, 0x1);
+} else {
+esp_set_tc(s, esp_get_stc(s));
+}
 } else {
 s->dma = 0;
 }
-- 
2.20.1




  1   2   3   4   >