Hi, Keir,
In our nightly test, most of the windows guests cannot boot. And
my colleague and I checked this issue and find that this issue is caused
by C/S 17693. Some code in that changeset is:
@@ -196,6 +191,12 @@ static void pci_setup(void)
switch ( class )
{
+ case 0x0300:
+ if ( (vendor_id == 0x1234) && (device_id == 0x1111) )
+ virtual_vga = VGA_std;
+ if ( (vendor_id == 0x1013) && (device_id == 0xb8) )
+ virtual_vga = VGA_cirrus;
+ break;
case 0x0680:
ASSERT((vendor_id == 0x8086) && (device_id == 0x7113));
/*
In "case 0x0300", the code should not break, and it shall execute the
following default code to do memory mappings.
Best Regards,
--Dongxiao
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On 22/5/08 10:37, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:> Hi, Keir, > In our nightly test, most of the windows guests cannot boot. And > my colleague and I checked this issue and find that this issue is caused > by C/S 17693. Some code in that changeset is:Argh. My stupid mistake and an ugly switch statement. :-) -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 22/5/08 10:44, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:> On 22/5/08 10:37, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote: > >> Hi, Keir, >> In our nightly test, most of the windows guests cannot boot. And >> my colleague and I checked this issue and find that this issue is caused >> by C/S 17693. Some code in that changeset is: > > Argh. My stupid mistake and an ugly switch statement. :-)Okay, I fixed it in c/s 17697, but still this should not have caused a hypervisor crash as described in bugzilla bug #1259. There must be some assumption in the vram dirty tracking code that the SVGA BAR is mapped. I''ve asked Samuel to take a look into the underlying hypervisor crash. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Samuel Thibault
2008-May-22 11:01 UTC
Re: [Xen-devel] Re: The issue of booting windows guest
Keir Fraser, le Thu 22 May 2008 11:11:58 +0100, a écrit :> On 22/5/08 10:44, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote: > > > On 22/5/08 10:37, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote: > > > >> Hi, Keir, > >> In our nightly test, most of the windows guests cannot boot. And > >> my colleague and I checked this issue and find that this issue is caused > >> by C/S 17693. Some code in that changeset is: > > > > Argh. My stupid mistake and an ugly switch statement. :-) > > Okay, I fixed it in c/s 17697, but still this should not have caused a > hypervisor crash as described in bugzilla bug #1259. There must be some > assumption in the vram dirty tracking code that the SVGA BAR is mapped.Indeed, here is a patch Samuel shadow: check for gfn_to_mfn returning INVALID_MFN Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com> diff -r ce4fc4e03f8a xen/arch/x86/mm/shadow/common.c --- a/xen/arch/x86/mm/shadow/common.c Thu May 22 11:30:38 2008 +0100 +++ b/xen/arch/x86/mm/shadow/common.c Thu May 22 12:02:22 2008 +0100 @@ -2799,8 +2799,11 @@ if ( !d->dirty_vram ) { /* Just recount from start. */ - for ( i = begin_pfn; i < end_pfn; i++ ) - flush_tlb |= sh_remove_all_mappings(d->vcpu[0], gfn_to_mfn(d, i, &t)); + for ( i = begin_pfn; i < end_pfn; i++ ) { + mfn_t mfn = gfn_to_mfn(d, i, &t); + if (mfn_x(mfn) != INVALID_MFN) + flush_tlb |= sh_remove_all_mappings(d->vcpu[0], mfn); + } gdprintk(XENLOG_INFO, "tracking VRAM %lx - %lx\n", begin_pfn, end_pfn); @@ -2840,61 +2843,70 @@ /* Iterate over VRAM to track dirty bits. */ for ( i = 0; i < nr; i++ ) { mfn_t mfn = gfn_to_mfn(d, begin_pfn + i, &t); - struct page_info *page = mfn_to_page(mfn); - u32 count_info = page->u.inuse.type_info & PGT_count_mask; + struct page_info *page; + u32 count_info; int dirty = 0; paddr_t sl1ma = d->dirty_vram->sl1ma[i]; - switch (count_info) + if (mfn_x(mfn) == INVALID_MFN) { - case 0: - /* No guest reference, nothing to track. */ - break; - case 1: - /* One guest reference. */ - if ( sl1ma == INVALID_PADDR ) + dirty = 1; + } + else + { + page = mfn_to_page(mfn); + count_info = page->u.inuse.type_info & PGT_count_mask; + switch (count_info) { - /* We don''t know which sl1e points to this, too bad. */ - dirty = 1; - /* TODO: Heuristics for finding the single mapping of - * this gmfn */ - flush_tlb |= sh_remove_all_mappings(d->vcpu[0], gfn_to_mfn(d, begin_pfn + i, &t)); - } - else - { - /* Hopefully the most common case: only one mapping, - * whose dirty bit we can use. */ - l1_pgentry_t *sl1e; + case 0: + /* No guest reference, nothing to track. */ + break; + case 1: + /* One guest reference. */ + if ( sl1ma == INVALID_PADDR ) + { + /* We don''t know which sl1e points to this, too bad. */ + dirty = 1; + /* TODO: Heuristics for finding the single mapping of + * this gmfn */ + flush_tlb |= sh_remove_all_mappings(d->vcpu[0], mfn); + } + else + { + /* Hopefully the most common case: only one mapping, + * whose dirty bit we can use. */ + l1_pgentry_t *sl1e; #ifdef __i386__ - void *sl1p = map_sl1p; - unsigned long sl1mfn = paddr_to_pfn(sl1ma); + void *sl1p = map_sl1p; + unsigned long sl1mfn = paddr_to_pfn(sl1ma); - if ( sl1mfn != map_mfn ) { - if ( map_sl1p ) - sh_unmap_domain_page(map_sl1p); - map_sl1p = sl1p = sh_map_domain_page(_mfn(sl1mfn)); - map_mfn = sl1mfn; - } - sl1e = sl1p + (sl1ma & ~PAGE_MASK); + if ( sl1mfn != map_mfn ) { + if ( map_sl1p ) + sh_unmap_domain_page(map_sl1p); + map_sl1p = sl1p = sh_map_domain_page(_mfn(sl1mfn)); + map_mfn = sl1mfn; + } + sl1e = sl1p + (sl1ma & ~PAGE_MASK); #else - sl1e = maddr_to_virt(sl1ma); + sl1e = maddr_to_virt(sl1ma); #endif - if ( l1e_get_flags(*sl1e) & _PAGE_DIRTY ) - { - dirty = 1; - /* Note: this is atomic, so we may clear a - * _PAGE_ACCESSED set by another processor. */ - l1e_remove_flags(*sl1e, _PAGE_DIRTY); - flush_tlb = 1; + if ( l1e_get_flags(*sl1e) & _PAGE_DIRTY ) + { + dirty = 1; + /* Note: this is atomic, so we may clear a + * _PAGE_ACCESSED set by another processor. */ + l1e_remove_flags(*sl1e, _PAGE_DIRTY); + flush_tlb = 1; + } } + break; + default: + /* More than one guest reference, + * we don''t afford tracking that. */ + dirty = 1; + break; } - break; - default: - /* More than one guest reference, - * we don''t afford tracking that. */ - dirty = 1; - break; } if ( dirty ) @@ -2916,8 +2928,11 @@ { /* was clean for more than two seconds, try to disable guest * write access */ - for ( i = begin_pfn; i < end_pfn; i++ ) - flush_tlb |= sh_remove_write_access(d->vcpu[0], gfn_to_mfn(d, i, &t), 1, 0); + for ( i = begin_pfn; i < end_pfn; i++ ) { + mfn_t mfn = gfn_to_mfn(d, i, &t); + if (mfn_x(mfn) != INVALID_MFN) + flush_tlb |= sh_remove_write_access(d->vcpu[0], mfn, 1, 0); + } d->dirty_vram->last_dirty = -1; } rc = 0; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel