[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH for-4.14 v2] x86/rtc: provide mediated access to RTC for PVH dom0
On Fri, Jun 5, 2020 at 4:03 AM Roger Pau Monne <roger.pau@xxxxxxxxxx> wrote: > > Mediated access to the RTC was provided for PVHv1 dom0 using the PV > code paths (guest_io_{write/read}), but those accesses where never > implemented for PVHv2 dom0. This patch provides such mediated accesses > to the RTC for PVH dom0, just like it's provided for a classic PV > dom0. > > Pull out some of the RTC logic from guest_io_{read/write} into > specific helpers that can be used by both PV and HVM guests. The > setup of the handlers for PVH is done in rtc_init, which is already > used to initialize the fully emulated RTC. > > Without this a Linux PVH dom0 will read garbage when trying to access > the RTC, and one vCPU will be constantly looping in > rtc_timer_do_work. > > Note that such issue doesn't happen on domUs because the ACPI > NO_CMOS_RTC flag is set in FADT, which prevents the OS from accessing > the RTC. Also the X86_EMU_RTC flag is not set for PVH dom0, as the > accesses are not emulated but rather forwarded to the physical > hardware. > > No functional change expected for classic PV dom0. For the dense guys like me: what is the user-visible feature that is now being offered with this? Would really appreciate a pointer or two. Thanks, Roman. > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> > --- > for-4.14 reasoning: the fix is mostly isolated to PVH dom0, and as > such the risk is very low of causing issues to other guests types, but > without this fix one vCPU when using a Linux dom0 will be constantly > looping over rtc_timer_do_work with 100% CPU usage, at least when > using Linux 4.19 or newer. > --- > Changes since v1: > - Share the code with PV. > - Add access checks to the IO ports. > --- > xen/arch/x86/hvm/rtc.c | 34 ++++++++++++++++++ > xen/arch/x86/pv/emul-priv-op.c | 28 +++------------ > xen/arch/x86/time.c | 59 +++++++++++++++++++++++++++++++ > xen/include/asm-x86/mc146818rtc.h | 3 ++ > 4 files changed, 100 insertions(+), 24 deletions(-) > > diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c > index 5bbbdc0e0f..9f304a0fb4 100644 > --- a/xen/arch/x86/hvm/rtc.c > +++ b/xen/arch/x86/hvm/rtc.c > @@ -22,6 +22,7 @@ > * IN THE SOFTWARE. > */ > > +#include <asm/iocap.h> > #include <asm/mc146818rtc.h> > #include <asm/hvm/vpt.h> > #include <asm/hvm/io.h> > @@ -808,10 +809,43 @@ void rtc_reset(struct domain *d) > s->pt.source = PTSRC_isa; > } > > +/* RTC mediator for HVM hardware domain. */ > +static int hw_rtc_io(int dir, unsigned int port, unsigned int size, > + uint32_t *val) > +{ > + if ( dir == IOREQ_READ ) > + *val = ~0; > + > + if ( size != 1 ) > + { > + gdprintk(XENLOG_WARNING, "bad RTC access size (%u)\n", size); > + return X86EMUL_OKAY; > + } > + if ( !ioports_access_permitted(current->domain, port, port) ) > + { > + gdprintk(XENLOG_WARNING, "access to RTC port %x not allowed\n", > port); > + return X86EMUL_OKAY; > + } > + > + if ( dir == IOREQ_WRITE ) > + rtc_guest_write(port, *val); > + else > + *val = rtc_guest_read(port); > + > + return X86EMUL_OKAY; > +} > + > void rtc_init(struct domain *d) > { > RTCState *s = domain_vrtc(d); > > + if ( is_hardware_domain(d) ) > + { > + /* Hardware domain gets mediated access to the physical RTC. */ > + register_portio_handler(d, RTC_PORT(0), 2, hw_rtc_io); > + return; > + } > + > if ( !has_vrtc(d) ) > return; > > diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c > index fad6c754ad..1597f27ed9 100644 > --- a/xen/arch/x86/pv/emul-priv-op.c > +++ b/xen/arch/x86/pv/emul-priv-op.c > @@ -280,19 +280,10 @@ static uint32_t guest_io_read(unsigned int port, > unsigned int bytes, > { > sub_data = pv_pit_handler(port, 0, 0); > } > - else if ( port == RTC_PORT(0) ) > - { > - sub_data = currd->arch.cmos_idx; > - } > - else if ( (port == RTC_PORT(1)) && > + else if ( (port == RTC_PORT(0) || port == RTC_PORT(1)) && > ioports_access_permitted(currd, RTC_PORT(0), RTC_PORT(1)) ) > { > - unsigned long flags; > - > - spin_lock_irqsave(&rtc_lock, flags); > - outb(currd->arch.cmos_idx & 0x7f, RTC_PORT(0)); > - sub_data = inb(RTC_PORT(1)); > - spin_unlock_irqrestore(&rtc_lock, flags); > + sub_data = rtc_guest_read(port); > } > else if ( (port == 0xcf8) && (bytes == 4) ) > { > @@ -413,21 +404,10 @@ static void guest_io_write(unsigned int port, unsigned > int bytes, > { > pv_pit_handler(port, (uint8_t)data, 1); > } > - else if ( port == RTC_PORT(0) ) > - { > - currd->arch.cmos_idx = data; > - } > - else if ( (port == RTC_PORT(1)) && > + else if ( (port == RTC_PORT(0) || 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); > + rtc_guest_write(port, data); > } > else if ( (port == 0xcf8) && (bytes == 4) ) > { > diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c > index bbaea3aa65..e1c81067ce 100644 > --- a/xen/arch/x86/time.c > +++ b/xen/arch/x86/time.c > @@ -27,6 +27,7 @@ > #include <xen/keyhandler.h> > #include <xen/guest_access.h> > #include <asm/io.h> > +#include <asm/iocap.h> > #include <asm/msr.h> > #include <asm/mpspec.h> > #include <asm/processor.h> > @@ -1110,6 +1111,64 @@ static unsigned long get_cmos_time(void) > return mktime(rtc.year, rtc.mon, rtc.day, rtc.hour, rtc.min, rtc.sec); > } > > +/* Helpers for guest accesses to the physical RTC. */ > +unsigned int rtc_guest_read(unsigned int port) > +{ > + const struct domain *currd = current->domain; > + unsigned long flags; > + unsigned int data = ~0; > + > + ASSERT(port == RTC_PORT(0) || port == RTC_PORT(1)); > + if ( !ioports_access_permitted(currd, port, port) ) > + { > + ASSERT_UNREACHABLE(); > + return data; > + } > + > + switch ( port ) > + { > + case RTC_PORT(0): > + data = currd->arch.cmos_idx; > + break; > + > + case RTC_PORT(1): > + spin_lock_irqsave(&rtc_lock, flags); > + outb(currd->arch.cmos_idx & 0x7f, RTC_PORT(0)); > + data = inb(RTC_PORT(1)); > + spin_unlock_irqrestore(&rtc_lock, flags); > + break; > + } > + > + return data; > +} > + > +void rtc_guest_write(unsigned int port, unsigned int data) > +{ > + struct domain *currd = current->domain; > + unsigned long flags; > + > + ASSERT(port == RTC_PORT(0) || port == RTC_PORT(1)); > + if ( !ioports_access_permitted(currd, port, port) ) > + { > + ASSERT_UNREACHABLE(); > + return; > + } > + > + switch ( port ) > + { > + case RTC_PORT(0): > + currd->arch.cmos_idx = data; > + break; > + > + case RTC_PORT(1): > + 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); > + break; > + } > +} > + > static unsigned long get_wallclock_time(void) > { > #ifdef CONFIG_XEN_GUEST > diff --git a/xen/include/asm-x86/mc146818rtc.h > b/xen/include/asm-x86/mc146818rtc.h > index 8758528f7c..803b236c0a 100644 > --- a/xen/include/asm-x86/mc146818rtc.h > +++ b/xen/include/asm-x86/mc146818rtc.h > @@ -110,4 +110,7 @@ outb_p((val),RTC_PORT(1)); \ > > #define RTC_IRQ 8 > > +unsigned int rtc_guest_read(unsigned int port); > +void rtc_guest_write(unsigned int port, unsigned int data); > + > #endif /* _ASM_MC146818RTC_H */ > -- > 2.26.2 > >
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |