Thanks for your comments!
At Thu, 11 May 2006 16:29:57 -0500,
Hollis Blanchard wrote:
> > static inline int
> > paging_enabled(vcpu_guest_context_t *v)
> > {
> > - unsigned long cr0 = v->ctrlreg[0];
> > - return (cr0 & X86_CR0_PE) && (cr0 & X86_CR0_PG);
> > + /* This check can be removed when Xen places the correct values in
> > + * cr0 for paravirtualized guests.
> > + */
> > + if ( (v->flags & VGCF_HVM_GUEST) == 1 ) {
> > + unsigned long cr0 = v->ctrlreg[0];
> > +
> > + return (cr0 & X86_CR0_PE) && (cr0 & X86_CR0_PG);
> > + }
> > + return 1;
> > }
>
> Instead of this, please look at the patch Ryan sent (Re: [Xen-devel]
> [PATCH] paging_enabled and non-HVM guests)? You'll need to clean up the
> whitespace at least though.
I'll comment Ryans patch below too make sure I understand what you
mean.
> diff -r c4eead8a925b tools/libxc/xc_ptrace.c
> --- a/tools/libxc/xc_ptrace.c Sun Apr 16 14:41:31 2006
> +++ b/tools/libxc/xc_ptrace.c Thu Apr 20 22:44:35 2006
> @@ -281,8 +281,10 @@
> uint64_t *l4, *l3, *l2, *l1;
> static void *v;
>
> +#if 0
> if ((ctxt[cpu].ctrlreg[4] & 0x20) == 0 ) /* legacy ia32 mode */
> return map_domain_va_32(xc_handle, cpu, guest_va, perm);
> +#endif
Is this check valid for non-HVM guests, i.e., is it possible to have
32-bit PV-guests on a 64-bit hypervisor?
> [3 setup_sane_cr0.patch <text/plain; us-ascii (7bit)>]
> diff -r c4eead8a925b tools/libxc/xc_linux_build.c
> --- a/tools/libxc/xc_linux_build.c Sun Apr 16 14:41:31 2006
> +++ b/tools/libxc/xc_linux_build.c Thu Apr 20 22:45:21 2006
> @@ -45,6 +45,11 @@
> #ifdef __ia64__
> #define probe_aout9(image,image_size,load_funcs) 1
> #endif
> +
> +/* from xc_ptrace.h */
> +#define X86_CR0_PE 0x00000001 /* Enable Protected Mode (RW)
> */
> +#define X86_CR0_PG 0x80000000 /* Paging (RW)
> */
> +
>
> struct initrd_info {
> enum { INITRD_none, INITRD_file, INITRD_mem } type;
> @@ -1174,6 +1179,8 @@
> ctxt->failsafe_callback_eip = 0;
> ctxt->syscall_callback_eip = 0;
> #endif
> + /* set sane cr0 bits, protected and paging enabled */
> + ctxt->ctrlreg[0] = X86_CR0_PE|X86_CR0_PG;
> #endif /* x86 */
> [...]
> --- a/linux-2.6-xen-sparse/drivers/xen/core/smpboot.c Sun Apr 16 14:41:31 2006
> +++ b/linux-2.6-xen-sparse/drivers/xen/core/smpboot.c Thu Apr 20 22:45:36 2006
> @@ -216,6 +216,8 @@
>
> ctxt.gs_base_kernel = (unsigned long)(cpu_pda(vcpu));
> #endif
> + /* set sane cr0 bits, protected and paging enabled */
> + ctxt.ctrlreg[0] = 0x80000001;
>
> BUG_ON(HYPERVISOR_vcpu_op(VCPUOP_initialise, vcpu, &ctxt));
> }
I'll add this to the patch.
> > /*
> > @@ -157,6 +164,43 @@ static long nr_pages
> > static long nr_pages = 0;
> > static unsigned long *page_array = NULL;
> >
> > +static unsigned long
> > +va_to_ma64(int cpu,
> > + uint64_t *table,
> > + unsigned long idx)
> > +{
> > + unsigned long out;
> > +
> > + /* Paravirtualized domains store machine addresses in tables while
> > + * HVM domains keep pseudo-physical addresses. HVM domains
> > + * therefore need one extra translation.
> > + */
> > + if ( (out = table[idx]) == 0)
>
> Isn't 0 as a physical or machine address valid?
>
> >+ return 0;
>
> Couldn't 0 be a valid machine address?
>
> Can you leave these checks and the table indexing to the callers, where
> they are now?
True, I'll just return the value and revert the table indexing. I'm a
bit unsure about the checks-for-0 though. map_domain_va_32 does this,
but map_domain_va_pae and map_domain_va_64 contained no checks. Maybe
the proper fix is just to remove the checks in map_domain_va_32 as 0
can be a valid physical/machine address?
> Since you don't have an HVM system to test on, you should probably CC
> the people who do who've worked on xc_ptrace.c. hg log suggests Kamble,
> Nitin A <nitin.a.kamble@xxxxxxxxx>.
I'll send a reworked patch. Nitin: could you test the next patch?
// Simon
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home