Re: [Qemu-devel] [PATCH] RFC: vmcoreinfo device

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 14 Jun 2017 10:46:08 +0000
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> Hi
> 
> On Mon, May 29, 2017 at 4:44 PM Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Fri, 26 May 2017 13:59:09 +0000
> > Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> >  
> > > Hi
> > >
> > > On Thu, May 4, 2017 at 5:41 PM Igor Mammedov <imammedo@redhat.com>  
> > wrote:  
> > >  
> > > > On Tue, 02 May 2017 19:03:15 +0000
> > > > Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> > > >  
> > > > > Hi
> > > > >
> > > > > On Tue, May 2, 2017 at 11:17 AM Igor Mammedov <imammedo@redhat.com>  
> > > > wrote:  
> > > > >  
> > > > > > On Fri, 28 Apr 2017 14:28:38 +0000
> > > > > > Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> > > > > >  
> > > > > > > Hi
> > > > > > >
> > > > > > > On Fri, Apr 28, 2017 at 6:12 PM Ladi Prosek <lprosek@redhat.com>  
> > > > wrote:  
> > > > > > >  
> > > > > > > > On Mon, Apr 24, 2017 at 3:03 PM, Marc-André Lureau
> > > > > > > > <marcandre.lureau@redhat.com> wrote:  
> > > > > > > > > The VM coreinfo (vmcoreinfo) device is an emulated device  
> > which  
> > > > > > > > > exposes a 4k memory range to the guest to store various  
> > > > informations  
> > > > > > > > > useful to debug the guest OS. (it is greatly inspired by the  
> > > > VMGENID  
> > > > > > > > > device implementation)
> > > > > > > > >
> > > > > > > > > This is an early-boot alternative to the qemu-ga VMDUMP_INFO  
> > > > event  
> > > > > > > > > proposed in "[PATCH 00/21] WIP: dump: add kaslr support".
> > > > > > > > >
> > > > > > > > > If deemed more appropriate, we can consider writing to fw_cfg  
> > > > > > directly  
> > > > > > > > > instead of guest memory, now that qemu 2.9 supports it again.
> > > > > > > > >
> > > > > > > > > The proof-of-concept kernel module:
> > > > > > > > >  
> > > > https://github.com/elmarco/vmgenid-test/blob/master/qemuvmci-test.c  
> > > > > > > >
> > > > > > > > Here's a proof-of-concept Windows driver:
> > > > > > > >
> > > > > > > >  
> > > > > >  
> > > >  
> > https://github.com/ladipro/kvm-guest-drivers-windows/tree/vmcoreinfo/vmcoreinfo  
> > > > > > > >
> > > > > > > > I just wanted to be sure that it's possible to evaluate the  
> > ADDR  
> > > > > > > > method in Windows.
> > > > > > > >
> > > > > > > > From a practical point of view it is unfortunate that this  
> > would  
> > > > be a  
> > > > > > > > completely new device. For Windows guests it means another  
> > driver  
> > > > > > > > binary and all the overhead associated with deploying it on  
> > VMs.  
> > > > Would  
> > > > > > > > it be too crazy to add this functionality to the pvpanic  
> > device?  
> > > > The  
> > > > > > > > mechanics could stay the same but it would be done under the  
> > > > existing  
> > > > > > > > ACPI\QEMU0001 device.
> > > > > > > >  
> > > > > > >
> > > > > > > pvpanic is under _SB.PCI0.ISA, that could be problematic
> > > > > > >
> > > > > > > and _STA is a name field.
> > > > > > >
> > > > > > > Someone with more experience with ACPI could tell us if that make  
> > > > sense  
> > > > > > to  
> > > > > > > merge both and how.
> > > > > > >
> > > > > > > Can't you handle 2 ACPI devices in the same windows driver  
> > instead?  
> > > > > > we use QEMU0001 to reserve IO ports for pvpanic device,
> > > > > > ASL wise there shouldn't problems with adding _ADDR method to it
> > > > > >
> > > > > > but then we probably should fold vmcoreinfo into pvpanic device
> > > > > > as well (QEMU and linux driver)
> > > > > >
> > > > > >  
> > > > > pvpanic is x86-only afaict.  
> > > > There is nothing that forces it to be x86 specific (beside being ISA
> > > > device),
> > > > ARM also can benefit from/use pvpanic if you make it pci device or just
> > > > plain
> > > > Device.
> > > >  
> > >
> > > Could someone advise on a recommended solution here?
> > >
> > > Should we make a PCI variant of pvpanic and add the proposed vmcoreinfo
> > > ACPI/mem to it ?
> > >
> > > Tbh, I think I prefer to have seperate devices since they work  
> > differently  
> > > and vmcoreinfo doesn't require any bus device.  
> > Considering pvpanic and vmcore are closely related, I'd prefer
> > it being one device. But I won't insist on it.
> >
> > Also instead of PCI device, I'd use plain Device as vmgenid does,
> > that would leave open question how to make legacy ISA based pvpanic
> > on top of it in case both are merged.
> >
> >  
> Can we instead try to fit pvpanic functionality in the proposed vmcoreinfo
> device? This could be done in a future revision of the device. This way we
> don't have to deal with legacy ISA, and can make pvpanic functionality more
> portable.
it would complicate host/guest side, as there will be 2 drivers per OS and later
device will have to support capability negotiation/reporting to enable
pvpanic functionality in vmcoreinfo + corresponding changes in guest driver.
That might lead to a bigger maintenance burden than if vmcoreinfo were
integrated with pvpanic from the very beginning.


> Tbh, I have no idea how to do the merging of the legacy ISA pvpanic with
> vmcoreinfo without breaking things, nor do I have the motivation to do so.
> 
> More comments welcome (I would like to resend the patch as non-rfc this
> time, I hope we can make it in 2.10)
> 
> Thanks
>