Hi jean,
On 9/8/21 11:54 AM, Jean-Philippe Brucker wrote:
> On Mon, Sep 06, 2021 at 04:57:39PM +0200, Eric Auger wrote:
>> Hi Jean,
>>
>> On 9/3/21 4:32 PM, Jean-Philippe Brucker wrote:
>>> From: Eric Auger <eric.au...@redhat.com>
>>>
>>> Add a hotplug handler for virtio-iommu on x86 and set the necessary
>>> reserved region property. On x86, the [0xfee00000, 0xfeefffff] DMA
>>> region is reserved for MSIs. DMA transactions to this range either
>>> trigger IRQ remapping in the IOMMU or bypasses IOMMU translation.
>> s/bypasses/bypass.
>>> Although virtio-iommu does not support IRQ remapping it must be informed
>>> of the reserved region so that it can forward DMA transactions targeting
>>> this region.
>>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>> Feel free to remove my SoB. I have done much here.
>>> Signed-off-by: Jean-Philippe Brucker <jean-phili...@linaro.org>
>>> ---
>>> hw/i386/pc.c | 21 ++++++++++++++++++++-
>>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> index 694fc9ce07..c1e1cffe16 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
>>> @@ -797,6 +797,11 @@ void pc_machine_done(Notifier *notifier, void *data)
>>> "irqchip support.");
>>> exit(EXIT_FAILURE);
>>> }
>>> +
>>> + if (pcms->virtio_iommu && x86_iommu_get_default()) {
>>> + error_report("QEMU does not support multiple vIOMMUs for x86
>>> yet.");
>>> + exit(EXIT_FAILURE);
>>> + }
>>> }
>> I think you shall detect the case of dual instantiation of intel_iommu
>> and virtio-iommu.
> Those are handled by x86_iommu_set_default() and
Oh I did not notice the x86_iommu_get_default() stuff.
sorry for the noise
Eric
> pc_machine_device_plug_cb() (below) respectively. I considered factoring
> it but I think it just adds complexity.
>
> Thanks,
> Jean
>
>> Maybe pc_hotplug_allowed() can be used for that. Note
>> that both devices can be refered to in either order.
>>>
>>> void pc_guest_info_init(PCMachineState *pcms)
>>> @@ -1376,6 +1381,14 @@ static void
>>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
>>> object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
>>> pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
>>> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>>> + /* Declare the reserved MSI region */
>>> + char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
>>> + VIRTIO_IOMMU_RESV_MEM_T_MSI);
>>> +
>>> + qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
>>> + qdev_prop_set_string(dev, "reserved-regions[0]", resv_prop_str);
>>> + g_free(resv_prop_str);
>>> }
>>> }
>>>
>>> @@ -1393,6 +1406,11 @@ static void pc_machine_device_plug_cb(HotplugHandler
>>> *hotplug_dev,
>>> PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>>> PCIDevice *pdev = PCI_DEVICE(dev);
>>>
>>> + if (pcms->virtio_iommu) {
>>> + error_setg(errp,
>>> + "QEMU does not support multiple vIOMMUs for x86
>>> yet.");
>>> + return;
>>> + }
>>> pcms->virtio_iommu = true;
>>> pcms->virtio_iommu_bdf = pci_get_bdf(pdev);
>>> }
>>> @@ -1436,7 +1454,8 @@ static HotplugHandler
>>> *pc_get_hotplug_handler(MachineState *machine,
>>> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
>>> object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
>>> object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
>>> - object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
>>> + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) ||
>>> + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>>> return HOTPLUG_HANDLER(machine);
>>> }
>>>
>> Thanks
>>
>> Eric
>>
