All of lore.kernel.org
 help / color / mirror / Atom feed
* The issue of booting windows guest
@ 2008-05-22  9:37 Xu, Dongxiao
  2008-05-22  9:44 ` Keir Fraser
  0 siblings, 1 reply; 4+ messages in thread
From: Xu, Dongxiao @ 2008-05-22  9:37 UTC (permalink / raw)
  To: Keir Fraser, xen-devel; +Cc: Zhang, Li

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: The issue of booting windows guest
  2008-05-22  9:37 The issue of booting windows guest Xu, Dongxiao
@ 2008-05-22  9:44 ` Keir Fraser
  2008-05-22 10:11   ` Keir Fraser
  0 siblings, 1 reply; 4+ messages in thread
From: Keir Fraser @ 2008-05-22  9:44 UTC (permalink / raw)
  To: Xu, Dongxiao, xen-devel; +Cc: Zhang, Li




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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Re: The issue of booting windows guest
  2008-05-22  9:44 ` Keir Fraser
@ 2008-05-22 10:11   ` Keir Fraser
  2008-05-22 11:01     ` Samuel Thibault
  0 siblings, 1 reply; 4+ messages in thread
From: Keir Fraser @ 2008-05-22 10:11 UTC (permalink / raw)
  To: Xu, Dongxiao, xen-devel; +Cc: Zhang, Li, Samuel Thibault

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Re: The issue of booting windows guest
  2008-05-22 10:11   ` Keir Fraser
@ 2008-05-22 11:01     ` Samuel Thibault
  0 siblings, 0 replies; 4+ messages in thread
From: Samuel Thibault @ 2008-05-22 11:01 UTC (permalink / raw)
  To: Keir Fraser; +Cc: Xu, Dongxiao, xen-devel, Zhang, Li

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;

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-05-22 11:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-22  9:37 The issue of booting windows guest Xu, Dongxiao
2008-05-22  9:44 ` Keir Fraser
2008-05-22 10:11   ` Keir Fraser
2008-05-22 11:01     ` Samuel Thibault

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.