diff mbox

hw/arm/virt: mark the PCIe host controller as DMA coherent in the DT

Message ID 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org
State Superseded
Headers show

Commit Message

Ard Biesheuvel June 28, 2016, 5:14 p.m. UTC 
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(+)

-- 
2.7.4

Comments

Peter Maydell June 30, 2016, 2:09 p.m. UTC | #1 
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
Andrew Jones July 1, 2016, 11:40 a.m. UTC | #2 
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
Ard Biesheuvel July 1, 2016, 1:07 p.m. UTC | #3 
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?)
Peter Maydell July 1, 2016, 1:31 p.m. UTC | #4 
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 mbox

Patch

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",