On Tue, Jul 18, 2017 at 6:18 PM, Marc-André Lureau <marcandre.lur...@gmail.com> wrote: > Hi > > On Tue, Jul 18, 2017 at 6:05 PM Ladi Prosek <lpro...@redhat.com> wrote: >> >> >> > I would like to hear from Ladi how he intended to use the device in >> > the future, and if he would also prefer ACPI methods and what those >> > methods should be. >> >> We should be able to drive pretty much anything from Windows. I wrote >> a dummy driver for your earlier prototype just to be sure that ACPI >> methods are fine, as I had not done or seen that before. >> >> There are constraints which may be unique to Windows, though. If the >> dump-support data is kept in guest-allocated memory, the guest must be >> able to revoke it (set / call the method with NULL PA?) because >> Windows drivers must free everything on unload. Unlike Linux, I don't > > > Well, the currently proposed vmcoreinfo device has a 4k memory region to put > anything you want, Windows shouldn't be allowed to touch it directly (e820 > regions iirc)
Got it. And we would likely use it to put just addr/size there to make updates atomic. I think we're supposed to update our dump-support data on memory hot-plug for example. >> think we can get a piece of memory at startup and keep it for as long >> as the OS running. It would be flagged as a memory leak. But that >> should be easy to have. Can't really think of anything else. > > > The question is what kind of data you want to give to the host to help with > debug. Is this something that can be as simple as addr/size, or you would > rather have a 4k page to put various things there. The size of the dump header as currently provided by Windows (that's the dump-support data we want to pass to the host) is 4k. So I wouldn't put it directly in the page anyway. That, plus the desire to update the data at least somewhat atomically, would make me prefer a simple addr/size pair I think. >> >> >> >>> >> >>> >> > instead of exporting a physical addess and storing address there. >> >>> >> > This >> >>> >> > way you can add more methods as you add functionality. >> >>> >> >> >>> >> I'm not saying this is a bad idea (especially because I don't fully >> >>> >> understand your point), but I will say that I'm quite sad that you >> >>> >> are >> >>> >> sending Marc-André back to the drawing board after he posted v4 -- >> >>> >> also >> >>> >> invalidating my review efforts. :/ >> >>> >> >> >>> >> Laszlo >> >>> > >> >>> > You are right, I should have looked at this sooner. Early RFC >> >>> > suggested writing into fw cfg directly. I couldn't find any >> >>> > discussion around this - why was this abandoned? >> >>> >> >>> Violation (or rather abuse) of layers iirc >> >> >> >> Hmm. I tried to find discussion about that and failed. It is >> >> available >> >> through QEMU0002 in ACPI - would it be OK if guests went through that? >> > >> > I wouldn't mind, although merging functionality in a single device >> > isn't what I would think of first. I guess Ladi would be happier with >> > a single device. I suppose it shouldn't break drivers if we had >> > memory, io, methods etc to the device? >> >> Yeah, it would be nice if this was part of pvpanic. Even something >> really simple like: >> >> /* The bit of supported pv event */ >> #define PVPANIC_F_PANICKED 0 >> +#define PVPANIC_F_DUMP_INFO_SET 1 > > > QEMU0002 is fw_cfg Ah, sorry, I got confused. >> >> >> - memory_region_init_io(&s->io, OBJECT(s), &pvpanic_ops, s, "pvpanic", >> 1); >> + memory_region_init_io(&s->io, OBJECT(s), &pvpanic_ops, s, >> "pvpanic", 32); // or whatever >> >> The guest writes to two or three registers: PA, size, type?, then sets >> the PVPANIC_F_DUMP_INFO_SET bit. >> >> Although not sure if extending the I/O region is OK. And of course I >> only need this on x86 :p >> > > I would rather have a solution that works on various archs. It's a shame > pvpanic was designed with x86 only in mind imho. Understood. Thanks! > > -- > Marc-André Lureau
