All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
@ 2017-02-08 13:31 Alexander Graf
  2017-02-08 15:29 ` Laszlo Ersek
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Graf @ 2017-02-08 13:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-arm, mst

QEMU emulated hardware is always dma coherent with its guest. We do
annotate that correctly on the PCI host controller, but left out
virtio-mmio.

Recent kernels have started to interpret that flag rather than take
dma coherency as granted with virtio-mmio. While that is considered
a kernel bug, as it breaks previously working systems, it showed that
our dt description is incomplete.

This patch adds the respective marker that allows guest OSs to evaluate
that our virtio-mmio devices are indeed cache coherent.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/arm/vexpress.c | 1 +
 hw/arm/virt.c     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 58760f4..e057568 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -452,6 +452,7 @@ static int add_virtio_mmio_node(void *fdt, uint32_t acells, uint32_t scells,
                                        acells, addr, scells, size);
     qemu_fdt_setprop_cells(fdt, nodename, "interrupt-parent", intc);
     qemu_fdt_setprop_cells(fdt, nodename, "interrupts", 0, irq, 1);
+    qemu_fdt_setprop(fdt, nodename, "dma-coherent", NULL, 0);
     g_free(nodename);
     if (rc) {
         return -1;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 1f216cf..14881fa 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -797,6 +797,7 @@ static void create_virtio_devices(const VirtMachineState *vms, qemu_irq *pic)
         qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
                                GIC_FDT_IRQ_TYPE_SPI, irq,
                                GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
+        qemu_fdt_setprop(vms->fdt, nodename, "dma-coherent", NULL, 0);
         g_free(nodename);
     }
 }
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-08 13:31 [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt Alexander Graf
@ 2017-02-08 15:29 ` Laszlo Ersek
  2017-02-08 16:12   ` Alexander Graf
  0 siblings, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2017-02-08 15:29 UTC (permalink / raw)
  To: Alexander Graf, qemu-devel; +Cc: qemu-arm, mst, Ard Biesheuvel, Shannon Zhao

CC'ing Ard and Shannon (I recall this property from earlier):

On 02/08/17 14:31, Alexander Graf wrote:
> QEMU emulated hardware is always dma coherent with its guest. We do
> annotate that correctly on the PCI host controller, but left out
> virtio-mmio.

I recommend to reference the following commit here:

commit 5d636e21c44ecf982a22a7bc4ca89186079ac283
Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Mon Jul 4 13:06:36 2016 +0100

    hw/arm/virt: mark the PCIe host controller as DMA coherent in the DT
    
    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.
    
    This brings the DT description into line with the ACPI description,
    which already marks the PCI bridge as cache coherent (see commit
    bc64b96c984abf).
    
    Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
    Message-id: 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org
    Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
    Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> 
> Recent kernels have started to interpret that flag rather than take
> dma coherency as granted with virtio-mmio. While that is considered
> a kernel bug, as it breaks previously working systems, it showed that
> our dt description is incomplete.
> 
> This patch adds the respective marker that allows guest OSs to evaluate
> that our virtio-mmio devices are indeed cache coherent.

As noted above, commit bc64b96c984a ("hw/arm/virt-acpi-build: _CCA attribute is compulsory", 2015-11-03) had done the same in the ACPI description of the PCIe host controller.

Thus, do we need _CCA in the ACPI description of the virtio-mmio transports, to parallel the DT change? See the LNRO0005 device in acpi_dsdt_add_virtio().

If that's the case, then I propose that either the patch please fix both DT and ACPI, or that at least we file a bug "somewhere", for adding _CCA in acpi_dsdt_add_virtio().

Ard, Shannon, any comments?

Thanks,
Laszlo


> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/arm/vexpress.c | 1 +
>  hw/arm/virt.c     | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> index 58760f4..e057568 100644
> --- a/hw/arm/vexpress.c
> +++ b/hw/arm/vexpress.c
> @@ -452,6 +452,7 @@ static int add_virtio_mmio_node(void *fdt, uint32_t acells, uint32_t scells,
>                                         acells, addr, scells, size);
>      qemu_fdt_setprop_cells(fdt, nodename, "interrupt-parent", intc);
>      qemu_fdt_setprop_cells(fdt, nodename, "interrupts", 0, irq, 1);
> +    qemu_fdt_setprop(fdt, nodename, "dma-coherent", NULL, 0);
>      g_free(nodename);
>      if (rc) {
>          return -1;
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 1f216cf..14881fa 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -797,6 +797,7 @@ static void create_virtio_devices(const VirtMachineState *vms, qemu_irq *pic)
>          qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
>                                 GIC_FDT_IRQ_TYPE_SPI, irq,
>                                 GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
> +        qemu_fdt_setprop(vms->fdt, nodename, "dma-coherent", NULL, 0);
>          g_free(nodename);
>      }
>  }
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-08 15:29 ` Laszlo Ersek
@ 2017-02-08 16:12   ` Alexander Graf
  2017-02-08 16:17     ` Laszlo Ersek
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Graf @ 2017-02-08 16:12 UTC (permalink / raw)
  To: Laszlo Ersek, qemu-devel; +Cc: qemu-arm, mst, Ard Biesheuvel, Shannon Zhao

On 02/08/2017 04:29 PM, Laszlo Ersek wrote:
> CC'ing Ard and Shannon (I recall this property from earlier):
>
> On 02/08/17 14:31, Alexander Graf wrote:
>> QEMU emulated hardware is always dma coherent with its guest. We do
>> annotate that correctly on the PCI host controller, but left out
>> virtio-mmio.
> I recommend to reference the following commit here:
>
> commit 5d636e21c44ecf982a22a7bc4ca89186079ac283
> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date:   Mon Jul 4 13:06:36 2016 +0100
>
>      hw/arm/virt: mark the PCIe host controller as DMA coherent in the DT
>      
>      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.
>      
>      This brings the DT description into line with the ACPI description,
>      which already marks the PCI bridge as cache coherent (see commit
>      bc64b96c984abf).
>      
>      Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>      Message-id: 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org
>      Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>      Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
>> Recent kernels have started to interpret that flag rather than take
>> dma coherency as granted with virtio-mmio. While that is considered
>> a kernel bug, as it breaks previously working systems, it showed that
>> our dt description is incomplete.
>>
>> This patch adds the respective marker that allows guest OSs to evaluate
>> that our virtio-mmio devices are indeed cache coherent.
> As noted above, commit bc64b96c984a ("hw/arm/virt-acpi-build: _CCA attribute is compulsory", 2015-11-03) had done the same in the ACPI description of the PCIe host controller.
>
> Thus, do we need _CCA in the ACPI description of the virtio-mmio transports, to parallel the DT change? See the LNRO0005 device in acpi_dsdt_add_virtio().

