[RFC 0/4] Add a 'in_mmio' device flag to avoid the DMA to MMIO

Li Qiang posted 4 patches 3 years, 7 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200908164157.47108-1-liq3ea@163.com
Maintainers: Dmitry Fleytman <dmitry.fleytman@gmail.com>, Eduardo Habkost <ehabkost@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Jason Wang <jasowang@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
hw/net/e1000e.c        |  8 ++++----
hw/usb/hcd-xhci.c      | 25 ++++++++++++++---------
include/exec/memory.h  |  9 +++++++++
include/hw/qdev-core.h |  1 +
softmmu/memory.c       | 46 +++++++++++++++++++++++++++++++++++++++---
5 files changed, 72 insertions(+), 17 deletions(-)
[RFC 0/4] Add a 'in_mmio' device flag to avoid the DMA to MMIO
Posted by Li Qiang 3 years, 7 months ago
Currently the qemu device fuzzer find some DMA to MMIO issue. If the
device handling MMIO currently trigger a DMA which the address is MMIO,
this will reenter the device MMIO handler. As some of the device doesn't
consider this it will sometimes crash the qemu.

This patch tries to solve this by adding a per-device flag 'in_mmio'.
When the memory core dispatch MMIO it will check/set this flag and when
it leaves it will clean this flag.


Li Qiang (4):
  memory: add memory_region_init_io_with_dev interface
  memory: avoid reenter the device's MMIO handler while processing MMIO
  e1000e: use the new memory_region_init_io_with_dev interface
  hcd-xhci: use the new memory_region_init_io_with_dev interface

 hw/net/e1000e.c        |  8 ++++----
 hw/usb/hcd-xhci.c      | 25 ++++++++++++++---------
 include/exec/memory.h  |  9 +++++++++
 include/hw/qdev-core.h |  1 +
 softmmu/memory.c       | 46 +++++++++++++++++++++++++++++++++++++++---
 5 files changed, 72 insertions(+), 17 deletions(-)

-- 
2.17.1


Re: [RFC 0/4] Add a 'in_mmio' device flag to avoid the DMA to MMIO
Posted by Jason Wang 3 years, 7 months ago
On 2020/9/9 上午12:41, Li Qiang wrote:
> Currently the qemu device fuzzer find some DMA to MMIO issue. If the
> device handling MMIO currently trigger a DMA which the address is MMIO,
> this will reenter the device MMIO handler. As some of the device doesn't
> consider this it will sometimes crash the qemu.
>
> This patch tries to solve this by adding a per-device flag 'in_mmio'.
> When the memory core dispatch MMIO it will check/set this flag and when
> it leaves it will clean this flag.


What's the plan for fixing the irq issues pointed out by Peter?

Thanks


>
>
> Li Qiang (4):
>    memory: add memory_region_init_io_with_dev interface
>    memory: avoid reenter the device's MMIO handler while processing MMIO
>    e1000e: use the new memory_region_init_io_with_dev interface
>    hcd-xhci: use the new memory_region_init_io_with_dev interface
>
>   hw/net/e1000e.c        |  8 ++++----
>   hw/usb/hcd-xhci.c      | 25 ++++++++++++++---------
>   include/exec/memory.h  |  9 +++++++++
>   include/hw/qdev-core.h |  1 +
>   softmmu/memory.c       | 46 +++++++++++++++++++++++++++++++++++++++---
>   5 files changed, 72 insertions(+), 17 deletions(-)
>


Re: [RFC 0/4] Add a 'in_mmio' device flag to avoid the DMA to MMIO
Posted by Li Qiang 3 years, 7 months ago
Jason Wang <jasowang@redhat.com> 于2020年9月9日周三 上午10:17写道:
>
>
> On 2020/9/9 上午12:41, Li Qiang wrote:
> > Currently the qemu device fuzzer find some DMA to MMIO issue. If the
> > device handling MMIO currently trigger a DMA which the address is MMIO,
> > this will reenter the device MMIO handler. As some of the device doesn't
> > consider this it will sometimes crash the qemu.
> >
> > This patch tries to solve this by adding a per-device flag 'in_mmio'.
> > When the memory core dispatch MMIO it will check/set this flag and when
> > it leaves it will clean this flag.
>
>
> What's the plan for fixing the irq issues pointed out by Peter?
>

Just have a basic idea. Just like this we can add a per-device flag,
'in_mmio' or 'in_emulation'
or some other names. The device need solve the irq handler/mmio and
anything other reenter issue by themself
or they can just check/set/clean this flag. This way we may can define
a principle which Peter mentioned that the device emulation should
obey.



Thanks,
Li Qiang


