[Qemu-devel] [PATCH v2 0/4] target-arm: Add some omitted dma cache coherency flags

Alexander Graf posted 4 patches 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1486644810-33181-1-git-send-email-agraf@suse.de
Test checkpatch passed
Test docker passed
Test s390x passed
hw/arm/vexpress.c        | 1 +
hw/arm/virt-acpi-build.c | 2 ++
hw/arm/virt.c            | 2 ++
3 files changed, 5 insertions(+)
[Qemu-devel] [PATCH v2 0/4] target-arm: Add some omitted dma cache coherency flags
Posted by Alexander Graf 7 years, 1 month ago
ARM is amazing when it comes to cache coherency and VMs. While any sane
architecture allows the host to override the guest's caching attributes,
that's very hard to do on ARM.

That means that the guest may directly access guest memory bypassing the
cache while QEMU happily writes to / reads from cache. The end result is
very nasty, because both sides see very different views of the world.
 
That means that we need to be very cautious to tell guests that devices
that QEMU emulates are going to use data in the cache rather than directly
on memory.

We added this to PCI a while back for DT (5d636e21 "hw/arm/virt: mark the PCIe
host controller as DMA coherent in the DT") and ACPI (bc64b96 "hw/arm/virt-
acpi-build: _CCA attribute is compulsory") but never updated virtio-mmio or
fw-cfg in DT or ACPI tables.

This patch set adds the respective cache coherency flags for them in both DT and
ACPI.

Fortunately, no guests except for Linux 4.9.7 and 4.9.8 are broken because of
this. Upstream realized quickly enough that every user of virtio-mmio out there
describes its cache coherency incorrectly and reverted the patch that would
require said dma coherency flag. But we should be safe for the future and "do
the right thing".

Alexander Graf (4):
  target-arm: Declare virtio-mmio as dma-coherent in dt
  hw/arm/virt: Declare virtio-mmio as dma cache coherent in ACPI
  hw/arm/virt: Declare fwcfg as dma cache coherent in ACPI
  hw/arm/virt: Declare fwcfg as dma cache coherent in dt

 hw/arm/vexpress.c        | 1 +
 hw/arm/virt-acpi-build.c | 2 ++
 hw/arm/virt.c            | 2 ++
 3 files changed, 5 insertions(+)

-- 
2.10.0


Re: [Qemu-devel] [PATCH v2 0/4] target-arm: Add some omitted dma cache coherency flags
Posted by Laszlo Ersek 7 years, 1 month ago
On 02/09/17 13:53, Alexander Graf wrote:
> ARM is amazing when it comes to cache coherency and VMs. While any sane
> architecture allows the host to override the guest's caching attributes,
> that's very hard to do on ARM.
> 
> That means that the guest may directly access guest memory bypassing the
> cache while QEMU happily writes to / reads from cache. The end result is
> very nasty, because both sides see very different views of the world.
>  
> That means that we need to be very cautious to tell guests that devices
> that QEMU emulates are going to use data in the cache rather than directly
> on memory.
> 
> We added this to PCI a while back for DT (5d636e21 "hw/arm/virt: mark the PCIe
> host controller as DMA coherent in the DT") and ACPI (bc64b96 "hw/arm/virt-
> acpi-build: _CCA attribute is compulsory") but never updated virtio-mmio or
> fw-cfg in DT or ACPI tables.
> 
> This patch set adds the respective cache coherency flags for them in both DT and
> ACPI.
> 
> Fortunately, no guests except for Linux 4.9.7 and 4.9.8 are broken because of
> this. Upstream realized quickly enough that every user of virtio-mmio out there
> describes its cache coherency incorrectly and reverted the patch that would
> require said dma coherency flag. But we should be safe for the future and "do
> the right thing".
> 
> Alexander Graf (4):
>   target-arm: Declare virtio-mmio as dma-coherent in dt
>   hw/arm/virt: Declare virtio-mmio as dma cache coherent in ACPI
>   hw/arm/virt: Declare fwcfg as dma cache coherent in ACPI
>   hw/arm/virt: Declare fwcfg as dma cache coherent in dt
> 
>  hw/arm/vexpress.c        | 1 +
>  hw/arm/virt-acpi-build.c | 2 ++
>  hw/arm/virt.c            | 2 ++
>  3 files changed, 5 insertions(+)
> 

Famous last words:
series
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Should we replicate patch #3 to QEMU0002 / FWCF in
"hw/i386/acpi-build.c" too? Or is it that we couldn't care less about
_CCA on x86? :) (Can't really muster the energy right now to look it up
in the ACPI spec, sorry!)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2 0/4] target-arm: Add some omitted dma cache coherency flags
Posted by Michael S. Tsirkin 7 years, 1 month ago
On Thu, Feb 09, 2017 at 02:15:36PM +0100, Laszlo Ersek wrote:
> On 02/09/17 13:53, Alexander Graf wrote:
> > ARM is amazing when it comes to cache coherency and VMs. While any sane
> > architecture allows the host to override the guest's caching attributes,
> > that's very hard to do on ARM.
> > 
> > That means that the guest may directly access guest memory bypassing the
> > cache while QEMU happily writes to / reads from cache. The end result is
> > very nasty, because both sides see very different views of the world.
> >  
> > That means that we need to be very cautious to tell guests that devices
> > that QEMU emulates are going to use data in the cache rather than directly
> > on memory.
> > 
> > We added this to PCI a while back for DT (5d636e21 "hw/arm/virt: mark the PCIe
> > host controller as DMA coherent in the DT") and ACPI (bc64b96 "hw/arm/virt-
> > acpi-build: _CCA attribute is compulsory") but never updated virtio-mmio or
> > fw-cfg in DT or ACPI tables.
> > 
> > This patch set adds the respective cache coherency flags for them in both DT and
> > ACPI.
> > 
> > Fortunately, no guests except for Linux 4.9.7 and 4.9.8 are broken because of
> > this. Upstream realized quickly enough that every user of virtio-mmio out there
> > describes its cache coherency incorrectly and reverted the patch that would
> > require said dma coherency flag. But we should be safe for the future and "do
> > the right thing".
> > 
> > Alexander Graf (4):
> >   target-arm: Declare virtio-mmio as dma-coherent in dt
> >   hw/arm/virt: Declare virtio-mmio as dma cache coherent in ACPI
> >   hw/arm/virt: Declare fwcfg as dma cache coherent in ACPI
> >   hw/arm/virt: Declare fwcfg as dma cache coherent in dt
> > 
> >  hw/arm/vexpress.c        | 1 +
> >  hw/arm/virt-acpi-build.c | 2 ++
> >  hw/arm/virt.c            | 2 ++
> >  3 files changed, 5 insertions(+)
> > 
> 
> Famous last words:
> series
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> Should we replicate patch #3 to QEMU0002 / FWCF in
> "hw/i386/acpi-build.c" too? Or is it that we couldn't care less about
> _CCA on x86? :) (Can't really muster the energy right now to look it up
> in the ACPI spec, sorry!)
> 
> Thanks
> Laszlo

