>On 18 October 2018 at 14:04, Philippe Mathieu-Daudé <phi...@redhat.com> wrote: >> Signed-off-by: Peng Hao <peng.h...@zte.com.cn> >> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> >> [PMD: Use TYPE_PVPANIC definition, split in 2 patches] >> --- >> default-configs/arm-softmmu.mak | 2 +- >> hw/arm/virt.c | 21 +++++++++++++++++++++ >> include/hw/arm/virt.h | 1 + >> 3 files changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/default-configs/arm-softmmu.mak >> b/default-configs/arm-softmmu.mak >> index 2420491aac..def07fa095 100644 >> --- a/default-configs/arm-softmmu.mak >> +++ b/default-configs/arm-softmmu.mak >> @@ -43,7 +43,7 @@ CONFIG_USB_MUSB=y >> CONFIG_USB_EHCI_SYSBUS=y >> CONFIG_PLATFORM_BUS=y >> CONFIG_VIRTIO_MMIO=y >> - >> +CONFIG_PVPANIC=y >> CONFIG_ARM11MPCORE=y >> CONFIG_A9MPCORE=y >> CONFIG_A15MPCORE=y >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 9f677825f9..40469e6785 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -59,6 +59,7 @@ >> #include "qapi/visitor.h" >> #include "standard-headers/linux/input.h" >> #include "hw/arm/smmuv3.h" >> +#include "hw/misc/pvpanic.h" >> >> #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \ >> static void virt_##major##_##minor##_class_init(ObjectClass *oc, \ >> @@ -140,6 +141,7 @@ static const MemMapEntry a15memmap[] = { >> [VIRT_UART] = { 0x09000000, 0x00001000 }, >> [VIRT_RTC] = { 0x09010000, 0x00001000 }, >> [VIRT_FW_CFG] = { 0x09020000, 0x00000018 }, >> + [VIRT_PVPANIC_MMIO] = { 0x09020018, 0x00000002 }, > >This shouldn't be in the same physical 64K page as the preceding device: >we carefully keep them all separate so that the guest can give them >separate MMU permissions if it wants. > I never notice that. Thanks.
>> [VIRT_GPIO] = { 0x09030000, 0x00001000 }, >> [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, >> [VIRT_SMMU] = { 0x09050000, 0x00020000 }, >> @@ -802,6 +804,23 @@ static void create_gpio(const VirtMachineState *vms, >> qemu_irq *pic) >> g_free(nodename); >> } >> >> +static void create_pvpanic_device(const VirtMachineState *vms) >> +{ >> + char *nodename; >> + hwaddr base = vms->memmap[VIRT_PVPANIC_MMIO].base; >> + hwaddr size = vms->memmap[VIRT_PVPANIC_MMIO].size; >> + >> + sysbus_create_simple(TYPE_PVPANIC, base, NULL); >> + >> + nodename = g_strdup_printf("/pvpanic-mmio@%" PRIx64, base); >> + qemu_fdt_add_subnode(vms->fdt, nodename); >> + qemu_fdt_setprop_string(vms->fdt, nodename, >> + "compatible", "pvpanic,mmio"); >> + qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg", >> + 2, base, 2, size); > >This doesn't seem to be an official DT binding. It would need to be >accepted upstream (ie in the kernel's Documentation/devicetree/bindings/) >before we could add the device in QEMU. The patch I submitted to the kernel about pvpanic-mmio driver is also pointing out the same problem. I can use "qemu,pvpanic-mmio" ? > >What about an ACPI table entry? I must add an ACPI table entry for pvpanic-mmio? I tested whether adding a ACPI entry does not seem to affect the function of pvpanic. now I emulate the pvpanic-mmio device as a sysbus device and use fdt to send mmio address info. >Does this really need to be an MMIO device, rather than a PCI device? >Being a PCI device would avoid all of these issues: >* having to specify a dt binding >* having to specify an ACPI binding >* having to either have the thing always present, or some custom >way for the user to enable/disable it >plus it means it's available for other boards and architectures than >just the Arm virt board. > I thought about pci device , If so, it seems that I can only use ACPI to pass MMIO information, just like pvpanic isa device did. I don't want to rely on ACPI like pvpanic isa device . I can use fdt to transmit mmio info to kernel for pci device ? another reason, the function of pvpanic is simple. Thanks. >I'd also like some confirmation from folks more familiar with the >current state of the art in guest-to-management-layer communication >that pvpanic is still the recommended way to achieve this goal, >and hasn't been obsoleted by something else. > >thanks >-- PMM