| Message ID | 59f26856-d23d-bb69-0403-39e77acbf85c@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: RTC handling adjustments | expand |
> -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 15 July 2020 10:47 > To: xen-devel@lists.xenproject.org > Cc: Andrew Cooper <andrew.cooper3@citrix.com>; Paul Durrant <paul@xen.org>; Wei Liu <wl@xen.org>; > Roger Pau Monné <roger.pau@citrix.com> > Subject: [PATCH v2 1/2] x86: restore pv_rtc_handler() invocation > > This was lost when making the logic accessible to PVH Dom0. > > Fixes: 835d8d69d96a ("x86/rtc: provide mediated access to RTC for PVH dom0") > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/time.c > +++ b/xen/arch/x86/time.c > @@ -1160,6 +1160,10 @@ void rtc_guest_write(unsigned int port, > case RTC_PORT(1): > if ( !ioports_access_permitted(currd, RTC_PORT(0), RTC_PORT(1)) ) > break; > + > + if ( pv_rtc_handler ) > + pv_rtc_handler(currd->arch.cmos_idx & 0x7f, data); > + This appears to be semantically slightly different to the old code in that it is only done for a write to RC_PORT(1), whereas it would have been done on a write to either RTC_POR(0) or RTC_PORT(1) before. Is that of any concern? Paul > spin_lock_irqsave(&rtc_lock, flags); > outb(currd->arch.cmos_idx & 0x7f, RTC_PORT(0)); > outb(data, RTC_PORT(1));
On 15.07.2020 12:01, Paul Durrant wrote: >> -----Original Message----- >> From: Jan Beulich <jbeulich@suse.com> >> Sent: 15 July 2020 10:47 >> To: xen-devel@lists.xenproject.org >> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; Paul Durrant <paul@xen.org>; Wei Liu <wl@xen.org>; >> Roger Pau Monné <roger.pau@citrix.com> >> Subject: [PATCH v2 1/2] x86: restore pv_rtc_handler() invocation >> >> This was lost when making the logic accessible to PVH Dom0. >> >> Fixes: 835d8d69d96a ("x86/rtc: provide mediated access to RTC for PVH dom0") >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/arch/x86/time.c >> +++ b/xen/arch/x86/time.c >> @@ -1160,6 +1160,10 @@ void rtc_guest_write(unsigned int port, >> case RTC_PORT(1): >> if ( !ioports_access_permitted(currd, RTC_PORT(0), RTC_PORT(1)) ) >> break; >> + >> + if ( pv_rtc_handler ) >> + pv_rtc_handler(currd->arch.cmos_idx & 0x7f, data); >> + > > This appears to be semantically slightly different to the old code in that it is only done for a write to RC_PORT(1), whereas it would have been done on a write to either RTC_POR(0) or RTC_PORT(1) before. Is that of any concern? The old code was (quoting plain 4.13.1) else if ( port == RTC_PORT(0) ) { currd->arch.cmos_idx = data; } else if ( (port == RTC_PORT(1)) && ioports_access_permitted(currd, RTC_PORT(0), RTC_PORT(1)) ) { unsigned long flags; if ( pv_rtc_handler ) pv_rtc_handler(currd->arch.cmos_idx & 0x7f, data); spin_lock_irqsave(&rtc_lock, flags); outb(currd->arch.cmos_idx & 0x7f, RTC_PORT(0)); outb(data, RTC_PORT(1)); spin_unlock_irqrestore(&rtc_lock, flags); } which I think similarly invoked the hook for RTC_PORT(1) only. Jan
--- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -1160,6 +1160,10 @@ void rtc_guest_write(unsigned int port, case RTC_PORT(1): if ( !ioports_access_permitted(currd, RTC_PORT(0), RTC_PORT(1)) ) break; + + if ( pv_rtc_handler ) + pv_rtc_handler(currd->arch.cmos_idx & 0x7f, data); + spin_lock_irqsave(&rtc_lock, flags); outb(currd->arch.cmos_idx & 0x7f, RTC_PORT(0)); outb(data, RTC_PORT(1));
This was lost when making the logic accessible to PVH Dom0. Fixes: 835d8d69d96a ("x86/rtc: provide mediated access to RTC for PVH dom0") Signed-off-by: Jan Beulich <jbeulich@suse.com>