ACPI spec says:
On platforms for which existing default cache-coherency behavior of the OS is not adequate, _CCA
enables the OS to adapt to the differences

So I think we don't need it on x86.

-- 
MST

Re: [Qemu-devel] [PATCH v2 0/4] target-arm: Add some omitted dma cache coherency flags
Posted by Alexander Graf 7 years, 1 month ago

> Am 09.02.2017 um 19:13 schrieb Michael S. Tsirkin <mst@redhat.com>:
> 
>> On Thu, Feb 09, 2017 at 02:15:36PM +0100, Laszlo Ersek wrote:
>>> On 02/09/17 13:53, Alexander Graf wrote:
>>> ARM is amazing when it comes to cache coherency and VMs. While any sane
>>> architecture allows the host to override the guest's caching attributes,
>>> that's very hard to do on ARM.
>>> 
>>> That means that the guest may directly access guest memory bypassing the
>>> cache while QEMU happily writes to / reads from cache. The end result is
>>> very nasty, because both sides see very different views of the world.
>>> 
>>> That means that we need to be very cautious to tell guests that devices
>>> that QEMU emulates are going to use data in the cache rather than directly
>>> on memory.
>>> 
>>> We added this to PCI a while back for DT (5d636e21 "hw/arm/virt: mark the PCIe
>>> host controller as DMA coherent in the DT") and ACPI (bc64b96 "hw/arm/virt-
>>> acpi-build: _CCA attribute is compulsory") but never updated virtio-mmio or
>>> fw-cfg in DT or ACPI tables.
>>> 
>>> This patch set adds the respective cache coherency flags for them in both DT and
>>> ACPI.
>>> 
>>> Fortunately, no guests except for Linux 4.9.7 and 4.9.8 are broken because of
>>> this. Upstream realized quickly enough that every user of virtio-mmio out there
>>> describes its cache coherency incorrectly and reverted the patch that would
>>> require said dma coherency flag. But we should be safe for the future and "do
>>> the right thing".
>>> 
>>> Alexander Graf (4):
>>>  target-arm: Declare virtio-mmio as dma-coherent in dt
>>>  hw/arm/virt: Declare virtio-mmio as dma cache coherent in ACPI
>>>  hw/arm/virt: Declare fwcfg as dma cache coherent in ACPI
>>>  hw/arm/virt: Declare fwcfg as dma cache coherent in dt
>>> 
>>> hw/arm/vexpress.c        | 1 +
>>> hw/arm/virt-acpi-build.c | 2 ++
>>> hw/arm/virt.c            | 2 ++
>>> 3 files changed, 5 insertions(+)
>>> 
>> 
>> Famous last words:
>> series
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>> 
>> Should we replicate patch #3 to QEMU0002 / FWCF in
>> "hw/i386/acpi-build.c" too? Or is it that we couldn't care less about
>> _CCA on x86? :) (Can't really muster the energy right now to look it up
>> in the ACPI spec, sorry!)
>> 
>> Thanks
>> Laszlo
> 
> ACPI spec says:
> On platforms for which existing default cache-coherency behavior of the OS is not adequate, _CCA
> enables the OS to adapt to the differences
> 
> So I think we don't need it on x86.

