[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v6] x86: detect CMOS aliasing on ports other than 0x70/0x71
On Wed, Apr 19, 2023 at 03:58:10PM +0200, Jan Beulich wrote: > On 18.04.2023 13:35, Roger Pau Monné wrote: > > On Tue, Apr 18, 2023 at 11:24:19AM +0200, Jan Beulich wrote: > >> ... in order to also intercept Dom0 accesses through the alias ports. > >> > >> Also stop intercepting accesses to the CMOS ports if we won't ourselves > >> use the CMOS RTC, because of there being none. > >> > >> Note that rtc_init() deliberately uses 16 as the upper loop bound, > >> despite probe_cmos_alias() using 8: The higher bound is benign now, but > >> would save us touching the code (or, worse, missing to touch it) in case > >> the lower one was doubled. > >> > >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > > > > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> > > Before committing I went back to read through doc and earlier comments, > in particular regarding the NMI disable. As a result I'm now inclined > to follow your earlier request and fold in the change below. Thoughts? It was unclear to me whether port 0x70 also had this NMI disabling behavior when the RTC/CMOS is not present but it seems that port is shared between the RTC index and the NMI logic, so lack of RTC doesn't imply lack of the NMI bit. > Jan > > --- a/xen/arch/x86/time.c > +++ b/xen/arch/x86/time.c > @@ -1305,6 +1305,13 @@ bool is_cmos_port(unsigned int port, uns > { > unsigned int offs; > > + /* > + * While not really CMOS-related, port 0x70 always needs intercepting > + * to deal with the NMI disable bit. > + */ > + if ( port <= RTC_PORT(0) && port + bytes > RTC_PORT(0) ) > + return true; It might make it clearer to move this after the !is_hardware_domain(d) check, as non-hardware domains don't get access to that port anyway? > + > if ( !is_hardware_domain(d) ) > return port <= RTC_PORT(1) && port + bytes > RTC_PORT(0); > > @@ -1342,6 +1349,17 @@ unsigned int rtc_guest_read(unsigned int > * underlying hardware would permit doing so. > */ > data = currd->arch.cmos_idx & (0xff >> (port == RTC_PORT(0))); > + > + /* > + * When there's (supposedly) no RTC/CMOS, we don't intercept the > other > + * ports. While reading the index register isn't normally possible, > + * play safe and return back whatever can be read (just in case a > value > + * written through an alias would be attempted to be read back here). > + */ > + if ( port == RTC_PORT(0) && > + (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) && > + ioports_access_permitted(currd, port, port) ) > + data = inb(port) & 0x7f; Do we really need to mask the high bit here? We don't allow setting that bit in the first place. Thanks, Roger.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |