qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v12 3/4] hw/arm/virt: add dynamic sysbus device


From: Eric Auger
Subject: Re: [Qemu-devel] [PATCH v12 3/4] hw/arm/virt: add dynamic sysbus device support
Date: Mon, 27 Apr 2015 15:54:34 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0

Hi Alex
On 04/27/2015 03:41 PM, Alexander Graf wrote:
> On 04/24/2015 05:19 PM, Eric Auger wrote:
>> Allows sysbus devices to be instantiated from command line by
>> using -device option. Machvirt creates a platform bus at init.
>> The dynamic sysbus devices are attached to this platform bus device.
>>
>> The platform bus device registers a machine init done notifier
>> whose role will be to bind the dynamic sysbus devices. Indeed
>> dynamic sysbus devices are created after machine init.
>>
>> machvirt also registers a notifier that will build the device
>> tree nodes for the platform bus and its children dynamic sysbus
>> devices.
>>
>> Signed-off-by: Alexander Graf <address@hidden>
> 
> Hrm, are you sure this code is from me? :)

Well I just wanted to emphasize this originates from your e500.c etsec
integration. through this is true most of it now is in sysbus-fdt.c. I
will remove your Signed-off-by if this is your will ;-)

Best Regards

Eric
> 
>> Signed-off-by: Eric Auger <address@hidden>
>> Reviewed-by: Alex Bennée <address@hidden>
>>
>> ---
>> v11 -> v12:
>> - resize PLATFORM_BUS_NUM_IRQS to 64 instead of 32
>> - overall NUM_IRQS changed to 256. This leaves space for MSI
>>    looming addition
>> - VIRT_PLATFORM_BUS size changed from 4MB to 32MB and base set at
>>    0xc000000
>> - Add Alex R-b
>>
>> v8 -> v9:
>> - PLATFORM_BUS_NUM_IRQS set to 32 instead of 20
>> - platform bus irq now start at 64 instead of 48
>> - remove change of indentation in a15memmap
>> - correct misc style issues
>>
>> v7 -> v8:
>> - rebase on 2.2.0
>> - in machvirt_init, create_platform_bus simply is added
>>    after the arm_load_kernel call instead of moving this latter.
>>    Related comment slighly reworded.
>> - Due to those changes I dropped Alex and Shannon's Reviewed-by
>>
>> v6 -> v7:
>> Take into account Shannon comments:
>> - remove PLATFORM_BUS_FIRST_IRQ macro
>> - correct platform bus size to 0x400000
>> - add an additional comment in a15irqmap related to
>>    PLATFORM_BUS_NUM_IRQS
>>
>> v5 -> v6:
>> - Take into account Peter's comments:
>>    - platform_bus_params initialized from vbi->memmap and vbi->irqmap.
>>      As a consequence, const is removed. Also alignment in a15memmap
>>      is slightly changed.
>>    - ARMPlatformBusSystemParams handle removed from create_platform_bus
>>      prototype
>> - arm_load_kernel has become a machine init done notifier registration.
>>    It must be called before platform_bus_create to guarantee the correct
>>    notifier execution sequence
>>
>> v4 -> v5:
>> - platform_bus_params becomes static const
>> - reword comment in create_platform_bus
>> - reword the commit message
>>
>> v3 -> v4:
>> - use platform bus object, instantiated in create_platform_bus
>> - device tree generation for platform bus and children dynamic
>>    sysbus devices is no more handled at reset but in a
>>    machine_init_done_notifier (due to the change in implementaion
>>    of ARM load dtb using rom_add_blob_fixed).
>> - device tree enhancement now takes into account the case of
>>    user provided dtb. Before the user dtb was overwritten which
>>    was wrong. However in case the dtb is provided by the user,
>>    dynamic sysbus nodes are not added there.
>> - renaming of MACHVIRT_PLATFORM defines
>> - MACHVIRT_PLATFORM_PAGE_SHIFT and SIZE_PAGES not needed anymore,
>>    hence removed.
>> - DynSysbusParams struct renamed into ARMPlatformBusSystemParams
>>    and above params removed.
>> - separation of dt creation and QEMU binding is not mandated anymore
>>    since the device tree is not created from scratch anymore. Instead
>>    the modify_dtb function is used.
>> - create_platform_bus registers another machine init done notifier
>>    to start VFIO IRQ handling. This latter executes after the
>>    dynamic sysbus device binding.
>>
>> v2 -> v3:
>> - renaming of arm_platform_bus_create_devtree and arm_load_dtb
>> - add copyright in hw/arm/dyn_sysbus_devtree.c
>>
>> v1 -> v2:
>> - remove useless vfio-platform.h include file
>> - s/MACHVIRT_PLATFORM_HOLE/MACHVIRT_PLATFORM_SIZE
>> - use dyn_sysbus_binding and dyn_sysbus_devtree
>> - dynamic sysbus platform buse size shrinked to 4MB and
>>    moved between RTC and MMIO
>>
>> v1:
>>
>> Inspired from what Alex Graf did in ppc e500
>> https://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00012.html
>> ---
>>   hw/arm/virt.c | 61
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 565f573..efa3216 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -43,11 +43,13 @@
>>   #include "qemu/bitops.h"
>>   #include "qemu/error-report.h"
>>   #include "hw/pci-host/gpex.h"
>> +#include "hw/arm/sysbus-fdt.h"
>> +#include "hw/platform-bus.h"
>>     #define NUM_VIRTIO_TRANSPORTS 32
>>     /* Number of external interrupt lines to configure the GIC with */
>> -#define NUM_IRQS 128
>> +#define NUM_IRQS 256
>>     #define GIC_FDT_IRQ_TYPE_SPI 0
>>   #define GIC_FDT_IRQ_TYPE_PPI 1
>> @@ -60,6 +62,8 @@
>>   #define GIC_FDT_IRQ_PPI_CPU_START 8
>>   #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
>>   +#define PLATFORM_BUS_NUM_IRQS 64
>> +
>>   enum {
>>       VIRT_FLASH,
>>       VIRT_MEM,
>> @@ -71,8 +75,11 @@ enum {
>>       VIRT_RTC,
>>       VIRT_FW_CFG,
>>       VIRT_PCIE,
>> +    VIRT_PLATFORM_BUS,
>>   };
>>   +static ARMPlatformBusSystemParams platform_bus_params;
>> +
>>   typedef struct MemMapEntry {
>>       hwaddr base;
>>       hwaddr size;
>> @@ -131,6 +138,7 @@ static const MemMapEntry a15memmap[] = {
>>       [VIRT_FW_CFG] =     { 0x09020000, 0x0000000a },
>>       [VIRT_MMIO] =       { 0x0a000000, 0x00000200 },
>>       /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of
>> that size */
>> +    [VIRT_PLATFORM_BUS] =   { 0x0c000000, 0x02000000 },
> 
> Peter, would you have a hard time if we just get rid of VIRT_MMIO
> completely and allow users to create the mmio-virtio bridges using
> -device for -M virt-2.4 and above?
> 
> At the end of the day, I'm fairly sure people will end up virtio-pci
> anyway and it's just a big waste of address space to keep VIRT_MMIO
> around, no?
> 
> That said, if you want to keep virtio-mmio support in the way it is
> today, I won't object. The rest of the patch looks good to me.
> 
> 
> Alex
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]