> Thanks
>
>
> >
> >
> > Li Qiang (4):
> >    memory: add memory_region_init_io_with_dev interface
> >    memory: avoid reenter the device's MMIO handler while processing MMIO
> >    e1000e: use the new memory_region_init_io_with_dev interface
> >    hcd-xhci: use the new memory_region_init_io_with_dev interface
> >
> >   hw/net/e1000e.c        |  8 ++++----
> >   hw/usb/hcd-xhci.c      | 25 ++++++++++++++---------
> >   include/exec/memory.h  |  9 +++++++++
> >   include/hw/qdev-core.h |  1 +
> >   softmmu/memory.c       | 46 +++++++++++++++++++++++++++++++++++++++---
> >   5 files changed, 72 insertions(+), 17 deletions(-)
> >
>

Re: [RFC 0/4] Add a 'in_mmio' device flag to avoid the DMA to MMIO
Posted by Paolo Bonzini 3 years, 7 months ago
On 08/09/20 18:41, Li Qiang wrote:
> Currently the qemu device fuzzer find some DMA to MMIO issue. If the
> device handling MMIO currently trigger a DMA which the address is MMIO,
> this will reenter the device MMIO handler. As some of the device doesn't
> consider this it will sometimes crash the qemu.
> 
> This patch tries to solve this by adding a per-device flag 'in_mmio'.
> When the memory core dispatch MMIO it will check/set this flag and when
> it leaves it will clean this flag.
> 
> 
> Li Qiang (4):
>   memory: add memory_region_init_io_with_dev interface
>   memory: avoid reenter the device's MMIO handler while processing MMIO
>   e1000e: use the new memory_region_init_io_with_dev interface
>   hcd-xhci: use the new memory_region_init_io_with_dev interface
> 
>  hw/net/e1000e.c        |  8 ++++----
>  hw/usb/hcd-xhci.c      | 25 ++++++++++++++---------
>  include/exec/memory.h  |  9 +++++++++
>  include/hw/qdev-core.h |  1 +
>  softmmu/memory.c       | 46 +++++++++++++++++++++++++++++++++++++++---
>  5 files changed, 72 insertions(+), 17 deletions(-)
> 

I don't think this is a good solution.  These are device bugs and they
need to be fixed.

Paolo


Re: [RFC 0/4] Add a 'in_mmio' device flag to avoid the DMA to MMIO
Posted by Peter Maydell 3 years, 7 months ago
On Sun, 20 Sep 2020 at 08:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 08/09/20 18:41, Li Qiang wrote:
> > Currently the qemu device fuzzer find some DMA to MMIO issue. If the
> > device handling MMIO currently trigger a DMA which the address is MMIO,
> > this will reenter the device MMIO handler. As some of the device doesn't
> > consider this it will sometimes crash the qemu.

> I don't think this is a good solution.  These are device bugs and they
> need to be fixed.

Do you have an opinion on what the right approach to fixing them is?
It seems like a hard problem to me; my brain has been too full of
cotton wool recently and I haven't felt up to sitting down and
trying to think through whether there's a clean way to handle the
reentrancy-into-device-code problem in the general case...

thanks
-- PMM

Re: [RFC 0/4] Add a 'in_mmio' device flag to avoid the DMA to MMIO
Posted by Li Qiang 3 years, 7 months ago
Paolo Bonzini <pbonzini@redhat.com> 于2020年9月20日周日 下午3:56写道:
>
> On 08/09/20 18:41, Li Qiang wrote:
> > Currently the qemu device fuzzer find some DMA to MMIO issue. If the
> > device handling MMIO currently trigger a DMA which the address is MMIO,
> > this will reenter the device MMIO handler. As some of the device doesn't
> > consider this it will sometimes crash the qemu.
> >
> > This patch tries to solve this by adding a per-device flag 'in_mmio'.
> > When the memory core dispatch MMIO it will check/set this flag and when
> > it leaves it will clean this flag.
> >
> >
> > Li Qiang (4):
> >   memory: add memory_region_init_io_with_dev interface
> >   memory: avoid reenter the device's MMIO handler while processing MMIO
> >   e1000e: use the new memory_region_init_io_with_dev interface
> >   hcd-xhci: use the new memory_region_init_io_with_dev interface
> >
> >  hw/net/e1000e.c        |  8 ++++----
> >  hw/usb/hcd-xhci.c      | 25 ++++++++++++++---------
> >  include/exec/memory.h  |  9 +++++++++
> >  include/hw/qdev-core.h |  1 +
> >  softmmu/memory.c       | 46 +++++++++++++++++++++++++++++++++++++++---
> >  5 files changed, 72 insertions(+), 17 deletions(-)
> >
>
> I don't think this is a good solution.  These are device bugs and they
> need to be fixed.

I agree with this the device should finally handle their cases. But we
can do something in a pattern if the device hasn't
do that.
I have posted a patchset:
-->https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg00906.html

This is add flags in the Device State. And check/record the flag when
doing reentrancy code.
Once the device has fixed the reentrancy issue, they can remove this flag.

Thanks,
Li QIang

>
> Paolo
>