From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [V10 PATCH 18/23] PVH xen: add hypercall support for PVH Date: Thu, 8 Aug 2013 10:20:39 +0100 Message-ID: <520362E7.9070000@eu.citrix.com> References: <1374631171-15224-1-git-send-email-mukesh.rathor@oracle.com> <1374631171-15224-19-git-send-email-mukesh.rathor@oracle.com> <20130807191213.0f51cf5c@mantra.us.oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20130807191213.0f51cf5c@mantra.us.oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Mukesh Rathor Cc: "xen-devel@lists.xensource.com" , "keir.xen@gmail.com" List-Id: xen-devel@lists.xenproject.org On 08/08/13 03:12, Mukesh Rathor wrote: > On Wed, 7 Aug 2013 17:43:54 +0100 > George Dunlap wrote: > >> On Wed, Jul 24, 2013 at 2:59 AM, Mukesh Rathor >> wrote: >>> This patch expands HVM hcall support to include PVH. >>> >>> Changes in v8: >>> - Carve out PVH support of hvm_op to a small function. >>> >>> Signed-off-by: Mukesh Rathor >>> --- >>> xen/arch/x86/hvm/hvm.c | 80 >>> +++++++++++++++++++++++++++++++++++++------ >>> xen/arch/x86/x86_64/traps.c | 2 +- 2 files changed, 70 >>> insertions(+), 12 deletions(-) >>> >>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c >>> index 383c5cd..6af020e 100644 >>> --- a/xen/arch/x86/hvm/hvm.c >>> +++ b/xen/arch/x86/hvm/hvm.c >>> @@ -3192,6 +3192,17 @@ static long hvm_vcpu_op( >>> case VCPUOP_register_vcpu_time_memory_area: >>> rc = do_vcpu_op(cmd, vcpuid, arg); >>> break; >>> + >>> + case VCPUOP_is_up: >>> + case VCPUOP_up: >>> + case VCPUOP_initialise: >>> + /* PVH fixme: this white list should be removed eventually >>> */ >> What do you mean by this? That PVH won't need these in the future, or >> that you'll have some other way? > Just not have these checks here, but just support them all, whatever > makese sense. Sorry, I still don't understand -- do you mean you want to eventually just allow all VCPUOPs for PVH? > >>> + if ( is_pvh_vcpu(current) ) >>> + rc = do_vcpu_op(cmd, vcpuid, arg); >>> + else >>> + rc = -ENOSYS; >>> + break; >>> + >>> default: >>> rc = -ENOSYS; >>> break; >>> @@ -3312,6 +3323,24 @@ static hvm_hypercall_t *const >>> hvm_hypercall32_table[NR_hypercalls] = { HYPERCALL(tmem_op) >>> }; >>> >>> +/* PVH 32bitfixme. */ >>> +static hvm_hypercall_t *const pvh_hypercall64_table[NR_hypercalls] >>> = { >>> + HYPERCALL(platform_op), >>> + HYPERCALL(memory_op), >>> + HYPERCALL(xen_version), >>> + HYPERCALL(console_io), >>> + [ __HYPERVISOR_grant_table_op ] = (hvm_hypercall_t >>> *)hvm_grant_table_op, >>> + [ __HYPERVISOR_vcpu_op ] = (hvm_hypercall_t >>> *)hvm_vcpu_op, >>> + HYPERCALL(mmuext_op), >>> + HYPERCALL(xsm_op), >>> + HYPERCALL(sched_op), >>> + HYPERCALL(event_channel_op), >>> + [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t >>> *)hvm_physdev_op, >>> + HYPERCALL(hvm_op), >>> + HYPERCALL(sysctl), >>> + HYPERCALL(domctl) >>> +}; >> It would be nice if this list were in the same order as the other >> lists, so that it is easy to figure out what calls are common and what >> calls are different. > These are ordered by the hcall number, and assists in the debug. That makes sense. What about adding a "prep" patch which re-organizes the other lists by hcall number? I'm not particular about which order, I just think they should be the same. -George