[Qemu-devel] [PATCH for-1.7 0/2] revert master abort related patches

[Qemu-devel] [PATCH for-1.7 0/2] revert master abort related patches

[Marcel Apfelbaum]

The master-abort patch introduced a background memory region
covering all 64 bit pci address space, the visible parts
being the unused pci-holes addresses.

The patch revealed the following issues:
 1. Some memory regions have INT64_MAX size, but the size
    was supposed to be UINT64_MAX (meaning that the
    region covers all 64 bit address space). Having
    a region that is not even a multiple of PAGE_SIZE
    is really not what we want.
 2. exec.c does not support all the 64 bit address range
    and when using an unsupported address, it leads to
    page tables corruption.
 3. Some memory regions overlap and the visible region
    is selected by chance (the algorithm implementation)
    and not by the memory API:
    - selecting a proper priority
    - arrange the regions that are not supposed to overlap.
    
This series reverts this patch and another related patch
because the impact for 1.7 is too big.
After the issues above are solved, the patch can finally
be applied.

Marcel Apfelbaum (1):
  Revert "hw/pci: partially handle pci master abort"

Michael S. Tsirkin (1):
  Revert "exec: limit system memory size"

 exec.c                   |  7 +------
 hw/pci/pci.c             | 26 --------------------------
 include/hw/pci/pci_bus.h |  1 -
 3 files changed, 1 insertion(+), 33 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH for-1.7 1/2] Revert "hw/pci: partially handle pci master abort"

[Marcel Apfelbaum]

This reverts commit a53ae8e934cd54686875b5bcfc2f434244ee55d6.

The master-abort patch introduced a background memory region
covering all 64 bit pci address space, the visible parts
being the unused pci-holes addresses.

The patch revealed the following issues:
 1. Some memory regions have INT64_MAX size, but the size
    was supposed to be UINT64_MAX (meaning that the
    region covers all 64 bit address space). Having
    a region that is not even a multiple of PAGE_SIZE
    is really not what we want.
 2. exec.c does not support all the 64 bit address range
    and when using an unsupported address, it leads to
    page tables corruption.
 3. Some memory regions overlap and the visible region
    is selected by chance (the algorithm implementation)
    and not by the memory API:
    - selecting a proper priority
    - arrange the regions that are not supposed to overlap.
    
The patch is reverted because the impact for 1.7 is too big.
After the issues above are solved, the patch can finally
be applied.


Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/pci/pci.c             | 26 --------------------------
 include/hw/pci/pci_bus.h |  1 -
 2 files changed, 27 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index a98c8a0..ed32059 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -283,24 +283,6 @@ const char *pci_root_bus_path(PCIDevice *dev)
     return rootbus->qbus.name;
 }
 
-static uint64_t master_abort_mem_read(void *opaque, hwaddr addr, unsigned size)
-{
-   return -1ULL;
-}
-
-static void master_abort_mem_write(void *opaque, hwaddr addr, uint64_t val,
-                                   unsigned size)
-{
-}
-
-static const MemoryRegionOps master_abort_mem_ops = {
-    .read = master_abort_mem_read,
-    .write = master_abort_mem_write,
-    .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-#define MASTER_ABORT_MEM_PRIORITY INT_MIN
-
 static void pci_bus_init(PCIBus *bus, DeviceState *parent,
                          const char *name,
                          MemoryRegion *address_space_mem,
@@ -312,14 +294,6 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
     bus->address_space_mem = address_space_mem;
     bus->address_space_io = address_space_io;
 
-
-    memory_region_init_io(&bus->master_abort_mem, OBJECT(bus),
-                          &master_abort_mem_ops, bus, "pci-master-abort",
-                          memory_region_size(bus->address_space_mem));
-    memory_region_add_subregion_overlap(bus->address_space_mem,
-                                        0, &bus->master_abort_mem,
-                                        MASTER_ABORT_MEM_PRIORITY);
-
     /* host bridge */
     QLIST_INIT(&bus->child);
 
diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 2ad5edb..9df1788 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -23,7 +23,6 @@ struct PCIBus {
     PCIDevice *parent_dev;
     MemoryRegion *address_space_mem;
     MemoryRegion *address_space_io;
-    MemoryRegion master_abort_mem;
 
     QLIST_HEAD(, PCIBus) child; /* this will be replaced by qdev later */
     QLIST_ENTRY(PCIBus) sibling;/* this will be replaced by qdev later */
-- 
1.8.3.1

[Qemu-devel] [PATCH for-1.7 2/2] Revert "exec: limit system memory size"

[Marcel Apfelbaum]

From: "Michael S. Tsirkin" <mst@redhat.com>

This reverts commit 818f86b88394b7b2b59d313e51043fe15a8004db.

This was a work-around for bugs elsewhere in the system,
exposed by commit a53ae8e934cd54686875b5bcfc2f434244ee55d6:
    "hw/pci: partially handle pci master abort"
since that's reverted now, the work-around is not required for 1.7
anymore.
The proper fix is supporting full 64 bit addresses in the radix tree.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 exec.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index 79610ce..b453713 100644
--- a/exec.c
+++ b/exec.c
@@ -1741,12 +1741,7 @@ void address_space_destroy_dispatch(AddressSpace *as)
 static void memory_map_init(void)
 {
     system_memory = g_malloc(sizeof(*system_memory));
-
-    assert(TARGET_PHYS_ADDR_SPACE_BITS <= 64);
-
-    memory_region_init(system_memory, NULL, "system",
-                       TARGET_PHYS_ADDR_SPACE_BITS == 64 ?
-                       UINT64_MAX : (0x1ULL << TARGET_PHYS_ADDR_SPACE_BITS));
+    memory_region_init(system_memory, NULL, "system", INT64_MAX);
     address_space_init(&address_space_memory, system_memory, "memory");
 
     system_io = g_malloc(sizeof(*system_io));
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-1.7 0/2] revert master abort related patches

[Michael S. Tsirkin]

On Sun, Nov 10, 2013 at 02:15:23PM +0200, Marcel Apfelbaum wrote:
> The master-abort patch introduced a background memory region
> covering all 64 bit pci address space, the visible parts
> being the unused pci-holes addresses.
> 
> The patch revealed the following issues:
>  1. Some memory regions have INT64_MAX size, but the size
>     was supposed to be UINT64_MAX (meaning that the
>     region covers all 64 bit address space). Having
>     a region that is not even a multiple of PAGE_SIZE
>     is really not what we want.
>  2. exec.c does not support all the 64 bit address range
>     and when using an unsupported address, it leads to
>     page tables corruption.
>  3. Some memory regions overlap and the visible region
>     is selected by chance (the algorithm implementation)
>     and not by the memory API:
>     - selecting a proper priority
>     - arrange the regions that are not supposed to overlap.
>     
> This series reverts this patch and another related patch
> because the impact for 1.7 is too big.
> After the issues above are solved, the patch can finally
> be applied.

I edited the commit log slightly and applied this, thanks.

> Marcel Apfelbaum (1):
>   Revert "hw/pci: partially handle pci master abort"
> 
> Michael S. Tsirkin (1):
>   Revert "exec: limit system memory size"
> 
>  exec.c                   |  7 +------
>  hw/pci/pci.c             | 26 --------------------------
>  include/hw/pci/pci_bus.h |  1 -
>  3 files changed, 1 insertion(+), 33 deletions(-)
> 
> -- 
> 1.8.3.1