| Message ID | 1402419999-26059-3-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
On 10.06.2014 19:06, Peter Maydell wrote: > UEFI mandates that the platform must include an RTC, so provide > one in 'virt', using the PL031. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/arm/virt.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index e658eb0..b60928e 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -66,6 +66,7 @@ enum { > VIRT_GIC_CPU, > VIRT_UART, > VIRT_MMIO, > + VIRT_RTC, > }; > > typedef struct MemMapEntry { > @@ -93,6 +94,8 @@ typedef struct VirtBoardInfo { > * high memory region beyond 4GB). > * This represents a compromise between how much RAM can be given to > * a 32 bit VM and leaving space for expansion and in particular for PCI. > + * Note that devices should generally be placed at multiples of 0x10000, > + * to accommodate guests using 64K pages. > */ > static const MemMapEntry a15memmap[] = { > [VIRT_FLASH] = { 0, 0x8000000 }, > @@ -101,6 +104,7 @@ static const MemMapEntry a15memmap[] = { > [VIRT_GIC_DIST] = { 0x8000000, 0x10000 }, > [VIRT_GIC_CPU] = { 0x8010000, 0x10000 }, > [VIRT_UART] = { 0x9000000, 0x1000 }, > + [VIRT_RTC] = { 0x90010000, 0x1000 }, > [VIRT_MMIO] = { 0xa000000, 0x200 }, > /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ > /* 0x10000000 .. 0x40000000 reserved for PCI */ > @@ -109,6 +113,7 @@ static const MemMapEntry a15memmap[] = { > > static const int a15irqmap[] = { > [VIRT_UART] = 1, > + [VIRT_RTC] = 2, > [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ > }; > > @@ -340,6 +345,29 @@ static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic) > g_free(nodename); > } > > +static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic) > +{ > + char *nodename; > + hwaddr base = vbi->memmap[VIRT_RTC].base; > + hwaddr size = vbi->memmap[VIRT_RTC].size; > + int irq = vbi->irqmap[VIRT_RTC]; > + const char compat[] = "arm,pl031\0arm,primecell"; > + > + sysbus_create_simple("pl031", base, pic[irq]); > + > + nodename = g_strdup_printf("/pl031@%" PRIx64, base); > + qemu_fdt_add_subnode(vbi->fdt, nodename); > + qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat)); > + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", > + 2, base, 2, size); > + qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts", > + GIC_FDT_IRQ_TYPE_SPI, irq, > + GIC_FDT_IRQ_FLAGS_EDGE_LO_HI); > + qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle); > + qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk"); > + g_free(nodename); > +} > + > static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic) > { > int i; > @@ -524,6 +552,8 @@ static void machvirt_init(MachineState *machine) > > create_uart(vbi, pic); > > + create_rtc(vbi, pic); > + > /* Create mmio transports, so the user can create virtio backends > * (which will be automatically plugged in to the transports). If > * no backend is created the transport will just sit harmlessly idle. > I am quite happy with the RTC device being added to the virt platform, as this will move me from 1970 in the guest, where I am at the moment. :) One question I would have is, what would be the best/recommended way as a user of the virt platform to add buses and devices to the platform? Is using virt as the base platform, and extending it with additional buses and devices a sensible thing to do? In my case I am particularly interested in the possibility to add a PCI-E bus to the platform in some (any) way, so that QEMU provides support for that, one that does not mean maintaining a separate patchset. Is extending via --device a viable option? New machine model? Thank you for any advice, Claudio
On Thu, Jun 12, 2014 at 2:01 AM, Claudio Fontana <claudio.fontana@huawei.com> wrote: > On 10.06.2014 19:06, Peter Maydell wrote: >> UEFI mandates that the platform must include an RTC, so provide >> one in 'virt', using the PL031. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> hw/arm/virt.c | 30 ++++++++++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index e658eb0..b60928e 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -66,6 +66,7 @@ enum { >> VIRT_GIC_CPU, >> VIRT_UART, >> VIRT_MMIO, >> + VIRT_RTC, >> }; >> >> typedef struct MemMapEntry { >> @@ -93,6 +94,8 @@ typedef struct VirtBoardInfo { >> * high memory region beyond 4GB). >> * This represents a compromise between how much RAM can be given to >> * a 32 bit VM and leaving space for expansion and in particular for PCI. >> + * Note that devices should generally be placed at multiples of 0x10000, >> + * to accommodate guests using 64K pages. >> */ >> static const MemMapEntry a15memmap[] = { >> [VIRT_FLASH] = { 0, 0x8000000 }, >> @@ -101,6 +104,7 @@ static const MemMapEntry a15memmap[] = { >> [VIRT_GIC_DIST] = { 0x8000000, 0x10000 }, >> [VIRT_GIC_CPU] = { 0x8010000, 0x10000 }, >> [VIRT_UART] = { 0x9000000, 0x1000 }, >> + [VIRT_RTC] = { 0x90010000, 0x1000 }, >> [VIRT_MMIO] = { 0xa000000, 0x200 }, >> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ >> /* 0x10000000 .. 0x40000000 reserved for PCI */ >> @@ -109,6 +113,7 @@ static const MemMapEntry a15memmap[] = { >> >> static const int a15irqmap[] = { >> [VIRT_UART] = 1, >> + [VIRT_RTC] = 2, >> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ >> }; >> >> @@ -340,6 +345,29 @@ static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic) >> g_free(nodename); >> } >> >> +static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic) >> +{ >> + char *nodename; >> + hwaddr base = vbi->memmap[VIRT_RTC].base; >> + hwaddr size = vbi->memmap[VIRT_RTC].size; >> + int irq = vbi->irqmap[VIRT_RTC]; >> + const char compat[] = "arm,pl031\0arm,primecell"; >> + >> + sysbus_create_simple("pl031", base, pic[irq]); >> + >> + nodename = g_strdup_printf("/pl031@%" PRIx64, base); >> + qemu_fdt_add_subnode(vbi->fdt, nodename); >> + qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat)); >> + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", >> + 2, base, 2, size); >> + qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts", >> + GIC_FDT_IRQ_TYPE_SPI, irq, >> + GIC_FDT_IRQ_FLAGS_EDGE_LO_HI); >> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle); >> + qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk"); >> + g_free(nodename); >> +} >> + >> static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic) >> { >> int i; >> @@ -524,6 +552,8 @@ static void machvirt_init(MachineState *machine) >> >> create_uart(vbi, pic); >> >> + create_rtc(vbi, pic); >> + >> /* Create mmio transports, so the user can create virtio backends >> * (which will be automatically plugged in to the transports). If >> * no backend is created the transport will just sit harmlessly idle. >> > > I am quite happy with the RTC device being added to the virt platform, as this will move me from 1970 in the guest, where I am at the moment. :) > > One question I would have is, what would be the best/recommended way as a user of the virt platform > to add buses and devices to the platform? Is using virt as the base platform, and extending it with additional buses and devices a sensible thing to do? > Yes. > In my case I am particularly interested in the possibility to add a PCI-E bus to the platform in some (any) way, so that QEMU provides support for that, one that does not mean maintaining a separate patchset. Is extending via --device a viable option? > New machine model? For PCI support it should just be a case of adding a PCI controller to the virt board, then -device should just work for pci devs. Regards, Peter > > Thank you for any advice, > > Claudio > >
On 14.06.2014 01:10, Peter Crosthwaite wrote: > On Thu, Jun 12, 2014 at 2:01 AM, Claudio Fontana > <claudio.fontana@huawei.com> wrote: >> On 10.06.2014 19:06, Peter Maydell wrote: >>> UEFI mandates that the platform must include an RTC, so provide >>> one in 'virt', using the PL031. >>> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >>> --- >>> hw/arm/virt.c | 30 ++++++++++++++++++++++++++++++ >>> 1 file changed, 30 insertions(+) >>> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>> index e658eb0..b60928e 100644 >>> --- a/hw/arm/virt.c >>> +++ b/hw/arm/virt.c >>> @@ -66,6 +66,7 @@ enum { >>> VIRT_GIC_CPU, >>> VIRT_UART, >>> VIRT_MMIO, >>> + VIRT_RTC, >>> }; >>> >>> typedef struct MemMapEntry { >>> @@ -93,6 +94,8 @@ typedef struct VirtBoardInfo { >>> * high memory region beyond 4GB). >>> * This represents a compromise between how much RAM can be given to >>> * a 32 bit VM and leaving space for expansion and in particular for PCI. >>> + * Note that devices should generally be placed at multiples of 0x10000, >>> + * to accommodate guests using 64K pages. >>> */ >>> static const MemMapEntry a15memmap[] = { >>> [VIRT_FLASH] = { 0, 0x8000000 }, >>> @@ -101,6 +104,7 @@ static const MemMapEntry a15memmap[] = { >>> [VIRT_GIC_DIST] = { 0x8000000, 0x10000 }, >>> [VIRT_GIC_CPU] = { 0x8010000, 0x10000 }, >>> [VIRT_UART] = { 0x9000000, 0x1000 }, >>> + [VIRT_RTC] = { 0x90010000, 0x1000 }, >>> [VIRT_MMIO] = { 0xa000000, 0x200 }, >>> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ >>> /* 0x10000000 .. 0x40000000 reserved for PCI */ >>> @@ -109,6 +113,7 @@ static const MemMapEntry a15memmap[] = { >>> >>> static const int a15irqmap[] = { >>> [VIRT_UART] = 1, >>> + [VIRT_RTC] = 2, >>> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ >>> }; >>> >>> @@ -340,6 +345,29 @@ static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic) >>> g_free(nodename); >>> } >>> >>> +static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic) >>> +{ >>> + char *nodename; >>> + hwaddr base = vbi->memmap[VIRT_RTC].base; >>> + hwaddr size = vbi->memmap[VIRT_RTC].size; >>> + int irq = vbi->irqmap[VIRT_RTC]; >>> + const char compat[] = "arm,pl031\0arm,primecell"; >>> + >>> + sysbus_create_simple("pl031", base, pic[irq]); >>> + >>> + nodename = g_strdup_printf("/pl031@%" PRIx64, base); >>> + qemu_fdt_add_subnode(vbi->fdt, nodename); >>> + qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat)); >>> + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", >>> + 2, base, 2, size); >>> + qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts", >>> + GIC_FDT_IRQ_TYPE_SPI, irq, >>> + GIC_FDT_IRQ_FLAGS_EDGE_LO_HI); >>> + qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle); >>> + qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk"); >>> + g_free(nodename); >>> +} >>> + >>> static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic) >>> { >>> int i; >>> @@ -524,6 +552,8 @@ static void machvirt_init(MachineState *machine) >>> >>> create_uart(vbi, pic); >>> >>> + create_rtc(vbi, pic); >>> + >>> /* Create mmio transports, so the user can create virtio backends >>> * (which will be automatically plugged in to the transports). If >>> * no backend is created the transport will just sit harmlessly idle. >>> >> >> I am quite happy with the RTC device being added to the virt platform, as this will move me from 1970 in the guest, where I am at the moment. :) >> >> One question I would have is, what would be the best/recommended way as a user of the virt platform >> to add buses and devices to the platform? Is using virt as the base platform, and extending it with additional buses and devices a sensible thing to do? >> > > Yes. > >> In my case I am particularly interested in the possibility to add a PCI-E bus to the platform in some (any) way, so that QEMU provides support for that, one that does not mean maintaining a separate patchset. Is extending via --device a viable option? >> New machine model? > > For PCI support it should just be a case of adding a PCI controller to > the virt board, then -device should just work for pci devs. Thanks for your answer; that seems straightforward enough, so would a patch to add pci to the virt board be acceptable? virt.c shows that there is already a reserved space in the mem map for that. Thank you, Claudio
On 16 June 2014 10:18, Claudio Fontana <claudio.fontana@huawei.com> wrote: > On 14.06.2014 01:10, Peter Crosthwaite wrote: >> For PCI support it should just be a case of adding a PCI controller to >> the virt board, then -device should just work for pci devs. > > Thanks for your answer; that seems straightforward enough, > so would a patch to add pci to the virt board be acceptable? You would need to implement a sensible PCI controller first. Actually Rob Herring has some prototype patches which do this (using the kernel's new support for a PCI controller which can be defined entirely using a device tree). They have some bugs still, though (the lsi SCSI controller device doesn't seem to work, for instance). Rob, would you mind posting your PCI controller patch as an RFC? That way Claudio could have a look at the issues with DMA if he wants to move forward here. thanks -- PMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index e658eb0..b60928e 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -66,6 +66,7 @@ enum { VIRT_GIC_CPU, VIRT_UART, VIRT_MMIO, + VIRT_RTC, }; typedef struct MemMapEntry { @@ -93,6 +94,8 @@ typedef struct VirtBoardInfo { * high memory region beyond 4GB). * This represents a compromise between how much RAM can be given to * a 32 bit VM and leaving space for expansion and in particular for PCI. + * Note that devices should generally be placed at multiples of 0x10000, + * to accommodate guests using 64K pages. */ static const MemMapEntry a15memmap[] = { [VIRT_FLASH] = { 0, 0x8000000 }, @@ -101,6 +104,7 @@ static const MemMapEntry a15memmap[] = { [VIRT_GIC_DIST] = { 0x8000000, 0x10000 }, [VIRT_GIC_CPU] = { 0x8010000, 0x10000 }, [VIRT_UART] = { 0x9000000, 0x1000 }, + [VIRT_RTC] = { 0x90010000, 0x1000 }, [VIRT_MMIO] = { 0xa000000, 0x200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ /* 0x10000000 .. 0x40000000 reserved for PCI */ @@ -109,6 +113,7 @@ static const MemMapEntry a15memmap[] = { static const int a15irqmap[] = { [VIRT_UART] = 1, + [VIRT_RTC] = 2, [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ }; @@ -340,6 +345,29 @@ static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic) g_free(nodename); } +static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic) +{ + char *nodename; + hwaddr base = vbi->memmap[VIRT_RTC].base; + hwaddr size = vbi->memmap[VIRT_RTC].size; + int irq = vbi->irqmap[VIRT_RTC]; + const char compat[] = "arm,pl031\0arm,primecell"; + + sysbus_create_simple("pl031", base, pic[irq]); + + nodename = g_strdup_printf("/pl031@%" PRIx64, base); + qemu_fdt_add_subnode(vbi->fdt, nodename); + qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat)); + qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", + 2, base, 2, size); + qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts", + GIC_FDT_IRQ_TYPE_SPI, irq, + GIC_FDT_IRQ_FLAGS_EDGE_LO_HI); + qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle); + qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk"); + g_free(nodename); +} + static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic) { int i; @@ -524,6 +552,8 @@ static void machvirt_init(MachineState *machine) create_uart(vbi, pic); + create_rtc(vbi, pic); + /* Create mmio transports, so the user can create virtio backends * (which will be automatically plugged in to the transports). If * no backend is created the transport will just sit harmlessly idle.
UEFI mandates that the platform must include an RTC, so provide one in 'virt', using the PL031. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)