[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-1.7 v2 0/8] fix address space size issue
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH for-1.7 v2 0/8] fix address space size issue |
Date: |
Thu, 07 Nov 2013 15:08:35 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130923 Thunderbird/17.0.9 |
Il 07/11/2013 11:41, Marcel Apfelbaum ha scritto:
> A bug reported by Luiz Capitulino let us to find
> several bugs in memory address space setup.
>
> One issue is that gdb stub can give us arbitrary addresses
> and we'll try to access them.
> Since our lookup ignored high bits in the address,
> we hit a wrong section and got a crash.
> In fact, PCI devices can access arbitrary addresses too,
> so we should just make lookup robust against this case.
>
> Another issue has to do with size of regions.
> memory API uses UINT64_MAX so say "all 64 bit" but
> some devices mistakenly used INT64_MAX.
>
> It should not affect most systems in practice as
> everything should be limited by address space size,
> but it's an API misuse that we should not keep around,
> and it will become a problem if a system with 64 bit
> target address hits this path.
>
> Patch 1 introduces TARGET_PHYS_ADDR_SPACE_MAX that is
> the max size for memory regions rendered by exec.
> Patches 2-3 limits the size of memory regions used by exec.c.
> Patch 4 fixes an actual bug.
> The rest of patches make code cleaner and more robust.
I think this is wrong.
There is no reason why boards should be using TARGET_PHYS_ADDR_SPACE_MAX
if the PCI address space is 64-bit. Many files in hw/ do not even have
access to TARGET_PHYS_ADDR_SPACE_MAX, which is only available to files
that are compiled per-target.
The fact that you have to reduce the IOMMU address spaces from 64 bits
(UINT64_MAX, not the buggy INT64_MAX) to something else is a red flag.
If the IOMMU supports 64-bit addressing, the IOMMU regions should have
2^64 bits as their length. Yes, it works by chance now, but it _does_
work for 2^64-bit size regions: you can do DMA to a high address (say
0xf << 60), the IOMMU tables will convert to a low address that fits
within TARGET_PHYS_ADDR_SPACE_BITS, and the everything will be fine.
Patches 4 and 6 change that.
So, ack for patch 5-7-8, which should also be enough to fix the problem
that Luiz reported. For everything else the solution is to make memory
dispatch use the whole 64 bits, as in
http://permalink.gmane.org/gmane.comp.emulators.qemu/240229. If you
want to delay that to 1.8 that's fine.
Paolo
>
> Marcel Apfelbaum (3):
> exec: declare TARGET_PHYS_ADDR_SPACE_MAX to limit memory regions
> rendered by exec
> hw/alpha: limit iommu-typhoon memory size
> hw/ppc: limit iommu-spapr memory size
>
> Michael S. Tsirkin (4):
> exec: don't ignore high address bits on lookup
> pci: fix address space size for bridge
> exec: don't ignore high address bits on set
> spapr_pci: s/INT64_MAX/UINT64_MAX/
>
> Paolo Bonzini (1):
> pc: s/INT64_MAX/UINT64_MAX/
>
> exec.c | 18 ++++++++++++++++++
> hw/alpha/typhoon.c | 2 +-
> hw/i386/pc_piix.c | 2 +-
> hw/i386/pc_q35.c | 2 +-
> hw/pci/pci_bridge.c | 2 +-
> hw/ppc/spapr_iommu.c | 2 +-
> hw/ppc/spapr_pci.c | 2 +-
> include/exec/address-spaces.h | 4 ++++
> 8 files changed, 28 insertions(+), 6 deletions(-)
>
- [Qemu-devel] [PATCH for-1.7 v2 3/8] hw/ppc: limit iommu-spapr memory size, (continued)
- [Qemu-devel] [PATCH for-1.7 v2 3/8] hw/ppc: limit iommu-spapr memory size, Marcel Apfelbaum, 2013/11/07
- [Qemu-devel] [PATCH for-1.7 v2 4/8] exec: don't ignore high address bits on lookup, Marcel Apfelbaum, 2013/11/07
- [Qemu-devel] [PATCH for-1.7 v2 5/8] pci: fix address space size for bridge, Marcel Apfelbaum, 2013/11/07
- [Qemu-devel] [PATCH for-1.7 v2 7/8] pc: s/INT64_MAX/UINT64_MAX/, Marcel Apfelbaum, 2013/11/07
- [Qemu-devel] [PATCH for-1.7 v2 8/8] spapr_pci: s/INT64_MAX/UINT64_MAX/, Marcel Apfelbaum, 2013/11/07
- [Qemu-devel] [PATCH for-1.7 v2 2/8] hw/alpha: limit iommu-typhoon memory size, Marcel Apfelbaum, 2013/11/07
- [Qemu-devel] [PATCH for-1.7 v2 6/8] exec: don't ignore high address bits on set, Marcel Apfelbaum, 2013/11/07
- Re: [Qemu-devel] [PATCH for-1.7 v2 0/8] fix address space size issue, Alexander Graf, 2013/11/07
- Re: [Qemu-devel] [PATCH for-1.7 v2 0/8] fix address space size issue,
Paolo Bonzini <=