Yes, we should also annotate it correctly in the DSDT. Today it's not a 
deal breaker as Linux always assumes virtio-mmio to be dma coherent, but 
it would make our platform description more accurate.

> If that's the case, then I propose that either the patch please fix both DT and ACPI, or that at least we file a bug "somewhere", for adding _CCA in acpi_dsdt_add_virtio().

I agree that it should happen in the same patch (set). While I don't 
care a lot about ACPI right now (since dt is preferred on upstream 
kernels), I can take a look.


Alex

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-08 16:12   ` Alexander Graf
@ 2017-02-08 16:17     ` Laszlo Ersek
  2017-02-08 16:27       ` Ard Biesheuvel
                         ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Laszlo Ersek @ 2017-02-08 16:17 UTC (permalink / raw)
  To: Alexander Graf, qemu-devel; +Cc: qemu-arm, mst, Ard Biesheuvel, Shannon Zhao

On 02/08/17 17:12, Alexander Graf wrote:
> On 02/08/2017 04:29 PM, Laszlo Ersek wrote:
>> CC'ing Ard and Shannon (I recall this property from earlier):
>>
>> On 02/08/17 14:31, Alexander Graf wrote:
>>> QEMU emulated hardware is always dma coherent with its guest. We do
>>> annotate that correctly on the PCI host controller, but left out
>>> virtio-mmio.
>> I recommend to reference the following commit here:
>>
>> commit 5d636e21c44ecf982a22a7bc4ca89186079ac283
>> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Date:   Mon Jul 4 13:06:36 2016 +0100
>>
>>      hw/arm/virt: mark the PCIe host controller as DMA coherent in the DT
>>           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.
>>           This brings the DT description into line with the ACPI
>> description,
>>      which already marks the PCI bridge as cache coherent (see commit
>>      bc64b96c984abf).
>>           Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>      Message-id:
>> 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org
>>      Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>      Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>
>>> Recent kernels have started to interpret that flag rather than take
>>> dma coherency as granted with virtio-mmio. While that is considered
>>> a kernel bug, as it breaks previously working systems, it showed that
>>> our dt description is incomplete.
>>>
>>> This patch adds the respective marker that allows guest OSs to evaluate
>>> that our virtio-mmio devices are indeed cache coherent.
>> As noted above, commit bc64b96c984a ("hw/arm/virt-acpi-build: _CCA
>> attribute is compulsory", 2015-11-03) had done the same in the ACPI
>> description of the PCIe host controller.
>>
>> Thus, do we need _CCA in the ACPI description of the virtio-mmio
>> transports, to parallel the DT change? See the LNRO0005 device in
>> acpi_dsdt_add_virtio().
> 
> Yes, we should also annotate it correctly in the DSDT. Today it's not a
> deal breaker as Linux always assumes virtio-mmio to be dma coherent, but
> it would make our platform description more accurate.
> 
>> If that's the case, then I propose that either the patch please fix
>> both DT and ACPI, or that at least we file a bug "somewhere", for
>> adding _CCA in acpi_dsdt_add_virtio().
> 
> I agree that it should happen in the same patch (set). While I don't
> care a lot about ACPI right now (since dt is preferred on upstream
> kernels), I can take a look.

Thank you!
Laszlo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-08 16:17     ` Laszlo Ersek
@ 2017-02-08 16:27       ` Ard Biesheuvel
  2017-02-08 18:23         ` [Qemu-devel] [Qemu-arm] " Peter Maydell
  2017-02-09 12:24       ` [Qemu-devel] " Alexander Graf
  2017-02-09 12:29       ` Alexander Graf
  2 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2017-02-08 16:27 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: Alexander Graf, qemu-devel, qemu-arm, mst, Shannon Zhao


