Re: [PATCH for-4.14] x86/rtc: provide mediated access to RTC for PVH dom0

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Jun 05, 2020 at 09:03:11AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> > Sent: 05 June 2020 08:50
> > To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: paul@xxxxxxx; Roger Pau Monne <roger.pau@xxxxxxxxxx>; Jan Beulich 
> > <jbeulich@xxxxxxxx>; Andrew
> > Cooper <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>
> > Subject: [PATCH for-4.14] x86/rtc: provide mediated access to RTC for PVH 
> > dom0
> > 
> > At some point (maybe PVHv1?) mediated access to the RTC was provided
> > for PVH dom0 using the PV code paths (guest_io_{write/read}). At some
> > point this code has been made PV specific and unhooked from the
> > current PVH IO path. This patch provides such mediated access to the
> > RTC for PVH dom0, just like it's provided for a classic PV dom0.
> > 
> > Instead of re-using the PV paths implement such handler together with
> > the vRTC code for HVM, so that calling rtc_init will setup the
> > appropriate handlers for all HVM based guests.
> > 
> > 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.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> > for-4.14 reasoning: the fix is completely 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.
> > ---
> 
> I can't say I'm a big fan of the code duplication with emul-priv-op.c, but 
> it's not much code and it does keep this patch small. If the maintainers are 
> happy to ack then consider this...

Calling guest_io_{write/read} straight away is no acceptable IMO (and it
would have to be moved out of emul-priv-op.c), and splitting the RTC
accessors into separate helpers seemed like a lot of churn for the
actual code that such helpers would contain.

> Release-acked-by: Paul Durrant <paul@xxxxxxx>

Thanks.