Patchwork [v2,2/5] xen: disable rtc_clock

login
register
mail settings
Submitter Stefano Stabellini
Date Jan. 27, 2012, 12:26 p.m.
Message ID <1327667215-5411-2-git-send-email-stefano.stabellini@eu.citrix.com>
Download mbox | patch
Permalink /patch/138215/
State New
Headers show

Comments

Stefano Stabellini - Jan. 27, 2012, 12:26 p.m.
rtc_clock is only used by the RTC emulator (mc146818rtc.c), however Xen
has its own RTC emulator in the hypervisor so we can disable it.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen-all.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Paolo Bonzini - Jan. 27, 2012, 2:41 p.m.
On 01/27/2012 01:26 PM, Stefano Stabellini wrote:
> rtc_clock is only used by the RTC emulator (mc146818rtc.c), however Xen
> has its own RTC emulator in the hypervisor so we can disable it.
>
> Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
> ---
>   xen-all.c |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/xen-all.c b/xen-all.c
> index d1fc597..bf183f7 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -513,6 +513,7 @@ void xen_vcpu_init(void)
>           qemu_register_reset(xen_reset_vcpu, first_cpu);
>           xen_reset_vcpu(first_cpu);
>       }
> +    qemu_clock_enable(rtc_clock, false);
>   }
>
>   /* get the ioreq packets from share mem */

Depending on "-rtc clock=vm" or "-rtc clock=rt", this may not be true.

Why do you need to instantiate an RTC at all?

Paolo
Stefano Stabellini - Jan. 30, 2012, 11:56 a.m.
On Fri, 27 Jan 2012, Paolo Bonzini wrote:
> On 01/27/2012 01:26 PM, Stefano Stabellini wrote:
> > rtc_clock is only used by the RTC emulator (mc146818rtc.c), however Xen
> > has its own RTC emulator in the hypervisor so we can disable it.
> >
> > Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
> > ---
> >   xen-all.c |    1 +
> >   1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/xen-all.c b/xen-all.c
> > index d1fc597..bf183f7 100644
> > --- a/xen-all.c
> > +++ b/xen-all.c
> > @@ -513,6 +513,7 @@ void xen_vcpu_init(void)
> >           qemu_register_reset(xen_reset_vcpu, first_cpu);
> >           xen_reset_vcpu(first_cpu);
> >       }
> > +    qemu_clock_enable(rtc_clock, false);
> >   }
> >
> >   /* get the ioreq packets from share mem */
> 
> Depending on "-rtc clock=vm" or "-rtc clock=rt", this may not be true.

Good point.
I should check for rtc_clock == host_clock.


> Why do you need to instantiate an RTC at all?

I don't, in fact in previous versions of this patch series I tried to do
that but it is difficult and makes the common code worse, so I followed
Anthony's suggestion of just disabling the rtc_clock.
Paolo Bonzini - Jan. 30, 2012, 11:59 a.m.
On 01/30/2012 12:56 PM, Stefano Stabellini wrote:
>> >  Depending on "-rtc clock=vm" or "-rtc clock=rt", this may not be true.
>
> Good point.
> I should check for rtc_clock == host_clock.
>
>> >  Why do you need to instantiate an RTC at all?
> I don't, in fact in previous versions of this patch series I tried to do
> that but it is difficult and makes the common code worse, so I followed
> Anthony's suggestion of just disabling the rtc_clock.

I see.  It shouldn't surprise you that I completely disagree with 
Anthony on this. :)

Paolo
Stefano Stabellini - Jan. 30, 2012, 6:53 p.m.
On Mon, 30 Jan 2012, Paolo Bonzini wrote:
> On 01/30/2012 12:56 PM, Stefano Stabellini wrote:
> >> >  Depending on "-rtc clock=vm" or "-rtc clock=rt", this may not be true.
> >
> > Good point.
> > I should check for rtc_clock == host_clock.
> >
> >> >  Why do you need to instantiate an RTC at all?
> > I don't, in fact in previous versions of this patch series I tried to do
> > that but it is difficult and makes the common code worse, so I followed
> > Anthony's suggestion of just disabling the rtc_clock.
> 
> I see.  It shouldn't surprise you that I completely disagree with 
> Anthony on this. :)

I am OK with both approaches, but I can see the benefits of reducing
this patch to (almost) a one-liner.
Anthony Liguori - Jan. 30, 2012, 7:28 p.m.
On 01/30/2012 05:59 AM, Paolo Bonzini wrote:
> On 01/30/2012 12:56 PM, Stefano Stabellini wrote:
>>> > Depending on "-rtc clock=vm" or "-rtc clock=rt", this may not be true.
>>
>> Good point.
>> I should check for rtc_clock == host_clock.
>>
>>> > Why do you need to instantiate an RTC at all?
>> I don't, in fact in previous versions of this patch series I tried to do
>> that but it is difficult and makes the common code worse, so I followed
>> Anthony's suggestion of just disabling the rtc_clock.
>
> I see. It shouldn't surprise you that I completely disagree with Anthony on
> this. :)

Give me some credit at least...

The original patches didn't disable the RTC, it introduced a half-neutered Xen 
specific RTC.

My original suggestion (which I still think is the best approach) is to change 
the RTC emulation to not use a second timer at all.

Regards,

Anthony Liguori

> Paolo
>
Paolo Bonzini - Jan. 31, 2012, 7:30 a.m.
On 01/30/2012 08:28 PM, Anthony Liguori wrote:
>>
>> I see. It shouldn't surprise you that I completely disagree with
>> Anthony on this. :)
>
> Give me some credit at least...
>
> The original patches didn't disable the RTC, it introduced a
> half-neutered Xen specific RTC.

Ah. :)

> My original suggestion (which I still think is the best approach) is to
> change the RTC emulation to not use a second timer at all.

Yes, that would be really the best approach.  I tried and it's pretty 
hard, but we can give it a second try.

Paolo

Patch

diff --git a/xen-all.c b/xen-all.c
index d1fc597..bf183f7 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -513,6 +513,7 @@  void xen_vcpu_init(void)
         qemu_register_reset(xen_reset_vcpu, first_cpu);
         xen_reset_vcpu(first_cpu);
     }
+    qemu_clock_enable(rtc_clock, false);
 }
 
 /* get the ioreq packets from share mem */