Re: [PATCH 2/6] xen/pvh: Extend vcpu_guest_context, p2m, event, andxenbus to support PVH.

From: Konrad Rzeszutek Wilk
Date: Mon Oct 22 2012 - 16:27:03 EST


On Mon, Oct 22, 2012 at 11:31:54AM -0700, Mukesh Rathor wrote:
> On Mon, 22 Oct 2012 14:44:40 +0100
> Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> wrote:
>
> > On Sat, 20 Oct 2012, Konrad Rzeszutek Wilk wrote:
> > > From: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> > >
> > > make gdt_frames[]/gdt_ents into a union with {gdtaddr, gdtsz}, as
> > > PVH only needs to send down gdtaddr and gdtsz.
> > >
> > > For interrupts, PVH uses native_irq_ops.
> > > vcpu hotplug is currently not available for PVH.
> > >
> > > For events we follow what PVHVM does - to use callback vector.
> > > Lastly, also use HVM path to setup XenBus.
> > >
> > > Signed-off-by: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > > ---
> > > return true;
> > > }
> > > - xen_copy_trap_info(ctxt->trap_ctxt);
> > > + /* check for autoxlated to get it right for 32bit kernel */
> >
> > I am not sure what this comment means, considering that in another
> > comment below you say that we don't support 32bit PVH kernels.
>
> Function is common to both 32bit and 64bit kernels. We need to check
> for auto xlated also in the if statement in addition to supervisor
> mode kernel, so 32 bit doesn't go down the wrong path.

Can one just make it #ifdef CONFIG_X86_64 for the whole thing?
You are either way during bootup doing a 'BUG' when booting as 32-bit?


>
> PVH is not supported for 32bit kernels, and gs_base_user doesn't exist
> in the structure for 32bit so it needs to be if'def'd 64bit which is
> ok because PVH is not supprted on 32bit kernel.
>
> > > + (unsigned
> > > long)xen_hypervisor_callback;
> > > + ctxt->failsafe_callback_eip =
> > > + (unsigned
> > > long)xen_failsafe_callback;
> > > + }
> > > + ctxt->user_regs.cs = __KERNEL_CS;
> > > + ctxt->user_regs.esp = idle->thread.sp0 - sizeof(struct
> > > pt_regs);
> > > per_cpu(xen_cr3, cpu) = __pa(swapper_pg_dir);
> > > ctxt->ctrlreg[3] =
> > > xen_pfn_to_cr3(virt_to_mfn(swapper_pg_dir));
> >
> > The tradional path looks the same as before, however it is hard to
> > tell whether the PVH path is correct without the Xen side. For
> > example, what is gdtsz?
>
> gdtsz is GUEST_GDTR_LIMIT and gdtaddr is GUEST_GDTR_BASE in the vmcs.

looking at this I figured it could be a bit neater. So I split it in
two patches which should make it easier to read the PVH one.