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.