According to acpi 6.1, x86 explicitly defaults to dma coherent if _CCA is omitted. It's only illegal for ARM.

Alex



Re: [Qemu-devel] [PATCH v2 0/4] target-arm: Add some omitted dma cache coherency flags
Posted by Laszlo Ersek 7 years, 1 month ago
On 02/09/17 19:27, Alexander Graf wrote:
> 
> 
>> Am 09.02.2017 um 19:13 schrieb Michael S. Tsirkin <mst@redhat.com>:
>>
>>> On Thu, Feb 09, 2017 at 02:15:36PM +0100, Laszlo Ersek wrote:
>>>> On 02/09/17 13:53, Alexander Graf wrote:
>>>> ARM is amazing when it comes to cache coherency and VMs. While any sane
>>>> architecture allows the host to override the guest's caching attributes,
>>>> that's very hard to do on ARM.
>>>>
>>>> That means that the guest may directly access guest memory bypassing the
>>>> cache while QEMU happily writes to / reads from cache. The end result is
>>>> very nasty, because both sides see very different views of the world.
>>>>
>>>> That means that we need to be very cautious to tell guests that devices
>>>> that QEMU emulates are going to use data in the cache rather than directly
>>>> on memory.
>>>>
>>>> We added this to PCI a while back for DT (5d636e21 "hw/arm/virt: mark the PCIe
>>>> host controller as DMA coherent in the DT") and ACPI (bc64b96 "hw/arm/virt-
>>>> acpi-build: _CCA attribute is compulsory") but never updated virtio-mmio or
>>>> fw-cfg in DT or ACPI tables.
>>>>
>>>> This patch set adds the respective cache coherency flags for them in both DT and
>>>> ACPI.
>>>>
>>>> Fortunately, no guests except for Linux 4.9.7 and 4.9.8 are broken because of
>>>> this. Upstream realized quickly enough that every user of virtio-mmio out there
>>>> describes its cache coherency incorrectly and reverted the patch that would
>>>> require said dma coherency flag. But we should be safe for the future and "do
>>>> the right thing".
>>>>
>>>> Alexander Graf (4):
>>>>  target-arm: Declare virtio-mmio as dma-coherent in dt
>>>>  hw/arm/virt: Declare virtio-mmio as dma cache coherent in ACPI
>>>>  hw/arm/virt: Declare fwcfg as dma cache coherent in ACPI
>>>>  hw/arm/virt: Declare fwcfg as dma cache coherent in dt
>>>>
>>>> hw/arm/vexpress.c        | 1 +
>>>> hw/arm/virt-acpi-build.c | 2 ++
>>>> hw/arm/virt.c            | 2 ++
>>>> 3 files changed, 5 insertions(+)
>>>>
>>>
>>> Famous last words:
>>> series
>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>>
>>> Should we replicate patch #3 to QEMU0002 / FWCF in
>>> "hw/i386/acpi-build.c" too? Or is it that we couldn't care less about
>>> _CCA on x86? :) (Can't really muster the energy right now to look it up
>>> in the ACPI spec, sorry!)
>>>
>>> Thanks
>>> Laszlo
>>
>> ACPI spec says:
>> On platforms for which existing default cache-coherency behavior of the OS is not adequate, _CCA
>> enables the OS to adapt to the differences
>>
>> So I think we don't need it on x86.
> 
> According to acpi 6.1, x86 explicitly defaults to dma coherent if _CCA is omitted. It's only illegal for ARM.

