[RFC PATCH 0/8] softmmu: Restrict CPU I/O instructions

[RFC PATCH 0/8] softmmu: Restrict CPU I/O instructions

[Philippe Mathieu-Daudé]

An attempt to restrict CPU I/O instructions to targets
where it makes sense. If it does, I'll send the next
series which restrict the I/O address space to X86/AVR.

Based-on: <20210314225308.2582284-1-f4bug@amsat.org>

Philippe Mathieu-Daudé (8):
  softmmu/physmem: Rename io_mem_unassigned -> unassigned_mr
  exec: Extract CPU I/O instructions to "cpu-io.h"
  target: Introduce TARGET_HAS_IOPORT
  qtest/fuzz: Restrict CPU I/O instructions
  qtest/libqos: Restrict CPU I/O instructions
  qtest: Restrict CPU I/O instructions
  monitor: Restrict CPU I/O instructions
  softmmu: Restrict CPU I/O instructions

 default-configs/targets/avr-softmmu.mak    |  1 +
 default-configs/targets/i386-softmmu.mak   |  1 +
 default-configs/targets/x86_64-softmmu.mak |  1 +
 include/exec/cpu-io.h                      | 30 ++++++++
 include/exec/ioport.h                      |  7 --
 tests/qtest/libqos/fw_cfg.h                |  3 +
 hw/i386/xen/xen-hvm.c                      |  1 +
 monitor/misc.c                             |  6 +-
 softmmu/cpu-io.c                           | 88 ++++++++++++++++++++++
 softmmu/ioport.c                           | 60 ---------------
 softmmu/physmem.c                          | 12 +--
 softmmu/qtest.c                            | 11 ++-
 tests/qtest/fuzz/generic_fuzz.c            | 16 ++--
 tests/qtest/fuzz/qtest_wrappers.c          |  7 +-
 tests/qtest/libqos/fw_cfg.c                |  2 +
 hmp-commands.hx                            |  2 +
 softmmu/meson.build                        |  3 +-
 17 files changed, 167 insertions(+), 84 deletions(-)
 create mode 100644 include/exec/cpu-io.h
 create mode 100644 softmmu/cpu-io.c

-- 
2.26.2

[RFC PATCH 1/8] softmmu/physmem: Rename io_mem_unassigned -> unassigned_mr

[Philippe Mathieu-Daudé]

'io_mem_unassigned' memory region is not specific to I/O,
rename it using a more generic 'unassigned_mr', matching
the '_mr' suffix pattern used in various places in the code
base.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/physmem.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 7e8b0fab89a..115679f8d1a 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -93,7 +93,7 @@ static MemoryRegion *system_io;
 AddressSpace address_space_io;
 AddressSpace address_space_memory;
 
-static MemoryRegion io_mem_unassigned;
+static MemoryRegion unassigned_mr;
 
 typedef struct PhysPageEntry PhysPageEntry;
 
@@ -458,7 +458,7 @@ static MemoryRegionSection address_space_translate_iommu(IOMMUMemoryRegion *iomm
     return *section;
 
 unassigned:
-    return (MemoryRegionSection) { .mr = &io_mem_unassigned };
+    return (MemoryRegionSection) { .mr = &unassigned_mr };
 }
 
 /**
@@ -534,7 +534,7 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
                                     attrs);
 
     /* Illegal translation */
-    if (section.mr == &io_mem_unassigned) {
+    if (section.mr == &unassigned_mr) {
         goto iotlb_fail;
     }
 
@@ -1212,7 +1212,7 @@ static void register_subpage(FlatView *fv, MemoryRegionSection *section)
     };
     hwaddr start, end;
 
-    assert(existing->mr->subpage || existing->mr == &io_mem_unassigned);
+    assert(existing->mr->subpage || existing->mr == &unassigned_mr);
 
     if (!(existing->mr->subpage)) {
         subpage = subpage_init(fv, base);
@@ -2573,7 +2573,7 @@ MemoryRegionSection *iotlb_to_section(CPUState *cpu,
 
 static void io_mem_init(void)
 {
-    memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,
+    memory_region_init_io(&unassigned_mr, NULL, &unassigned_mem_ops, NULL,
                           NULL, UINT64_MAX);
 }
 
@@ -2582,7 +2582,7 @@ AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv)
     AddressSpaceDispatch *d = g_new0(AddressSpaceDispatch, 1);
     uint16_t n;
 
-    n = dummy_section(&d->map, fv, &io_mem_unassigned);
+    n = dummy_section(&d->map, fv, &unassigned_mr);
     assert(n == PHYS_SECTION_UNASSIGNED);
 
     d->phys_map  = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };
-- 
2.26.2

[RFC PATCH 2/8] exec: Extract CPU I/O instructions to "cpu-io.h"

Not all architectures use an I/O bus. Extract the CPU I/O
instruction helpers into a specific unit named cpu-io.c
(and its equivalent "cpu-io.h" header).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/exec/cpu-io.h             | 30 +++++++++++
 include/exec/ioport.h             |  7 ---
 hw/i386/xen/xen-hvm.c             |  1 +
 monitor/misc.c                    |  2 +-
 softmmu/cpu-io.c                  | 88 +++++++++++++++++++++++++++++++
 softmmu/ioport.c                  | 60 ---------------------
 softmmu/qtest.c                   |  1 +
 tests/qtest/fuzz/qtest_wrappers.c |  3 +-
 softmmu/meson.build               |  3 +-
 9 files changed, 125 insertions(+), 70 deletions(-)
 create mode 100644 include/exec/cpu-io.h
 create mode 100644 softmmu/cpu-io.c

