| Message ID | 3301c5bc-7b47-1b0e-8ce4-30435057a276@web.de |
|---|---|
| State | New |
| Headers | show |
| Series | hw/arm/virt: Add linux,pci-domain property | expand |
On 23 April 2018 at 06:18, Jan Kiszka <jan.kiszka@web.de> wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > This allows to pin the host controller in the Linux PCI domain space. > Linux requires that property to be available consistently or not at all, > in which case the domain number becomes unstable on additions/removals. > Adding it here won't make a difference in practice for most setups as we > only expose one controller. > > However, enabling Jailhouse on top may introduce another controller, and > that one would like to have stable address as well. So the property is > needed for the first controller as well. Am I right in thinking that for ACPI the PCI domain number is communicated via the _SEG method? If so, looks like we already set that, and we set it to 0, which matches what we're doing here in the DT, so that's good. > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > hw/arm/virt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 94dcb125d3..943371b75e 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -1023,6 +1023,7 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic) > qemu_fdt_setprop_string(vms->fdt, nodename, "device_type", "pci"); > qemu_fdt_setprop_cell(vms->fdt, nodename, "#address-cells", 3); > qemu_fdt_setprop_cell(vms->fdt, nodename, "#size-cells", 2); > + qemu_fdt_setprop_cell(vms->fdt, nodename, "linux,pci-domain", 0); > qemu_fdt_setprop_cells(vms->fdt, nodename, "bus-range", 0, > nr_pcie_buses - 1); > qemu_fdt_setprop(vms->fdt, nodename, "dma-coherent", NULL, 0); > -- Drew -- is minor changes to the DTC something we can do without having to condition it on machine version? I forget... thanks -- PMM
On 2018-04-23 15:11, Peter Maydell wrote: > On 23 April 2018 at 06:18, Jan Kiszka <jan.kiszka@web.de> wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> This allows to pin the host controller in the Linux PCI domain space. >> Linux requires that property to be available consistently or not at all, >> in which case the domain number becomes unstable on additions/removals. >> Adding it here won't make a difference in practice for most setups as we >> only expose one controller. >> >> However, enabling Jailhouse on top may introduce another controller, and >> that one would like to have stable address as well. So the property is >> needed for the first controller as well. > > Am I right in thinking that for ACPI the PCI domain number is > communicated via the _SEG method? If so, looks like we already > set that, and we set it to 0, which matches what we're doing here > in the DT, so that's good. Looking at the related arm64 kernel code (not the spec), it seems __SEG is involved, yet. Jan > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> hw/arm/virt.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 94dcb125d3..943371b75e 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -1023,6 +1023,7 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic) >> qemu_fdt_setprop_string(vms->fdt, nodename, "device_type", "pci"); >> qemu_fdt_setprop_cell(vms->fdt, nodename, "#address-cells", 3); >> qemu_fdt_setprop_cell(vms->fdt, nodename, "#size-cells", 2); >> + qemu_fdt_setprop_cell(vms->fdt, nodename, "linux,pci-domain", 0); >> qemu_fdt_setprop_cells(vms->fdt, nodename, "bus-range", 0, >> nr_pcie_buses - 1); >> qemu_fdt_setprop(vms->fdt, nodename, "dma-coherent", NULL, 0); >> -- > > Drew -- is minor changes to the DTC something we can do without > having to condition it on machine version? I forget... > > thanks > -- PMM >
On Mon, Apr 23, 2018 at 02:11:40PM +0100, Peter Maydell wrote: > On 23 April 2018 at 06:18, Jan Kiszka <jan.kiszka@web.de> wrote: > > From: Jan Kiszka <jan.kiszka@siemens.com> > > > > This allows to pin the host controller in the Linux PCI domain space. > > Linux requires that property to be available consistently or not at all, > > in which case the domain number becomes unstable on additions/removals. > > Adding it here won't make a difference in practice for most setups as we > > only expose one controller. > > > > However, enabling Jailhouse on top may introduce another controller, and > > that one would like to have stable address as well. So the property is > > needed for the first controller as well. > > Am I right in thinking that for ACPI the PCI domain number is > communicated via the _SEG method? If so, looks like we already > set that, and we set it to 0, which matches what we're doing here > in the DT, so that's good. _SEG and linux,pci-domain are similar in definition, but don't appear to be equivalent, as the same _SEG number is permitted across multiple host bridges, but linux,pci-domain must be unique for each host bridge. I think the (_SEG, _BBN) pair may form the equivalent. We also set _BBN to zero, so I think we're fine anyway. > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > --- > > hw/arm/virt.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > index 94dcb125d3..943371b75e 100644 > > --- a/hw/arm/virt.c > > +++ b/hw/arm/virt.c > > @@ -1023,6 +1023,7 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic) > > qemu_fdt_setprop_string(vms->fdt, nodename, "device_type", "pci"); > > qemu_fdt_setprop_cell(vms->fdt, nodename, "#address-cells", 3); > > qemu_fdt_setprop_cell(vms->fdt, nodename, "#size-cells", 2); > > + qemu_fdt_setprop_cell(vms->fdt, nodename, "linux,pci-domain", 0); > > qemu_fdt_setprop_cells(vms->fdt, nodename, "bus-range", 0, > > nr_pcie_buses - 1); > > qemu_fdt_setprop(vms->fdt, nodename, "dma-coherent", NULL, 0); > > -- > > Drew -- is minor changes to the DTC something we can do without > having to condition it on machine version? I forget... Yes, and even less minor changes than this one can be made to ACPI and DT generation, per the "if a firwmare update could change it, then don't worry about it" policy. (I've CC'ed Igor in case he wants to chime in to correct anything I've said.) Thanks, drew
On Tue, Apr 24, 2018 at 04:12:58PM +0200, Andrew Jones wrote: > On Mon, Apr 23, 2018 at 02:11:40PM +0100, Peter Maydell wrote: > > On 23 April 2018 at 06:18, Jan Kiszka <jan.kiszka@web.de> wrote: > > > From: Jan Kiszka <jan.kiszka@siemens.com> > > > > > > This allows to pin the host controller in the Linux PCI domain space. > > > Linux requires that property to be available consistently or not at all, > > > in which case the domain number becomes unstable on additions/removals. > > > Adding it here won't make a difference in practice for most setups as we > > > only expose one controller. > > > > > > However, enabling Jailhouse on top may introduce another controller, and > > > that one would like to have stable address as well. So the property is > > > needed for the first controller as well. > > > > Am I right in thinking that for ACPI the PCI domain number is > > communicated via the _SEG method? If so, looks like we already > > set that, and we set it to 0, which matches what we're doing here > > in the DT, so that's good. > > _SEG and linux,pci-domain are similar in definition, but don't appear to > be equivalent, as the same _SEG number is permitted across multiple host > bridges, but linux,pci-domain must be unique for each host bridge. I > think the (_SEG, _BBN) pair may form the equivalent. We also set _BBN > to zero, so I think we're fine anyway. > > > > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > > --- > > > hw/arm/virt.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > > index 94dcb125d3..943371b75e 100644 > > > --- a/hw/arm/virt.c > > > +++ b/hw/arm/virt.c > > > @@ -1023,6 +1023,7 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic) > > > qemu_fdt_setprop_string(vms->fdt, nodename, "device_type", "pci"); > > > qemu_fdt_setprop_cell(vms->fdt, nodename, "#address-cells", 3); > > > qemu_fdt_setprop_cell(vms->fdt, nodename, "#size-cells", 2); > > > + qemu_fdt_setprop_cell(vms->fdt, nodename, "linux,pci-domain", 0); > > > qemu_fdt_setprop_cells(vms->fdt, nodename, "bus-range", 0, > > > nr_pcie_buses - 1); > > > qemu_fdt_setprop(vms->fdt, nodename, "dma-coherent", NULL, 0); > > > -- > > > > Drew -- is minor changes to the DTC something we can do without > > having to condition it on machine version? I forget... > > Yes, and even less minor changes than this one can be made to ACPI and DT > generation, per the "if a firwmare update could change it, then don't > worry about it" policy. > > (I've CC'ed Igor in case he wants to chime in to correct anything I've > said.) Eh, now I've CC'ed him...
On 24 April 2018 at 15:12, Andrew Jones <drjones@redhat.com> wrote: > On Mon, Apr 23, 2018 at 02:11:40PM +0100, Peter Maydell wrote: >> On 23 April 2018 at 06:18, Jan Kiszka <jan.kiszka@web.de> wrote: >> > From: Jan Kiszka <jan.kiszka@siemens.com> >> > >> > This allows to pin the host controller in the Linux PCI domain space. >> > Linux requires that property to be available consistently or not at all, >> > in which case the domain number becomes unstable on additions/removals. >> > Adding it here won't make a difference in practice for most setups as we >> > only expose one controller. >> > >> > However, enabling Jailhouse on top may introduce another controller, and >> > that one would like to have stable address as well. So the property is >> > needed for the first controller as well. >> >> Am I right in thinking that for ACPI the PCI domain number is >> communicated via the _SEG method? If so, looks like we already >> set that, and we set it to 0, which matches what we're doing here >> in the DT, so that's good. > > _SEG and linux,pci-domain are similar in definition, but don't appear to > be equivalent, as the same _SEG number is permitted across multiple host > bridges, but linux,pci-domain must be unique for each host bridge. I > think the (_SEG, _BBN) pair may form the equivalent. We also set _BBN > to zero, so I think we're fine anyway. > >> >> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> > --- >> > hw/arm/virt.c | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> > index 94dcb125d3..943371b75e 100644 >> > --- a/hw/arm/virt.c >> > +++ b/hw/arm/virt.c >> > @@ -1023,6 +1023,7 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic) >> > qemu_fdt_setprop_string(vms->fdt, nodename, "device_type", "pci"); >> > qemu_fdt_setprop_cell(vms->fdt, nodename, "#address-cells", 3); >> > qemu_fdt_setprop_cell(vms->fdt, nodename, "#size-cells", 2); >> > + qemu_fdt_setprop_cell(vms->fdt, nodename, "linux,pci-domain", 0); >> > qemu_fdt_setprop_cells(vms->fdt, nodename, "bus-range", 0, >> > nr_pcie_buses - 1); >> > qemu_fdt_setprop(vms->fdt, nodename, "dma-coherent", NULL, 0); >> > -- >> >> Drew -- is minor changes to the DTC something we can do without >> having to condition it on machine version? I forget... > > Yes, and even less minor changes than this one can be made to ACPI and DT > generation, per the "if a firwmare update could change it, then don't > worry about it" policy. Cool; applied to target-arm.next, then. -- PMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 94dcb125d3..943371b75e 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1023,6 +1023,7 @@ static void create_pcie(const VirtMachineState *vms, qemu_irq *pic) qemu_fdt_setprop_string(vms->fdt, nodename, "device_type", "pci"); qemu_fdt_setprop_cell(vms->fdt, nodename, "#address-cells", 3); qemu_fdt_setprop_cell(vms->fdt, nodename, "#size-cells", 2); + qemu_fdt_setprop_cell(vms->fdt, nodename, "linux,pci-domain", 0); qemu_fdt_setprop_cells(vms->fdt, nodename, "bus-range", 0, nr_pcie_buses - 1); qemu_fdt_setprop(vms->fdt, nodename, "dma-coherent", NULL, 0);