Re: [Qemu-devel] [PATCH 1/4] hw/pci-host/bonito.c: Add PCI mem region mapped at the correct address

[Philippe Mathieu-Daudé]

On 2/11/19 5:01 AM, BALATON Zoltan wrote:
> Stop using system memory as PCI memory otherwise devices such as VGA
> that have regions mapped to PCI memory clash with RAM. Use a separate
> memory region for PCI memory and map it to the correct address in
> system memory which allows PCI mem regions to show at the correct
> address where clients expect them.
> 
> Signed-off-by: BALATON Zoltan <address@hidden>
> ---
>  hw/pci-host/bonito.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 9f33582706..c940ec6e48 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -598,11 +598,14 @@ static const VMStateDescription vmstate_bonito = {
>  static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
>  {
>      PCIHostState *phb = PCI_HOST_BRIDGE(dev);
> +    MemoryRegion *mr = g_new0(MemoryRegion, 1);

No need to alloc, move that to BonitoState, see [*].

>  
> +    memory_region_init(mr, OBJECT(dev), "pci.mem", BONITO_PCILO_SIZE);
>      phb->bus = pci_register_root_bus(DEVICE(dev), "pci",
>                                       pci_bonito_set_irq, pci_bonito_map_irq,
> -                                     dev, get_system_memory(), 
> get_system_io(),
> +                                     dev, mr, get_system_io(),
>                                       0x28, 32, TYPE_PCI_BUS);
> +    memory_region_add_subregion(get_system_memory(), BONITO_PCILO_BASE, mr);
>  }
>  
>  static void bonito_realize(PCIDevice *dev, Error **errp)
> 

[*]:
-- >8 --
@@ -217,6 +217,7 @@ struct BonitoState {
     PCIHostState parent_obj;
     qemu_irq *pic;
     PCIBonitoState *pci_dev;
+    MemoryRegion pci_mmio;
 };

 #define TYPE_BONITO_PCI_HOST_BRIDGE "Bonito-pcihost"
@@ -598,14 +599,15 @@ static const VMStateDescription vmstate_bonito = {
 static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
 {
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
-    MemoryRegion *mr = g_new0(MemoryRegion, 1);
+    BonitoState *s = BONITO_PCI_HOST_BRIDGE(dev);

-    memory_region_init(mr, OBJECT(dev), "pci.mem", BONITO_PCILO_SIZE);
+    memory_region_init(&s->pci_mmio, OBJECT(dev), "pci.mem",
BONITO_PCILO_SIZE);
     phb->bus = pci_register_root_bus(DEVICE(dev), "pci",
                                      pci_bonito_set_irq,
pci_bonito_map_irq,
-                                     dev, mr, get_system_io(),
+                                     dev, &s->pci_mmio, get_system_io(),
                                      0x28, 32, TYPE_PCI_BUS);
-    memory_region_add_subregion(get_system_memory(), BONITO_PCILO_BASE,
mr);
+    memory_region_add_subregion(get_system_memory(),
+                                BONITO_PCILO_BASE, &s->pci_mmio);
 }
---

Using snippet:
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Tested-by: Philippe Mathieu-Daudé <address@hidden>