From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v2 4/6] xen/x86/pvh: Set up descriptors for 32-bit PVH guests Date: Tue, 21 Jul 2015 11:32:43 -0400 Message-ID: <55AE661B.90909__36086.8055992343$1437492883$gmane$org@oracle.com> References: <1437083021-24488-1-git-send-email-boris.ostrovsky@oracle.com> <1437083021-24488-5-git-send-email-boris.ostrovsky@oracle.com> <20150717152113.GB18085@l.oracle.com> <55A920FD.6050101@oracle.com> <20150717164331.GA19827@l.oracle.com> <55A940CC.6090509@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZHZXj-0004ah-Gi for xen-devel@lists.xenproject.org; Tue, 21 Jul 2015 15:33:19 +0000 In-Reply-To: <55A940CC.6090509@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: Konrad Rzeszutek Wilk Cc: elena.ufimtseva@oracle.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, tim@xen.org, linux-kernel@vger.kernel.org, david.vrabel@citrix.com, jbeulich@suse.com, xen-devel@lists.xenproject.org, ian.jackson@eu.citrix.com, roger.pau@citrix.com List-Id: xen-devel@lists.xenproject.org On 07/17/2015 01:52 PM, Boris Ostrovsky wrote: > On 07/17/2015 12:43 PM, Konrad Rzeszutek Wilk wrote: >> On Fri, Jul 17, 2015 at 11:36:29AM -0400, Boris Ostrovsky wrote: >>> On 07/17/2015 11:21 AM, Konrad Rzeszutek Wilk wrote: >>>> On Thu, Jul 16, 2015 at 05:43:39PM -0400, Boris Ostrovsky wrote: >>>>> Signed-off-by: Boris Ostrovsky >>>>> --- >>>>> Changes in v2: >>>>> * Set segment selectors using loadsegment() instead of assembly >>>>> >>>>> arch/x86/xen/enlighten.c | 15 ++++++++++----- >>>>> 1 file changed, 10 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c >>>>> index f8dc398..d665b1d 100644 >>>>> --- a/arch/x86/xen/enlighten.c >>>>> +++ b/arch/x86/xen/enlighten.c >>>>> @@ -1362,12 +1362,12 @@ static void __init >>>>> xen_boot_params_init_edd(void) >>>>> static void __ref xen_setup_gdt(int cpu) >>>>> { >>>>> if (xen_feature(XENFEAT_auto_translated_physmap)) { >>>>> -#ifdef CONFIG_X86_64 >>>>> - unsigned long dummy; >>>>> + unsigned long __attribute__((unused)) dummy; >>>>> - load_percpu_segment(cpu); /* We need to access per-cpu >>>>> area */ >>>> You removed that - where are we going to do that? As the >>>> 'switch_to_new_gdt' uses the per-cpu GDT table. >>> load_percpu_segment() is part of switch_to_new_gdt(), so I thought >>> there is >>> no need to call it here. >>> >>> But you are right --- switch_to_new_gdt() starts with >>> get_cpu_gdt_table() >>> which accesses per-CPU area. How did this manage to work then? >> I was surprised as well - I was expecting your patch to have blow up. >> Unless we are doing something fancy for CPU0 and for the other CPUs we >> already have the per-cpu segment setup during bootup (copied from BSP)? > > > No, %fs is zero when we enter xen_setup_gdt() (for 32-bit). > > In any case, I should put load_percpu_segment() back. No, I shouldn't. Until the new GDT is loaded we can't load selectors since current GDT doesn't have descriptors set up for them. And so any attempt to load uninitialized selectors results in a fault. This worked for 64-bit guests because there we load zero into %gs and that is allowed (processor doesn't perform descriptor checks for the first 4 indexes). But for 32-bit guests we load %fs with 0xd8. And the reason the code worked before was because we are using "master" per-cpu area and because GDT is the same for all CPUs at that point. Or so I think. -boris