> On 8 Feb 2017, at 16:17, Laszlo Ersek <lersek@redhat.com> wrote:
> 
>> On 02/08/17 17:12, Alexander Graf wrote:
>>> On 02/08/2017 04:29 PM, Laszlo Ersek wrote:
>>> CC'ing Ard and Shannon (I recall this property from earlier):
>>> 
>>>> On 02/08/17 14:31, Alexander Graf wrote:
>>>> QEMU emulated hardware is always dma coherent with its guest. We do
>>>> annotate that correctly on the PCI host controller, but left out
>>>> virtio-mmio.
>>> I recommend to reference the following commit here:
>>> 
>>> commit 5d636e21c44ecf982a22a7bc4ca89186079ac283
>>> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Date:   Mon Jul 4 13:06:36 2016 +0100
>>> 
>>>     hw/arm/virt: mark the PCIe host controller as DMA coherent in the DT
>>>          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.
>>>          This brings the DT description into line with the ACPI
>>> description,
>>>     which already marks the PCI bridge as cache coherent (see commit
>>>     bc64b96c984abf).
>>>          Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>     Message-id:
>>> 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org
>>>     Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>>     Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> 
>>>> Recent kernels have started to interpret that flag rather than take
>>>> dma coherency as granted with virtio-mmio. While that is considered
>>>> a kernel bug, as it breaks previously working systems, it showed that
>>>> our dt description is incomplete.
>>>> 
>>>> This patch adds the respective marker that allows guest OSs to evaluate
>>>> that our virtio-mmio devices are indeed cache coherent.
>>> As noted above, commit bc64b96c984a ("hw/arm/virt-acpi-build: _CCA
>>> attribute is compulsory", 2015-11-03) had done the same in the ACPI
>>> description of the PCIe host controller.
>>> 
>>> Thus, do we need _CCA in the ACPI description of the virtio-mmio
>>> transports, to parallel the DT change? See the LNRO0005 device in
>>> acpi_dsdt_add_virtio().
>> 
>> Yes, we should also annotate it correctly in the DSDT. Today it's not a
>> deal breaker as Linux always assumes virtio-mmio to be dma coherent, but
>> it would make our platform description more accurate.
>> 

The _CCA property is mandatory on arm64, and there is no default. So this is a bug nonetheless, even if the de facto default matches the platform. So yes, let's get this fixed please.


>>> If that's the case, then I propose that either the patch please fix
>>> both DT and ACPI, or that at least we file a bug "somewhere", for
>>> adding _CCA in acpi_dsdt_add_virtio().
>> 
>> I agree that it should happen in the same patch (set). While I don't
>> care a lot about ACPI right now (since dt is preferred on upstream
>> kernels), I can take a look.
> 
> Thank you!
> Laszlo
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-08 16:27       ` Ard Biesheuvel
@ 2017-02-08 18:23         ` Peter Maydell
  2017-02-08 18:47           ` Laszlo Ersek
  2017-02-09 10:35           ` Ard Biesheuvel
  0 siblings, 2 replies; 16+ messages in thread
From: Peter Maydell @ 2017-02-08 18:23 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Laszlo Ersek, Shannon Zhao, qemu-arm, Michael S. Tsirkin,
	Alexander Graf, QEMU Developers

On 8 February 2017 at 16:27, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> The _CCA property is mandatory on arm64, and there is no default.

Is there a tool that can check this kind of requirement
and complain about issues in the ACPI tables (and
ditto, device tree)? It's really easy to produce a
dt or ACPI table that works with current kernels and
then turns out to have a problem six or twelve
months down the line :-(

thanks
-- PMM

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-08 18:23         ` [Qemu-devel] [Qemu-arm] " Peter Maydell
@ 2017-02-08 18:47           ` Laszlo Ersek
  2017-02-09 12:04             ` Heyi Guo
  2017-02-09 10:35           ` Ard Biesheuvel
  1 sibling, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2017-02-08 18:47 UTC (permalink / raw)
  To: Peter Maydell, Ard Biesheuvel
  Cc: Shannon Zhao, qemu-arm, Michael S. Tsirkin, Alexander Graf,
	QEMU Developers, Heyi Guo

On 02/08/17 19:23, Peter Maydell wrote:
> On 8 February 2017 at 16:27, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> The _CCA property is mandatory on arm64, and there is no default.
> 
> Is there a tool that can check this kind of requirement
> and complain about issues in the ACPI tables (and
> ditto, device tree)? It's really easy to produce a
> dt or ACPI table that works with current kernels and
> then turns out to have a problem six or twelve
> months down the line :-(

I think the "bios bits" project or the FWTS (firmware test suite)
project might do some ACPI sanity checks. They could be run in guests.

https://biosbits.org/
https://wiki.ubuntu.com/FirmwareTestSuite

However, I'm unsure if they support aarch64. Also... it's not like those
projects are stationary. Even the ACPI spec is a moving target (and
spec-level regressions exist).

I believe Heyi Guo @ Linaro used to work with FWTS on aarch64. CC'd.

No clue about DT conformance testing. Is there an industry standard or a
working group behind DT?

Thanks
Laszlo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-08 18:23         ` [Qemu-devel] [Qemu-arm] " Peter Maydell
  2017-02-08 18:47           ` Laszlo Ersek
@ 2017-02-09 10:35           ` Ard Biesheuvel
  1 sibling, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 10:35 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Laszlo Ersek, Shannon Zhao, qemu-arm, Michael S. Tsirkin,
	Alexander Graf, QEMU Developers

(+Leif, Graeme)

On 8 February 2017 at 18:23, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 8 February 2017 at 16:27, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> The _CCA property is mandatory on arm64, and there is no default.
>
> Is there a tool that can check this kind of requirement
> and complain about issues in the ACPI tables (and
> ditto, device tree)? It's really easy to produce a
> dt or ACPI table that works with current kernels and
> then turns out to have a problem six or twelve
> months down the line :-(
>

Perhaps Graeme or Leif could shed some light on this? I know this is
on our roadmap, but I am not up to date with the latest status.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-08 18:47           ` Laszlo Ersek
@ 2017-02-09 12:04             ` Heyi Guo
  2017-02-10  1:40               ` Alex Hung
  0 siblings, 1 reply; 16+ messages in thread
From: Heyi Guo @ 2017-02-09 12:04 UTC (permalink / raw)
  To: Laszlo Ersek, Peter Maydell, Ard Biesheuvel
  Cc: Shannon Zhao, qemu-arm, Michael S. Tsirkin, Alexander Graf,
	QEMU Developers, Alex Hung

Hi all,

Just to confirm that FWTS is supporting aarch64.

Alex is the maintainer of FWTS and may provide more information.

Regards,

Gary (Heyi Guo)


在 2/9/2017 2:47 AM, Laszlo Ersek 写道:
> On 02/08/17 19:23, Peter Maydell wrote:
>> On 8 February 2017 at 16:27, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>> The _CCA property is mandatory on arm64, and there is no default.
>> Is there a tool that can check this kind of requirement
>> and complain about issues in the ACPI tables (and
>> ditto, device tree)? It's really easy to produce a
>> dt or ACPI table that works with current kernels and
>> then turns out to have a problem six or twelve
>> months down the line :-(
> I think the "bios bits" project or the FWTS (firmware test suite)
> project might do some ACPI sanity checks. They could be run in guests.
>
> https://biosbits.org/
> https://wiki.ubuntu.com/FirmwareTestSuite
>
> However, I'm unsure if they support aarch64. Also... it's not like those
> projects are stationary. Even the ACPI spec is a moving target (and
> spec-level regressions exist).
>
> I believe Heyi Guo @ Linaro used to work with FWTS on aarch64. CC'd.
>
> No clue about DT conformance testing. Is there an industry standard or a
> working group behind DT?
>
> Thanks
> Laszlo
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-08 16:17     ` Laszlo Ersek
  2017-02-08 16:27       ` Ard Biesheuvel
@ 2017-02-09 12:24       ` Alexander Graf
  2017-02-09 12:30         ` Ard Biesheuvel
  2017-02-09 12:41         ` Andrew Jones
  2017-02-09 12:29       ` Alexander Graf
  2 siblings, 2 replies; 16+ messages in thread
From: Alexander Graf @ 2017-02-09 12:24 UTC (permalink / raw)
  To: Laszlo Ersek, qemu-devel; +Cc: qemu-arm, mst, Ard Biesheuvel, Shannon Zhao



On 08/02/2017 17:17, Laszlo Ersek wrote:
> On 02/08/17 17:12, Alexander Graf wrote:
>> On 02/08/2017 04:29 PM, Laszlo Ersek wrote:
>>> CC'ing Ard and Shannon (I recall this property from earlier):
>>>
>>> On 02/08/17 14:31, Alexander Graf wrote:
>>>> QEMU emulated hardware is always dma coherent with its guest. We do
>>>> annotate that correctly on the PCI host controller, but left out
>>>> virtio-mmio.
>>> I recommend to reference the following commit here:
>>>
>>> commit 5d636e21c44ecf982a22a7bc4ca89186079ac283
>>> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Date:   Mon Jul 4 13:06:36 2016 +0100
>>>
>>>      hw/arm/virt: mark the PCIe host controller as DMA coherent in the DT
>>>           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.
>>>           This brings the DT description into line with the ACPI
>>> description,
>>>      which already marks the PCI bridge as cache coherent (see commit
>>>      bc64b96c984abf).
>>>           Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>      Message-id:
>>> 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org
>>>      Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>>      Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>
>>>> Recent kernels have started to interpret that flag rather than take
>>>> dma coherency as granted with virtio-mmio. While that is considered
>>>> a kernel bug, as it breaks previously working systems, it showed that
>>>> our dt description is incomplete.
>>>>
>>>> This patch adds the respective marker that allows guest OSs to evaluate
>>>> that our virtio-mmio devices are indeed cache coherent.
>>> As noted above, commit bc64b96c984a ("hw/arm/virt-acpi-build: _CCA
>>> attribute is compulsory", 2015-11-03) had done the same in the ACPI
>>> description of the PCIe host controller.
>>>
>>> Thus, do we need _CCA in the ACPI description of the virtio-mmio
>>> transports, to parallel the DT change? See the LNRO0005 device in
>>> acpi_dsdt_add_virtio().
>>
>> Yes, we should also annotate it correctly in the DSDT. Today it's not a
>> deal breaker as Linux always assumes virtio-mmio to be dma coherent, but
>> it would make our platform description more accurate.
>>
>>> If that's the case, then I propose that either the patch please fix
>>> both DT and ACPI, or that at least we file a bug "somewhere", for
>>> adding _CCA in acpi_dsdt_add_virtio().
>>
>> I agree that it should happen in the same patch (set). While I don't
>> care a lot about ACPI right now (since dt is preferred on upstream
>> kernels), I can take a look.
>
> Thank you!

Is ACPI boot supposed to work at all with 4.9?

Booting Linux on physical CPU 0x0
Linux version 4.9.6-00018-g13e39d5 (agraf@achrid) (gcc version 6.1.1 
20160711 (Linaro GCC 6.1-2016.08) ) #68 SMP PREEMPT Wed Feb 8 13:25:23 
CET 2017
Boot CPU: AArch64 Processor [500f0001]
efi: Getting EFI parameters from FDT:
efi: UEFI not found.
cma: Reserved 16 MiB at 0x00000000bf000000
ACPI: Early table checksum verification disabled
fACPI: Failed to init ACPI tables
On node 0 totalpages: 524288
   DMA zone: 8192 pages used for memmap
   DMA zone: 0 pages reserved
   DMA zone: 524288 pages, LIFO batch:31
psci: is not implemented in ACPI.
ACPI: APIC not present
fmissing boot CPU MPIDR, not enabling secondaries
percpu: Embedded 21 pages/cpu @ffff80007efdd000 s47896 r8192 d29928 u86016
pcpu-alloc: s47896 r8192 d29928 u86016 alloc=21*4096
pcpu-alloc: [0] 0
Detected PIPT I-cache on CPU0
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 516096
Kernel command line: acpi=force console=ttyAMA0
PID hash table entries: 4096 (order: 3, 32768 bytes)
Dentry cache hash table entries: 262144 (order: 9, 2097152 bytes)
Inode-cache hash table entries: 131072 (order: 8, 1048576 bytes)
Memory: 2030472K/2097152K available (8380K kernel code, 858K rwdata, 
3632K rodata, 1024K init, 280K bss, 50296K reserved, 16384K cma-reserved)
Virtual kernel memory layout:
     modules : 0xffff000000000000 - 0xffff000008000000   (   128 MB)
     vmalloc : 0xffff000008000000 - 0xffff7dffbfff0000   (129022 GB)
       .text : 0xffff000008080000 - 0xffff0000088b0000   (  8384 KB)
     .rodata : 0xffff0000088b0000 - 0xffff000008c40000   (  3648 KB)
       .init : 0xffff000008c40000 - 0xffff000008d40000   (  1024 KB)
       .data : 0xffff000008d40000 - 0xffff000008e16a00   (   859 KB)
        .bss : 0xffff000008e16a00 - 0xffff000008e5cc3c   (   281 KB)
     fixed   : 0xffff7dfffe7fd000 - 0xffff7dfffec00000   (  4108 KB)
     PCI I/O : 0xffff7dfffee00000 - 0xffff7dffffe00000   (    16 MB)
     vmemmap : 0xffff7e0000000000 - 0xffff800000000000   (  2048 GB maximum)
               0xffff7e0000000000 - 0xffff7e0002000000   (    32 MB actual)
     memory  : 0xffff800000000000 - 0xffff800080000000   (  2048 MB)
SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
Preemptible hierarchical RCU implementation.
	Build-time adjustment of leaf fanout to 64.
	RCU restricting CPUs from NR_CPUS=64 to nr_cpu_ids=1.
RCU: Adjusting geometry for rcu_fanout_leaf=64, nr_cpu_ids=1
NR_IRQS:64 nr_irqs:64 0
ACPI: APIC not present
ACPI: APIC not present
ACPI: APIC not present
ACPI: APIC not present
ACPI: APIC not present
Kernel panic - not syncing: No interrupt controller found.
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.9.6-00018-g13e39d5 #68
Hardware name: linux,dummy-virt (DT)
Call trace:
[<ffff000008088500>] dump_backtrace+0x0/0x1a0
[<ffff0000080886b4>] show_stack+0x14/0x20
[<ffff000008374e54>] dump_stack+0x94/0xb8
[<ffff0000081658fc>] panic+0x110/0x278
[<ffff000008c424a8>] init_IRQ+0x24/0x2c
[<ffff000008c409f0>] start_kernel+0x230/0x38c
[<ffff000008c401d8>] __primary_switched+0x5c/0x64
---[ end Kernel panic - not syncing: No interrupt controller found.
linux,dummy-virt (DT)


Alex

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-08 16:17     ` Laszlo Ersek
  2017-02-08 16:27       ` Ard Biesheuvel
  2017-02-09 12:24       ` [Qemu-devel] " Alexander Graf
@ 2017-02-09 12:29       ` Alexander Graf
  2017-02-09 12:31         ` Ard Biesheuvel
  2 siblings, 1 reply; 16+ messages in thread
From: Alexander Graf @ 2017-02-09 12:29 UTC (permalink / raw)
  To: Laszlo Ersek, qemu-devel; +Cc: qemu-arm, mst, Ard Biesheuvel, Shannon Zhao



On 08/02/2017 17:17, Laszlo Ersek wrote:
> On 02/08/17 17:12, Alexander Graf wrote:
>> On 02/08/2017 04:29 PM, Laszlo Ersek wrote:
>>> CC'ing Ard and Shannon (I recall this property from earlier):
>>>
>>> On 02/08/17 14:31, Alexander Graf wrote:
>>>> QEMU emulated hardware is always dma coherent with its guest. We do
>>>> annotate that correctly on the PCI host controller, but left out
>>>> virtio-mmio.
>>> I recommend to reference the following commit here:
>>>
>>> commit 5d636e21c44ecf982a22a7bc4ca89186079ac283
>>> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Date:   Mon Jul 4 13:06:36 2016 +0100
>>>
>>>      hw/arm/virt: mark the PCIe host controller as DMA coherent in the DT
>>>           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.
>>>           This brings the DT description into line with the ACPI
>>> description,
>>>      which already marks the PCI bridge as cache coherent (see commit
>>>      bc64b96c984abf).
>>>           Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>      Message-id:
>>> 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org
>>>      Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>>      Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>
>>>> Recent kernels have started to interpret that flag rather than take
>>>> dma coherency as granted with virtio-mmio. While that is considered
>>>> a kernel bug, as it breaks previously working systems, it showed that
>>>> our dt description is incomplete.
>>>>
>>>> This patch adds the respective marker that allows guest OSs to evaluate
>>>> that our virtio-mmio devices are indeed cache coherent.
>>> As noted above, commit bc64b96c984a ("hw/arm/virt-acpi-build: _CCA
>>> attribute is compulsory", 2015-11-03) had done the same in the ACPI
>>> description of the PCIe host controller.
>>>
>>> Thus, do we need _CCA in the ACPI description of the virtio-mmio
>>> transports, to parallel the DT change? See the LNRO0005 device in
>>> acpi_dsdt_add_virtio().
>>
>> Yes, we should also annotate it correctly in the DSDT. Today it's not a
>> deal breaker as Linux always assumes virtio-mmio to be dma coherent, but
>> it would make our platform description more accurate.
>>
>>> If that's the case, then I propose that either the patch please fix
>>> both DT and ACPI, or that at least we file a bug "somewhere", for
>>> adding _CCA in acpi_dsdt_add_virtio().
>>
>> I agree that it should happen in the same patch (set). While I don't
>> care a lot about ACPI right now (since dt is preferred on upstream
>> kernels), I can take a look.
>
> Thank you!

In fact, don't some other devices also suffer from this? Fw-cfg is now 
DMA capable IIUC, so that should also get a _CCA attribute for example.


Alex

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-09 12:24       ` [Qemu-devel] " Alexander Graf
@ 2017-02-09 12:30         ` Ard Biesheuvel
  2017-02-09 12:43           ` Alexander Graf
  2017-02-09 12:41         ` Andrew Jones
  1 sibling, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 12:30 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Laszlo Ersek, QEMU Developers, qemu-arm, Michael S. Tsirkin,
	Shannon Zhao

On 9 February 2017 at 12:24, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 08/02/2017 17:17, Laszlo Ersek wrote:
>>
>> On 02/08/17 17:12, Alexander Graf wrote:
>>>
>>> On 02/08/2017 04:29 PM, Laszlo Ersek wrote:
>>>>
>>>> CC'ing Ard and Shannon (I recall this property from earlier):
>>>>
>>>> On 02/08/17 14:31, Alexander Graf wrote:
>>>>>
>>>>> QEMU emulated hardware is always dma coherent with its guest. We do
>>>>> annotate that correctly on the PCI host controller, but left out
>>>>> virtio-mmio.
>>>>
>>>> I recommend to reference the following commit here:
>>>>
>>>> commit 5d636e21c44ecf982a22a7bc4ca89186079ac283
>>>> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> Date:   Mon Jul 4 13:06:36 2016 +0100
>>>>
>>>>      hw/arm/virt: mark the PCIe host controller as DMA coherent in the
>>>> DT
>>>>           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.
>>>>           This brings the DT description into line with the ACPI
>>>> description,
>>>>      which already marks the PCI bridge as cache coherent (see commit
>>>>      bc64b96c984abf).
>>>>           Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>      Message-id:
>>>> 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org
>>>>      Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>>>      Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>>
>>>>> Recent kernels have started to interpret that flag rather than take
>>>>> dma coherency as granted with virtio-mmio. While that is considered
>>>>> a kernel bug, as it breaks previously working systems, it showed that
>>>>> our dt description is incomplete.
>>>>>
>>>>> This patch adds the respective marker that allows guest OSs to evaluate
>>>>> that our virtio-mmio devices are indeed cache coherent.
>>>>
>>>> As noted above, commit bc64b96c984a ("hw/arm/virt-acpi-build: _CCA
>>>> attribute is compulsory", 2015-11-03) had done the same in the ACPI
>>>> description of the PCIe host controller.
>>>>
>>>> Thus, do we need _CCA in the ACPI description of the virtio-mmio
>>>> transports, to parallel the DT change? See the LNRO0005 device in
>>>> acpi_dsdt_add_virtio().
>>>
>>>
>>> Yes, we should also annotate it correctly in the DSDT. Today it's not a
>>> deal breaker as Linux always assumes virtio-mmio to be dma coherent, but
>>> it would make our platform description more accurate.
>>>
>>>> If that's the case, then I propose that either the patch please fix
>>>> both DT and ACPI, or that at least we file a bug "somewhere", for
>>>> adding _CCA in acpi_dsdt_add_virtio().
>>>
>>>
>>> I agree that it should happen in the same patch (set). While I don't
>>> care a lot about ACPI right now (since dt is preferred on upstream
>>> kernels), I can take a look.
>>
>>
>> Thank you!
>
>
> Is ACPI boot supposed to work at all with 4.9?
>

Yes, but only if you boot via UEFI

> Booting Linux on physical CPU 0x0
> Linux version 4.9.6-00018-g13e39d5 (agraf@achrid) (gcc version 6.1.1
> 20160711 (Linaro GCC 6.1-2016.08) ) #68 SMP PREEMPT Wed Feb 8 13:25:23 CET
> 2017
> Boot CPU: AArch64 Processor [500f0001]
> efi: Getting EFI parameters from FDT:
> efi: UEFI not found.
> cma: Reserved 16 MiB at 0x00000000bf000000
> ACPI: Early table checksum verification disabled
> fACPI: Failed to init ACPI tables
> On node 0 totalpages: 524288
>   DMA zone: 8192 pages used for memmap
>   DMA zone: 0 pages reserved
>   DMA zone: 524288 pages, LIFO batch:31
> psci: is not implemented in ACPI.
> ACPI: APIC not present
> fmissing boot CPU MPIDR, not enabling secondaries
> percpu: Embedded 21 pages/cpu @ffff80007efdd000 s47896 r8192 d29928 u86016
> pcpu-alloc: s47896 r8192 d29928 u86016 alloc=21*4096
> pcpu-alloc: [0] 0
> Detected PIPT I-cache on CPU0
> Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 516096
> Kernel command line: acpi=force console=ttyAMA0
> PID hash table entries: 4096 (order: 3, 32768 bytes)
> Dentry cache hash table entries: 262144 (order: 9, 2097152 bytes)
> Inode-cache hash table entries: 131072 (order: 8, 1048576 bytes)
> Memory: 2030472K/2097152K available (8380K kernel code, 858K rwdata, 3632K
> rodata, 1024K init, 280K bss, 50296K reserved, 16384K cma-reserved)
> Virtual kernel memory layout:
>     modules : 0xffff000000000000 - 0xffff000008000000   (   128 MB)
>     vmalloc : 0xffff000008000000 - 0xffff7dffbfff0000   (129022 GB)
>       .text : 0xffff000008080000 - 0xffff0000088b0000   (  8384 KB)
>     .rodata : 0xffff0000088b0000 - 0xffff000008c40000   (  3648 KB)
>       .init : 0xffff000008c40000 - 0xffff000008d40000   (  1024 KB)
>       .data : 0xffff000008d40000 - 0xffff000008e16a00   (   859 KB)
>        .bss : 0xffff000008e16a00 - 0xffff000008e5cc3c   (   281 KB)
>     fixed   : 0xffff7dfffe7fd000 - 0xffff7dfffec00000   (  4108 KB)
>     PCI I/O : 0xffff7dfffee00000 - 0xffff7dffffe00000   (    16 MB)
>     vmemmap : 0xffff7e0000000000 - 0xffff800000000000   (  2048 GB maximum)
>               0xffff7e0000000000 - 0xffff7e0002000000   (    32 MB actual)
>     memory  : 0xffff800000000000 - 0xffff800080000000   (  2048 MB)
> SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
> Preemptible hierarchical RCU implementation.
>         Build-time adjustment of leaf fanout to 64.
>         RCU restricting CPUs from NR_CPUS=64 to nr_cpu_ids=1.
> RCU: Adjusting geometry for rcu_fanout_leaf=64, nr_cpu_ids=1
> NR_IRQS:64 nr_irqs:64 0
> ACPI: APIC not present
> ACPI: APIC not present
> ACPI: APIC not present
> ACPI: APIC not present
> ACPI: APIC not present
> Kernel panic - not syncing: No interrupt controller found.
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.9.6-00018-g13e39d5 #68
> Hardware name: linux,dummy-virt (DT)
> Call trace:
> [<ffff000008088500>] dump_backtrace+0x0/0x1a0
> [<ffff0000080886b4>] show_stack+0x14/0x20
> [<ffff000008374e54>] dump_stack+0x94/0xb8
> [<ffff0000081658fc>] panic+0x110/0x278
> [<ffff000008c424a8>] init_IRQ+0x24/0x2c
> [<ffff000008c409f0>] start_kernel+0x230/0x38c
> [<ffff000008c401d8>] __primary_switched+0x5c/0x64
> ---[ end Kernel panic - not syncing: No interrupt controller found.
> linux,dummy-virt (DT)
>
>
> Alex

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-09 12:29       ` Alexander Graf
@ 2017-02-09 12:31         ` Ard Biesheuvel
  0 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 12:31 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Laszlo Ersek, QEMU Developers, qemu-arm, Michael S. Tsirkin,
	Shannon Zhao

On 9 February 2017 at 12:29, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 08/02/2017 17:17, Laszlo Ersek wrote:
>>
>> On 02/08/17 17:12, Alexander Graf wrote:
>>>
>>> On 02/08/2017 04:29 PM, Laszlo Ersek wrote:
>>>>
>>>> CC'ing Ard and Shannon (I recall this property from earlier):
>>>>
>>>> On 02/08/17 14:31, Alexander Graf wrote:
>>>>>
>>>>> QEMU emulated hardware is always dma coherent with its guest. We do
>>>>> annotate that correctly on the PCI host controller, but left out
>>>>> virtio-mmio.
>>>>
>>>> I recommend to reference the following commit here:
>>>>
>>>> commit 5d636e21c44ecf982a22a7bc4ca89186079ac283
>>>> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> Date:   Mon Jul 4 13:06:36 2016 +0100
>>>>
>>>>      hw/arm/virt: mark the PCIe host controller as DMA coherent in the
>>>> DT
>>>>           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.
>>>>           This brings the DT description into line with the ACPI
>>>> description,
>>>>      which already marks the PCI bridge as cache coherent (see commit
>>>>      bc64b96c984abf).
>>>>           Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>      Message-id:
>>>> 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org
>>>>      Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>>>      Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>>
>>>>> Recent kernels have started to interpret that flag rather than take
>>>>> dma coherency as granted with virtio-mmio. While that is considered
>>>>> a kernel bug, as it breaks previously working systems, it showed that
>>>>> our dt description is incomplete.
>>>>>
>>>>> This patch adds the respective marker that allows guest OSs to evaluate
>>>>> that our virtio-mmio devices are indeed cache coherent.
>>>>
>>>> As noted above, commit bc64b96c984a ("hw/arm/virt-acpi-build: _CCA
>>>> attribute is compulsory", 2015-11-03) had done the same in the ACPI
>>>> description of the PCIe host controller.
>>>>
>>>> Thus, do we need _CCA in the ACPI description of the virtio-mmio
>>>> transports, to parallel the DT change? See the LNRO0005 device in
>>>> acpi_dsdt_add_virtio().
>>>
>>>
>>> Yes, we should also annotate it correctly in the DSDT. Today it's not a
>>> deal breaker as Linux always assumes virtio-mmio to be dma coherent, but
>>> it would make our platform description more accurate.
>>>
>>>> If that's the case, then I propose that either the patch please fix
>>>> both DT and ACPI, or that at least we file a bug "somewhere", for
>>>> adding _CCA in acpi_dsdt_add_virtio().
>>>
>>>
>>> I agree that it should happen in the same patch (set). While I don't
>>> care a lot about ACPI right now (since dt is preferred on upstream
>>> kernels), I can take a look.
>>
>>
>> Thank you!
>
>
> In fact, don't some other devices also suffer from this? Fw-cfg is now DMA
> capable IIUC, so that should also get a _CCA attribute for example.
>

Correct. Any described DMA capable device should have a _CCA attribute

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-09 12:24       ` [Qemu-devel] " Alexander Graf
  2017-02-09 12:30         ` Ard Biesheuvel
@ 2017-02-09 12:41         ` Andrew Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Andrew Jones @ 2017-02-09 12:41 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Laszlo Ersek, qemu-devel, Ard Biesheuvel, qemu-arm, Shannon Zhao, mst

On Thu, Feb 09, 2017 at 01:24:02PM +0100, Alexander Graf wrote:
> 
> 
> On 08/02/2017 17:17, Laszlo Ersek wrote:
> > On 02/08/17 17:12, Alexander Graf wrote:
> > > On 02/08/2017 04:29 PM, Laszlo Ersek wrote:
> > > > CC'ing Ard and Shannon (I recall this property from earlier):
> > > > 
> > > > On 02/08/17 14:31, Alexander Graf wrote:
> > > > > QEMU emulated hardware is always dma coherent with its guest. We do
> > > > > annotate that correctly on the PCI host controller, but left out
> > > > > virtio-mmio.
> > > > I recommend to reference the following commit here:
> > > > 
> > > > commit 5d636e21c44ecf982a22a7bc4ca89186079ac283
> > > > Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > Date:   Mon Jul 4 13:06:36 2016 +0100
> > > > 
> > > >      hw/arm/virt: mark the PCIe host controller as DMA coherent in the DT
> > > >           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.
> > > >           This brings the DT description into line with the ACPI
> > > > description,
> > > >      which already marks the PCI bridge as cache coherent (see commit
> > > >      bc64b96c984abf).
> > > >           Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > >      Message-id:
> > > > 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org
> > > >      Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > > >      Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > > > 
> > > > > Recent kernels have started to interpret that flag rather than take
> > > > > dma coherency as granted with virtio-mmio. While that is considered
> > > > > a kernel bug, as it breaks previously working systems, it showed that
> > > > > our dt description is incomplete.
> > > > > 
> > > > > This patch adds the respective marker that allows guest OSs to evaluate
> > > > > that our virtio-mmio devices are indeed cache coherent.
> > > > As noted above, commit bc64b96c984a ("hw/arm/virt-acpi-build: _CCA
> > > > attribute is compulsory", 2015-11-03) had done the same in the ACPI
> > > > description of the PCIe host controller.
> > > > 
> > > > Thus, do we need _CCA in the ACPI description of the virtio-mmio
> > > > transports, to parallel the DT change? See the LNRO0005 device in
> > > > acpi_dsdt_add_virtio().
> > > 
> > > Yes, we should also annotate it correctly in the DSDT. Today it's not a
> > > deal breaker as Linux always assumes virtio-mmio to be dma coherent, but
> > > it would make our platform description more accurate.
> > > 
> > > > If that's the case, then I propose that either the patch please fix
> > > > both DT and ACPI, or that at least we file a bug "somewhere", for
> > > > adding _CCA in acpi_dsdt_add_virtio().
> > > 
> > > I agree that it should happen in the same patch (set). While I don't
> > > care a lot about ACPI right now (since dt is preferred on upstream
> > > kernels), I can take a look.
> > 
> > Thank you!
> 
> Is ACPI boot supposed to work at all with 4.9?

It does work. Although you may need to specific console=ttyAMA0.
I think there was/is an issue with getting the default console
out of the SPCR.

> 
> Booting Linux on physical CPU 0x0
> Linux version 4.9.6-00018-g13e39d5 (agraf@achrid) (gcc version 6.1.1
> 20160711 (Linaro GCC 6.1-2016.08) ) #68 SMP PREEMPT Wed Feb 8 13:25:23 CET
> 2017
> Boot CPU: AArch64 Processor [500f0001]
> efi: Getting EFI parameters from FDT:
> efi: UEFI not found.

Hmm, I'm not sure ACPI boot should work without AAVMF.

Thanks,
drew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-09 12:30         ` Ard Biesheuvel
@ 2017-02-09 12:43           ` Alexander Graf
  0 siblings, 0 replies; 16+ messages in thread
From: Alexander Graf @ 2017-02-09 12:43 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Laszlo Ersek, QEMU Developers, qemu-arm, Michael S. Tsirkin,
	Shannon Zhao



On 09/02/2017 13:30, Ard Biesheuvel wrote:
> On 9 February 2017 at 12:24, Alexander Graf <agraf@suse.de> wrote:
>>
>>
>> On 08/02/2017 17:17, Laszlo Ersek wrote:
>>>
>>> On 02/08/17 17:12, Alexander Graf wrote:
>>>>
>>>> On 02/08/2017 04:29 PM, Laszlo Ersek wrote:
>>>>>
>>>>> CC'ing Ard and Shannon (I recall this property from earlier):
>>>>>
>>>>> On 02/08/17 14:31, Alexander Graf wrote:
>>>>>>
>>>>>> QEMU emulated hardware is always dma coherent with its guest. We do
>>>>>> annotate that correctly on the PCI host controller, but left out
>>>>>> virtio-mmio.
>>>>>
>>>>> I recommend to reference the following commit here:
>>>>>
>>>>> commit 5d636e21c44ecf982a22a7bc4ca89186079ac283
>>>>> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>> Date:   Mon Jul 4 13:06:36 2016 +0100
>>>>>
>>>>>      hw/arm/virt: mark the PCIe host controller as DMA coherent in the
>>>>> DT
>>>>>           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.
>>>>>           This brings the DT description into line with the ACPI
>>>>> description,
>>>>>      which already marks the PCI bridge as cache coherent (see commit
>>>>>      bc64b96c984abf).
>>>>>           Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>      Message-id:
>>>>> 1467134090-5099-1-git-send-email-ard.biesheuvel@linaro.org
>>>>>      Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>>>>      Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>>>
>>>>>> Recent kernels have started to interpret that flag rather than take
>>>>>> dma coherency as granted with virtio-mmio. While that is considered
>>>>>> a kernel bug, as it breaks previously working systems, it showed that
>>>>>> our dt description is incomplete.
>>>>>>
>>>>>> This patch adds the respective marker that allows guest OSs to evaluate
>>>>>> that our virtio-mmio devices are indeed cache coherent.
>>>>>
>>>>> As noted above, commit bc64b96c984a ("hw/arm/virt-acpi-build: _CCA
>>>>> attribute is compulsory", 2015-11-03) had done the same in the ACPI
>>>>> description of the PCIe host controller.
>>>>>
>>>>> Thus, do we need _CCA in the ACPI description of the virtio-mmio
>>>>> transports, to parallel the DT change? See the LNRO0005 device in
>>>>> acpi_dsdt_add_virtio().
>>>>
>>>>
>>>> Yes, we should also annotate it correctly in the DSDT. Today it's not a
>>>> deal breaker as Linux always assumes virtio-mmio to be dma coherent, but
>>>> it would make our platform description more accurate.
>>>>
>>>>> If that's the case, then I propose that either the patch please fix
>>>>> both DT and ACPI, or that at least we file a bug "somewhere", for
>>>>> adding _CCA in acpi_dsdt_add_virtio().
>>>>
>>>>
>>>> I agree that it should happen in the same patch (set). While I don't
>>>> care a lot about ACPI right now (since dt is preferred on upstream
>>>> kernels), I can take a look.
>>>
>>>
>>> Thank you!
>>
>>
>> Is ACPI boot supposed to work at all with 4.9?
>>
>
> Yes, but only if you boot via UEFI

I see, thanks. I guess there's no boot protocol to fetch ACPI tables 
without UEFI.

Interestingly enough, omitting the _CCA property didn't actually make 
too much of a difference. The guest still seems to boot fine without it, 
so it seems to treat virtio-mmio as cache coherent. Only if I explicitly 
set _CCA as 0 it's broken the same as dt.

If that's the case, it's scary if we have different defaults for dt and 
acpi descriptions, no?

Also, we should probably explicitly warn in the kernel every time we see 
an ACPI device try to do DMA operations without _CCA set in its descriptor.


Alex

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt
  2017-02-09 12:04             ` Heyi Guo
@ 2017-02-10  1:40               ` Alex Hung
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Hung @ 2017-02-10  1:40 UTC (permalink / raw)
  To: Heyi Guo
  Cc: Laszlo Ersek, Peter Maydell, Ard Biesheuvel, Shannon Zhao,
	qemu-arm, Michael S. Tsirkin, Alexander Graf, QEMU Developers

Hi,

FWTS supports multiple architectures:

- amd64
- arm64
- armhf
- i386
- ppc64el
- s390x

Package details are available @
https://launchpad.net/~firmware-testing-team/+archive/ubuntu/ppa-fwts-stable/+packages

FWTS also supports device tree to some extent, thanks to contributions
from Jeremy Kerr & Deb McLemore.

Cheers,
Alex Hung

On Thu, Feb 9, 2017 at 8:04 PM, Heyi Guo <heyi.guo@linaro.org> wrote:
> Hi all,
>
> Just to confirm that FWTS is supporting aarch64.
>
> Alex is the maintainer of FWTS and may provide more information.
>
> Regards,
>
> Gary (Heyi Guo)
>
>
> 在 2/9/2017 2:47 AM, Laszlo Ersek 写道:
>>
>> On 02/08/17 19:23, Peter Maydell wrote:
>>>
>>> On 8 February 2017 at 16:27, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> wrote:
>>>>
>>>> The _CCA property is mandatory on arm64, and there is no default.
>>>
>>> Is there a tool that can check this kind of requirement
>>> and complain about issues in the ACPI tables (and
>>> ditto, device tree)? It's really easy to produce a
>>> dt or ACPI table that works with current kernels and
>>> then turns out to have a problem six or twelve
>>> months down the line :-(
>>
>> I think the "bios bits" project or the FWTS (firmware test suite)
>> project might do some ACPI sanity checks. They could be run in guests.
>>
>> https://biosbits.org/
>> https://wiki.ubuntu.com/FirmwareTestSuite
>>
>> However, I'm unsure if they support aarch64. Also... it's not like those
>> projects are stationary. Even the ACPI spec is a moving target (and
>> spec-level regressions exist).
>>
>> I believe Heyi Guo @ Linaro used to work with FWTS on aarch64. CC'd.
>>
>> No clue about DT conformance testing. Is there an industry standard or a
>> working group behind DT?
>>
>> Thanks
>> Laszlo
>>
>



-- 
Cheers,
Alex Hung

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-02-10  1:40 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-08 13:31 [Qemu-devel] [PATCH] target-arm: Declare virtio-mmio as dma-coherent in dt Alexander Graf
2017-02-08 15:29 ` Laszlo Ersek
2017-02-08 16:12   ` Alexander Graf
2017-02-08 16:17     ` Laszlo Ersek
2017-02-08 16:27       ` Ard Biesheuvel
2017-02-08 18:23         ` [Qemu-devel] [Qemu-arm] " Peter Maydell
2017-02-08 18:47           ` Laszlo Ersek
2017-02-09 12:04             ` Heyi Guo
2017-02-10  1:40               ` Alex Hung
2017-02-09 10:35           ` Ard Biesheuvel
2017-02-09 12:24       ` [Qemu-devel] " Alexander Graf
2017-02-09 12:30         ` Ard Biesheuvel
2017-02-09 12:43           ` Alexander Graf
2017-02-09 12:41         ` Andrew Jones
2017-02-09 12:29       ` Alexander Graf
2017-02-09 12:31         ` Ard Biesheuvel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.