diff --git a/include/exec/cpu-io.h b/include/exec/cpu-io.h
new file mode 100644
index 00000000000..6538c451177
--- /dev/null
+++ b/include/exec/cpu-io.h
@@ -0,0 +1,30 @@
+/*
+ * QEMU CPU I/O instructions
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef CPU_IO_H
+#define CPU_IO_H
+
+void cpu_outb(uint32_t addr, uint8_t val);
+void cpu_outw(uint32_t addr, uint16_t val);
+void cpu_outl(uint32_t addr, uint32_t val);
+uint8_t cpu_inb(uint32_t addr);
+uint16_t cpu_inw(uint32_t addr);
+uint32_t cpu_inl(uint32_t addr);
+
+#endif /* CPU_IO_H */
diff --git a/include/exec/ioport.h b/include/exec/ioport.h
index e34f668998d..e58f475a7f0 100644
--- a/include/exec/ioport.h
+++ b/include/exec/ioport.h
@@ -44,13 +44,6 @@ typedef struct MemoryRegionPortio {
 extern const MemoryRegionOps unassigned_io_ops;
 #endif
 
-void cpu_outb(uint32_t addr, uint8_t val);
-void cpu_outw(uint32_t addr, uint16_t val);
-void cpu_outl(uint32_t addr, uint32_t val);
-uint8_t cpu_inb(uint32_t addr);
-uint16_t cpu_inw(uint32_t addr);
-uint32_t cpu_inl(uint32_t addr);
-
 typedef struct PortioList {
     const struct MemoryRegionPortio *ports;
     Object *owner;
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 7ce672e5a5c..56e0018a50e 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -34,6 +34,7 @@
 #include "sysemu/xen-mapcache.h"
 #include "trace.h"
 #include "exec/address-spaces.h"
+#include "exec/cpu-io.h"
 
 #include <xen/hvm/ioreq.h>
 #include <xen/hvm/e820.h>
diff --git a/monitor/misc.c b/monitor/misc.c
index a7650ed7470..d40c7d5afc0 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -77,7 +77,7 @@
 #include "qapi/qmp-event.h"
 #include "sysemu/cpus.h"
 #include "qemu/cutils.h"
-
+#include "exec/cpu-io.h"
 #if defined(TARGET_S390X)
 #include "hw/s390x/storage-keys.h"
 #include "hw/s390x/storage-attributes.h"
diff --git a/softmmu/cpu-io.c b/softmmu/cpu-io.c
new file mode 100644
index 00000000000..6ee03555f2c
--- /dev/null
+++ b/softmmu/cpu-io.c
@@ -0,0 +1,88 @@
+/*
+ * QEMU CPU I/O instructions
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "exec/address-spaces.h"
+#include "exec/cpu-io.h"
+#include "cpu.h"
+#include "trace.h"
+
+void cpu_outb(uint32_t addr, uint8_t val)
+{
+    trace_cpu_out(addr, 'b', val);
+    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                        &val, 1);
+}
+
+void cpu_outw(uint32_t addr, uint16_t val)
+{
+    uint8_t buf[2];
+
+    trace_cpu_out(addr, 'w', val);
+    stw_p(buf, val);
+    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                        buf, 2);
+}
+
+void cpu_outl(uint32_t addr, uint32_t val)
+{
+    uint8_t buf[4];
+
+    trace_cpu_out(addr, 'l', val);
+    stl_p(buf, val);
+    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                        buf, 4);
+}
+
+uint8_t cpu_inb(uint32_t addr)
+{
+    uint8_t val;
+
+    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                       &val, 1);
+    trace_cpu_in(addr, 'b', val);
+    return val;
+}
+
+uint16_t cpu_inw(uint32_t addr)
+{
+    uint8_t buf[2];
+    uint16_t val;
+
+    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 2);
+    val = lduw_p(buf);
+    trace_cpu_in(addr, 'w', val);
+    return val;
+}
+
+uint32_t cpu_inl(uint32_t addr)
+{
+    uint8_t buf[4];
+    uint32_t val;
+
+    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 4);
+    val = ldl_p(buf);
+    trace_cpu_in(addr, 'l', val);
+    return val;
+}
diff --git a/softmmu/ioport.c b/softmmu/ioport.c
index cb8adb0b936..69b13eed5e6 100644
--- a/softmmu/ioport.c
+++ b/softmmu/ioport.c
@@ -26,7 +26,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "cpu.h"
 #include "exec/ioport.h"
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
@@ -54,65 +53,6 @@ const MemoryRegionOps unassigned_io_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-void cpu_outb(uint32_t addr, uint8_t val)
-{
-    trace_cpu_out(addr, 'b', val);
-    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
-                        &val, 1);
-}
-
-void cpu_outw(uint32_t addr, uint16_t val)
-{
-    uint8_t buf[2];
-
-    trace_cpu_out(addr, 'w', val);
-    stw_p(buf, val);
-    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
-                        buf, 2);
-}
-
-void cpu_outl(uint32_t addr, uint32_t val)
-{
-    uint8_t buf[4];
-
-    trace_cpu_out(addr, 'l', val);
-    stl_p(buf, val);
-    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
-                        buf, 4);
-}
-
-uint8_t cpu_inb(uint32_t addr)
-{
-    uint8_t val;
-
-    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
-                       &val, 1);
-    trace_cpu_in(addr, 'b', val);
-    return val;
-}
-
-uint16_t cpu_inw(uint32_t addr)
-{
-    uint8_t buf[2];
-    uint16_t val;
-
-    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 2);
-    val = lduw_p(buf);
-    trace_cpu_in(addr, 'w', val);
-    return val;
-}
-
-uint32_t cpu_inl(uint32_t addr)
-{
-    uint8_t buf[4];
-    uint32_t val;
-
-    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 4);
-    val = ldl_p(buf);
-    trace_cpu_in(addr, 'l', val);
-    return val;
-}
-
 void portio_list_init(PortioList *piolist,
                       Object *owner,
                       const MemoryRegionPortio *callbacks,
diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 130c3666154..ff253068657 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -31,6 +31,7 @@
 #ifdef CONFIG_PSERIES
 #include "hw/ppc/spapr_rtas.h"
 #endif
+#include "exec/cpu-io.h"
 
 #define MAX_IRQ 256
 
diff --git a/tests/qtest/fuzz/qtest_wrappers.c b/tests/qtest/fuzz/qtest_wrappers.c
index 0580f8df860..921d1e5ed3a 100644
--- a/tests/qtest/fuzz/qtest_wrappers.c
+++ b/tests/qtest/fuzz/qtest_wrappers.c
@@ -12,8 +12,9 @@
  */
 
 #include "qemu/osdep.h"
+#include "exec/address-spaces.h"
+#include "exec/cpu-io.h"
 #include "hw/core/cpu.h"
-#include "exec/ioport.h"
 
 #include "fuzz.h"
 
diff --git a/softmmu/meson.build b/softmmu/meson.build
index d8e03018abf..ebf063b8990 100644
--- a/softmmu/meson.build
+++ b/softmmu/meson.build
@@ -2,11 +2,11 @@
   'arch_init.c',
   'balloon.c',
   'cpus.c',
+  'cpu-io.c',
   'cpu-throttle.c',
   'datadir.c',
   'globals.c',
   'physmem.c',
-  'ioport.c',
   'rtc.c',
   'runstate.c',
   'memory.c',
@@ -24,6 +24,7 @@
 softmmu_ss.add(files(
   'bootdevice.c',
   'dma-helpers.c',
+  'ioport.c',
   'qdev-monitor.c',
 ), sdl, libpmem, libdaxctl)
 
-- 
2.26.2

[RFC PATCH 2/8] exec: Extract CPU I/O instructions to "cpu-io.h"

[Philippe Mathieu-Daudé]

Not all architectures use an I/O bus. Extract the CPU I/O
instruction helpers into a specific unit named cpu-io.c
(and its equivalent "cpu-io.h" header).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/exec/cpu-io.h             | 30 +++++++++++
 include/exec/ioport.h             |  7 ---
 hw/i386/xen/xen-hvm.c             |  1 +
 monitor/misc.c                    |  2 +-
 softmmu/cpu-io.c                  | 88 +++++++++++++++++++++++++++++++
 softmmu/ioport.c                  | 60 ---------------------
 softmmu/qtest.c                   |  1 +
 tests/qtest/fuzz/qtest_wrappers.c |  3 +-
 softmmu/meson.build               |  3 +-
 9 files changed, 125 insertions(+), 70 deletions(-)
 create mode 100644 include/exec/cpu-io.h
 create mode 100644 softmmu/cpu-io.c

diff --git a/include/exec/cpu-io.h b/include/exec/cpu-io.h
new file mode 100644
index 00000000000..6538c451177
--- /dev/null
+++ b/include/exec/cpu-io.h
@@ -0,0 +1,30 @@
+/*
+ * QEMU CPU I/O instructions
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef CPU_IO_H
+#define CPU_IO_H
+
+void cpu_outb(uint32_t addr, uint8_t val);
+void cpu_outw(uint32_t addr, uint16_t val);
+void cpu_outl(uint32_t addr, uint32_t val);
+uint8_t cpu_inb(uint32_t addr);
+uint16_t cpu_inw(uint32_t addr);
+uint32_t cpu_inl(uint32_t addr);
+
+#endif /* CPU_IO_H */
diff --git a/include/exec/ioport.h b/include/exec/ioport.h
index e34f668998d..e58f475a7f0 100644
--- a/include/exec/ioport.h
+++ b/include/exec/ioport.h
@@ -44,13 +44,6 @@ typedef struct MemoryRegionPortio {
 extern const MemoryRegionOps unassigned_io_ops;
 #endif
 
-void cpu_outb(uint32_t addr, uint8_t val);
-void cpu_outw(uint32_t addr, uint16_t val);
-void cpu_outl(uint32_t addr, uint32_t val);
-uint8_t cpu_inb(uint32_t addr);
-uint16_t cpu_inw(uint32_t addr);
-uint32_t cpu_inl(uint32_t addr);
-
 typedef struct PortioList {
     const struct MemoryRegionPortio *ports;
     Object *owner;
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 7ce672e5a5c..56e0018a50e 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -34,6 +34,7 @@
 #include "sysemu/xen-mapcache.h"
 #include "trace.h"
 #include "exec/address-spaces.h"
+#include "exec/cpu-io.h"
 
 #include <xen/hvm/ioreq.h>
 #include <xen/hvm/e820.h>
diff --git a/monitor/misc.c b/monitor/misc.c
index a7650ed7470..d40c7d5afc0 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -77,7 +77,7 @@
 #include "qapi/qmp-event.h"
 #include "sysemu/cpus.h"
 #include "qemu/cutils.h"
-
+#include "exec/cpu-io.h"
 #if defined(TARGET_S390X)
 #include "hw/s390x/storage-keys.h"
 #include "hw/s390x/storage-attributes.h"
diff --git a/softmmu/cpu-io.c b/softmmu/cpu-io.c
new file mode 100644
index 00000000000..6ee03555f2c
--- /dev/null
+++ b/softmmu/cpu-io.c
@@ -0,0 +1,88 @@
+/*
+ * QEMU CPU I/O instructions
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "exec/address-spaces.h"
+#include "exec/cpu-io.h"
+#include "cpu.h"
+#include "trace.h"
+
+void cpu_outb(uint32_t addr, uint8_t val)
+{
+    trace_cpu_out(addr, 'b', val);
+    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                        &val, 1);
+}
+
+void cpu_outw(uint32_t addr, uint16_t val)
+{
+    uint8_t buf[2];
+
+    trace_cpu_out(addr, 'w', val);
+    stw_p(buf, val);
+    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                        buf, 2);
+}
+
+void cpu_outl(uint32_t addr, uint32_t val)
+{
+    uint8_t buf[4];
+
+    trace_cpu_out(addr, 'l', val);
+    stl_p(buf, val);
+    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                        buf, 4);
+}
+
+uint8_t cpu_inb(uint32_t addr)
+{
+    uint8_t val;
+
+    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                       &val, 1);
+    trace_cpu_in(addr, 'b', val);
+    return val;
+}
+
+uint16_t cpu_inw(uint32_t addr)
+{
+    uint8_t buf[2];
+    uint16_t val;
+
+    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 2);
+    val = lduw_p(buf);
+    trace_cpu_in(addr, 'w', val);
+    return val;
+}
+
+uint32_t cpu_inl(uint32_t addr)
+{
+    uint8_t buf[4];
+    uint32_t val;
+
+    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 4);
+    val = ldl_p(buf);
+    trace_cpu_in(addr, 'l', val);
+    return val;
+}
diff --git a/softmmu/ioport.c b/softmmu/ioport.c
index cb8adb0b936..69b13eed5e6 100644
--- a/softmmu/ioport.c
+++ b/softmmu/ioport.c
@@ -26,7 +26,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "cpu.h"
 #include "exec/ioport.h"
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
@@ -54,65 +53,6 @@ const MemoryRegionOps unassigned_io_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-void cpu_outb(uint32_t addr, uint8_t val)
-{
-    trace_cpu_out(addr, 'b', val);
-    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
-                        &val, 1);
-}
-
-void cpu_outw(uint32_t addr, uint16_t val)
-{
-    uint8_t buf[2];
-
-    trace_cpu_out(addr, 'w', val);
-    stw_p(buf, val);
-    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
-                        buf, 2);
-}
-
-void cpu_outl(uint32_t addr, uint32_t val)
-{
-    uint8_t buf[4];
-
-    trace_cpu_out(addr, 'l', val);
-    stl_p(buf, val);
-    address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
-                        buf, 4);
-}
-
-uint8_t cpu_inb(uint32_t addr)
-{
-    uint8_t val;
-
-    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
-                       &val, 1);
-    trace_cpu_in(addr, 'b', val);
-    return val;
-}
-
-uint16_t cpu_inw(uint32_t addr)
-{
-    uint8_t buf[2];
-    uint16_t val;
-
-    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 2);
-    val = lduw_p(buf);
-    trace_cpu_in(addr, 'w', val);
-    return val;
-}
-
-uint32_t cpu_inl(uint32_t addr)
-{
-    uint8_t buf[4];
-    uint32_t val;
-
-    address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 4);
-    val = ldl_p(buf);
-    trace_cpu_in(addr, 'l', val);
-    return val;
-}
-
 void portio_list_init(PortioList *piolist,
                       Object *owner,
                       const MemoryRegionPortio *callbacks,
diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 130c3666154..ff253068657 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -31,6 +31,7 @@
 #ifdef CONFIG_PSERIES
 #include "hw/ppc/spapr_rtas.h"
 #endif
+#include "exec/cpu-io.h"
 
 #define MAX_IRQ 256
 
diff --git a/tests/qtest/fuzz/qtest_wrappers.c b/tests/qtest/fuzz/qtest_wrappers.c
index 0580f8df860..921d1e5ed3a 100644
--- a/tests/qtest/fuzz/qtest_wrappers.c
+++ b/tests/qtest/fuzz/qtest_wrappers.c
@@ -12,8 +12,9 @@
  */
 
 #include "qemu/osdep.h"
+#include "exec/address-spaces.h"
+#include "exec/cpu-io.h"
 #include "hw/core/cpu.h"
-#include "exec/ioport.h"
 
 #include "fuzz.h"
 
diff --git a/softmmu/meson.build b/softmmu/meson.build
index d8e03018abf..ebf063b8990 100644
--- a/softmmu/meson.build
+++ b/softmmu/meson.build
@@ -2,11 +2,11 @@
   'arch_init.c',
   'balloon.c',
   'cpus.c',
+  'cpu-io.c',
   'cpu-throttle.c',
   'datadir.c',
   'globals.c',
   'physmem.c',
-  'ioport.c',
   'rtc.c',
   'runstate.c',
   'memory.c',
@@ -24,6 +24,7 @@
 softmmu_ss.add(files(
   'bootdevice.c',
   'dma-helpers.c',
+  'ioport.c',
   'qdev-monitor.c',
 ), sdl, libpmem, libdaxctl)
 
-- 
2.26.2

[RFC PATCH 3/8] target: Introduce TARGET_HAS_IOPORT

[Philippe Mathieu-Daudé]

Have target architectures providing CPU access to I/O bus
define TARGET_HAS_IOPORT.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 default-configs/targets/avr-softmmu.mak    | 1 +
 default-configs/targets/i386-softmmu.mak   | 1 +
 default-configs/targets/x86_64-softmmu.mak | 1 +
 3 files changed, 3 insertions(+)

diff --git a/default-configs/targets/avr-softmmu.mak b/default-configs/targets/avr-softmmu.mak
index e3f921c0199..4652ce136bc 100644
--- a/default-configs/targets/avr-softmmu.mak
+++ b/default-configs/targets/avr-softmmu.mak
@@ -1,2 +1,3 @@
 TARGET_ARCH=avr
+TARGET_HAS_IOPORT=y
 TARGET_XML_FILES= gdb-xml/avr-cpu.xml
diff --git a/default-configs/targets/i386-softmmu.mak b/default-configs/targets/i386-softmmu.mak
index 5babf71895d..969008859b6 100644
--- a/default-configs/targets/i386-softmmu.mak
+++ b/default-configs/targets/i386-softmmu.mak
@@ -1,3 +1,4 @@
 TARGET_ARCH=i386
+TARGET_HAS_IOPORT=y
 TARGET_SUPPORTS_MTTCG=y
 TARGET_XML_FILES= gdb-xml/i386-32bit.xml
diff --git a/default-configs/targets/x86_64-softmmu.mak b/default-configs/targets/x86_64-softmmu.mak
index 75e42bc8404..2d35270f020 100644
--- a/default-configs/targets/x86_64-softmmu.mak
+++ b/default-configs/targets/x86_64-softmmu.mak
@@ -1,4 +1,5 @@
 TARGET_ARCH=x86_64
 TARGET_BASE_ARCH=i386
+TARGET_HAS_IOPORT=y
 TARGET_SUPPORTS_MTTCG=y
 TARGET_XML_FILES= gdb-xml/i386-64bit.xml
-- 
2.26.2

[RFC PATCH 4/8] qtest/fuzz: Restrict CPU I/O instructions

[Philippe Mathieu-Daudé]

Restrict CPU I/O instructions to architectures providing
I/O bus.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/qtest/fuzz/generic_fuzz.c   | 16 ++++++++++------
 tests/qtest/fuzz/qtest_wrappers.c |  4 ++++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index ee8c17a04c4..3e0089f4a63 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -304,6 +304,13 @@ static bool get_io_address(address_range *result, AddressSpace *as,
     return cb_info.found;
 }
 
+static bool get_mmio_address(address_range *result,
+                             uint8_t index, uint32_t offset)
+{
+    return get_io_address(result, &address_space_memory, index, offset);
+}
+
+#ifdef TARGET_HAS_IOPORT
 static bool get_pio_address(address_range *result,
                             uint8_t index, uint16_t offset)
 {
@@ -318,12 +325,6 @@ static bool get_pio_address(address_range *result,
     return result->addr <= 0xFFFF ? found : false;
 }
 
-static bool get_mmio_address(address_range *result,
-                             uint8_t index, uint32_t offset)
-{
-    return get_io_address(result, &address_space_memory, index, offset);
-}
-
 static void op_in(QTestState *s, const unsigned char * data, size_t len)
 {
     enum Sizes {Byte, Word, Long, end_sizes};
@@ -395,6 +396,7 @@ static void op_out(QTestState *s, const unsigned char * data, size_t len)
         break;
     }
 }
+#endif /* TARGET_HAS_IOPORT */
 
 static void op_read(QTestState *s, const unsigned char * data, size_t len)
 {
@@ -626,8 +628,10 @@ static void handle_timeout(int sig)
 static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
 {
     void (*ops[]) (QTestState *s, const unsigned char* , size_t) = {
+#ifdef TARGET_HAS_IOPORT
         [OP_IN]                 = op_in,
         [OP_OUT]                = op_out,
+#endif /* TARGET_HAS_IOPORT */
         [OP_READ]               = op_read,
         [OP_WRITE]              = op_write,
         [OP_PCI_READ]           = op_pci_read,
diff --git a/tests/qtest/fuzz/qtest_wrappers.c b/tests/qtest/fuzz/qtest_wrappers.c
index 921d1e5ed3a..d56dda9e9b8 100644
--- a/tests/qtest/fuzz/qtest_wrappers.c
+++ b/tests/qtest/fuzz/qtest_wrappers.c
@@ -24,12 +24,14 @@ static bool serialize = true;
     RET_TYPE __wrap_##NAME_AND_ARGS;\
     RET_TYPE __real_##NAME_AND_ARGS;
 
+#ifdef TARGET_HAS_IOPORT
 WRAP(uint8_t  , qtest_inb(QTestState *s, uint16_t addr))
 WRAP(uint16_t , qtest_inw(QTestState *s, uint16_t addr))
 WRAP(uint32_t , qtest_inl(QTestState *s, uint16_t addr))
 WRAP(void     , qtest_outb(QTestState *s, uint16_t addr, uint8_t value))
 WRAP(void     , qtest_outw(QTestState *s, uint16_t addr, uint16_t value))
 WRAP(void     , qtest_outl(QTestState *s, uint16_t addr, uint32_t value))
+#endif /* TARGET_HAS_IOPORT */
 WRAP(uint8_t  , qtest_readb(QTestState *s, uint64_t addr))
 WRAP(uint16_t , qtest_readw(QTestState *s, uint64_t addr))
 WRAP(uint32_t , qtest_readl(QTestState *s, uint64_t addr))
@@ -50,6 +52,7 @@ WRAP(void,      qtest_memset(QTestState *s, uint64_t addr,
                              uint8_t patt, size_t size))
 
 
+#ifdef TARGET_HAS_IOPORT
 uint8_t __wrap_qtest_inb(QTestState *s, uint16_t addr)
 {
     if (!serialize) {
@@ -103,6 +106,7 @@ void __wrap_qtest_outl(QTestState *s, uint16_t addr, uint32_t value)
         __real_qtest_outl(s, addr, value);
     }
 }
+#endif /* TARGET_HAS_IOPORT */
 
 uint8_t __wrap_qtest_readb(QTestState *s, uint64_t addr)
 {
-- 
2.26.2

[RFC PATCH 5/8] qtest/libqos: Restrict CPU I/O instructions

[Philippe Mathieu-Daudé]

Restrict CPU I/O instructions to architectures providing
I/O bus.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/qtest/libqos/fw_cfg.h | 3 +++
 tests/qtest/libqos/fw_cfg.c | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/tests/qtest/libqos/fw_cfg.h b/tests/qtest/libqos/fw_cfg.h
index c6a7cf8cf05..3bfb6d6d55b 100644
--- a/tests/qtest/libqos/fw_cfg.h
+++ b/tests/qtest/libqos/fw_cfg.h
@@ -36,6 +36,8 @@ size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
 
 QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
 void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
+
+#ifdef TARGET_HAS_IOPORT
 QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
 void io_fw_cfg_uninit(QFWCFG *fw_cfg);
 
@@ -48,6 +50,7 @@ static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
 {
     io_fw_cfg_uninit(fw_cfg);
 }
+#endif /* TARGET_HAS_IOPORT */
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(QFWCFG, mm_fw_cfg_uninit)
 
diff --git a/tests/qtest/libqos/fw_cfg.c b/tests/qtest/libqos/fw_cfg.c
index 6b8e1babe51..db2b83f5212 100644
--- a/tests/qtest/libqos/fw_cfg.c
+++ b/tests/qtest/libqos/fw_cfg.c
@@ -131,6 +131,7 @@ void mm_fw_cfg_uninit(QFWCFG *fw_cfg)
     g_free(fw_cfg);
 }
 
+#ifdef TARGET_HAS_IOPORT
 static void io_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
 {
     qtest_outw(fw_cfg->qts, fw_cfg->base, key);
@@ -162,3 +163,4 @@ void io_fw_cfg_uninit(QFWCFG *fw_cfg)
 {
     g_free(fw_cfg);
 }
+#endif /* TARGET_HAS_IOPORT */
-- 
2.26.2

[RFC PATCH 6/8] qtest: Restrict CPU I/O instructions

[Philippe Mathieu-Daudé]

Restrict CPU I/O instructions to architectures providing
I/O bus.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/qtest.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index ff253068657..51fe256297a 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -31,7 +31,9 @@
 #ifdef CONFIG_PSERIES
 #include "hw/ppc/spapr_rtas.h"
 #endif
+#ifdef TARGET_HAS_IOPORT
 #include "exec/cpu-io.h"
+#endif
 
 #define MAX_IRQ 256
 
@@ -462,7 +464,9 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         qemu_set_irq(irq, level);
         qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
-    } else if (strcmp(words[0], "outb") == 0 ||
+    }
+#ifdef TARGET_HAS_IOPORT
+    else if (strcmp(words[0], "outb") == 0 ||
                strcmp(words[0], "outw") == 0 ||
                strcmp(words[0], "outl") == 0) {
         unsigned long addr;
@@ -506,7 +510,9 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         }
         qtest_send_prefix(chr);
         qtest_sendf(chr, "OK 0x%04x\n", value);
-    } else if (strcmp(words[0], "writeb") == 0 ||
+    }
+#endif /* TARGET_HAS_IOPORT */
+    else if (strcmp(words[0], "writeb") == 0 ||
                strcmp(words[0], "writew") == 0 ||
                strcmp(words[0], "writel") == 0 ||
                strcmp(words[0], "writeq") == 0) {
-- 
2.26.2

[RFC PATCH 7/8] monitor: Restrict CPU I/O instructions

[Philippe Mathieu-Daudé]

Restrict CPU I/O instructions to architectures providing
I/O bus.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 monitor/misc.c  | 4 ++++
 hmp-commands.hx | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/monitor/misc.c b/monitor/misc.c
index d40c7d5afc0..b59f11433eb 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -77,7 +77,9 @@
 #include "qapi/qmp-event.h"
 #include "sysemu/cpus.h"
 #include "qemu/cutils.h"
+#ifdef TARGET_HAS_IOPORT
 #include "exec/cpu-io.h"
+#endif
 #if defined(TARGET_S390X)
 #include "hw/s390x/storage-keys.h"
 #include "hw/s390x/storage-attributes.h"
@@ -884,6 +886,7 @@ static void hmp_mouse_button(Monitor *mon, const QDict *qdict)
     mouse_button_state = button_state;
 }
 
+#ifdef TARGET_HAS_IOPORT
 static void hmp_ioport_read(Monitor *mon, const QDict *qdict)
 {
     int size = qdict_get_int(qdict, "size");
@@ -939,6 +942,7 @@ static void hmp_ioport_write(Monitor *mon, const QDict *qdict)
         break;
     }
 }
+#endif /* TARGET_HAS_IOPORT */
 
 static void hmp_boot_set(Monitor *mon, const QDict *qdict)
 {
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d4001f9c5dc..cedcedf7d8d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -561,6 +561,7 @@ SRST
   used.
 ERST
 
+#ifdef TARGET_HAS_IOPORT
     {
         .name       = "i",
         .args_type  = "fmt:/,addr:i,index:i.",
@@ -586,6 +587,7 @@ SRST
 ``o/``\ *fmt* *addr* *val*
   Write to I/O port.
 ERST
+#endif /* TARGET_HAS_IOPORT */
 
     {
         .name       = "sendkey",
-- 
2.26.2

[RFC PATCH 8/8] softmmu: Restrict CPU I/O instructions

[Philippe Mathieu-Daudé]

Restrict CPU I/O instructions to architectures providing
I/O bus.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/softmmu/meson.build b/softmmu/meson.build
index ebf063b8990..3df196917a0 100644
--- a/softmmu/meson.build
+++ b/softmmu/meson.build
@@ -2,7 +2,6 @@
   'arch_init.c',
   'balloon.c',
   'cpus.c',
-  'cpu-io.c',
   'cpu-throttle.c',
   'datadir.c',
   'globals.c',
@@ -16,6 +15,7 @@
   'cpu-timers.c',
   'runstate-action.c',
 )])
+specific_ss.add(when: 'TARGET_HAS_IOPORT', if_true: files('cpu-io.c'))
 
 specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TCG'], if_true: [files(
   'icount.c'
-- 
2.26.2

Re: [RFC PATCH 4/8] qtest/fuzz: Restrict CPU I/O instructions

[Alexander Bulekov]

On 210315 0029, Philippe Mathieu-Daudé wrote:
> Restrict CPU I/O instructions to architectures providing
> I/O bus.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/qtest/fuzz/generic_fuzz.c   | 16 ++++++++++------
>  tests/qtest/fuzz/qtest_wrappers.c |  4 ++++
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index ee8c17a04c4..3e0089f4a63 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -304,6 +304,13 @@ static bool get_io_address(address_range *result, AddressSpace *as,
>      return cb_info.found;
>  }
>  
> +static bool get_mmio_address(address_range *result,
> +                             uint8_t index, uint32_t offset)
> +{
> +    return get_io_address(result, &address_space_memory, index, offset);
> +}
> +
> +#ifdef TARGET_HAS_IOPORT
>  static bool get_pio_address(address_range *result,
>                              uint8_t index, uint16_t offset)
>  {
> @@ -318,12 +325,6 @@ static bool get_pio_address(address_range *result,
>      return result->addr <= 0xFFFF ? found : false;
>  }
>  
> -static bool get_mmio_address(address_range *result,
> -                             uint8_t index, uint32_t offset)
> -{
> -    return get_io_address(result, &address_space_memory, index, offset);
> -}
> -
>  static void op_in(QTestState *s, const unsigned char * data, size_t len)
>  {
>      enum Sizes {Byte, Word, Long, end_sizes};
> @@ -395,6 +396,7 @@ static void op_out(QTestState *s, const unsigned char * data, size_t len)
>          break;
>      }
>  }
> +#endif /* TARGET_HAS_IOPORT */
>  
>  static void op_read(QTestState *s, const unsigned char * data, size_t len)
>  {
> @@ -626,8 +628,10 @@ static void handle_timeout(int sig)
>  static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
>  {
>      void (*ops[]) (QTestState *s, const unsigned char* , size_t) = {
> +#ifdef TARGET_HAS_IOPORT
>          [OP_IN]                 = op_in,
>          [OP_OUT]                = op_out,

I think op_pci_read and op_pci_write would need to be disabled as well
(at least the way they are implemented now).

> +#endif /* TARGET_HAS_IOPORT */
>          [OP_READ]               = op_read,
>          [OP_WRITE]              = op_write,
>          [OP_PCI_READ]           = op_pci_read,
> diff --git a/tests/qtest/fuzz/qtest_wrappers.c b/tests/qtest/fuzz/qtest_wrappers.c
> index 921d1e5ed3a..d56dda9e9b8 100644
> --- a/tests/qtest/fuzz/qtest_wrappers.c
> +++ b/tests/qtest/fuzz/qtest_wrappers.c
> @@ -24,12 +24,14 @@ static bool serialize = true;
>      RET_TYPE __wrap_##NAME_AND_ARGS;\
>      RET_TYPE __real_##NAME_AND_ARGS;
>  
> +#ifdef TARGET_HAS_IOPORT
>  WRAP(uint8_t  , qtest_inb(QTestState *s, uint16_t addr))
>  WRAP(uint16_t , qtest_inw(QTestState *s, uint16_t addr))
>  WRAP(uint32_t , qtest_inl(QTestState *s, uint16_t addr))
>  WRAP(void     , qtest_outb(QTestState *s, uint16_t addr, uint8_t value))
>  WRAP(void     , qtest_outw(QTestState *s, uint16_t addr, uint16_t value))
>  WRAP(void     , qtest_outl(QTestState *s, uint16_t addr, uint32_t value))
> +#endif /* TARGET_HAS_IOPORT */
>  WRAP(uint8_t  , qtest_readb(QTestState *s, uint64_t addr))
>  WRAP(uint16_t , qtest_readw(QTestState *s, uint64_t addr))
>  WRAP(uint32_t , qtest_readl(QTestState *s, uint64_t addr))
> @@ -50,6 +52,7 @@ WRAP(void,      qtest_memset(QTestState *s, uint64_t addr,
>                               uint8_t patt, size_t size))
>  
>  
> +#ifdef TARGET_HAS_IOPORT
>  uint8_t __wrap_qtest_inb(QTestState *s, uint16_t addr)
>  {
>      if (!serialize) {
> @@ -103,6 +106,7 @@ void __wrap_qtest_outl(QTestState *s, uint16_t addr, uint32_t value)
>          __real_qtest_outl(s, addr, value);
>      }
>  }
> +#endif /* TARGET_HAS_IOPORT */
>  
>  uint8_t __wrap_qtest_readb(QTestState *s, uint64_t addr)
>  {
> -- 
> 2.26.2
> 

Re: [RFC PATCH 4/8] qtest/fuzz: Restrict CPU I/O instructions

[Thomas Huth]

On 15/03/2021 00.29, Philippe Mathieu-Daudé wrote:
> Restrict CPU I/O instructions to architectures providing
> I/O bus.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   tests/qtest/fuzz/generic_fuzz.c   | 16 ++++++++++------
>   tests/qtest/fuzz/qtest_wrappers.c |  4 ++++
>   2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index ee8c17a04c4..3e0089f4a63 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -304,6 +304,13 @@ static bool get_io_address(address_range *result, AddressSpace *as,
>       return cb_info.found;
>   }
>   
> +static bool get_mmio_address(address_range *result,
> +                             uint8_t index, uint32_t offset)
> +{
> +    return get_io_address(result, &address_space_memory, index, offset);
> +}
> +
> +#ifdef TARGET_HAS_IOPORT

Sorry, but the qtests are generic code, I don't think we should introduce 
target specific ifdefs here...?

  Thomas

Re: [RFC PATCH 4/8] qtest/fuzz: Restrict CPU I/O instructions

[Philippe Mathieu-Daudé]

On 3/15/21 6:14 AM, Thomas Huth wrote:
> On 15/03/2021 00.29, Philippe Mathieu-Daudé wrote:
>> Restrict CPU I/O instructions to architectures providing
>> I/O bus.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   tests/qtest/fuzz/generic_fuzz.c   | 16 ++++++++++------
>>   tests/qtest/fuzz/qtest_wrappers.c |  4 ++++
>>   2 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/qtest/fuzz/generic_fuzz.c
>> b/tests/qtest/fuzz/generic_fuzz.c
>> index ee8c17a04c4..3e0089f4a63 100644
>> --- a/tests/qtest/fuzz/generic_fuzz.c
>> +++ b/tests/qtest/fuzz/generic_fuzz.c
>> @@ -304,6 +304,13 @@ static bool get_io_address(address_range *result,
>> AddressSpace *as,
>>       return cb_info.found;
>>   }
>>   +static bool get_mmio_address(address_range *result,
>> +                             uint8_t index, uint32_t offset)
>> +{
>> +    return get_io_address(result, &address_space_memory, index, offset);
>> +}
>> +
>> +#ifdef TARGET_HAS_IOPORT
> 
> Sorry, but the qtests are generic code, I don't think we should
> introduce target specific ifdefs here...?
My view is if you want to generically access an I/O bus, you need
to do it via its address space, not the CPU architecture-specific
interface.

I.e., if an I/O bus is exposed by the PCI function of a south bridge,
if you use the correct PCI AS view you can run your test on any
architecture providing a PCI bus, not only X86.

So yes you are right, and the current code is abusing it. Yes it is
fixable but is it worthwhile? Apparently nobody cared, so probably
not worthwhile. Let's disregard this series for now.

Regards,

Phil.

Re: [RFC PATCH 5/8] qtest/libqos: Restrict CPU I/O instructions

[Laszlo Ersek]

(+Peter, comment below)

On 03/15/21 00:29, Philippe Mathieu-Daudé wrote:
> Restrict CPU I/O instructions to architectures providing
> I/O bus.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/qtest/libqos/fw_cfg.h | 3 +++
>  tests/qtest/libqos/fw_cfg.c | 2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/tests/qtest/libqos/fw_cfg.h b/tests/qtest/libqos/fw_cfg.h
> index c6a7cf8cf05..3bfb6d6d55b 100644
> --- a/tests/qtest/libqos/fw_cfg.h
> +++ b/tests/qtest/libqos/fw_cfg.h
> @@ -36,6 +36,8 @@ size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
>  
>  QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
>  void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
> +
> +#ifdef TARGET_HAS_IOPORT
>  QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
>  void io_fw_cfg_uninit(QFWCFG *fw_cfg);
>  
> @@ -48,6 +50,7 @@ static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
>  {
>      io_fw_cfg_uninit(fw_cfg);
>  }
> +#endif /* TARGET_HAS_IOPORT */
>  
>  G_DEFINE_AUTOPTR_CLEANUP_FUNC(QFWCFG, mm_fw_cfg_uninit)
>  
> diff --git a/tests/qtest/libqos/fw_cfg.c b/tests/qtest/libqos/fw_cfg.c
> index 6b8e1babe51..db2b83f5212 100644
> --- a/tests/qtest/libqos/fw_cfg.c
> +++ b/tests/qtest/libqos/fw_cfg.c
> @@ -131,6 +131,7 @@ void mm_fw_cfg_uninit(QFWCFG *fw_cfg)
>      g_free(fw_cfg);
>  }
>  
> +#ifdef TARGET_HAS_IOPORT
>  static void io_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
>  {
>      qtest_outw(fw_cfg->qts, fw_cfg->base, key);
> @@ -162,3 +163,4 @@ void io_fw_cfg_uninit(QFWCFG *fw_cfg)
>  {
>      g_free(fw_cfg);
>  }
> +#endif /* TARGET_HAS_IOPORT */
> 

I'm not sure the macro name is ideal; the PCI host on aarch64/"virt"
emulates IO Ports (it's possible to allocate PCI IO resources on
"virt"). From patch#3, TARGET_HAS_IOPORT does not seem to extend to arm64.

I guess the intent is OK in both patches #3 and #5.

Thanks
Laszlo

Re: [RFC PATCH 5/8] qtest/libqos: Restrict CPU I/O instructions

[Richard Henderson]

On 3/16/21 9:37 AM, Laszlo Ersek wrote:
> (+Peter, comment below)
> 
> On 03/15/21 00:29, Philippe Mathieu-Daudé wrote:
>> Restrict CPU I/O instructions to architectures providing
>> I/O bus.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   tests/qtest/libqos/fw_cfg.h | 3 +++
>>   tests/qtest/libqos/fw_cfg.c | 2 ++
>>   2 files changed, 5 insertions(+)
>>
>> diff --git a/tests/qtest/libqos/fw_cfg.h b/tests/qtest/libqos/fw_cfg.h
>> index c6a7cf8cf05..3bfb6d6d55b 100644
>> --- a/tests/qtest/libqos/fw_cfg.h
>> +++ b/tests/qtest/libqos/fw_cfg.h
>> @@ -36,6 +36,8 @@ size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
>>   
>>   QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
>>   void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
>> +
>> +#ifdef TARGET_HAS_IOPORT
>>   QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
>>   void io_fw_cfg_uninit(QFWCFG *fw_cfg);
>>   
>> @@ -48,6 +50,7 @@ static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
>>   {
>>       io_fw_cfg_uninit(fw_cfg);
>>   }
>> +#endif /* TARGET_HAS_IOPORT */
>>   
>>   G_DEFINE_AUTOPTR_CLEANUP_FUNC(QFWCFG, mm_fw_cfg_uninit)
>>   
>> diff --git a/tests/qtest/libqos/fw_cfg.c b/tests/qtest/libqos/fw_cfg.c
>> index 6b8e1babe51..db2b83f5212 100644
>> --- a/tests/qtest/libqos/fw_cfg.c
>> +++ b/tests/qtest/libqos/fw_cfg.c
>> @@ -131,6 +131,7 @@ void mm_fw_cfg_uninit(QFWCFG *fw_cfg)
>>       g_free(fw_cfg);
>>   }
>>   
>> +#ifdef TARGET_HAS_IOPORT
>>   static void io_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
>>   {
>>       qtest_outw(fw_cfg->qts, fw_cfg->base, key);
>> @@ -162,3 +163,4 @@ void io_fw_cfg_uninit(QFWCFG *fw_cfg)
>>   {
>>       g_free(fw_cfg);
>>   }
>> +#endif /* TARGET_HAS_IOPORT */
>>
> 
> I'm not sure the macro name is ideal; the PCI host on aarch64/"virt"
> emulates IO Ports (it's possible to allocate PCI IO resources on
> "virt"). From patch#3, TARGET_HAS_IOPORT does not seem to extend to arm64.

Correct, aarch64 has memory-mapped pci io resources, they are not on a separate 
ioport address space as for x86 and avr.


r~

Re: [RFC PATCH 5/8] qtest/libqos: Restrict CPU I/O instructions

[Philippe Mathieu-Daudé]

Hi Richard and Laszlo,

On 3/16/21 4:43 PM, Richard Henderson wrote:
> On 3/16/21 9:37 AM, Laszlo Ersek wrote:
>> (+Peter, comment below)
>>
>> On 03/15/21 00:29, Philippe Mathieu-Daudé wrote:
>>> Restrict CPU I/O instructions to architectures providing
>>> I/O bus.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>   tests/qtest/libqos/fw_cfg.h | 3 +++
>>>   tests/qtest/libqos/fw_cfg.c | 2 ++
>>>   2 files changed, 5 insertions(+)
>>>
>>> diff --git a/tests/qtest/libqos/fw_cfg.h b/tests/qtest/libqos/fw_cfg.h
>>> index c6a7cf8cf05..3bfb6d6d55b 100644
>>> --- a/tests/qtest/libqos/fw_cfg.h
>>> +++ b/tests/qtest/libqos/fw_cfg.h
>>> @@ -36,6 +36,8 @@ size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char
>>> *filename,
>>>     QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
>>>   void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
>>> +
>>> +#ifdef TARGET_HAS_IOPORT
>>>   QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
>>>   void io_fw_cfg_uninit(QFWCFG *fw_cfg);
>>>   @@ -48,6 +50,7 @@ static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
>>>   {
>>>       io_fw_cfg_uninit(fw_cfg);
>>>   }
>>> +#endif /* TARGET_HAS_IOPORT */
>>>     G_DEFINE_AUTOPTR_CLEANUP_FUNC(QFWCFG, mm_fw_cfg_uninit)
>>>   diff --git a/tests/qtest/libqos/fw_cfg.c b/tests/qtest/libqos/fw_cfg.c
>>> index 6b8e1babe51..db2b83f5212 100644
>>> --- a/tests/qtest/libqos/fw_cfg.c
>>> +++ b/tests/qtest/libqos/fw_cfg.c
>>> @@ -131,6 +131,7 @@ void mm_fw_cfg_uninit(QFWCFG *fw_cfg)
>>>       g_free(fw_cfg);
>>>   }
>>>   +#ifdef TARGET_HAS_IOPORT
>>>   static void io_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
>>>   {
>>>       qtest_outw(fw_cfg->qts, fw_cfg->base, key);
>>> @@ -162,3 +163,4 @@ void io_fw_cfg_uninit(QFWCFG *fw_cfg)
>>>   {
>>>       g_free(fw_cfg);
>>>   }
>>> +#endif /* TARGET_HAS_IOPORT */
>>>
>>
>> I'm not sure the macro name is ideal; the PCI host on aarch64/"virt"
>> emulates IO Ports (it's possible to allocate PCI IO resources on
>> "virt"). From patch#3, TARGET_HAS_IOPORT does not seem to extend to
>> arm64.
> 
> Correct, aarch64 has memory-mapped pci io resources, they are not on a
> separate ioport address space as for x86 and avr.

I first wrote TARGET_CPU_HAS_IOPORT but realized architecture
and CPU are linked, so I elided _CPU_.

What I'd like to clear from the QTest API is the idea that the CPU has
direct access to the I/O bus via I/O specific instructions.

Any machine able to use a host <-> PCI bus chipset is able to access
the I/O function from the PCI bus.

The fact that on X86 the first PCI function is wired to the same I/O
bus than the CPU is a machine implementation detail.

When accessing PCI I/O ressources on Aarch64, you don't have to use
dedicated I/O instructions.

Anyway for now Thomas discarded this series, as QTest is a generic API,
and we never had to worry about mixing address spaces so far, so not in
a hurry to clean this (although it would be useful to change address
space to access DMA or secure-CPU-view from QTest).

Regards,

Phil.

Re: [RFC PATCH 5/8] qtest/libqos: Restrict CPU I/O instructions

[Laszlo Ersek]

On 03/16/21 16:55, Philippe Mathieu-Daudé wrote:
> Hi Richard and Laszlo,
> 
> On 3/16/21 4:43 PM, Richard Henderson wrote:
>> On 3/16/21 9:37 AM, Laszlo Ersek wrote:
>>> (+Peter, comment below)
>>>
>>> On 03/15/21 00:29, Philippe Mathieu-Daudé wrote:
>>>> Restrict CPU I/O instructions to architectures providing
>>>> I/O bus.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>>   tests/qtest/libqos/fw_cfg.h | 3 +++
>>>>   tests/qtest/libqos/fw_cfg.c | 2 ++
>>>>   2 files changed, 5 insertions(+)
>>>>
>>>> diff --git a/tests/qtest/libqos/fw_cfg.h b/tests/qtest/libqos/fw_cfg.h
>>>> index c6a7cf8cf05..3bfb6d6d55b 100644
>>>> --- a/tests/qtest/libqos/fw_cfg.h
>>>> +++ b/tests/qtest/libqos/fw_cfg.h
>>>> @@ -36,6 +36,8 @@ size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char
>>>> *filename,
>>>>     QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
>>>>   void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
>>>> +
>>>> +#ifdef TARGET_HAS_IOPORT
>>>>   QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
>>>>   void io_fw_cfg_uninit(QFWCFG *fw_cfg);
>>>>   @@ -48,6 +50,7 @@ static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
>>>>   {
>>>>       io_fw_cfg_uninit(fw_cfg);
>>>>   }
>>>> +#endif /* TARGET_HAS_IOPORT */
>>>>     G_DEFINE_AUTOPTR_CLEANUP_FUNC(QFWCFG, mm_fw_cfg_uninit)
>>>>   diff --git a/tests/qtest/libqos/fw_cfg.c b/tests/qtest/libqos/fw_cfg.c
>>>> index 6b8e1babe51..db2b83f5212 100644
>>>> --- a/tests/qtest/libqos/fw_cfg.c
>>>> +++ b/tests/qtest/libqos/fw_cfg.c
>>>> @@ -131,6 +131,7 @@ void mm_fw_cfg_uninit(QFWCFG *fw_cfg)
>>>>       g_free(fw_cfg);
>>>>   }
>>>>   +#ifdef TARGET_HAS_IOPORT
>>>>   static void io_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
>>>>   {
>>>>       qtest_outw(fw_cfg->qts, fw_cfg->base, key);
>>>> @@ -162,3 +163,4 @@ void io_fw_cfg_uninit(QFWCFG *fw_cfg)
>>>>   {
>>>>       g_free(fw_cfg);
>>>>   }
>>>> +#endif /* TARGET_HAS_IOPORT */
>>>>
>>>
>>> I'm not sure the macro name is ideal; the PCI host on aarch64/"virt"
>>> emulates IO Ports (it's possible to allocate PCI IO resources on
>>> "virt"). From patch#3, TARGET_HAS_IOPORT does not seem to extend to
>>> arm64.
>>
>> Correct, aarch64 has memory-mapped pci io resources, they are not on a
>> separate ioport address space as for x86 and avr.
> 
> I first wrote TARGET_CPU_HAS_IOPORT but realized architecture
> and CPU are linked, so I elided _CPU_.
> 
> What I'd like to clear from the QTest API is the idea that the CPU has
> direct access to the I/O bus via I/O specific instructions.
> 
> Any machine able to use a host <-> PCI bus chipset is able to access
> the I/O function from the PCI bus.
> 
> The fact that on X86 the first PCI function is wired to the same I/O
> bus than the CPU is a machine implementation detail.
> 
> When accessing PCI I/O ressources on Aarch64, you don't have to use
> dedicated I/O instructions.
> 
> Anyway for now Thomas discarded this series, as QTest is a generic API,
> and we never had to worry about mixing address spaces so far, so not in
> a hurry to clean this (although it would be useful to change address
> space to access DMA or secure-CPU-view from QTest).

If this is about an "IO Bus" or "IO instructions", then we should call
the macro TARGET_HAS_IO_BUS or "TARGET_ISA_HAS_IO" (or
"TARGET_HAS_IO_INSNS"), or something like those. My only confusion was
about the "IO Port" expression in the macro name; the idea is OK from my
perspective otherwise.

Thanks
Laszlo

Re: [RFC PATCH 5/8] qtest/libqos: Restrict CPU I/O instructions

[Philippe Mathieu-Daudé]

On 3/17/21 4:59 PM, Laszlo Ersek wrote:
> On 03/16/21 16:55, Philippe Mathieu-Daudé wrote:
>> Hi Richard and Laszlo,
>>
>> On 3/16/21 4:43 PM, Richard Henderson wrote:
>>> On 3/16/21 9:37 AM, Laszlo Ersek wrote:
>>>> (+Peter, comment below)
>>>>
>>>> On 03/15/21 00:29, Philippe Mathieu-Daudé wrote:
>>>>> Restrict CPU I/O instructions to architectures providing
>>>>> I/O bus.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>> ---
>>>>>   tests/qtest/libqos/fw_cfg.h | 3 +++
>>>>>   tests/qtest/libqos/fw_cfg.c | 2 ++
>>>>>   2 files changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/tests/qtest/libqos/fw_cfg.h b/tests/qtest/libqos/fw_cfg.h
>>>>> index c6a7cf8cf05..3bfb6d6d55b 100644
>>>>> --- a/tests/qtest/libqos/fw_cfg.h
>>>>> +++ b/tests/qtest/libqos/fw_cfg.h
>>>>> @@ -36,6 +36,8 @@ size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char
>>>>> *filename,
>>>>>     QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
>>>>>   void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
>>>>> +
>>>>> +#ifdef TARGET_HAS_IOPORT
>>>>>   QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
>>>>>   void io_fw_cfg_uninit(QFWCFG *fw_cfg);
>>>>>   @@ -48,6 +50,7 @@ static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
>>>>>   {
>>>>>       io_fw_cfg_uninit(fw_cfg);
>>>>>   }
>>>>> +#endif /* TARGET_HAS_IOPORT */
>>>>>     G_DEFINE_AUTOPTR_CLEANUP_FUNC(QFWCFG, mm_fw_cfg_uninit)
>>>>>   diff --git a/tests/qtest/libqos/fw_cfg.c b/tests/qtest/libqos/fw_cfg.c
>>>>> index 6b8e1babe51..db2b83f5212 100644
>>>>> --- a/tests/qtest/libqos/fw_cfg.c
>>>>> +++ b/tests/qtest/libqos/fw_cfg.c
>>>>> @@ -131,6 +131,7 @@ void mm_fw_cfg_uninit(QFWCFG *fw_cfg)
>>>>>       g_free(fw_cfg);
>>>>>   }
>>>>>   +#ifdef TARGET_HAS_IOPORT
>>>>>   static void io_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
>>>>>   {
>>>>>       qtest_outw(fw_cfg->qts, fw_cfg->base, key);
>>>>> @@ -162,3 +163,4 @@ void io_fw_cfg_uninit(QFWCFG *fw_cfg)
>>>>>   {
>>>>>       g_free(fw_cfg);
>>>>>   }
>>>>> +#endif /* TARGET_HAS_IOPORT */
>>>>>
>>>>
>>>> I'm not sure the macro name is ideal; the PCI host on aarch64/"virt"
>>>> emulates IO Ports (it's possible to allocate PCI IO resources on
>>>> "virt"). From patch#3, TARGET_HAS_IOPORT does not seem to extend to
>>>> arm64.
>>>
>>> Correct, aarch64 has memory-mapped pci io resources, they are not on a
>>> separate ioport address space as for x86 and avr.
>>
>> I first wrote TARGET_CPU_HAS_IOPORT but realized architecture
>> and CPU are linked, so I elided _CPU_.
>>
>> What I'd like to clear from the QTest API is the idea that the CPU has
>> direct access to the I/O bus via I/O specific instructions.
>>
>> Any machine able to use a host <-> PCI bus chipset is able to access
>> the I/O function from the PCI bus.
>>
>> The fact that on X86 the first PCI function is wired to the same I/O
>> bus than the CPU is a machine implementation detail.
>>
>> When accessing PCI I/O ressources on Aarch64, you don't have to use
>> dedicated I/O instructions.
>>
>> Anyway for now Thomas discarded this series, as QTest is a generic API,
>> and we never had to worry about mixing address spaces so far, so not in
>> a hurry to clean this (although it would be useful to change address
>> space to access DMA or secure-CPU-view from QTest).
> 
> If this is about an "IO Bus" or "IO instructions", then we should call
> the macro TARGET_HAS_IO_BUS or "TARGET_ISA_HAS_IO" (or
> "TARGET_HAS_IO_INSNS"), or something like those. My only confusion was
> about the "IO Port" expression in the macro name; the idea is OK from my
> perspective otherwise.

TARGET_HAS_IO_BUS / TARGET_HAS_IO_INSNS LGTM
(ISA bus is not particularly relevant for the AVR target).

Thanks for the feedback :)

Re: [RFC PATCH 5/8] qtest/libqos: Restrict CPU I/O instructions

[Laszlo Ersek]

On 03/17/21 17:24, Philippe Mathieu-Daudé wrote:
> On 3/17/21 4:59 PM, Laszlo Ersek wrote:

>> If this is about an "IO Bus" or "IO instructions", then we should call
>> the macro TARGET_HAS_IO_BUS or "TARGET_ISA_HAS_IO" (or
>> "TARGET_HAS_IO_INSNS"), or something like those. My only confusion was
>> about the "IO Port" expression in the macro name; the idea is OK from my
>> perspective otherwise.
> 
> TARGET_HAS_IO_BUS / TARGET_HAS_IO_INSNS LGTM
> (ISA bus is not particularly relevant for the AVR target).

Apologies for being unclear -- by "ISA", I meant "instruction set
architecture".

So "TARGET_ISA_HAS_IO" was a synonym for "TARGET_HAS_IO_INSNS" -- i.e.,
no IO-specific machine code instructions.

> Thanks for the feedback :)

My pleasure :)
Laszlo

Re: [RFC PATCH 4/8] qtest/fuzz: Restrict CPU I/O instructions

[Paolo Bonzini]

On 15/03/21 06:14, Thomas Huth wrote:
>> diff --git a/tests/qtest/fuzz/generic_fuzz.c 
>> b/tests/qtest/fuzz/generic_fuzz.c
>> index ee8c17a04c4..3e0089f4a63 100644
>> --- a/tests/qtest/fuzz/generic_fuzz.c
>> +++ b/tests/qtest/fuzz/generic_fuzz.c
>> @@ -304,6 +304,13 @@ static bool get_io_address(address_range *result, 
>> AddressSpace *as,
>>       return cb_info.found;
>>   }
>> +static bool get_mmio_address(address_range *result,
>> +                             uint8_t index, uint32_t offset)
>> +{
>> +    return get_io_address(result, &address_space_memory, index, offset);
>> +}
>> +
>> +#ifdef TARGET_HAS_IOPORT
> 
> Sorry, but the qtests are generic code, I don't think we should 
> introduce target specific ifdefs here...?

FWIW this is not a qtest, it's a separate emulator executable and this 
file is compiled per-target.

That said, your objection does apply to patch 5 since libqos is compiled 
only once for all targets.

Paolo