| Message ID | 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 28 June 2016 at 18:14, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > Since QEMU performs cacheable accesses to guest memory when doing DMA > as part of the implementation of emulated PCI devices, guest drivers > should use cacheable accesses as well when running under KVM. Since this > essentially means that emulated PCI devices are DMA coherent, set the > 'dma-coherent' DT property on the PCIe host controller DT node. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > hw/arm/virt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index c5c125e9204a..6e098afd1fe5 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -1021,6 +1021,7 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic, > qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 2); > qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0, > nr_pcie_buses - 1); > + qemu_fdt_setprop(vbi->fdt, nodename, "dma-coherent", NULL, 0); > > if (vbi->v2m_phandle) { > qemu_fdt_setprop_cells(vbi->fdt, nodename, "msi-parent", > -- > 2.7.4 Applied, thanks. -- PMM
On Thu, Jun 30, 2016 at 03:09:53PM +0100, Peter Maydell wrote: > On 28 June 2016 at 18:14, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > Since QEMU performs cacheable accesses to guest memory when doing DMA > > as part of the implementation of emulated PCI devices, guest drivers > > should use cacheable accesses as well when running under KVM. Since this > > essentially means that emulated PCI devices are DMA coherent, set the > > 'dma-coherent' DT property on the PCIe host controller DT node. > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > hw/arm/virt.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > index c5c125e9204a..6e098afd1fe5 100644 > > --- a/hw/arm/virt.c > > +++ b/hw/arm/virt.c > > @@ -1021,6 +1021,7 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic, > > qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 2); > > qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0, > > nr_pcie_buses - 1); > > + qemu_fdt_setprop(vbi->fdt, nodename, "dma-coherent", NULL, 0); > > > > if (vbi->v2m_phandle) { > > qemu_fdt_setprop_cells(vbi->fdt, nodename, "msi-parent", > > -- > > 2.7.4 > > Applied, thanks. > > -- PMM > I might have mentioned in the commit message that the ACPI generation already does this, as _CCA is set to 1, added with commit bc64b96c (assuming I'm right, and a value of 1 there is the ACPI equivalent of this patch) bc64b96c's commit message is also lacking, in the fact it doesn't state why the value of 1 is chosen, only that the attribute is compulsory, which I presume could have been added with the value 0 to satisfy that. Anyway, I just wanted to point out that I *think* we're fine for ACPI. Perhaps I'm the only one who didn't know that already though... Thanks, drew
On 1 July 2016 at 13:40, Andrew Jones <drjones@redhat.com> wrote: > On Thu, Jun 30, 2016 at 03:09:53PM +0100, Peter Maydell wrote: >> On 28 June 2016 at 18:14, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> > Since QEMU performs cacheable accesses to guest memory when doing DMA >> > as part of the implementation of emulated PCI devices, guest drivers >> > should use cacheable accesses as well when running under KVM. Since this >> > essentially means that emulated PCI devices are DMA coherent, set the >> > 'dma-coherent' DT property on the PCIe host controller DT node. >> > >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > --- >> > hw/arm/virt.c | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> > index c5c125e9204a..6e098afd1fe5 100644 >> > --- a/hw/arm/virt.c >> > +++ b/hw/arm/virt.c >> > @@ -1021,6 +1021,7 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic, >> > qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 2); >> > qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0, >> > nr_pcie_buses - 1); >> > + qemu_fdt_setprop(vbi->fdt, nodename, "dma-coherent", NULL, 0); >> > >> > if (vbi->v2m_phandle) { >> > qemu_fdt_setprop_cells(vbi->fdt, nodename, "msi-parent", >> > -- >> > 2.7.4 >> >> Applied, thanks. >> >> -- PMM >> > > I might have mentioned in the commit message that the ACPI generation > already does this, as _CCA is set to 1, added with commit bc64b96c > (assuming I'm right, and a value of 1 there is the ACPI equivalent of > this patch) > > bc64b96c's commit message is also lacking, in the fact it doesn't > state why the value of 1 is chosen, only that the attribute is > compulsory, which I presume could have been added with the value 0 > to satisfy that. > > Anyway, I just wanted to point out that I *think* we're fine for > ACPI. Perhaps I'm the only one who didn't know that already though... > I did a quick test to compare: (a) mach-virt in DT mode without this patch (b) mach-virt in DT mode with this patch (c) mach-virt in ACPI mode (with bc64b96c applied) with '-device nec-usb-xhci -device usb-kbd' added to the QEMU command line (b) and (c) detect the keyboard, whereas (a) does not. So I guess that solves the USB mystery (but perhaps Alex could confirm?)
On 1 July 2016 at 12:40, Andrew Jones <drjones@redhat.com> wrote: > I might have mentioned in the commit message that the ACPI generation > already does this, as _CCA is set to 1, added with commit bc64b96c > (assuming I'm right, and a value of 1 there is the ACPI equivalent of > this patch) > > bc64b96c's commit message is also lacking, in the fact it doesn't > state why the value of 1 is chosen, only that the attribute is > compulsory, which I presume could have been added with the value 0 > to satisfy that. > > Anyway, I just wanted to point out that I *think* we're fine for > ACPI. Perhaps I'm the only one who didn't know that already though... Thanks for the update: I've edited the commit message to add: # This brings the DT description into line with the ACPI description, # which already marks the PCI bridge as cache coherent (see commit # bc64b96c984abf). thanks -- PMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index c5c125e9204a..6e098afd1fe5 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1021,6 +1021,7 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic, qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 2); qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0, nr_pcie_buses - 1); + qemu_fdt_setprop(vbi->fdt, nodename, "dma-coherent", NULL, 0); if (vbi->v2m_phandle) { qemu_fdt_setprop_cells(vbi->fdt, nodename, "msi-parent",
Since QEMU performs cacheable accesses to guest memory when doing DMA as part of the implementation of emulated PCI devices, guest drivers should use cacheable accesses as well when running under KVM. Since this essentially means that emulated PCI devices are DMA coherent, set the 'dma-coherent' DT property on the PCIe host controller DT node. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- hw/arm/virt.c | 1 + 1 file changed, 1 insertion(+)