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é <f4...@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