From: Jes Sorensen <Jes.Sorensen@redhat.com> To: "Kevin O'Connor" <kevin@koconnor.net> Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, Avi Kivity <avi@redhat.com>, Anthony Liguori <aliguori@us.ibm.com>, seabios@seabios.org, Alexander Graf <agraf@suse.de>, Gleb Natapov <gleb@redhat.com> Subject: Re: [PATCH] Seabios - read e820 table from qemu_cfg Date: Fri, 29 Jan 2010 10:03:55 +0100 [thread overview] Message-ID: <4B62A47B.5070807@redhat.com> (raw) In-Reply-To: <20100128043917.GA3623@morn.localdomain> On 01/28/10 05:39, Kevin O'Connor wrote: > I think defining accessor functions for every piece of data passed > through qemu-cfg interface is going to get tiring. I'd prefer to > extend the existing qemu-cfg "file" interface for new content. > > For example, add a helper with something like: > > int qemu_cfg_get_file(const char *name, void *dest, int maxsize); Hi Kevin, I think switching qemu_cfg to use a file name based interface would be a nice feature, but I think it should be independent of this patch. I am CC'ing Gleb on this as he did the original design I believe. >> - if (kvm_para_available()) >> - // 4 pages before the bios, 3 pages for vmx tss pages, the >> - // other page for EPT real mode pagetable >> - add_e820(0xfffbc000, 4*4096, E820_RESERVED); >> + if (kvm_para_available()) { >> + u32 count; >> + >> + count = qemu_cfg_e820_entries(); >> + if (count) { >> + struct e820_entry entry; >> + int i; >> + >> + for (i = 0; i< count; i++) { >> + qemu_cfg_e820_load_next(&entry); >> + add_e820(entry.address, entry.length, entry.type); >> + } > > and then this becomes: > > struct e820entry map[128]; > int len = qemu_cfg_get_file("e820map",&map, sizeof(map)); > if (len>= 0) > for (i=0; i<len / sizeof(map[0]); i++) > add_e820(map[i].start, map[i].size, map[i].type); > > The advantage being that it should be possible to write one set of > helper functions in both qemu and seabios that can then be used to > pass arbitrary content. The only issue here is that I designed the Seabios portion to not rely on the size of the struct, to avoid having to statically reserve it like in your example. Having the qemu_cfg_get_file() function return a pointer to a file descriptor and then have a qemu_cfg_read() helper that takes the descriptor as it's first argument would avoid this problem. > As a side note, it should probably do the e820 map check even for qemu > users (ie, not just kvm). Ah I didn't realize Seabios would try to use the fw_cfg interface if it wasn't running on top of QEMU. That would be good to do. Cheers, Jes
WARNING: multiple messages have this Message-ID (diff)
From: Jes Sorensen <Jes.Sorensen@redhat.com> To: Kevin O'Connor <kevin@koconnor.net> Cc: Anthony Liguori <aliguori@us.ibm.com>, Gleb Natapov <gleb@redhat.com>, kvm@vger.kernel.org, seabios@seabios.org, Alexander Graf <agraf@suse.de>, qemu-devel@nongnu.org, Avi Kivity <avi@redhat.com> Subject: [Qemu-devel] Re: [PATCH] Seabios - read e820 table from qemu_cfg Date: Fri, 29 Jan 2010 10:03:55 +0100 [thread overview] Message-ID: <4B62A47B.5070807@redhat.com> (raw) In-Reply-To: <20100128043917.GA3623@morn.localdomain> On 01/28/10 05:39, Kevin O'Connor wrote: > I think defining accessor functions for every piece of data passed > through qemu-cfg interface is going to get tiring. I'd prefer to > extend the existing qemu-cfg "file" interface for new content. > > For example, add a helper with something like: > > int qemu_cfg_get_file(const char *name, void *dest, int maxsize); Hi Kevin, I think switching qemu_cfg to use a file name based interface would be a nice feature, but I think it should be independent of this patch. I am CC'ing Gleb on this as he did the original design I believe. >> - if (kvm_para_available()) >> - // 4 pages before the bios, 3 pages for vmx tss pages, the >> - // other page for EPT real mode pagetable >> - add_e820(0xfffbc000, 4*4096, E820_RESERVED); >> + if (kvm_para_available()) { >> + u32 count; >> + >> + count = qemu_cfg_e820_entries(); >> + if (count) { >> + struct e820_entry entry; >> + int i; >> + >> + for (i = 0; i< count; i++) { >> + qemu_cfg_e820_load_next(&entry); >> + add_e820(entry.address, entry.length, entry.type); >> + } > > and then this becomes: > > struct e820entry map[128]; > int len = qemu_cfg_get_file("e820map",&map, sizeof(map)); > if (len>= 0) > for (i=0; i<len / sizeof(map[0]); i++) > add_e820(map[i].start, map[i].size, map[i].type); > > The advantage being that it should be possible to write one set of > helper functions in both qemu and seabios that can then be used to > pass arbitrary content. The only issue here is that I designed the Seabios portion to not rely on the size of the struct, to avoid having to statically reserve it like in your example. Having the qemu_cfg_get_file() function return a pointer to a file descriptor and then have a qemu_cfg_read() helper that takes the descriptor as it's first argument would avoid this problem. > As a side note, it should probably do the e820 map check even for qemu > users (ie, not just kvm). Ah I didn't realize Seabios would try to use the fw_cfg interface if it wasn't running on top of QEMU. That would be good to do. Cheers, Jes
next prev parent reply other threads:[~2010-01-29 9:04 UTC|newest] Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top 2010-01-25 16:46 [PATCH] Seabios - read e820 reserve from qemu_cfg Jes Sorensen 2010-01-25 16:46 ` [Qemu-devel] " Jes Sorensen 2010-01-25 16:49 ` [PATCH] QEMU-KVM - provide e820 reserve through qemu_cfg Jes Sorensen 2010-01-25 16:49 ` [Qemu-devel] " Jes Sorensen 2010-01-25 16:52 ` [PATCH] QEMU " Jes Sorensen 2010-01-25 16:52 ` [Qemu-devel] " Jes Sorensen 2010-01-25 16:58 ` Alexander Graf 2010-01-25 16:58 ` [Qemu-devel] " Alexander Graf 2010-01-25 17:13 ` Jes Sorensen 2010-01-25 17:13 ` [Qemu-devel] " Jes Sorensen 2010-01-25 17:28 ` Alexander Graf 2010-01-25 17:28 ` [Qemu-devel] " Alexander Graf 2010-01-25 17:46 ` Jes Sorensen 2010-01-25 17:46 ` [Qemu-devel] " Jes Sorensen 2010-01-25 20:04 ` Alexander Graf 2010-01-25 20:04 ` [Qemu-devel] " Alexander Graf 2010-01-25 20:14 ` Anthony Liguori 2010-01-25 20:14 ` [Qemu-devel] " Anthony Liguori 2010-01-25 21:05 ` Jes Sorensen 2010-01-25 21:05 ` [Qemu-devel] " Jes Sorensen 2010-01-25 21:08 ` Alexander Graf 2010-01-25 21:08 ` [Qemu-devel] " Alexander Graf 2010-01-25 21:24 ` Jes Sorensen 2010-01-25 21:24 ` [Qemu-devel] " Jes Sorensen 2010-01-26 6:46 ` Gleb Natapov 2010-01-26 6:46 ` [Qemu-devel] " Gleb Natapov 2010-01-26 8:36 ` Jes Sorensen 2010-01-26 8:36 ` [Qemu-devel] " Jes Sorensen 2010-01-26 0:24 ` [PATCH] Seabios - read e820 reserve from qemu_cfg Kevin O'Connor 2010-01-26 0:24 ` [Qemu-devel] " Kevin O'Connor 2010-01-26 21:52 ` [PATCH] Seabios - read e820 table " Jes Sorensen 2010-01-26 21:52 ` [Qemu-devel] " Jes Sorensen 2010-01-26 21:53 ` [PATCH] QEMU-KVM - provide e820 table via fw_cfg Jes Sorensen 2010-01-26 21:53 ` [Qemu-devel] " Jes Sorensen 2010-01-26 21:55 ` Alexander Graf 2010-01-26 21:55 ` [Qemu-devel] " Alexander Graf 2010-01-28 4:39 ` [PATCH] Seabios - read e820 table from qemu_cfg Kevin O'Connor 2010-01-28 4:39 ` [Qemu-devel] " Kevin O'Connor 2010-01-29 9:03 ` Jes Sorensen [this message] 2010-01-29 9:03 ` Jes Sorensen 2010-01-29 16:08 ` Gleb Natapov 2010-01-29 16:08 ` [Qemu-devel] " Gleb Natapov 2010-01-30 3:35 ` Kevin O'Connor 2010-01-30 3:35 ` [Qemu-devel] " Kevin O'Connor 2010-02-08 10:31 ` Jes Sorensen 2010-02-08 10:31 ` [Qemu-devel] " Jes Sorensen 2010-02-14 3:16 ` Kevin O'Connor 2010-02-14 3:16 ` [Qemu-devel] " Kevin O'Connor
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4B62A47B.5070807@redhat.com \ --to=jes.sorensen@redhat.com \ --cc=agraf@suse.de \ --cc=aliguori@us.ibm.com \ --cc=avi@redhat.com \ --cc=gleb@redhat.com \ --cc=kevin@koconnor.net \ --cc=kvm@vger.kernel.org \ --cc=qemu-devel@nongnu.org \ --cc=seabios@seabios.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.