Incredible; a finding that, for a change, does not create more work.

Thanks.
Laszlo


Re: [Qemu-devel] [PATCH v2 0/4] target-arm: Add some omitted dma cache coherency flags
Posted by Ard Biesheuvel 7 years, 1 month ago
On 9 February 2017 at 12:53, Alexander Graf <agraf@suse.de> wrote:
> ARM is amazing when it comes to cache coherency and VMs. While any sane
> architecture allows the host to override the guest's caching attributes,
> that's very hard to do on ARM.
>
> That means that the guest may directly access guest memory bypassing the
> cache while QEMU happily writes to / reads from cache. The end result is
> very nasty, because both sides see very different views of the world.
>
> That means that we need to be very cautious to tell guests that devices
> that QEMU emulates are going to use data in the cache rather than directly
> on memory.
>
> We added this to PCI a while back for DT (5d636e21 "hw/arm/virt: mark the PCIe
> host controller as DMA coherent in the DT") and ACPI (bc64b96 "hw/arm/virt-
> acpi-build: _CCA attribute is compulsory") but never updated virtio-mmio or
> fw-cfg in DT or ACPI tables.
>
> This patch set adds the respective cache coherency flags for them in both DT and
> ACPI.
>
> Fortunately, no guests except for Linux 4.9.7 and 4.9.8 are broken because of
> this. Upstream realized quickly enough that every user of virtio-mmio out there
> describes its cache coherency incorrectly and reverted the patch that would
> require said dma coherency flag. But we should be safe for the future and "do
> the right thing".
>
> Alexander Graf (4):
>   target-arm: Declare virtio-mmio as dma-coherent in dt
>   hw/arm/virt: Declare virtio-mmio as dma cache coherent in ACPI
>   hw/arm/virt: Declare fwcfg as dma cache coherent in ACPI
>   hw/arm/virt: Declare fwcfg as dma cache coherent in dt
>

This looks correct to me.

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 0/4] target-arm: Add some omitted dma cache coherency flags
Posted by Peter Maydell 7 years, 1 month ago
On 9 February 2017 at 12:53, Alexander Graf <agraf@suse.de> wrote:
> ARM is amazing when it comes to cache coherency and VMs. While any sane
> architecture allows the host to override the guest's caching attributes,
> that's very hard to do on ARM.
>
> That means that the guest may directly access guest memory bypassing the
> cache while QEMU happily writes to / reads from cache. The end result is
> very nasty, because both sides see very different views of the world.
>
> That means that we need to be very cautious to tell guests that devices
> that QEMU emulates are going to use data in the cache rather than directly
> on memory.
>
> We added this to PCI a while back for DT (5d636e21 "hw/arm/virt: mark the PCIe
> host controller as DMA coherent in the DT") and ACPI (bc64b96 "hw/arm/virt-
> acpi-build: _CCA attribute is compulsory") but never updated virtio-mmio or
> fw-cfg in DT or ACPI tables.
>
> This patch set adds the respective cache coherency flags for them in both DT and
> ACPI.
>
> Fortunately, no guests except for Linux 4.9.7 and 4.9.8 are broken because of
> this. Upstream realized quickly enough that every user of virtio-mmio out there
> describes its cache coherency incorrectly and reverted the patch that would
> require said dma coherency flag. But we should be safe for the future and "do
> the right thing".
>
> Alexander Graf (4):
>   target-arm: Declare virtio-mmio as dma-coherent in dt
>   hw/arm/virt: Declare virtio-mmio as dma cache coherent in ACPI
>   hw/arm/virt: Declare fwcfg as dma cache coherent in ACPI
>   hw/arm/virt: Declare fwcfg as dma cache coherent in dt
>
>  hw/arm/vexpress.c        | 1 +
>  hw/arm/virt-acpi-build.c | 2 ++
>  hw/arm/virt.c            | 2 ++
>  3 files changed, 5 insertions(+)

The patches in this series have more lines of Reviewed-by: tags than
they do actual code changes :-)

Thanks, applied to target-arm.next.

-- PMM