[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v14 08/11] virtio-iommu-pci: Introduce the x-dt-binding optio
From: |
Auger Eric |
Subject: |
Re: [PATCH v14 08/11] virtio-iommu-pci: Introduce the x-dt-binding option |
Date: |
Fri, 7 Feb 2020 14:36:20 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
Hi Jean,
On 2/7/20 12:15 PM, Jean-Philippe Brucker wrote:
> On Fri, Feb 07, 2020 at 11:51:55AM +0100, Auger Eric wrote:
>> Hi,
>>
>> On 2/7/20 11:23 AM, Michael S. Tsirkin wrote:
>>> On Fri, Feb 07, 2020 at 10:32:00AM +0100, Eric Auger wrote:
>>>> At the moment, the kernel only supports device tree
>>>> integration of the virtio-iommu. DT bindings between the
>>>> PCI root complex and the IOMMU must be created by the machine
>>>> in conformance to:
>>>>
>>>> Documentation/devicetree/bindings/virtio/iommu.txt.
>>>>
>>>> To make sure the end-user is aware of this, force him to use the
>>>> temporary device option "x-dt-binding" and also double check the
>>>> machine has a hotplug handler for the virtio-iommu-pci device.
>>>> This hotplug handler is in charge of creating those DT bindings.
>>>>
>>>> Signed-off-by: Eric Auger <address@hidden>
>>>> Suggested-by: Michael S. Tsirkin <address@hidden>
>>>
>>> how about setting it by default from machine class?
>> Do you mean in ARM virt machine class? But this wouldn't prevent a user
>> from launching an ACPI booted guest. I thought you wanted the end-user
>> to know what he does.
>>
>> I don't figure out a way to know if the guest is booted in dt or acpi
>> mode. I can get access to those info:
>> - whether acpi is enabled
>
> That's the default on virt machine right?
I think that's the default for qemu. you can siwtch it off on the cmd
line using "-no-acpi"
Then if there is a FW it can
> choose either ACPI or DT?
yes that's why I can't actually know which one is going to be used.
>
>> - whether a FW is loaded
In case there is no FW I am sure this is dt mode. So I would be inclined
to only support that mode atm.
>>
>> But a FW can be loaded, acpi enabled and eventually the guest is DT
>> booted with acpi=off in kernel opts.
>>
>> Maybe at this point I could only support the case where no FW is loaded.
>> In machvirt I would not register the virtio-iommu-pci hotplug handler in
>> case a FW is loaded. Then I could get rid of the new x-dt-binding prop.
>>
>> Thoughts?
>
> Yes, I'm hoping we can get the topology description into next version of
> Linux, and it would be nicer not to introduce backward-compatible baggage
> for something that should be solved within a few month
I agree. So I am inclined to remove the x-dt-binding prop then I just
expose allow the virtio-iommu-pci to be instantiated if no FW is loaded.
The constraint will be removed when we get topology description support
in kernel.
Thanks
Eric
If we have to
> warn the user then checking the FW seems like a good compromise, and easy
> to remove later.
>
> Thanks,
> Jean
>
>>
>> Eric
>>> See
>>> [PATCH 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later
>>> [PATCH 2/2] spapr: Enable virtio iommu_platform=on by default
>>> which does it for spapr.
>>
>>>
>>>> ---
>>>>
>>>> May be squashed with previous patch
>>>> ---
>>>> hw/virtio/virtio-iommu-pci.c | 18 ++++++++++++++++++
>>>> 1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
>>>> index d539fcce75..3d06e14000 100644
>>>> --- a/hw/virtio/virtio-iommu-pci.c
>>>> +++ b/hw/virtio/virtio-iommu-pci.c
>>>> @@ -14,6 +14,7 @@
>>>> #include "virtio-pci.h"
>>>> #include "hw/virtio/virtio-iommu.h"
>>>> #include "hw/qdev-properties.h"
>>>> +#include "qapi/error.h"
>>>>
>>>> typedef struct VirtIOIOMMUPCI VirtIOIOMMUPCI;
>>>>
>>>> @@ -27,10 +28,12 @@ typedef struct VirtIOIOMMUPCI VirtIOIOMMUPCI;
>>>> struct VirtIOIOMMUPCI {
>>>> VirtIOPCIProxy parent_obj;
>>>> VirtIOIOMMU vdev;
>>>> + bool dt_binding;
>>>> };
>>>>
>>>> static Property virtio_iommu_pci_properties[] = {
>>>> DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
>>>> + DEFINE_PROP_BOOL("x-dt-binding", VirtIOIOMMUPCI, dt_binding, false),
>>>> DEFINE_PROP_END_OF_LIST(),
>>>> };
>>>>
>>>> @@ -39,6 +42,21 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy
>>>> *vpci_dev, Error **errp)
>>>> VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(vpci_dev);
>>>> DeviceState *vdev = DEVICE(&dev->vdev);
>>>>
>>>> + if (!dev->dt_binding) {
>>>> + error_setg(errp,
>>>> + "Instantiation currently only is possible if the
>>>> machine "
>>>> + "creates device tree iommu-map bindings, ie. ACPI is
>>>> not "
>>>> + "yet supported");
>>>> + error_append_hint(errp, "use -virtio-iommu-pci,x-dt-binding\n");
>>>> + return;
>>>> + }
>>>> +
>>>> + if (!qdev_get_machine_hotplug_handler(DEVICE(vpci_dev))) {
>>>> + error_setg(errp,
>>>> + "The machine does not implement a virtio-iommu-pci
>>>> hotplug "
>>>> + " handler that creates the device tree iommu-map
>>>> bindings");
>>>> + return;
>>>> + }
>>>> qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
>>>> object_property_set_link(OBJECT(dev),
>>>> OBJECT(pci_get_bus(&vpci_dev->pci_dev)),
>>>> --
>>>> 2.20.1
>>>
>>
>
- Re: [PATCH v14 08/11] virtio-iommu-pci: Introduce the x-dt-binding option, (continued)
- Re: [PATCH v14 08/11] virtio-iommu-pci: Introduce the x-dt-binding option, Jean-Philippe Brucker, 2020/02/07
- Re: [PATCH v14 08/11] virtio-iommu-pci: Introduce the x-dt-binding option, Auger Eric, 2020/02/07
- Re: [PATCH v14 08/11] virtio-iommu-pci: Introduce the x-dt-binding option, Michael S. Tsirkin, 2020/02/07
- Re: [PATCH v14 08/11] virtio-iommu-pci: Introduce the x-dt-binding option, Auger Eric, 2020/02/07
- Re: [PATCH v14 08/11] virtio-iommu-pci: Introduce the x-dt-binding option, Peter Xu, 2020/02/07
- Re: [PATCH v14 08/11] virtio-iommu-pci: Introduce the x-dt-binding option, Michael S. Tsirkin, 2020/02/09
- Re: [PATCH v14 08/11] virtio-iommu-pci: Introduce the x-dt-binding option, Peter Xu, 2020/02/10
- Re: [PATCH v14 08/11] virtio-iommu-pci: Introduce the x-dt-binding option, Michael S. Tsirkin, 2020/02/07
- [PATCH v14 09/11] hw/arm/virt: Add the virtio-iommu device tree mappings, Eric Auger, 2020/02/07
- [PATCH v14 10/11] virtio-iommu: Support migration, Eric Auger, 2020/02/07
- [PATCH v14 11/11] tests: Add virtio-iommu test, Eric Auger, 2020/02/07
- Re: [PATCH v14 00/11] VIRTIO-IOMMU device, no-reply, 2020/02/07