Re: [Qemu-devel] [PATCH v5 27/36] sysbus: add a sysbus_mmio_unmap() helper

[Cédric Le Goater]

On 11/29/18 5:09 AM, David Gibson wrote:
> On Fri, Nov 16, 2018 at 11:57:20AM +0100, Cédric Le Goater wrote:
>> This will be used to remove the MMIO regions of the POWER9 XIVE
>> interrupt controller when the sPAPR machine is reseted.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
> 
> Reviewed-by: David Gibson <address@hidden>
> 
> Since the code looks sane.
> 
> Hoever, I think using memory_region_set_enabled() would be a better
> idea for our purposes than actually adding/deleting the subregion.

Yes and we might not need this one anymore. 

Thanks,

C.

>> ---
>>  include/hw/sysbus.h |  1 +
>>  hw/core/sysbus.c    | 10 ++++++++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
>> index 0b59a3b8d605..bc641984b5da 100644
>> --- a/include/hw/sysbus.h
>> +++ b/include/hw/sysbus.h
>> @@ -92,6 +92,7 @@ qemu_irq sysbus_get_connected_irq(SysBusDevice *dev, int 
>> n);
>>  void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr);
>>  void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr addr,
>>                               int priority);
>> +void sysbus_mmio_unmap(SysBusDevice *dev, int n);
>>  void sysbus_add_io(SysBusDevice *dev, hwaddr addr,
>>                     MemoryRegion *mem);
>>  MemoryRegion *sysbus_address_space(SysBusDevice *dev);
>> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
>> index 7ac36ad3e707..09f202167dcb 100644
>> --- a/hw/core/sysbus.c
>> +++ b/hw/core/sysbus.c
>> @@ -153,6 +153,16 @@ static void sysbus_mmio_map_common(SysBusDevice *dev, 
>> int n, hwaddr addr,
>>      }
>>  }
>>  
>> +void sysbus_mmio_unmap(SysBusDevice *dev, int n)
>> +{
>> +    assert(n >= 0 && n < dev->num_mmio);
>> +
>> +    if (dev->mmio[n].addr != (hwaddr)-1) {
>> +        memory_region_del_subregion(get_system_memory(), 
>> dev->mmio[n].memory);
>> +        dev->mmio[n].addr = (hwaddr)-1;
>> +    }
>> +}
>> +
>>  void sysbus_mmio_map(SysBusDevice *dev, int n, hwaddr addr)
>>  {
>>      sysbus_mmio_map_common(dev, n, addr, false, 0);
>