WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH] v2 (Re: cs:21768 causes guest spend more time on

To: "Zhang, Jianwu" <jianwu.zhang@xxxxxxxxx>, Tim Deegan <Tim.Deegan@xxxxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] v2 (Re: cs:21768 causes guest spend more time on boot up)
From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Date: Thu, 22 Jul 2010 14:20:57 +0100
Cc: "Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>, "Xu, Jiajun" <jiajun.xu@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Thu, 22 Jul 2010 06:21:59 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <5D8008F58939784290FAB48F549751981D10D5893E@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acsn98UCQwKTvsoLTyih8pgseubXLgABUUbKAF+NTiAACV4w/Q==
Thread-topic: [PATCH] v2 (Re: cs:21768 causes guest spend more time on boot up)
User-agent: Microsoft-Entourage/12.24.0.100205
I suggest to try zapping the entire shared-info page when hvmloader
finishes. There is nothing in there that is useful to keep across hvmloader
and guest OS; zapping will ensure that other flags with rising-edge
semantics such as per-vcpu evtchn selector words get reset; and doing
anything more than zeroing is pointless since e.g., the evtchn_mask array
offset and size is dependent on whether the guest OS is 32-bit or 64-bit. If
hvmloader were to set the mask to all 1s and then boot a 64-bit guest, the
rearranged shared_info would actually mean that hvmloader has set 1s in part
of the 64-bit extended evtchn_pending array!

Just a thought...

 -- Keir

On 22/07/2010 10:01, "Zhang, Jianwu" <jianwu.zhang@xxxxxxxxx> wrote:

> Hi Tim
> When we set the VCPUS parameter more than 2 in guest configure file, We meet
> this issue again . We try it on cs21837, and the guest os is rhel5u3.
> 
> Thanks 
> Jianwu
> 
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx]
> Sent: Tuesday, July 20, 2010 7:17 PM
> To: Tim Deegan
> Cc: Zhang, Yang Z; xen-devel@xxxxxxxxxxxxxxxxxxx; Zhang, Jianwu; Xu, Jiajun
> Subject: Re: [PATCH] v2 (Re: cs:21768 causes guest spend more time on boot up)
> 
> Ah, this is because you poll the ring and never actually use let alone clear
> the event channel rx port? This looks like a good fix, thanks.
> 
>  -- Keir
> 
> On 20/07/2010 11:38, "Tim Deegan" <Tim.Deegan@xxxxxxxxxxxxx> wrote:
> 
>> Here's another patch that also unsticks CentOS 5.5 boot for me, and
>> seems safer and saner (even if it turns out that the bug is somewhere
>> else and I'm just perturbing the inputs to some more complex system...)
>> 
>> Cheers,
>> 
>> Tim.
>> 
>> hvmloader: clear the xenbus event-channel when we're done with it.
>> Otherwise a later xenbus client that naively waits for the rising edge
>> could get stuck.
>> 
>> Signed-off-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
>> 
>> diff -r c4a83e3cc6b4 tools/firmware/hvmloader/util.c
>> --- a/tools/firmware/hvmloader/util.c Thu Jul 15 18:18:16 2010 +0100
>> +++ b/tools/firmware/hvmloader/util.c Tue Jul 20 11:34:06 2010 +0100
>> @@ -587,10 +587,28 @@
>>      return table;
>>  }
>>  
>> +struct shared_info *get_shared_info(void)
>> +{
>> +    static struct shared_info *shared_info = NULL;
>> +    struct xen_add_to_physmap xatp;
>> +
>> +    if ( shared_info == NULL )
>> +    {
>> +        /* Map shared-info page. */
>> +        xatp.domid = DOMID_SELF;
>> +        xatp.space = XENMAPSPACE_shared_info;
>> +        xatp.idx   = 0;
>> +        xatp.gpfn  = 0xfffff;
>> +        if ( hypercall_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
>> +            BUG();
>> +        shared_info = (struct shared_info *) (xatp.gpfn << 12);
>> +    }
>> +    return shared_info;
>> +}
>> +
>>  uint16_t get_cpu_mhz(void)
>>  {
>> -    struct xen_add_to_physmap xatp;
>> -    struct shared_info *shared_info = (struct shared_info *)0xfffff000;
>> +    struct shared_info *shared_info = get_shared_info();
>>      struct vcpu_time_info *info = &shared_info->vcpu_info[0].time;
>>      uint64_t cpu_khz;
>>      uint32_t tsc_to_nsec_mul, version;
>> @@ -599,14 +617,6 @@
>>      static uint16_t cpu_mhz;
>>      if ( cpu_mhz != 0 )
>>          return cpu_mhz;
>> -
>> -    /* Map shared-info page. */
>> -    xatp.domid = DOMID_SELF;
>> -    xatp.space = XENMAPSPACE_shared_info;
>> -    xatp.idx   = 0;
>> -    xatp.gpfn  = (unsigned long)shared_info >> 12;
>> -    if ( hypercall_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
>> -        BUG();
>>  
>>      /* Get a consistent snapshot of scale factor (multiplier and shift). */
>>      do {
>> diff -r c4a83e3cc6b4 tools/firmware/hvmloader/util.h
>> --- a/tools/firmware/hvmloader/util.h Thu Jul 15 18:18:16 2010 +0100
>> +++ b/tools/firmware/hvmloader/util.h Tue Jul 20 11:34:06 2010 +0100
>> @@ -68,6 +68,9 @@
>>  #define pci_writeb(devfn, reg, val) (pci_write(devfn, reg, 1, (uint8_t)
>> val))
>>  #define pci_writew(devfn, reg, val) (pci_write(devfn, reg, 2,
>> (uint16_t)val))
>>  #define pci_writel(devfn, reg, val) (pci_write(devfn, reg, 4,
>> (uint32_t)val))
>> +
>> +/* Get a pointer to the shared-info page */
>> +struct shared_info *get_shared_info(void);
>>  
>>  /* Get CPU speed in MHz. */
>>  uint16_t get_cpu_mhz(void);
>> diff -r c4a83e3cc6b4 tools/firmware/hvmloader/xenbus.c
>> --- a/tools/firmware/hvmloader/xenbus.c Thu Jul 15 18:18:16 2010 +0100
>> +++ b/tools/firmware/hvmloader/xenbus.c Tue Jul 20 11:34:06 2010 +0100
>> @@ -53,14 +53,20 @@
>>             (unsigned long) rings, (unsigned long) event);
>>  }
>>  
>> -/* Reset the xenbus connection so the next kernel can start again.
>> - * We zero out the whole ring -- the backend can handle this, and it's
>> - * not going to surprise any frontends since it's equivalent to never
>> - * having used the rings. */
>> +/* Reset the xenbus connection so the next kernel can start again. */
>>  void xenbus_shutdown(void)
>>  {
>>      ASSERT(rings != NULL);
>> +
>> +    /* We zero out the whole ring -- the backend can handle this, and it's
>> +     * not going to surprise any frontends since it's equivalent to never
>> +     * having used the rings. */
>>      memset(rings, 0, sizeof *rings);
>> +
>> +    /* Clear the xenbus event-channel too */
>> +    get_shared_info()->evtchn_pending[event / sizeof (unsigned long)]
>> +        &= ~(1UL << ((event % sizeof (unsigned long))));
>> +
>>      rings = NULL;
>>  }
>>  
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel