diff mbox series

[10/10] hw/arm/virt: Fix devicetree warnings about the virtio-iommu node

Message ID 20220824155113.286730-11-jean-philippe@linaro.org
State Superseded
Headers show
Series hw/arm/virt: Fix dt-schema warnings | expand

Commit Message

Jean-Philippe Brucker Aug. 24, 2022, 3:51 p.m. UTC
dt-validate and dtc throw a few warnings when parsing the virtio-iommu
node:

  pcie@10000000: virtio_iommu@16:compatible: ['virtio,pci-iommu'] does not contain items matching the given schema
  pcie@10000000: Unevaluated properties are not allowed (... 'virtio_iommu@16' were unexpected)
  From schema: linux/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
  pcie@10000000: virtio_iommu@16:compatible: ['virtio,pci-iommu'] does not contain items matching the given schema
  From schema: dtschema/schemas/pci/pci-bus.yaml

  Warning (pci_device_reg): /pcie@10000000/virtio_iommu@16: PCI unit address format error, expected "2,0"

The compatible property for a PCI child node should follow the rules
from "PCI Bus Binding to: IEEE Std 1275-1994". It should contain the
Vendor ID and Device ID (or class code).

The unit-name should be "device,function".

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
Note that this doesn't follow
linux/Documentation/devicetree/bindings/virtio/iommu.txt, I'll update
that document when converting it to yaml, hopefully this Linux cycle.
The "virtio,pci-iommu" compatible string is not actually used by any
driver and only QEMU implements it, so we can get rid of it.
---
 hw/arm/virt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Peter Maydell Aug. 24, 2022, 7:51 p.m. UTC | #1
On Wed, 24 Aug 2022 at 16:51, Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> dt-validate and dtc throw a few warnings when parsing the virtio-iommu
> node:
>
>   pcie@10000000: virtio_iommu@16:compatible: ['virtio,pci-iommu'] does not contain items matching the given schema
>   pcie@10000000: Unevaluated properties are not allowed (... 'virtio_iommu@16' were unexpected)
>   From schema: linux/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
>   pcie@10000000: virtio_iommu@16:compatible: ['virtio,pci-iommu'] does not contain items matching the given schema
>   From schema: dtschema/schemas/pci/pci-bus.yaml
>
>   Warning (pci_device_reg): /pcie@10000000/virtio_iommu@16: PCI unit address format error, expected "2,0"
>
> The compatible property for a PCI child node should follow the rules
> from "PCI Bus Binding to: IEEE Std 1275-1994". It should contain the
> Vendor ID and Device ID (or class code).
>
> The unit-name should be "device,function".
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> Note that this doesn't follow
> linux/Documentation/devicetree/bindings/virtio/iommu.txt, I'll update
> that document when converting it to yaml, hopefully this Linux cycle.
> The "virtio,pci-iommu" compatible string is not actually used by any
> driver and only QEMU implements it, so we can get rid of it.

I'm not sure you can just change the compat string like that,
unless you can guarantee that nobody anywhere has ever
looked for it in a dtb. Also, "virtio,pci-iommu" is much
clearer than "pci1af4,1057"...

-- PMM
Jean-Philippe Brucker Sept. 1, 2022, 3:04 p.m. UTC | #2
On Wed, Aug 24, 2022 at 08:51:18PM +0100, Peter Maydell wrote:
> On Wed, 24 Aug 2022 at 16:51, Jean-Philippe Brucker
> <jean-philippe@linaro.org> wrote:
> >
> > dt-validate and dtc throw a few warnings when parsing the virtio-iommu
> > node:
> >
> >   pcie@10000000: virtio_iommu@16:compatible: ['virtio,pci-iommu'] does not contain items matching the given schema
> >   pcie@10000000: Unevaluated properties are not allowed (... 'virtio_iommu@16' were unexpected)
> >   From schema: linux/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
> >   pcie@10000000: virtio_iommu@16:compatible: ['virtio,pci-iommu'] does not contain items matching the given schema
> >   From schema: dtschema/schemas/pci/pci-bus.yaml
> >
> >   Warning (pci_device_reg): /pcie@10000000/virtio_iommu@16: PCI unit address format error, expected "2,0"
> >
> > The compatible property for a PCI child node should follow the rules
> > from "PCI Bus Binding to: IEEE Std 1275-1994". It should contain the
> > Vendor ID and Device ID (or class code).
> >
> > The unit-name should be "device,function".
> >
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> > Note that this doesn't follow
> > linux/Documentation/devicetree/bindings/virtio/iommu.txt, I'll update
> > that document when converting it to yaml, hopefully this Linux cycle.
> > The "virtio,pci-iommu" compatible string is not actually used by any
> > driver and only QEMU implements it, so we can get rid of it.
> 
> I'm not sure you can just change the compat string like that,
> unless you can guarantee that nobody anywhere has ever
> looked for it in a dtb. Also, "virtio,pci-iommu" is much
> clearer than "pci1af4,1057"...

I'm pretty sure nobody ever looked for it, but can't guarantee it. And yes
the PCI notation is hideous but that's what the standard requires. So I
think changing this to 'compatible = "virtio,pci-iommu", "pci1af4,1057"'
would be better.

Thanks,
Jean
diff mbox series

Patch

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index de508d5329..08b79592eb 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1374,14 +1374,15 @@  static void create_smmu(const VirtMachineState *vms,
 
 static void create_virtio_iommu_dt_bindings(VirtMachineState *vms)
 {
-    const char compat[] = "virtio,pci-iommu";
+    const char compat[] = "pci1af4,1057";
     uint16_t bdf = vms->virtio_iommu_bdf;
     MachineState *ms = MACHINE(vms);
     char *node;
 
     vms->iommu_phandle = qemu_fdt_alloc_phandle(ms->fdt);
 
-    node = g_strdup_printf("%s/virtio_iommu@%d", vms->pciehb_nodename, bdf);
+    node = g_strdup_printf("%s/virtio_iommu@%x,%x", vms->pciehb_nodename,
+                           PCI_SLOT(bdf), PCI_FUNC(bdf));
     qemu_fdt_add_subnode(ms->fdt, node);
     qemu_fdt_setprop(ms->fdt, node, "compatible", compat, sizeof(compat));
     qemu_fdt_setprop_sized_cells(ms->fdt, node, "reg",