From patchwork Thu Apr 14 14:26:38 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konrad Rzeszutek Wilk X-Patchwork-Id: 8837691 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 83B4C9F36E for ; Thu, 14 Apr 2016 14:29:19 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7B6D520270 for ; Thu, 14 Apr 2016 14:29:18 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 97617202EB for ; Thu, 14 Apr 2016 14:29:15 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aqiEP-0005Xw-Qz; Thu, 14 Apr 2016 14:26:53 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aqiEO-0005Xq-6Z for xen-devel@lists.xenproject.org; Thu, 14 Apr 2016 14:26:52 +0000 Received: from [85.158.137.68] by server-6.bemta-3.messagelabs.com id 66/85-23864-BA8AF075; Thu, 14 Apr 2016 14:26:51 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrBIsWRWlGSWpSXmKPExsVyMfTOEd1VK/j DDR5c0rT4vmUykwOjx+EPV1gCGKNYM/OS8isSWDNmP77GWrDLsmJr40XmBsZ+vS5GLg4hgemM EgtOnmUGcVgElrFKNHVOYgdxJAQOsUp8WHKZtYuRE8iJkfja9pURwq6QeHtpJROILSSgJLFl8 mNGiFENTBIb298xgySEBfQkJn+7DdbAIqAqsflEO1icTUBf4unaa2C2iEAbo8T7h0ogzcwC7Y wSx3bOY4FotpfY9reTHcTmFbCS6Ll1lQVim5vEzLtrmCDighInZz4BizMLaEnc+PcSKM4BZEt LLP/HARLmFHCXePZtF1hYVEBF4tXB+gmMIrOQNM9C0jwLoXkBI/MqRvXi1KKy1CJdQ72kosz0 jJLcxMwcXUMDY73c1OLixPTUnMSkYr3k/NxNjMDwZwCCHYzLPzodYpTkYFIS5WXM4A8X4kvKT 6nMSCzOiC8qzUktPsQow8GhJMEbvxwoJ1iUmp5akZaZA4xEmLQEB4+SCK8CSJq3uCAxtzgzHS J1itGYY8vva2uZOLZNvbeWSYglLz8vVUqcVwWkVACkNKM0D24QLEFcYpSVEuZlBDpNiKcgtSg 3swRV/hWjOAejkjCvDsgUnsy8Erh9r4BOYQI6pewdL8gpJYkIKakGxlm97Q2q/jaRn83DQn01 L1zyj76yhEu5UtQ19kd69KoN09dNOxVz8fAPlY/mvFk9Obw/9tcv/7doruJZwwhr8yjHU80vL //5+Lrqo9eb1RK/qidGzTleX/Y66ND1HS3SRy9lBq44Ut0Y8rs/xFn2TZzC4f+pfCc3zzAMzb ontLT5hoTExfWveJVYijMSDbWYi4oTAURrbMELAwAA X-Env-Sender: ketuzsezr@gmail.com X-Msg-Ref: server-7.tower-31.messagelabs.com!1460644009!27318818!1 X-Originating-IP: [209.85.220.196] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 8.28; banners=-,-,- X-VirusChecked: Checked Received: (qmail 5971 invoked from network); 14 Apr 2016 14:26:50 -0000 Received: from mail-qk0-f196.google.com (HELO mail-qk0-f196.google.com) (209.85.220.196) by server-7.tower-31.messagelabs.com with AES128-GCM-SHA256 encrypted SMTP; 14 Apr 2016 14:26:50 -0000 Received: by mail-qk0-f196.google.com with SMTP id l68so439021qkf.3 for ; Thu, 14 Apr 2016 07:26:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=04dPanUHYoHvFx8nSwHCjeHKFxrJXZPwEvq1BCYzuNE=; b=tYs9CwTn1XQJPK/3j5AzG7tt4QTDRMWv3OiZrDHafGdHQp8jB7lQFAWnLUbXm/mU5a Mw5mtG7s3F4uz79eFVrYZ4bcB5XWrQyhiKK7zj2WsaXBp7VcMwr6TYdB29GtXLCoyLga nHvvlQY46c0kGVrw3IzQLo3TJ0xur0lcveyXrhnAwEektPY8QS6wIQRnZrBH/fMp0TON NK8gngiOYsjhsc8Ic3NHIajIC34Nv1aLI2YHFLuYMqZth8YTzIeVfImAABX/AMuDFG4S P3ucm4c8yRmrvd6FkTn6A9bQ4W6OcT64EBLxXcmcLAQsc4/q3j9qODdHqmD+uT/Cfat9 X54A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=04dPanUHYoHvFx8nSwHCjeHKFxrJXZPwEvq1BCYzuNE=; b=J9hEm/ouPwL8entJ84madM+X8QZFyb7KumyLSWTWxiIpuR+/R7If27vXJlV0be4ehL pjhQqCuBWtXXjXCl8KBrm62QY2vDj1XSdDv3692+LzVYepq6TkfjM4Uftb2ubC9GwsoD yLV6VKTSKEZqYjIzW2lNyu7c+dr0b0d8iN3dbNDI6XdLiSZRIi9ZKUWUokCm49NIT13m vjlE2XfK7uH7fd6SDKOdI7onKDv25rpGzNwGJ8znPeedMx5VgV8MQxYVUdEfQ6cdxNwU kgRSa1Ckkc6xK9muw1pHVK0/Z5eg/EWcMTFJI5AdvYPr1t/ryq0s9Td4zlaVD/ICdKFJ 82KQ== X-Gm-Message-State: AOPr4FWCb0589BuxdMPJ4YiLS2kAM7p0fwikAevj6ScVH6cvPjLC7LTOnPO25bmZYxq9+g== X-Received: by 10.55.81.87 with SMTP id f84mr18930593qkb.10.1460644008106; Thu, 14 Apr 2016 07:26:48 -0700 (PDT) Received: from localhost.localdomain ([172.58.217.62]) by smtp.gmail.com with ESMTPSA id c32sm18136101qgf.36.2016.04.14.07.26.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 14 Apr 2016 07:26:46 -0700 (PDT) Date: Thu, 14 Apr 2016 10:26:38 -0400 From: Konrad Rzeszutek Wilk To: Konrad Rzeszutek Wilk , Julien Grall , Stefano Stabellini Message-ID: <20160414142636.GA16521@localhost.localdomain> References: <1460584928-32440-1-git-send-email-konrad.wilk@oracle.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1460584928-32440-1-git-send-email-konrad.wilk@oracle.com> User-Agent: Mutt/1.5.24 (2015-08-30) Cc: xen-devel@lists.xenproject.org, andrew.cooper3@citrix.com, mpohlack@amazon.com, sasha.levin@oracle.com, ross.lagerwall@citrix.com Subject: Re: [Xen-devel] [PATCH v8.1] xSplice v1 design and implementation. X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP > Julien, Stefano, I dropped Julien's Ack on > > [PATCH v8.1 11/27] xsplice: Implement payload loading > > as Jan's expressed a desire not to have BITS_PER_LONG exposed > in the public headers. I've put CONFIG_ARM_32 in there. On IRC > Stefano was OK (relucantly) with it. > > Patchset has been tested on ARM (tweaking the Kconfig to build xSplice and then > booting and using it under ARM CubieTruck - granted there is no patching > yet but the hypercalls work), ARM64 (only built it), and x86 (legacy and EFI) > .. snip.. > > *Are there any TODOs left from v5,v6,v7 reviews?* > > Just how the sysctl.h definition for 'struct xsplice_patch_func' should be: > > #define XSPLICE_PAYLOAD_VERSION 1 > /* > * .xsplice.funcs structure layout defined in the `Payload format` > * section in the xSplice design document. > * > * The size should be exactly 64 bytes (64) or 52 bytes (32). > * > * We guard this with __XEN__ as toolstacks do not need to use it. > */ > #ifdef __XEN__ > struct xsplice_patch_func { > const char *name; /* Name of function to be patched. */ > #if CONFIG_ARM_32 > uint32_t new_addr; > uint32_t old_addr; > #else > uint64_t new_addr; > uint64_t old_addr; /* Can be zero and name will be looked up. */ > #endif > uint32_t new_size; > uint32_t old_size; > uint8_t version; /* MUST be XSPLICE_PAYLOAD_VERSION. */ > uint8_t pad[31]; /* MUST be zero filled. */ > }; > typedef struct xsplice_patch_func xsplice_patch_func_t; > #endif > > Please note the __XEN__ so even if toolstack does include the public/sysctl.h > it won't find it the structure. > > With this I am able to compile early-test-cases-prototype-work xSplice on ARM32. Jan suggested to just drop the uint32_t and uint64_t business and use a void pointer. It is nicer so I've put a new version up with this. The interdiff between this (v8.1) and (v9) is: Julien, Stefano, Are you guys OK with this? It does compile under ARM/ARM64 without trouble. If you are OK I will re-instate Julien's Ack on the patch that uses and introduces this: (xsplice: Implement support for applying/reverting/replacing patches.) diff --git a/docs/misc/xsplice.markdown b/docs/misc/xsplice.markdown index fb13a7d..d5abab0 100644 --- a/docs/misc/xsplice.markdown +++ b/docs/misc/xsplice.markdown @@ -298,13 +298,8 @@ which describe the functions to be patched: 
 struct xsplice_patch_func {  
     const char *name;  
-#if BITS_PER_LONG == 32  
-    uint32_t new_addr;  
-    uint32_t old_addr;  
-#else  
-    uint64_t new_addr;  
-    uint64_t old_addr;  
-#endif
+    void *new_addr;  
+    void *old_addr;  
     uint32_t new_size;  
     uint32_t old_size;  
     uint8_t version;  
@@ -312,8 +307,8 @@ struct xsplice_patch_func {
 };
-The size of the structure is 64 bytes or 52 bytes if compiled under 32-bit -hypervisors. +The size of the structure is 64 bytes on 64-bit hypervisors. It will be +52 on 32-bit hypervisors. * `name` is the symbol name of the old function. Only used if `old_addr` is zero, otherwise will be used during dynamic linking (when hypervisor loads @@ -353,8 +348,8 @@ A simple example of what a payload file can be: /* MUST be in sync with hypervisor. */ struct xsplice_patch_func { const char *name; - uint64_t new_addr; - uint64_t old_addr; + void *new_addr; + void *old_addr; uint32_t new_size; uint32_t old_size; uint8_t version; @@ -372,8 +367,8 @@ static unsigned char patch_this_fnc[] = "xen_extra_version"; struct xsplice_patch_func xsplice_hello_world = { .version = XSPLICE_PAYLOAD_VERSION, .name = patch_this_fnc, - .new_addr = (unsigned long)(xen_hello_world), - .old_addr = 0xffff82d08013963c, /* Extracted from xen-syms. */ + .new_addr = xen_hello_world, + .old_addr = (void *)0xffff82d08013963c, /* Extracted from xen-syms. */ .new_size = 13, /* To be be computed by scripts. */ .old_size = 13, /* -----------""--------------- */ } __attribute__((__section__(".xsplice.funcs"))); diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 73798c3..3a76f55 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -6102,7 +6102,7 @@ void modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf) v += PAGE_SIZE; /* - * If we not destroying mappings, or are not done with the L2E, + * If we are not destroying mappings, or not done with the L2E, * skip the empty&free check. */ if ( (nf & _PAGE_PRESENT) || ((v != e) && (l1_table_offset(v) != 0)) ) diff --git a/xen/arch/x86/test/xen_bye_world.c b/xen/arch/x86/test/xen_bye_world.c index b2f25bd..be83b5a 100644 --- a/xen/arch/x86/test/xen_bye_world.c +++ b/xen/arch/x86/test/xen_bye_world.c @@ -17,8 +17,8 @@ extern const char *xen_extra_version(void); struct xsplice_patch_func __section(".xsplice.funcs") xsplice_xen_bye_world = { .version = XSPLICE_PAYLOAD_VERSION, .name = bye_world_patch_this_fnc, - .new_addr = (unsigned long)(xen_bye_world), - .old_addr = (unsigned long)(xen_extra_version), + .new_addr = xen_bye_world, + .old_addr = (xen_extra_version, .new_size = NEW_CODE_SZ, .old_size = OLD_CODE_SZ, }; diff --git a/xen/arch/x86/test/xen_hello_world.c b/xen/arch/x86/test/xen_hello_world.c index 22fe2e7..a67b798 100644 --- a/xen/arch/x86/test/xen_hello_world.c +++ b/xen/arch/x86/test/xen_hello_world.c @@ -16,8 +16,8 @@ extern const char *xen_extra_version(void); struct xsplice_patch_func __section(".xsplice.funcs") xsplice_xen_hello_world = { .version = XSPLICE_PAYLOAD_VERSION, .name = hello_world_patch_this_fnc, - .new_addr = (unsigned long)(xen_hello_world), - .old_addr = (unsigned long)(xen_extra_version), + .new_addr = xen_hello_world, + .old_addr = xen_extra_version, .new_size = NEW_CODE_SZ, .old_size = OLD_CODE_SZ, }; diff --git a/xen/arch/x86/test/xen_replace_world.c b/xen/arch/x86/test/xen_replace_world.c index 70516bc..6030139 100644 --- a/xen/arch/x86/test/xen_replace_world.c +++ b/xen/arch/x86/test/xen_replace_world.c @@ -17,8 +17,8 @@ extern const char *xen_extra_version(void); struct xsplice_patch_func __section(".xsplice.funcs") xsplice_xen_replace_world = { .version = XSPLICE_PAYLOAD_VERSION, .name = xen_replace_world_name, - .new_addr = (unsigned long)(xen_replace_world), - .old_addr = (unsigned long)(xen_extra_version), + .new_addr = xen_replace_world, + .old_addr = xen_extra_version, .new_size = NEW_CODE_SZ, .old_size = OLD_CODE_SZ, }; diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h index 3d47b47..3876b04 100644 --- a/xen/include/public/sysctl.h +++ b/xen/include/public/sysctl.h @@ -845,13 +845,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_featureset_t); #ifdef __XEN__ struct xsplice_patch_func { const char *name; /* Name of function to be patched. */ -#if CONFIG_ARM_32 - uint32_t new_addr; - uint32_t old_addr; -#else - uint64_t new_addr; - uint64_t old_addr; /* Can be zero and name will be looked up. */ -#endif + void *new_addr; + void *old_addr; uint32_t new_size; uint32_t old_size; uint8_t version; /* MUST be XSPLICE_PAYLOAD_VERSION. */