This function (introduced quite a long time ago in
e7911109f4321e9ba0cc56a253b653600aa46bea - "disable qemu PCI
devices in HVM domains") appears to be completely broken, causing
the regression reported in
http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1805 (due to
the newly added caller of it in
56d7747a3cf811910c4cf865e1ebcb8b82502005 - "qemu: clean up
MSI-X table handling"). As it''s unclear how the function can ever
have fulfilled its purpose, and as I''m not seeing the log message
resulting from it being called on the sole original call path with SLE11
guests, I''d like to ask you or someone else at Citrix with access to a
suitable guest to double check that the below patch doesn''t break it.
Once verified that it fixes the problem at hand *and* doesn''t break
what it was supposed to be sued for originally, I''ll do a formal
submission.
Thanks, Jan
--- a/i386-dm/exec-dm.c
+++ b/i386-dm/exec-dm.c
@@ -360,7 +360,7 @@ void cpu_unregister_io_memory(int io_tab
int io_index = io_table_address >> IO_MEM_SHIFT;
for (i = 0; i < mmio_cnt; i++) {
- if (mmio[i].size && mmio[i].io_index == io_index) {
+ if (mmio[i].io_index == io_index) {
mmio[i].start = mmio[i].size = 0;
break;
}
@@ -466,12 +466,16 @@ static int iomem_index(target_phys_addr_
void unregister_iomem(target_phys_addr_t start)
{
- int index = iomem_index(start);
- if (index) {
+ unsigned int index;
+
+ for (index = 0; index < mmio_cnt; index++)
+ if (start == mmio[index].start)
+ break;
+ if (index < mmio_cnt) {
fprintf(logfile, "squash iomem [%lx, %lx).\n",
(unsigned long)(mmio[index].start),
(unsigned long)(mmio[index].start + mmio[index].size));
- mmio[index].start = mmio[index].size = 0;
+ mmio[index].size = 0;
}
}
On Tue, 31 Jan 2012, Jan Beulich wrote:> This function (introduced quite a long time ago in > e7911109f4321e9ba0cc56a253b653600aa46bea - "disable qemu PCI > devices in HVM domains") appears to be completely broken, causing > the regression reported in > http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1805 (due to > the newly added caller of it in > 56d7747a3cf811910c4cf865e1ebcb8b82502005 - "qemu: clean up > MSI-X table handling"). As it''s unclear how the function can ever > have fulfilled its purpose, and as I''m not seeing the log message > resulting from it being called on the sole original call path with SLE11 > guests, I''d like to ask you or someone else at Citrix with access to a > suitable guest to double check that the below patch doesn''t break it.Any upstream Linux kernel with PVHVM would work as a testcase. Just make sure you have CONFIG_XEN_PVHVM=y and the appropriate frontends in the kernel config.> Once verified that it fixes the problem at hand *and* doesn''t break > what it was supposed to be sued for originally, I''ll do a formal > submission.I have run a smoke test, here is the interesting part of the logs without this patch: region type 0 at [f3000000,f3020000). squash iomem [f3020000, f3021000). region type 1 at [c100,c140). and now with this patch: region type 0 at [f3000000,f3020000). squash iomem [f3000000, f3020000). region type 1 at [c100,c140).
>>> On 31.01.12 at 15:05, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > I have run a smoke test, here is the interesting part of the logsI take it that it worked?> without this patch: > > > region type 0 at [f3000000,f3020000). > squash iomem [f3020000, f3021000). > region type 1 at [c100,c140). > > > and now with this patch: > > > region type 0 at [f3000000,f3020000). > squash iomem [f3000000, f3020000). > region type 1 at [c100,c140).So it looks consistent now, while it apparently was inconsistent before (and no-one noticed). Jan
On Tue, 31 Jan 2012, Jan Beulich wrote:> >>> On 31.01.12 at 15:05, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > > I have run a smoke test, here is the interesting part of the logs > > I take it that it worked?Yes: the guest booted correctly, no emulated disks or nics present.