Wei Wang2
2009-Jul-20 14:03 UTC
[Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices
Hi, When guest code tries to get the block size of mmio, it will write all "1"s into pci bar register and then qemu will return all "0"s to the don''t care bits in the emulated bar register to indicate the block size to guest code. In this case, we should not create p2m mapping in pt_bar_reg_write() and pt_exp_rom_bar_reg_write(). Attached patch fixes this issue, additional comment can be found in the patch. Thanks, Wei Signed-off-by: Wei Wang <wei.wang2@amd.com> -- AMD GmbH, Germany Operating System Research Center Legal Information: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34 85609 Dornach b. München Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis München Registergericht München, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jiang, Yunhong
2009-Jul-20 14:26 UTC
RE: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices
I assume guest should disable the MMIO in PCI_COMMAND before writing all "1"s to bar register. Otherwise, what will happen on native if guest try to access 0xFFFFFFF0? And if we do update the P2M, will it cause trouble to Xen HV? Thanks Yunhong Jiang xen-devel-bounces@lists.xensource.com wrote:> Hi, > When guest code tries to get the block size of mmio, it will write > all "1"s into pci bar register and then qemu will return all "0"s to > the don''t care > bits in the emulated bar register to indicate the block size > to guest code. > In this case, we should not create p2m mapping in > pt_bar_reg_write() and > pt_exp_rom_bar_reg_write(). Attached patch fixes this issue, > additional comment can be found in the patch. > > Thanks, > Wei > > Signed-off-by: Wei Wang <wei.wang2@amd.com> > -- > AMD GmbH, Germany > Operating System Research Center > > Legal Information: > Advanced Micro Devices GmbH > Karl-Hammerschmidt-Str. 34 > 85609 Dornach b. München > > Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni > Sitz: Dornach, Gemeinde Aschheim, Landkreis München > Registergericht München, HRB Nr. 43632_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei Wang2
2009-Jul-20 14:47 UTC
Re: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices
Hi Yunhong My testing shows that Linux guests will try to probe pci bar with MMIO being enabled. When I assign a broadcom NIC with 32MB MMIO a Linux guest, guest will hang after remapping a guest address "0xfe000000" to physical mmio. However, windows and BSD guests do not have this issue. They alway probe mmio size after disabling mmio. Thanks, Wei On Monday 20 July 2009 16:26:00 Jiang, Yunhong wrote:> I assume guest should disable the MMIO in PCI_COMMAND before writing all > "1"s to bar register. Otherwise, what will happen on native if guest try to > access 0xFFFFFFF0? And if we do update the P2M, will it cause trouble to > Xen HV? > > Thanks > Yunhong Jiang > > xen-devel-bounces@lists.xensource.com wrote: > > Hi, > > When guest code tries to get the block size of mmio, it will write > > all "1"s into pci bar register and then qemu will return all "0"s to > > the don''t care > > bits in the emulated bar register to indicate the block size > > to guest code. > > In this case, we should not create p2m mapping in > > pt_bar_reg_write() and > > pt_exp_rom_bar_reg_write(). Attached patch fixes this issue, > > additional comment can be found in the patch. > > > > Thanks, > > Wei > > > > Signed-off-by: Wei Wang <wei.wang2@amd.com> > > -- > > AMD GmbH, Germany > > Operating System Research Center > > > > Legal Information: > > Advanced Micro Devices GmbH > > Karl-Hammerschmidt-Str. 34 > > 85609 Dornach b. München > > > > Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni > > Sitz: Dornach, Gemeinde Aschheim, Landkreis München > > Registergericht München, HRB Nr. 43632_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2009-Jul-20 14:48 UTC
Re: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices
Are you questioning whether this patch should be applied? -- Keir On 20/07/2009 15:26, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote:> I assume guest should disable the MMIO in PCI_COMMAND before writing all "1"s > to bar register. Otherwise, what will happen on native if guest try to access > 0xFFFFFFF0? > And if we do update the P2M, will it cause trouble to Xen HV? > > Thanks > Yunhong Jiang > > xen-devel-bounces@lists.xensource.com wrote: >> Hi, >> When guest code tries to get the block size of mmio, it will write >> all "1"s into pci bar register and then qemu will return all "0"s to >> the don''t care >> bits in the emulated bar register to indicate the block size >> to guest code. >> In this case, we should not create p2m mapping in >> pt_bar_reg_write() and >> pt_exp_rom_bar_reg_write(). Attached patch fixes this issue, >> additional comment can be found in the patch. >> >> Thanks, >> Wei >> >> Signed-off-by: Wei Wang <wei.wang2@amd.com> >> -- >> AMD GmbH, Germany >> Operating System Research Center >> >> Legal Information: >> Advanced Micro Devices GmbH >> Karl-Hammerschmidt-Str. 34 >> 85609 Dornach b. München >> >> Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni >> Sitz: Dornach, Gemeinde Aschheim, Landkreis München >> Registergericht München, HRB Nr. 43632 > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jiang, Yunhong
2009-Jul-20 15:48 UTC
RE: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices
Wei Wang2 wrote:> Hi Yunhong > My testing shows that Linux guests will try to probe pci bar with > MMIO being enabled. When I assign a broadcom NIC with 32MB MMIO a > Linux guest, guest will hang after remapping a guest addressYes, I remember I saw this bug in kernel before. I suspect the issue here is the local APIC address, which should not be intercepted before MMIO/RAM address in native. i.e. the p2m table should not cover the local APIC address as RAM, but I think your change in the qemu side is more straightforward (otherwise, we may need consider IOAPIC, HPET etc, which is complex). One thing left is, why it will hang, after all, guest will try to restore the BAR address later, and at that time, the local apic access can be intercepted again. --jyh> "0xfe000000" to physical mmio. However, windows and BSD guests do not > have this issue. They alway probe mmio size after disabling mmio. > Thanks, > Wei > > On Monday 20 July 2009 16:26:00 Jiang, Yunhong wrote: >> I assume guest should disable the MMIO in PCI_COMMAND before writing >> all "1"s to bar register. Otherwise, what will happen on native if >> guest try to access 0xFFFFFFF0? And if we do update the P2M, will it >> cause trouble to Xen HV? >> >> Thanks >> Yunhong Jiang >> >> xen-devel-bounces@lists.xensource.com wrote: >>> Hi, >>> When guest code tries to get the block size of mmio, it will write >>> all "1"s into pci bar register and then qemu will return all "0"s >>> to the don''t care bits in the emulated bar register to indicate the >>> block size >>> to guest code. >>> In this case, we should not create p2m mapping in >>> pt_bar_reg_write() and >>> pt_exp_rom_bar_reg_write(). Attached patch fixes this issue, >>> additional comment can be found in the patch. >>> >>> Thanks, >>> Wei >>> >>> Signed-off-by: Wei Wang <wei.wang2@amd.com> >>> -- >>> AMD GmbH, Germany >>> Operating System Research Center >>> >>> Legal Information: >>> Advanced Micro Devices GmbH >>> Karl-Hammerschmidt-Str. 34 >>> 85609 Dornach b. München >>> >>> Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni >>> Sitz: Dornach, Gemeinde Aschheim, Landkreis München >>> Registergericht München, HRB Nr. 43632_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jiang, Yunhong
2009-Jul-20 15:49 UTC
RE: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices
I''m just wondering if this is really needed. See my response to Wei Wang. Thanks --jyh Keir Fraser wrote:> Are you questioning whether this patch should be applied? > > -- Keir > > On 20/07/2009 15:26, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote: > >> I assume guest should disable the MMIO in PCI_COMMAND before writing >> all "1"s to bar register. Otherwise, what will happen on native if >> guest try to access 0xFFFFFFF0? And if we do update the P2M, will it >> cause trouble to Xen HV? >> >> Thanks >> Yunhong Jiang >> >> xen-devel-bounces@lists.xensource.com wrote: >>> Hi, >>> When guest code tries to get the block size of mmio, it will write >>> all "1"s into pci bar register and then qemu will return all "0"s >>> to the don''t care bits in the emulated bar register to indicate the >>> block size >>> to guest code. >>> In this case, we should not create p2m mapping in >>> pt_bar_reg_write() and >>> pt_exp_rom_bar_reg_write(). Attached patch fixes this issue, >>> additional comment can be found in the patch. >>> >>> Thanks, >>> Wei >>> >>> Signed-off-by: Wei Wang <wei.wang2@amd.com> >>> -- >>> AMD GmbH, Germany >>> Operating System Research Center >>> >>> Legal Information: >>> Advanced Micro Devices GmbH >>> Karl-Hammerschmidt-Str. 34 >>> 85609 Dornach b. München >>> >>> Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni >>> Sitz: Dornach, Gemeinde Aschheim, Landkreis München >>> Registergericht München, HRB Nr. 43632 >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei Wang2
2009-Jul-20 16:29 UTC
Re: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices
Hi Yunhong Thanks for the comment. Regarding the reason of guest hang, my guess is: guest OS might be trying to access guest memory at "0xfe000000" after remapping this address to mmio and guest memory may be corrupted after this access. For devices with large mmio, the complement code of mmio block size is more likely to be a real guest RAM. I saw the same issue when I assigned a graphic device with 256MB mmio to Linux guest. But devices with small mmio seems to work well. I did not see the problem on a broadcom NIC with 64K mmio. Thanks, Wei On Monday 20 July 2009 17:48:46 Jiang, Yunhong wrote:> Wei Wang2 wrote: > > Hi Yunhong > > My testing shows that Linux guests will try to probe pci bar with > > MMIO being enabled. When I assign a broadcom NIC with 32MB MMIO a > > Linux guest, guest will hang after remapping a guest address > > Yes, I remember I saw this bug in kernel before. > > I suspect the issue here is the local APIC address, which should not be > intercepted before MMIO/RAM address in native. i.e. the p2m table should > not cover the local APIC address as RAM, but I think your change in the > qemu side is more straightforward (otherwise, we may need consider IOAPIC, > HPET etc, which is complex). > > One thing left is, why it will hang, after all, guest will try to restore > the BAR address later, and at that time, the local apic access can be > intercepted again. > > --jyh > > > "0xfe000000" to physical mmio. However, windows and BSD guests do not > > have this issue. They alway probe mmio size after disabling mmio. > > Thanks, > > Wei > > > > On Monday 20 July 2009 16:26:00 Jiang, Yunhong wrote: > >> I assume guest should disable the MMIO in PCI_COMMAND before writing > >> all "1"s to bar register. Otherwise, what will happen on native if > >> guest try to access 0xFFFFFFF0? And if we do update the P2M, will it > >> cause trouble to Xen HV? > >> > >> Thanks > >> Yunhong Jiang > >> > >> xen-devel-bounces@lists.xensource.com wrote: > >>> Hi, > >>> When guest code tries to get the block size of mmio, it will write > >>> all "1"s into pci bar register and then qemu will return all "0"s > >>> to the don''t care bits in the emulated bar register to indicate the > >>> block size > >>> to guest code. > >>> In this case, we should not create p2m mapping in > >>> pt_bar_reg_write() and > >>> pt_exp_rom_bar_reg_write(). Attached patch fixes this issue, > >>> additional comment can be found in the patch. > >>> > >>> Thanks, > >>> Wei > >>> > >>> Signed-off-by: Wei Wang <wei.wang2@amd.com> > >>> -- > >>> AMD GmbH, Germany > >>> Operating System Research Center > >>> > >>> Legal Information: > >>> Advanced Micro Devices GmbH > >>> Karl-Hammerschmidt-Str. 34 > >>> 85609 Dornach b. München > >>> > >>> Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni > >>> Sitz: Dornach, Gemeinde Aschheim, Landkreis München > >>> Registergericht München, HRB Nr. 43632_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kay, Allen M
2009-Jul-20 23:39 UTC
RE: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices
Hi Wei, I thought the originally code already handles sizing of the BAR operations (writing all 1''s and reading the size back). How did it work before? Did it work because we are relying on the a well behaved guest to restore the original value of the BAR after the sizing operation? By reading the code, it is not obvious to me how does not doing r->addr = cfg_entry->data operation prevents calling of pt_bar_mapping_one() to create p2m mapping. Allen -----Original Message----- From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Wei Wang2 Sent: Monday, July 20, 2009 7:04 AM To: xen-devel@lists.xensource.com Subject: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices Hi, When guest code tries to get the block size of mmio, it will write all "1"s into pci bar register and then qemu will return all "0"s to the don''t care bits in the emulated bar register to indicate the block size to guest code. In this case, we should not create p2m mapping in pt_bar_reg_write() and pt_exp_rom_bar_reg_write(). Attached patch fixes this issue, additional comment can be found in the patch. Thanks, Wei Signed-off-by: Wei Wang <wei.wang2@amd.com> -- AMD GmbH, Germany Operating System Research Center Legal Information: Advanced Micro Devices GmbH Karl-Hammerschmidt-Str. 34 85609 Dornach b. München Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni Sitz: Dornach, Gemeinde Aschheim, Landkreis München Registergericht München, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jiang, Yunhong
2009-Jul-21 09:06 UTC
RE: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices
Thanks for your reply very much. BTW, I''m not object the patch , the idea of the patch is quite straightforward. In kernel side, the issue has been discussed a lot ( e.g http://kerneltrap.org/mailarchive/linux-kernel/2007/8/28/165632). The main challenge in kernel side is MMCFG (maybe because the MMIO for display is intercepted before MMCFG?? I''m not sure). So if the issue is caused by geust memory access, it may cause issue on native also. I think the 64K mmio is ok because it will change mapping to 0xFFxxxxx, which is higher than APIC/HPET range. I suspect 32M is the minimal size that will cause problem. Anyway, this is only for some clarification and should not matter much. Thanks Yunhong Jiang Wei Wang2 wrote:> Hi Yunhong > Thanks for the comment. Regarding the reason of guest hang, my > guess is: guest > OS might be trying to access guest memory at "0xfe000000" > after remapping > this address to mmio and guest memory may be corrupted after > this access. > For devices with large mmio, the complement code of mmio block > size is more > likely to be a real guest RAM. I saw the same issue when I > assigned a graphic > device with 256MB mmio to Linux guest. But devices with small > mmio seems to > work well. I did not see the problem on a broadcom NIC with 64K mmio. > Thanks, Wei > > On Monday 20 July 2009 17:48:46 Jiang, Yunhong wrote: >> Wei Wang2 wrote: >>> Hi Yunhong >>> My testing shows that Linux guests will try to probe pci bar with >>> MMIO being enabled. When I assign a broadcom NIC with 32MB MMIO a >>> Linux guest, guest will hang after remapping a guest address >> >> Yes, I remember I saw this bug in kernel before. >> >> I suspect the issue here is the local APIC address, which should not >> be intercepted before MMIO/RAM address in native. i.e. the p2m table >> should not cover the local APIC address as RAM, but I think your >> change in the qemu side is more straightforward (otherwise, we may >> need consider IOAPIC, HPET etc, which is complex). >> >> One thing left is, why it will hang, after all, guest will try to >> restore the BAR address later, and at that time, the local apic >> access can be intercepted again. >> >> --jyh >> >>> "0xfe000000" to physical mmio. However, windows and BSD guests do >>> not have this issue. They alway probe mmio size after disabling >>> mmio. Thanks, Wei >>> >>> On Monday 20 July 2009 16:26:00 Jiang, Yunhong wrote: >>>> I assume guest should disable the MMIO in PCI_COMMAND before >>>> writing all "1"s to bar register. Otherwise, what will happen on >>>> native if guest try to access 0xFFFFFFF0? And if we do update the >>>> P2M, will it cause trouble to Xen HV? >>>> >>>> Thanks >>>> Yunhong Jiang >>>> >>>> xen-devel-bounces@lists.xensource.com wrote: >>>>> Hi, >>>>> When guest code tries to get the block size of mmio, it will write >>>>> all "1"s into pci bar register and then qemu will return all "0"s >>>>> to the don''t care bits in the emulated bar register to indicate >>>>> the block size to guest code. >>>>> In this case, we should not create p2m mapping in >>>>> pt_bar_reg_write() and >>>>> pt_exp_rom_bar_reg_write(). Attached patch fixes this issue, >>>>> additional comment can be found in the patch. >>>>> >>>>> Thanks, >>>>> Wei >>>>> >>>>> Signed-off-by: Wei Wang <wei.wang2@amd.com> >>>>> -- >>>>> AMD GmbH, Germany >>>>> Operating System Research Center >>>>> >>>>> Legal Information: >>>>> Advanced Micro Devices GmbH >>>>> Karl-Hammerschmidt-Str. 34 >>>>> 85609 Dornach b. München >>>>> >>>>> Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni >>>>> Sitz: Dornach, Gemeinde Aschheim, Landkreis München >>>>> Registergericht München, HRB Nr. 43632_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei Wang2
2009-Jul-21 09:57 UTC
Re: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices
Hi Allen, Pls find my explanations inline.> I thought the originally code already handles sizing of the BAR operations > (writing all 1''s and reading the size back). How did it work before? Did it > work because we are relying on the a well behaved guest to restore the > original value of the BAR after the sizing operation?In early qemu, remapping mmio was only allowed by pt_cmd_reg_write(). But currently, guest code can also trigger mmio remapping from pt_bar_reg_write() and pt_exp_rom_bar_reg_write() and this will cause the problem.> By reading the code, it is not obvious to me how does not doing r->addr > cfg_entry->data operation prevents calling of pt_bar_mapping_one() to > create p2m mapping.Guest OS probes pci bar after guest bios doing this, so r->addr will still have the old mmio base address assigned by guest bios before guest OS writing ''1''s. If we prevent r->addr from being updated by cfg_entry->data, pt_bar_mapping_one() will not trigger any actual p2m updates because it will check whether r->addr has already been changed before calling r->map_func(). Thanks, Wei> Allen > > -----Original Message----- > From: xen-devel-bounces@lists.xensource.com > [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Wei Wang2 Sent: > Monday, July 20, 2009 7:04 AM > To: xen-devel@lists.xensource.com > Subject: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru > devices > > Hi, > When guest code tries to get the block size of mmio, it will write all "1"s > into pci bar register and then qemu will return all "0"s to the don''t care > bits in the emulated bar register to indicate the block size to guest code. > In this case, we should not create p2m mapping in pt_bar_reg_write() and > pt_exp_rom_bar_reg_write(). Attached patch fixes this issue, additional > comment can be found in the patch. > > Thanks, > Wei > > Signed-off-by: Wei Wang <wei.wang2@amd.com> > -- > AMD GmbH, Germany > Operating System Research Center > > Legal Information: > Advanced Micro Devices GmbH > Karl-Hammerschmidt-Str. 34 > 85609 Dornach b. München > > Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni > Sitz: Dornach, Gemeinde Aschheim, Landkreis München > Registergericht München, HRB Nr. 43632 > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2009-Jul-21 14:31 UTC
RE: [Xen-devel] [PATCH] passthru: Fix pci bar remapping for passthru devices
Jiang, Yunhong writes ("RE: [Xen-devel] [PATCH] passthru: Fix pci bar
remapping for passthru devices"):> Thanks for your reply very much. BTW, I''m not object the patch ,
the
> idea of the patch is quite straightforward.
Right, good, I''ll take that as an Ack and apply it.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel