cset 8690 blk driver regression

cset 8690 blk driver regression

[Alex Williamson]

Hi Steven,

  I'm seeing a regression on ia64 with this chunk of cset 8690:

--- a/xen/common/grant_table.c	Fri Jan 27 20:38:55 2006
+++ b/xen/common/grant_table.c	Fri Jan 27 20:57:07 2006
@@ -522,9 +522,7 @@
         ASSERT(d->grant_table != NULL);
         (void)put_user(GNTST_okay, &uop->status);
         for ( i = 0; i < op.nr_frames; i++ ) {
-            mfn = gnttab_shared_mfn(d, d->grant_table, i);
-            if (shadow_mode_translate(d))
-                mfn = __mfn_to_gpfn(d, mfn);
+            mfn = __mfn_to_gpfn(d, gnttab_shared_mfn(d, d->grant_table, i));
             (void)put_user(mfn, &op.frame_list[i]);
         }
     }

With this chunk, my domU no longer finds it's block device and fails to
boot:

VFS: Cannot open root device "hda1" or unknown-block(0,0)
Please append a correct "root=" boot option
Kernel panic - not syncing: VFS: Unable to mount root fs on
unknown-block(0,0)

Everything appears fine if I revert it.  Is this an intentional change?
It appears to be more of a functional change than the check-in comment
for this changeset would imply.  Thanks,

	Alex

-- 
Alex Williamson                             HP Linux & Open Source Lab

RE: [Xen-ia64-devel] cset 8690 blk driver regression

[Magenheimer, Dan (HP Labs Fort Collins)]

I can confirm this is broken on ia64 and that with this
reverted, with the xenlinux/ia64 subarch changes (just
checked in by Christian) and with the set_pfn_from_mfn
patch I posted an hour ago, Xen/ia64 is ready for 3.0.1.
(Assuming of course something else doesn't get broken
between now and then :-)

So... we need to get this fixed prior to 3.0.1!

To fix it so that it works on x86 (shadow and non-shadow)
and ia64, perhaps a macro call could be substituted for
the three removed lines and define'd differently for each
arch in asm/shadow.h?

Thanks,
Dan

> -----Original Message-----
> From: xen-ia64-devel-bounces@lists.xensource.com 
> [mailto:xen-ia64-devel-bounces@lists.xensource.com] On Behalf 
> Of Williamson, Alex (Linux Kernel Dev)
> Sent: Monday, January 30, 2006 2:08 PM
> To: sos22@cam.ac.uk
> Cc: xen-devel@lists.xensource.com; xen-ia64-devel@lists.xensource.com
> Subject: [Xen-ia64-devel] cset 8690 blk driver regression
> 
> Hi Steven,
> 
>   I'm seeing a regression on ia64 with this chunk of cset 8690:
> 
> --- a/xen/common/grant_table.c	Fri Jan 27 20:38:55 2006
> +++ b/xen/common/grant_table.c	Fri Jan 27 20:57:07 2006
> @@ -522,9 +522,7 @@
>          ASSERT(d->grant_table != NULL);
>          (void)put_user(GNTST_okay, &uop->status);
>          for ( i = 0; i < op.nr_frames; i++ ) {
> -            mfn = gnttab_shared_mfn(d, d->grant_table, i);
> -            if (shadow_mode_translate(d))
> -                mfn = __mfn_to_gpfn(d, mfn);
> +            mfn = __mfn_to_gpfn(d, gnttab_shared_mfn(d, 
> d->grant_table, i));
>              (void)put_user(mfn, &op.frame_list[i]);
>          }
>      }
> 
> With this chunk, my domU no longer finds it's block device 
> and fails to
> boot:
> 
> VFS: Cannot open root device "hda1" or unknown-block(0,0)
> Please append a correct "root=" boot option
> Kernel panic - not syncing: VFS: Unable to mount root fs on
> unknown-block(0,0)
> 
> Everything appears fine if I revert it.  Is this an 
> intentional change?
> It appears to be more of a functional change than the check-in comment
> for this changeset would imply.  Thanks,
> 
> 	Alex
> 
> -- 
> Alex Williamson                             HP Linux & Open Source Lab
> 
> 
> _______________________________________________
> Xen-ia64-devel mailing list
> Xen-ia64-devel@lists.xensource.com
> http://lists.xensource.com/xen-ia64-devel
> 

Re: cset 8690 blk driver regression

[Keir Fraser]

On 31 Jan 2006, at 20:11, Magenheimer, Dan (HP Labs Fort Collins) wrote:

> To fix it so that it works on x86 (shadow and non-shadow)
> and ia64, perhaps a macro call could be substituted for
> the three removed lines and define'd differently for each
> arch in asm/shadow.h?

I removed the dummy definition of __mfn_to_gpfn() from xen/shadow.h 
that was hiding ia64's own definition. I think Alex Williamson posted 
that as a fix. Is using that macro here not applicable for ia64 at all? 
If not and the tree is still broken for ia64 then we can perhaps define 
a gnttab_shared_gpfn() macro.

  -- Keir