Re: [PATCH] arm64/pvpanic-mmio : add pvpanic mmio device

[Mark Rutland]

On Thu, Oct 18, 2018 at 09:56:11AM +0800, peng.hao2@xxxxxxxxxx wrote:
> >Hi,
> >
> >[adding devicetree]
> >
> >On Wed, Oct 17, 2018 at 06:08:23PM +0800, Peng Hao wrote:

[...]

> >> +#define PVPANIC_MMIO_CRASHED    (1 << 0)
> >
> >This looks like it's identical to PVPANIC_PANICKED in the existing ACPI
> >IO pvpanic driver.
> >
> >I suspect on the QEMU side, there's one device, and the distinction is
> >only in how this is described.
> >
> in qemu:
> Firstly pvpanic(x86) use ioport, but arm don't support ioport.
> secondly pvpanic device is emulated as a isa bus device, but arm don't support
> isa bus.

I am aware of arm platforms with an ISA bus (well, LPC, at least).

> thirdly the realization of pvpanic device is depends on ACPI in linux
> kernel driver and in qemu the port info is passed through ACPI , but
> It is not necessary to configure ACPI for arm guest.

There are many people using ACPI with arm64 under QEMU, so while it's
not *necessary* to use ACPI, it is a common use case that we cannot rule
out.

> in linux : I don't want to depends on ACPI .

I'm not saying that you must depend on ACPI for your specific use-case.

> ARM virt machine  in qemu uses FDT, and I can easily 
> use FDT to get mmio information from QEMU. 
> >Can't one driver support both?
> Just like  virtio-mmio driver, It is mainly used for non x86 architecture.

So? I can build virtio-mmio for x86, too.

My concern is that other than the initial probing, the vast majority of
this is *identical* to the existing pvpanic driver (ignoring cosmetic
differences like the naming of variables).

I'm worried that there could be additions to the pvpanic device in
future (e.g. passing a pointer to some crash context), which would then
have to be duplicated across two drivers.

IMO we should:

a) Move the existing pvpanic driver to a neutral location (e.g.
   drivers/virt/qemu_pvpanic.c).

b) Teach the existing pvpanic driver to handle MMIO, even for ACPI.

c) Teach the existing pvpanic driver to handle DT, even if that DT
   binding only caters for MMIO.

Thanks,
Mark.